[Exporter.Prometheus] Fix scope metadata#7237
Conversation
- Emit OpenMetrics scope metadata as a single `otel_scope` metric family with `otel_scope_info` samples instead of repeating metadata for every scope. - Include instrumentation scope metadata on samples using `otel_scope_*` labels, including scope version, schema URL, and prefixed scope attributes. - Drop conflicting scope attributes named `name`, `version`, and `schema_url` to avoid collisions with generated scope labels.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7237 +/- ##
==========================================
- Coverage 90.04% 90.03% -0.02%
==========================================
Files 276 276
Lines 14271 14283 +12
==========================================
+ Hits 12850 12859 +9
- Misses 1421 1424 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Remove Go theme for .NET.
Add CHANGELOG entries.
There was a problem hiding this comment.
Pull request overview
This PR updates the Prometheus/OpenMetrics exporter serialization to be compliant with OpenMetrics/Prometheus scope metadata expectations by emitting scope metadata once per scrape and attaching instrumentation scope details to metric samples.
Changes:
- Emit OpenMetrics scope metadata as a single
otel_scopemetric family (withotel_scope_infosamples) instead of repeating TYPE/HELP per scope. - Add instrumentation scope metadata onto metric samples using
otel_scope_*labels (version, schema URL, and prefixed scope attributes). - Update/extend unit + integration tests and exporter changelogs to reflect the new output.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusSerializerTests.cs | Updates serializer expectations and adds tests for scope-label prefixing and reserved-name dropping. |
| test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs | Updates integration expected output for new scope metric family + prefixed scope attributes. |
| test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusCollectionManagerTests.cs | Adds coverage ensuring OpenMetrics scope metadata is written once as a single metric family. |
| test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs | Updates middleware integration expectations for new scope metric family + prefixed scope attributes. |
| src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusSerializerExt.cs | Adjusts histogram bucket serialization to account for WriteTags no longer leaving a trailing comma. |
| src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusSerializer.cs | Implements scope-metadata family split (metadata vs samples) and writes scope labels (version/schema/tags) with otel_scope_ prefix. |
| src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusCollectionManager.cs | Emits scope metadata once and deduplicates scope identities across metrics during OpenMetrics scrapes. |
| src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md | Documents the scope-metadata and scope-label behavior changes. |
| src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md | Documents the scope-metadata and scope-label behavior changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review feedback.
Add more test coverage for patch.
|
From Codex: src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusSerializer.cs:664 src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusCollectionManager.cs:264 |
- Fix missing handling for post-normalization collisions. - Fix metadata conflict handling being bypassed in some cases.
|
Both comments addressed. |
|
2 new codex findings: src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusSerializer.cs:441 |
Use canonical values for labels that are `float` or `double` to resolve TODO.
- Ensure `otel_scope_info` is sanitized. - Fix missing collision handling.
| [Theory] | ||
| [InlineData(false)] | ||
| [InlineData(true)] | ||
| public void WriteMetricConcatenatesPointTagsThatCollideWithScopeLabels(bool useOpenMetrics) |
There was a problem hiding this comment.
One more findings against spec complianace:
src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusSerializer.cs:725
CreateMetricPointLabels now concatenates point-tag collisions with existing scope labels, but it always appends the point value after the existing value. The OTel Prometheus compatibility spec requires colliding values to be ordered by the lexicographical order of the original keys. This works for the current test case, but fails for cases like scope attribute z="scope" plus point attribute otel_scope_z="point": both serialize to otel_scope_z, and the required order is point;scope, while this code emits scope;point.
This test could be changed to something like
[Theory]
[InlineData(false, "library.mascot", "dotnetbot", "otel_scope_library_mascot", "otter", "otel_scope_library_mascot", "dotnetbot;otter")]
[InlineData(true, "library.mascot", "dotnetbot", "otel_scope_library_mascot", "otter", "otel_scope_library_mascot", "dotnetbot;otter")]
[InlineData(false, "z", "scope", "otel_scope_z", "point", "otel_scope_z", "point;scope")]
[InlineData(true, "z", "scope", "otel_scope_z", "point", "otel_scope_z", "point;scope")]
public void WriteMetricConcatenatesPointTagsThatCollideWithScopeLabels(
bool useOpenMetrics,
string scopeTagKey,
string scopeTagValue,
string pointTagKey,
string pointTagValue,
string expectedLabelKey,
string expectedLabelValue)
{
There was a problem hiding this comment.
Should be addressed now.
# Conflicts: # src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md # src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md # src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusSerializer.cs
Fix SA1203 warning.
Update PrometheusSerializer so scope-tag collisions with point tags now preserve the scope tag’s raw original key through the final merge, which fixes lexicographic value ordering for cases like `z` vs. `otel_scope_z`.
Remove synthetic OpenMetrics `otel_scope`/`otel_scope_info` metadata family.
|
|
||
| #if !NET | ||
| #if NET | ||
| private static readonly ImmutableHashSet<string> ReservedScopeLabelNames = ["otel_scope_name", "otel_scope_schema_url", "otel_scope_version"]; |
There was a problem hiding this comment.
FrozenSet should be more performant
There was a problem hiding this comment.
edit: alternatively, all these 3 items could be made a cosnt, and the comparision could be done by swithc/case/==. Depends on the level of optimization you need
There was a problem hiding this comment.
Added where available.
Kielek
left a comment
There was a problem hiding this comment.
LGTM with some minor comment. Auto merge disabled dur to this potential change.
Use `FrozenSet<T>` for .NET 9+.
Changes
Scope-related fixes for OpenMetrics and Prometheus specification compliance.
otel_scope_*labels, including scope version, schema URL, and prefixed scope attributes.otel_scope_infoscope metadata metric.name,version, andschema_urlto avoid collisions with generated scope labels.Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial changesChanges in public API reviewed (if applicable)