Skip to content

[Exporter.Prometheus] Use dictionary to key views#7371

Open
martincostello wants to merge 8 commits into
open-telemetry:mainfrom
martincostello:gh-7156-refactor-serializer-reduce-fields
Open

[Exporter.Prometheus] Use dictionary to key views#7371
martincostello wants to merge 8 commits into
open-telemetry:mainfrom
martincostello:gh-7156-refactor-serializer-reduce-fields

Conversation

@martincostello
Copy link
Copy Markdown
Member

@martincostello martincostello commented Jun 5, 2026

Changes

Changes are about ~2% regression for the scrape endpoint, but that's still massively improved compared to before #7279.

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

- Instead of maintaining separate fields for reviews and timestamps for scrape responses, key on the `PrometheusProtocol` to support multiple formats and simplify passing options that affect the serializer around.
- Rename version properties for consistency.
Consolidate the buffer and view into a single state object, rather than storing across four dictionaries.
@github-actions github-actions Bot added pkg:OpenTelemetry.Exporter.Prometheus.AspNetCore Issues related to OpenTelemetry.Exporter.Prometheus.AspNetCore NuGet package pkg:OpenTelemetry.Exporter.Prometheus.HttpListener Issues related to OpenTelemetry.Exporter.Prometheus.HttpListener NuGet package labels Jun 5, 2026
Remove conditional `using`.
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 5, 2026

Codecov Report

❌ Patch coverage is 88.46154% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.89%. Comparing base (f159dfe) to head (1ac912f).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ner/Internal/Shared/PrometheusCollectionManager.cs 87.98% 25 Missing ⚠️
...HttpListener/Internal/Shared/PrometheusProtocol.cs 88.88% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7371      +/-   ##
==========================================
- Coverage   90.38%   89.89%   -0.50%     
==========================================
  Files         284      276       -8     
  Lines       15429    14521     -908     
==========================================
- Hits        13946    13054     -892     
+ Misses       1483     1467      -16     
Flag Coverage Δ
unittests-Project-Experimental 89.80% <87.17%> (-0.12%) ⬇️
unittests-Project-Stable 89.79% <87.17%> (-0.09%) ⬇️
unittests-Solution 89.86% <88.46%> (-0.08%) ⬇️
unittests-UnstableCoreLibraries-Experimental 49.46% <87.17%> (+0.43%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...metheus.AspNetCore/PrometheusExporterMiddleware.cs 100.00% <100.00%> (ø)
...s.HttpListener/Internal/PrometheusHeadersParser.cs 98.85% <100.00%> (ø)
...HttpListener/Internal/Shared/PrometheusExporter.cs 95.23% <ø> (-0.22%) ⬇️
....Prometheus.HttpListener/PrometheusHttpListener.cs 88.67% <100.00%> (ø)
...HttpListener/Internal/Shared/PrometheusProtocol.cs 96.00% <88.88%> (-4.00%) ⬇️
...ner/Internal/Shared/PrometheusCollectionManager.cs 90.80% <87.98%> (-2.59%) ⬇️

... and 10 files with indirect coverage changes

Ensure that the response received is for the same protocol the collection is for.
Add tests for `PrometheusProtocol`'s `IEquatable<PrometheusProtocol>` implementation.
Refactor collection to batch multiple protocol writes within the same lock to avoid queuing to serialize different protocols.
Fix deadlock during concurrent collections found in CI.
Fix test failing in CI.
@martincostello martincostello marked this pull request as ready for review June 5, 2026 20:24
@martincostello martincostello requested a review from a team as a code owner June 5, 2026 20:24
Copilot AI review requested due to automatic review settings June 5, 2026 20:24
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the Prometheus exporter scrape response caching/buffering so state is keyed by a richer PrometheusProtocol (media type, escaping, version, etc.), enabling concurrent support for multiple output formats and simplifying future option propagation.

Changes:

  • Refactors PrometheusCollectionManager to maintain per-protocol state in a dictionary keyed by PrometheusProtocol, returning a single View per negotiated protocol.
  • Extends PrometheusProtocol with IEquatable, GetHashCode, ToString, and adds debug-time validation to constrain key cardinality.
  • Updates HttpListener + ASP.NET Core endpoints and test suites to use the protocol-based collection APIs; adds new protocol-focused tests and links them into the ASP.NET Core test project.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusProtocolTests.cs Adds unit tests for PrometheusProtocol equality/hash behavior and dictionary-key usage.
test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusCollectionManagerTests.cs Updates tests to pass/track PrometheusProtocol and adds coverage for cross-protocol shared collection behavior.
test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests.csproj Links the new protocol test file into the ASP.NET Core test project.
src/Shared/Shims/Lock.cs Simplifies the Lock shim declaration used for pre-.NET9 locking compatibility.
src/OpenTelemetry.Exporter.Prometheus.HttpListener/PrometheusHttpListener.cs Switches scrape path to call EnterCollect(protocol) and write collectionResponse.View.
src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/Shared/PrometheusProtocol.cs Adds IEquatable, hashing, debug validation, and renames version constants to V{n}.
src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/Shared/PrometheusExporter.cs Removes OpenMetricsRequested mutable state in favor of protocol-driven collection.
src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/Shared/PrometheusCollectionManager.cs Major refactor: protocol-keyed per-format state, shared active collection across protocols, and new response/result plumbing.
src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusHeadersParser.cs Updates version constant names and adds protocol.Validate() after negotiation.
src/OpenTelemetry.Exporter.Prometheus.AspNetCore/PrometheusExporterMiddleware.cs Switches scrape path to EnterCollect(protocol) and validates protocol in parsing.
src/OpenTelemetry.Exporter.Prometheus.AspNetCore/OpenTelemetry.Exporter.Prometheus.AspNetCore.csproj Links Lock shim into the ASP.NET Core project to support shared code using Lock.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Shared/Shims/Lock.cs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg:OpenTelemetry.Exporter.Prometheus.AspNetCore Issues related to OpenTelemetry.Exporter.Prometheus.AspNetCore NuGet package pkg:OpenTelemetry.Exporter.Prometheus.HttpListener Issues related to OpenTelemetry.Exporter.Prometheus.HttpListener NuGet package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants