[Exporter.Prometheus] Fix Accept header handling#7208
Conversation
- Update the handling of the `Accept` header to follow the requirements documented in https://prometheus.io/docs/instrumenting/content_negotiation/. - Removed unused code.
| <Compile Include="$(RepoRoot)\src\OpenTelemetry.Exporter.Prometheus.HttpListener\Internal\PrometheusSerializerExt.cs" Link="Includes/PrometheusSerializerExt.cs" /> | ||
| <Compile Include="$(RepoRoot)\src\OpenTelemetry.Exporter.Prometheus.HttpListener\Internal\PrometheusType.cs" Link="Includes/PrometheusType.cs" /> | ||
| <Compile Include="$(RepoRoot)\src\OpenTelemetry.Exporter.Prometheus.HttpListener\Internal\PrometheusMetric.cs" Link="Includes/PrometheusMetric.cs" /> | ||
| <Compile Include="$(RepoRoot)\src\OpenTelemetry.Exporter.Prometheus.HttpListener\Internal\PrometheusHeadersParser.cs" Link="Includes/PrometheusHeadersParser.cs" /> |
There was a problem hiding this comment.
This was removed in favour of a native implementation as ASP.NET Core has built-in parsers for the Accept header and we were manually re-parsing it.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7208 +/- ##
=======================================
Coverage 89.67% 89.68%
=======================================
Files 272 272
Lines 13378 13434 +56
=======================================
+ Hits 11997 12048 +51
- Misses 1381 1386 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
|
- Add a constant for the OpenMetrics version. - Update CHANGELOGs with PR number.
There was a problem hiding this comment.
Pull request overview
Updates Prometheus exporter content negotiation to better follow Prometheus scrape protocol rules by improving Accept header parsing and OpenMetrics vs Prometheus text selection.
Changes:
- Enhanced
Acceptheader parsing to account for whitespace,qweights, and OpenMetricsversion(including quoted values). - Updated ASP.NET Core middleware to use typed
Acceptheaders (GetTypedHeaders().Accept) and select the preferred format by quality. - Expanded unit test coverage for header parsing/negotiation scenarios and updated changelogs.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHeadersParserTests.cs | Adds additional valid/invalid Accept header test cases (whitespace, quoted version, q weights). |
| test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs | Refactors some tests and adds a focused unit test for AcceptsOpenMetrics using typed header parsing. |
| src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusHeadersParser.cs | Implements Accept parsing with whitespace trimming, version parsing (quoted), and q-based negotiation. |
| src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md | Documents the improved Accept header handling change. |
| src/OpenTelemetry.Exporter.Prometheus.AspNetCore/PrometheusExporterMiddleware.cs | Switches to typed Accept parsing and applies q-based OpenMetrics vs text selection. |
| src/OpenTelemetry.Exporter.Prometheus.AspNetCore/OpenTelemetry.Exporter.Prometheus.AspNetCore.csproj | Stops linking the shared PrometheusHeadersParser into the AspNetCore project. |
| src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md | Documents the improved Accept header handling change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Parse `Accept` case-insensitively. - Check `quality` is between 0 and 1.
Changes
Update the handling of the
Acceptheader to follow the requirements documented in Scrape protocol content negotiation.The behaviour isn't 100% compliant yet - it requires some tweaks to the serializer to be fully-compliant as I've not included it here and are in #7209.
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial changesChanges in public API reviewed (if applicable)