Skip to content

fix: handle multi-type Accept headers in parseOpenMetricsVersion#1987

Merged
jaydeluca merged 1 commit intoprometheus:content-negotiation-flagfrom
zeitlinger:fix/parse-openmetrics-version
Mar 30, 2026
Merged

fix: handle multi-type Accept headers in parseOpenMetricsVersion#1987
jaydeluca merged 1 commit intoprometheus:content-negotiation-flagfrom
zeitlinger:fix/parse-openmetrics-version

Conversation

@zeitlinger
Copy link
Copy Markdown
Member

Summary

  • Fix bug where parseOpenMetricsVersion could pick up version parameters from unrelated media types in multi-type Accept headers (e.g. application/openmetrics-text, text/plain; version=0.0.4)
  • Split Accept header by , first to isolate individual media types before parsing parameters
  • Add two test cases covering multi-type Accept headers with and without an explicit openmetrics version

Test plan

  • All 55 existing ExpositionFormatsTest tests pass
  • New test: multi-type header where only text/plain has version → OM1 writer selected
  • New test: multi-type header where openmetrics has version=2.0.0 → OM2 writer selected

…ader

parseOpenMetricsVersion was splitting the entire Accept header by ";"
which meant version parameters from other media types (e.g. text/plain;
version=0.0.4) could be incorrectly attributed to openmetrics-text.

Now splits by "," first to isolate individual media types, finds the
openmetrics-text entry, then parses only its parameters for version.
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Copy link
Copy Markdown
Collaborator

@jaydeluca jaydeluca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could've left a code review comment and I could have done this, but thanks!

@jaydeluca jaydeluca merged commit b15d16e into prometheus:content-negotiation-flag Mar 30, 2026
13 checks passed
jaydeluca pushed a commit that referenced this pull request Mar 30, 2026
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants