Added ability to specify scrape protocols to accept during HTTP content type negotiation. by bwplotka · Pull Request #12738 · prometheus/prometheus

bwplotka · 2023-08-22T20:24:54Z

This is done via new option in GlobalConfig and ScrapeConfig: "scrape_protocols"

config/config.go

ArthurSens

LGTM, just one question :)

cmd/prometheus/main.go

config/config.go

bwplotka · 2023-08-25T16:48:48Z

Friendly ping @roidelapluie @beorn7 (:

config/config.go

roidelapluie · 2023-08-28T13:26:48Z

Wait..

scrape/scrape.go

roidelapluie · 2023-08-31T01:29:24Z

This is an open door to all sort of bad things. I would not have free text. Le jeu. 31 août 2023, 00:20, Bartlomiej Plotka ***@***.***> a écrit :

…

***@***.**** commented on this pull request. ------------------------------ In scrape/scrape.go <#12738 (comment)> : > @@ -807,10 +809,20 @@ type targetScraper struct { var errBodySizeLimit = errors.New("body size limit exceeded") -const ( - scrapeAcceptHeader = `application/openmetrics-text;version=1.0.0,application/openmetrics-text;version=0.0.1;q=0.75,text/plain;version=0.0.4;q=0.5,*/*;q=0.1` - scrapeAcceptHeaderWithProtobuf = `application/vnd.google.protobuf;proto=io.prometheus.client.MetricFamily;encoding=delimited,application/openmetrics-text;version=1.0.0;q=0.8,application/openmetrics-text;version=0.0.1;q=0.75,text/plain;version=0.0.4;q=0.5,*/*;q=0.1` -) +func acceptHeader(sps []config.ScrapeProtocol) string { + var vals []string + for _, sp := range sps { + switch { + case sp.Equals(config.PrometheusText): + vals = append(vals, "text/plain;version=0.0.4;q=0.5") + case sp.Equals(config.PrometheusProto): + vals = append(vals, "application/vnd.google.protobuf;proto=io.prometheus.client.MetricFamily;encoding=delimited") + case sp.Equals(config.OpenMetricsText): + vals = append(vals, "application/openmetrics-text;version=1.0.0,application/openmetrics-text;version=0.0.1;q=0.75,*/*;q=0.1") I consider changing to header values for more flexibility and simplicity: [image: image] <https://user-images.githubusercontent.com/6950331/264486651-67519d7c-2f82-4d31-a5ff-c16134997d08.png> WDYT? — Reply to this email directly, view it on GitHub <#12738 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACHHJRCF6CDZNVUYIQF463XX64C7ANCNFSM6AAAAAA32OPQII> . You are receiving this because you were mentioned.Message ID: ***@***.***>

bwplotka · 2023-09-03T17:31:21Z

Hm, what bad things?

bwplotka · 2023-09-03T17:47:39Z

The alternatives to my proposal are:

B) Multiple enum values like "OpenMetrics1.0.0", "OpenMetrics0.0.1", "PrometheusText0.0.4" "PrometheusProto" (proto is not versioned) and if new version of something comes in users are blocked (until code is changed). Also more work for us to maintain and update those.

C) Enums for types of format and free form semver version. This gives us ability to experiment with new versions of formats. More complex code, more flexibility for users.

I guess the benefit of B is that those are formats Prometheus supports and this is clearly guarded by those options.

🤔 @roidelapluie @beorn7

beorn7 · 2023-09-05T17:25:21Z

I haven't studied this in detail yet, but I like pragmatic fallbacks, something like:

if the version number exposed by the target doesn't fit exactly, but Prometheus recognizes the protocol, try something "smart" (something like trying the most recent parser for that protocol, but stay within the same major versions if one could be parsed).
if nothing fits, try classic Prometheus text format as the universal fallback (that's what we do right now, don't we?)

bwplotka · 2023-09-20T14:26:08Z

@beorn7 I think you are speaking about something else. We don't change parsing here, we only change Accept header that will be passed to clients. It's up to clients to give us something based on our priority as well as what they implement (support).

beorn7 · 2023-09-20T14:34:50Z

Yeah, sorry. I'll have a closer look ASAP (which might not be very soon in absolute terms, but let's see if somebody else beats me to it).

