Skip to content

Commit 52d0bae

Browse files
authored
Merge pull request #59 from prometheus/ci
fix: Make the check to actually fail on not formatted/broken links; fix PROM-52 proposal
2 parents 9f74f25 + 643c21f commit 52d0bae

33 files changed

+590
-516
lines changed

.github/.mdox.validator.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ version: 1
33
# Don't check external URLs, currently it can timeout a lot on CI.
44
validators:
55
- type: ignore
6-
regex: https://.*
6+
regex: http(s|)://.*

0000-template.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,5 +69,5 @@ The section stating potential alternatives. Highlight the objections reader shou
6969

7070
The tasks to do in order to migrate to the new idea.
7171

72-
* [ ] Task one <GH issue>
73-
* [ ] Task two <GH issue> ...
72+
* [ ] Task one `<GH issue>`
73+
* [ ] Task two `<GH issue>` ...

Makefile

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
include .bingo/Variables.mk
22

3-
.DEFAULT_GOAL := fmt
4-
MD_FILES_TO_FORMAT=$(shell find . -name "*.md")
3+
MD_FILES_TO_FORMAT=$(shell find . -name "*.md" | grep -v ".bingo/README.md")
54

65
help: ## Displays help.
76
@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m<target>\033[0m\n\nTargets:\n"} /^[a-zA-Z_-]+:.*?##/ { printf " \033[36m%-10s\033[0m %s\n", $$1, $$2 }' $(MAKEFILE_LIST)
@@ -10,10 +9,12 @@ help: ## Displays help.
109
fmt: ## Format docs, ensure GitHub format.
1110
fmt: $(MDOX)
1211
@echo "Formatting markdown files..."
13-
$(MDOX) fmt --soft-wraps $(MD_FILES_TO_FORMAT)
12+
$(MDOX) fmt --soft-wraps --links.validate --links.validate.config-file=.github/.mdox.validator.yaml $(MD_FILES_TO_FORMAT)
1413

1514
.PHONY: check
1615
check: ## Checks if doc is formatter and links are correct (don't check external links).
1716
check: $(MDOX)
18-
@echo "Checking markdown files. If changes are detected, try running `make` and trying again."
19-
$(MDOX) fmt --soft-wraps --check --links.validate.config-file=.github/.mdox.validator.yaml *.md $(MD_FILES_TO_FORMAT)
17+
@echo "Checking markdown file formatting and basic links."
18+
$(MDOX) fmt --soft-wraps --links.validate --links.validate.config-file=.github/.mdox.validator.yaml --check $(MD_FILES_TO_FORMAT) || (echo "🔥 Validation failed, files not formatted or links are broken. Try running 'make fmt' to fix formatting!" && exit 1)
19+
@echo "✅ Markdown files correctly formatted"
20+
bash ./scripts/proposals-filename-check.sh

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ The process of proposing a change via a design document is the following:
2929

3030
1. Fork `github.com/prometheus/proposals`.
3131
2. Copy the [template](0000-template.md) to [proposals directory](./proposals) as `./proposals/0000-<my-proposal>.md` where `<my-proposal>` is the relevant proposal title. Once a Pull Request is made, update the prefix number to match the PR ID. As a result your proposal will be referenced as `PROM-<PR ID>`.
32-
3. Fill the proposal details. Use the template as the guide for what sections should be present in the document.
32+
3. Fill the proposal details. Use the template as the guide for what sections should be present in the document.
3333
4. Create a GitHub Pull Request with the proposal, using `proposal:` prefix in the PR Title. Once the PR is proposed, a maintainer will assign a `proposal` label.
34-
1. If you prefer Google Docs to any other collaboration tool, feel free to use it in the initial state. We recommend the [Open Source Design document Template](https://docs.google.com/document/d/1zeElxolajNyGUB8J6aDXwxngHynh4iOuEzy3ylLc72U/edit#). However, the approval process will only happen officially in the Pull Request.
34+
1. If you prefer Google Docs to any other collaboration tool, feel free to use it in the initial state. We recommend the [Open Source Design document Template](https://docs.google.com/document/d/1zeElxolajNyGUB8J6aDXwxngHynh4iOuEzy3ylLc72U/edit#). However, the approval process will only happen officially in the Pull Request.
3535
5. An automatic formatter is enabled in the repository. Use `make` locally to trigger the formatting of all markdown documents (requires a working Go environment). Use `make check` to check all links (will be done by the CI pipeline, too).
3636
6. After a sufficient amount of discussion, the Prometheus team will try to reach a consensus of accepting or rejecting the proposal. In the former case, the PR gets merged. In the latter case, the PR gets closed with meaningful reasons why the proposal was rejected.
3737
1. If more eyes are needed or no consensus was made: Propose your idea as an agenda item for the [Prometheus DevSummit](https://docs.google.com/document/d/11LC3wJcVk00l8w5P3oLQ-m3Y37iom6INAMEu2ZAGIIE/edit) or announce it to the [developer mailing list](https://groups.google.com/forum/#!forum/prometheus-developers) to gather more information. You are welcome to start working on the design document before a bigger discussion—it is often easier to discuss with prior details provided. Be prepared that the idea might be rejected later. Still, the record of the document in the Pull Request is valuable even in a rejected state to inform about past decisions and opportunities considered.

SECURITY.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@
33
The Prometheus security policy, including how to report vulnerabilities, can be
44
found here:
55

6-
<https://prometheus.io/docs/operating/security/>
6+
https://prometheus.io/docs/operating/security/

proposals/0001-proposal-process.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ I propose to host in git only accepted proposals, simply in the root directory.
6262
Given that, the process of proposing change with the design doc would look as follows (we can use the below text as the initial instruction):
6363

6464
1. Fork `github.com/prometheus/proposals`.
65-
2. Create a GitHub Pull Request with a design document in markdown format to the repository's root directory. Make sure to use [template](../0000-00-00_template.md) as the guide for what sections should be present in the document. Put the creation date (the day you started preparing this design doc) as the prefix and some unique name as the suffix in the file name.
65+
2. Create a GitHub Pull Request with a design document in markdown format to the repository's root directory. Make sure to use [template](../0000-template.md) as the guide for what sections should be present in the document. Put the creation date (the day you started preparing this design doc) as the prefix and some unique name as the suffix in the file name.
6666
1. If you prefer Google Docs to any other collaboration tool, feel free to use it in the initial state. We recommend [Open Source Design Doc Template](https://docs.google.com/document/d/1zeElxolajNyGUB8J6aDXwxngHynh4iOuEzy3ylLc72U/edit#). However, the approval process will only happen officially in the Pull Request.
6767
3. Automatic formatter is enabled in the repository. Use `make` locally to format it. Use `make check` to check all links (will be done on the CI too).
6868
4. The design is accepted if the PR is merged into this repository. It's ok to eventually decide to reject the proposal and close the PR with meaningful reasons for why it was rejected.

proposals/0002-am-secure-cluster.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* **Related Issues and PRs:**
99
* https://github.com/prometheus/alertmanager/pull/2237
1010

11-
> NOTE(bwplotka): This proposal was moved from [Alertmanager repo](https://github.com/prometheus/alertmanager/blob/6ef6e6868dbeb7984d2d577dd4bf75c65bf1904f/doc/design/secure-cluster-traffic.md) before we had [unified proposal process](2022-11-23-proposal-process.md), so it does not follow consistent style guide. For new proposal see [README](/README.md).
11+
> NOTE(bwplotka): This proposal was moved from [Alertmanager repo](https://github.com/prometheus/alertmanager/blob/6ef6e6868dbeb7984d2d577dd4bf75c65bf1904f/doc/design/secure-cluster-traffic.md) before we had [unified proposal process](0001-proposal-process.md), so it does not follow consistent style guide. For new proposal see [README](/README.md).
1212
1313
## Status Quo
1414

proposals/0028-utf8.md

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,10 @@ We propose the following change in syntax to PromQL and the exposition format:
9494
* Quoted label name: `{"my.dotted.metric", "http.status"=~"5.."}`
9595

9696
* Classic metrics do not need to be treated differently:
97-
`sum(rate(regular_metric{region="east"}[5m]))`
97+
`sum(rate(regular_metric{region="east"}[5m]))`
9898

9999
* If the user decides to put a classic metric inside the selector, it still needs to be quoted:
100-
`sum(rate({"regular_metric", region="east"}[5m]))`
100+
`sum(rate({"regular_metric", region="east"}[5m]))`
101101

102102
* Escape syntax if the metric has a quote: `sum(rate({"my \"quoted\" metric", region="east"}[5m]))` or use single quotes in PromQL (not available in the exposition format): `sum(rate({'my "quoted" metric', region="east"}[5m]))`
103103

@@ -109,7 +109,7 @@ We propose the following change in syntax to PromQL and the exposition format:
109109
`{"my.metric.with.periods", region="east"}.sum`
110110

111111
* Compatible with possible future metric name operator syntax via prefixes (not being proposed here):
112-
```{~`a\.regex.*selector.*`, region="east"}``` or `{!"not.this.name", region="east"}`
112+
`{~`a\.regex.*selector.*`, region="east"}` or `{!"not.this.name", region="east"}`
113113

114114
### Unusual / Complex Characters
115115

@@ -126,8 +126,8 @@ Templating is used in alerts, and for dashboards that want to reference metric v
126126
There a few situations where we need to consider backwards compatibility:
127127

128128
1. Old metrics provider, new backend: Old providers will not serve UTF-8 metric names, so nothing needs to be done here.
129-
2. New metrics provider, old backend: Old versions of Prometheus will throw an error if they see the new syntax. Therefore we [propose a way](#Scrape-time) for the scraped target to serve previously-invalid metrics in a form that the old database can process.
130-
3. New backend, old query client: Errors will occur if an old client is fed UTF-8 metric names. Therefore we must [provide a way](#Text-escaping) for Prometheus to escape metric names such that old clients can query for and read them.
129+
2. New metrics provider, old backend: Old versions of Prometheus will throw an error if they see the new syntax. Therefore we [propose a way](#scrape-time) for the scraped target to serve previously-invalid metrics in a form that the old database can process.
130+
3. New backend, old query client: Errors will occur if an old client is fed UTF-8 metric names. Therefore we must [provide a way](#text-escaping) for Prometheus to escape metric names such that old clients can query for and read them.
131131
4. Old backend, new query client: as with the first case, nothing needs to be done.
132132

133133
### Text escaping
@@ -139,15 +139,15 @@ We propose an escaping syntax for metric names that contain UTF-8 characters for
139139
* All pre-existing underscores will become doubled: `__`.
140140
* If a string should start with "U__" already, it will need to be escaped: `U___55_____`. (That's `U__` + `_55_` (for `U`) + `__` + `__`).
141141

142-
#### Mixed-Block Scenario
142+
#### Mixed-Block Scenario
143143

144144
We must consider an edge case in which a newer client persists metrics to disk in an older database that does not support UTF-8. Those metrics will be written to disk with the U__ escaping format. If, later, the user upgrades their database software, new metrics will be written with the native UTF-8 characters. At query time, there will be a problem: newer blocks were written with UTF-8 and older blocks were written with the escaping format. The query code will not know which encoding to look for.
145145

146146
To avoid this confusion we propose to bump the version number in the tsdb meta.json file. On a per-block basis the query code can check the version number and look for UTF-8 metric names in native encoding for bumped version number 2, or in U__-escaped naming for earlier version 1.
147147

148148
Additionally, this means that an old metric or label name containing a valid escape sequence by chance would be parsed differently after enabling UTF-8 support. For example, a metric called `U___263a_` would appear as `` after the upgrade (if read from an old storage chunk, scraped from an old target, or received from an old remote-write client). This is an extremely unlikely scenario in practice so we believe a theoretically possible implicit name change is acceptable.
149149

150-
Lastly, if a metric is written as "U__" but does not pass the unescaping algorithm, then we assume it was meant to be read as-is and let it pass through.
150+
Lastly, if a metric is written as "U__" but does not pass the unescaping algorithm, then we assume it was meant to be read as-is and let it pass through.
151151

152152
### Scrape Time
153153

@@ -169,7 +169,7 @@ The HTTP endpoints will need to support HTTP-unquoting metric names in the URL p
169169

170170
## Implementation
171171

172-
Implementing this feature will require updates to several Prometheus libraries. These updates will be written so that functionality does not change for existing users and builds, and will only be enabled when certain flags are flipped or options turned on. There will be minimal read-only backwards compatibility for clients that do not support the new supported character set and syntax.
172+
Implementing this feature will require updates to several Prometheus libraries. These updates will be written so that functionality does not change for existing users and builds, and will only be enabled when certain flags are flipped or options turned on. There will be minimal read-only backwards compatibility for clients that do not support the new supported character set and syntax.
173173

174174
This implementation summary is based on the partial proof-of-concept work and may need to expand to accommodate any additional edge cases that are encountered. We do not expect the scope of work to expand significantly.
175175

@@ -219,11 +219,11 @@ We will also have to bump the version number of tsdb and add code to handle the
219219

220220
While this syntax could be made to work with simple additional characters like dots, it does not fulfill the goal of full UTF-8 support, which would include `{` or other special characters without a verbose and less-readable escaping mechanism, e.g.
221221

222-
`my.dotted.metric\{with\ braces\ and\ spaces\}{foo="bar"}`.
222+
`my.dotted.metric\{with\ braces\ and\ spaces\}{foo="bar"}`.
223223

224224
Versus our selected approach:
225225

226-
`{"my.dotted.metric{with braces and spaces}", foo="bar"}`
226+
`{"my.dotted.metric{with braces and spaces}", foo="bar"}`
227227

228228
Selecting this approach would introduce a convenient syntax for only a small new subset of UTF-8, so a quoting mechanism would still be needed for the full character set. Rather than implement this half-improvement we decided to only support a full quoting approach.
229229

@@ -239,7 +239,7 @@ See the [Discussion Doc](https://docs.google.com/document/d/1yFj5QSd1AgCYecZ9EJ8
239239

240240
### Backticked metric names outside the curly braces
241241

242-
``` `my.dotted.metric`{"label"="value"}```
242+
` `my.dotted.metric`{"label"="value"}`
243243

244244
Single, double, and backtick quotes are nearly equivalent in PromQL so this would just be the same as the above approach, or require changing the meaning of a particular quote character in a way that would break backward-compatibility. (Backticks prevent processing of escape sequences, which is otherwise enabled, see [PromQL documentation](https://prometheus.io/docs/prometheus/latest/querying/basics/#string-literals) for details.)
245245

@@ -260,4 +260,3 @@ By allowing non-escaped characters, this approach risks colliding with any new f
260260
2. Make updates to Prometheus and Grafana Agent.
261261
3. Launch as experimental feature, default off.
262262
4. Once stable and debugged, make UTF-8 the default (with fallback escaping for old clients always supported).
263-

proposals/0030-utf8-migration.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ The solution described here is valid for all forms of metrics ingestion, may it
4848
We must consider edge cases in which a blocks database has persisted metrics or labels that may have been written by different versions of code.
4949
There are multiple ways this can (and will) happen:
5050

51-
* An older version of Prometheus ingests names from a newer metrics producer. In this case, names would be escaped with any of the available escaping methods. If Prometheus is upgraded, newer blocks will be written in UTF-8.
51+
* An older version of Prometheus ingests names from a newer metrics producer. In this case, names would be escaped with any of the available escaping methods. If Prometheus is upgraded, newer blocks will be written in UTF-8.
5252
* A newer Prometheus receives names from an older producer, which is later upgraded. In this case, older names might be escaped using the replace-with-underscores method, and newer names will be UTF-8. This will often happen when Prometheus is receiving Open Telemetry metrics.
5353
* A newer Prometheus receives names from a mix of new and old producers, in which case the same block could contain escaped and UTF-8 data representing the same intended names.
5454

@@ -96,7 +96,7 @@ We propose to do this by expanding a lookup for a UTF-8 metric or label name int
9696

9797
1. **UTF-8**
9898
2. **underscore-replaced**: All unsupported characters are converted to underscores.
99-
3. **U__ escaping**: As described in the UTF-8 proposal, strings with invalid characters can be escaped by prepending `U__` and replacing all invalid characters with `_[UTF8 value]_`.
99+
3. **U__ escaping**: As described in the UTF-8 proposal, strings with invalid characters can be escaped by prepending `U__` and replacing all invalid characters with `_[UTF8 value]_`.
100100
4. **[Datadog proxy](https://github.com/grafana/mimir-proxies/blob/main/pkg/datadog/ddprom/naming.go#L30-L34) escaping pattern**: "`.`" becomes "`_dot_`" and "`_`" becomes "`__`".
101101

102102
In PromQL, the expansion would look something like this under the hood:
@@ -123,7 +123,7 @@ We will do performance testing to identify possible issues.
123123

124124
If the user is querying for metrics using a regex lookup for the `__name__` label, attempting to rewrite that query to account for other name encodings would be overly complex and error-prone.
125125
Therefore we will not try to rewrite the regex to account for multiple escaping methods and the regex will be passed through as-is.
126-
Users will need to write custom regex queries to account for metric name changes during the transition period in this case.
126+
Users will need to write custom regex queries to account for metric name changes during the transition period in this case.
127127
Since regex queries on metrics names are relatively rare and the domain of advanced users, we feel this is an acceptable approach.
128128

129129
### Name Collisions
@@ -178,4 +178,4 @@ That way queries for both the native UTF-8 name and the escaped name would succe
178178
When the migration was complete, users could turn off double-writing and only write UTF-8.
179179

180180
This approach would cause an explosion of on-disk usage.
181-
As disk is one of the most expensive resources, this approach was quickly discarded.
181+
As disk is one of the most expensive resources, this approach was quickly discarded.

0 commit comments

Comments
 (0)