roidelapluie · 2023-09-28T05:27:07Z

I want promethe to only send accept headers that are clearly defined. I do not really care about the enums we are using but let's not allow free text.

bwplotka · 2023-10-02T14:29:41Z

Ack, this is what we discussed in person too - moving to enums with version then - will do soon (:

bwplotka · 2023-10-09T13:21:28Z

PTAL, updated to enums. 🤗 cc @roidelapluie @beorn7

docs/configuration/configuration.md

…nt type negotiation. This is done via new option in GlobalConfig and ScrapeConfig: "scrape_protocol" Signed-off-by: bwplotka <bwplotka@gmail.com>

Signed-off-by: bwplotka <bwplotka@gmail.com>

@beorn7

Broken by #12738. We have to update both global variables (as GlobalConfig is not a pointer here). DefaultConfig is used when no global: section is provided, whereas DefaultGlobalConfig is used when it's provided and for individual scrape configsReported on #prometheus-dev (thanks to @beorn7): https://cloud-native.slack.com/archives/C01AUBA4PFE/p1697733267205649 Tested manually, it would be nice to add test at some point (quick fix for now). Signed-off-by: bwplotka <bwplotka@gmail.com>

@beorn7

Broken by #12738. We have to update both global variables (as GlobalConfig is not a pointer here). DefaultConfig is used when no global: section is provided, whereas DefaultGlobalConfig is used when it's provided and for individual scrape configs. Reported on #prometheus-dev (thanks to @beorn7): https://cloud-native.slack.com/archives/C01AUBA4PFE/p1697733267205649 Tested manually, it would be nice to add test at some point (quick fix for now). Signed-off-by: bwplotka <bwplotka@gmail.com>

@beorn7

…13010) Broken by #12738. We have to update both global variables (as GlobalConfig is not a pointer here). DefaultConfig is used when no global: section is provided, whereas DefaultGlobalConfig is used when it's provided and for individual scrape configs. Reported on #prometheus-dev (thanks to @beorn7): https://cloud-native.slack.com/archives/C01AUBA4PFE/p1697733267205649 Tested manually, it would be nice to add test at some point (quick fix for now). Signed-off-by: bwplotka <bwplotka@gmail.com>

…30934) Fixes #30883 Prometheus PR we need to adapt to: prometheus/prometheus#12738 The configuration option we were using to enable protobuf has been removed, so this PR removes the `enable_protobuf_negotiation` configuration on the prometheus receiver. In its place, the prometheus server now has a config option, `scrape_protocols`, which specifies which protocols it uses, including protobuf. Use `config.global.scrape_protocols = [ PrometheusProto, OpenMetricsText1.0.0, OpenMetricsText0.0.1, PrometheusText0.0.4 ]` to enable protobuf scraping instead of using our option. Prometheus PR we need to adapt to: prometheus/prometheus#12958 We now need to pass a prometheus.Registerer to the scrape manager to collect metrics about scrapes. This PR disables the collection of these scrape metrics from the prometheus server. We did not use the default registry, so we were previously collecting these metrics and never exposing them. This PR passes a no-op registerer to prevent using memory to store those metrics. In the future, we can consider using the prometheus -> OTel bridge to export these with the self-observability pipeline.

…nt type negotiation. (prometheus#12738) * Added ability to specify scrape protocols to accept during HTTP content type negotiation. This is done via new option in GlobalConfig and ScrapeConfig: "scrape_protocol" Signed-off-by: bwplotka <bwplotka@gmail.com> * Fixed readability and log message. Signed-off-by: bwplotka <bwplotka@gmail.com> --------- Signed-off-by: bwplotka <bwplotka@gmail.com>

bwplotka requested review from beorn7 and roidelapluie August 22, 2023 20:24

bwplotka force-pushed the formats branch 2 times, most recently from a127463 to c2e1ecf Compare August 22, 2023 20:44

bwplotka commented Aug 23, 2023

View reviewed changes

config/config.go Outdated Show resolved Hide resolved

ArthurSens approved these changes Aug 23, 2023

View reviewed changes

cmd/prometheus/main.go Outdated Show resolved Hide resolved

config/config.go Outdated Show resolved Hide resolved

bwplotka force-pushed the formats branch 2 times, most recently from 29b64ce to 0e99aa6 Compare August 24, 2023 11:30

roidelapluie approved these changes Aug 28, 2023

View reviewed changes

config/config.go Show resolved Hide resolved

roidelapluie reviewed Aug 28, 2023

View reviewed changes

scrape/scrape.go Outdated Show resolved Hide resolved

rexagod mentioned this pull request Sep 14, 2023

metrics: add exemplar support for counters kubernetes/kubernetes#119949

Merged

bwplotka marked this pull request as draft October 2, 2023 14:29

bwplotka mentioned this pull request Oct 2, 2023

Propose support for UTF-8 in metric and label names prometheus/proposals#28

Merged

bwplotka force-pushed the formats branch from 0e99aa6 to a56edaf Compare October 9, 2023 09:21

bwplotka marked this pull request as ready for review October 9, 2023 09:21

roidelapluie approved these changes Oct 9, 2023

View reviewed changes

bwplotka commented Oct 9, 2023

View reviewed changes

docs/configuration/configuration.md Outdated Show resolved Hide resolved

Added ability to specify scrape protocols to accept during HTTP conte…

0f7374e

…nt type negotiation. This is done via new option in GlobalConfig and ScrapeConfig: "scrape_protocol" Signed-off-by: bwplotka <bwplotka@gmail.com>

bwplotka force-pushed the formats branch from a56edaf to 0f7374e Compare October 9, 2023 15:13

Fixed readability and log message.

31140a7

Signed-off-by: bwplotka <bwplotka@gmail.com>

bwplotka merged commit 624b973 into main Oct 10, 2023

bwplotka deleted the formats branch October 10, 2023 10:16

dongjiang1989 mentioned this pull request Oct 11, 2023

[WIP] add scrape_protocols for GlobalConfig and ScrapeConfig prometheus-operator/prometheus-operator#5991

Closed

5 tasks

LeviHarrison mentioned this pull request Oct 15, 2023

Cherry pick latest main changes into 2.48 #12985

Merged

beorn7 mentioned this pull request Oct 17, 2023

scrape: make le and quantile normalization configurable #12984

Closed

bwplotka mentioned this pull request Oct 19, 2023

native-histograms: Fixed PrometheusProto scrape format preference. #13010

Merged

yu-shiba mentioned this pull request Dec 11, 2023

Quality values in Accept header in 2.48.0 are wrong #13268

Closed

rexagod mentioned this pull request Dec 13, 2023

Info and StateSet types cause errors when scrapped by Prometheus kubernetes/kube-state-metrics#2248

Closed

yu-shiba mentioned this pull request Dec 20, 2023

bug: Fix quality value in accept header #13313

Merged

dongjiang1989 mentioned this pull request Jan 16, 2024

feat: support scrape_protocols for GlobalConfig and ScrapeConfig prometheus-operator/prometheus-operator#6235

Merged

5 tasks

dashpole mentioned this pull request Feb 1, 2024

Update prometheus to v0.50.1 and remove enable_protobuf_negotiation open-telemetry/opentelemetry-collector-contrib#30934

Merged

ptodev mentioned this pull request Mar 22, 2024

Upgrade agent to prometheus 2.51 grafana/agent#6754

Closed

4 tasks

rexagod mentioned this pull request Apr 8, 2024

Add histogram exemplar support kubernetes/kubernetes#124013

Merged

Conversation

bwplotka commented Aug 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ArthurSens left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bwplotka commented Aug 25, 2023

Uh oh!

Uh oh!

roidelapluie commented Aug 28, 2023

Uh oh!

Uh oh!

roidelapluie commented Aug 31, 2023 via email

Uh oh!

bwplotka commented Sep 3, 2023

Uh oh!

bwplotka commented Sep 3, 2023

Uh oh!

beorn7 commented Sep 5, 2023

Uh oh!

bwplotka commented Sep 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

beorn7 commented Sep 20, 2023

Uh oh!

roidelapluie commented Sep 28, 2023

Uh oh!

bwplotka commented Oct 2, 2023

Uh oh!

bwplotka commented Oct 9, 2023

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bwplotka commented Aug 22, 2023 •

edited

Loading

bwplotka commented Sep 20, 2023 •

edited

Loading