feat: add support for configuring metric scope labels#5123
Conversation
MikeGoldsmith
left a comment
There was a problem hiding this comment.
This looks like it's on the right path. A few things to note:
-
_SCOPE_ATTR_CONFLICT_NAMESis defined but never used — we should either use it to explicitly filter conflicting scope attributes or remove it. -
PR description looks copy-pasted from #5122 — says "Resource metrics as Metric attributes" but this is about scope info.
-
Merge conflict concern — this, #5122, #5118, and #5117 all modify
PrometheusMetricReaderand_translate_to_prometheus. This is going to conflict with them, we need to coordinate what should be done in order or we'll be bumping into each other with rebases. -
nit: the nesting depth in
_translate_to_prometheusis getting deep — might be worth extracting the per-metric logic into a helper in a follow-up.
Sorry, got a little sloppy with this PR. I originally used |
|
@MikeGoldsmith I ended up doing a pretty major refactor of the exporter logic based on your suggestion, so we will likely want to merge this PR first. |
MikeGoldsmith
left a comment
There was a problem hiding this comment.
Looks good, thanks @herin049. Need to update PR title and description.
ef6fede to
4a590f0
Compare
4a590f0 to
be41170
Compare
|
Hey, just a heads-up from the Prometheus SIG: please hold off on the merge here. There's on going discussion happening on the PR that stabilizes this part of the spec open-telemetry/opentelemetry-specification#5056 |
|
Moving to draft until the SIG confirms this is the correct path. |
…en-telemetry#5056) Fixes open-telemetry#4989 ## Changes Stabilizes the configuration option `without_scope_info`, documented in the Prometheus exporter spec. It is implemented by a few SDKs already open-telemetry#4989 (comment), an open PR for Python(open-telemetry/opentelemetry-python#5123), and an issue for DotNet(open-telemetry/opentelemetry-dotnet#7157). Both prometheus exporters in the collector have the option as well[[1](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/exporter/prometheusexporter#:~:text=without_scope_info%3A%20(default%20%3D%20false)%3A%20If%20true%2C%20metrics%20will%20be%20exported%20without%20scope%20name%2C%20version%2C%20schemaURL%2C%20and%20attributes%20encoded%20as%20labels.)][[2](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/exporter/prometheusremotewriteexporter#:~:text=disable_scope_info%20(default%20%3D%20false)%3A%20If%20true%2C%20the%20scope%20info%20labels%20(otel_scope_name%2C%20otel_scope_version%20and%20otel_scope_...%20attributes)%20will%20not%20be%20exported.)] (Although remote write exporter uses `disable_scope_info` instead of `without_scope_info`. For non-trivial changes, follow the [change proposal process](https://github.com/open-telemetry/opentelemetry-specification/blob/main/CONTRIBUTING.md#proposing-a-change). * [X] Related issues open-telemetry#4989 * [ ] Related [OTEP(s)](https://github.com/open-telemetry/oteps) # * [ ] Links to the prototypes (when adding or changing features) * [X] [`CHANGELOG.md`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/CHANGELOG.md) file updated for non-trivial changes * For trivial changes, include `[chore]` in the PR title to skip the changelog check * [ ] [Spec compliance matrix](https://github.com/open-telemetry/opentelemetry-specification/blob/main/spec-compliance-matrix/template.yaml) updated if necessary * [ ] [Declarative config data model](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/data-model.md#overview) is updated if SDK config surface is changed --------- Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
|
This PR has been automatically marked as stale because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 days of this comment. |
There was a problem hiding this comment.
Pull request overview
Adds Prometheus exporter support for including (and optionally disabling) instrumentation scope labels on exported metrics, aligning the exporter with the spec’s Scope Info guidance, and refactors the OTel→Prometheus translation pipeline to be more modular.
Changes:
- Add scope label emission (
otel_scope_name,otel_scope_version,otel_scope_schema_url, andotel_scope_*scope attributes) with a toggle (scope_info_enabled). - Refactor metric translation into helper functions for family creation/population and data-point collection.
- Add/adjust unit tests and update Prometheus exporter documentation and changelog entry.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py | Adds tests for default/disabled scope labels and updates existing tests to account for new default behavior. |
| exporter/opentelemetry-exporter-prometheus/src/opentelemetry/exporter/prometheus/init.py | Implements scope label handling and refactors translation logic into smaller helpers. |
| docs/exporter/prometheus/prometheus.rst | Documents scope label behavior and configuration. |
| .changelog/5123.added | Notes the new Prometheus exporter feature. |
Comments suppressed due to low confidence (1)
exporter/opentelemetry-exporter-prometheus/src/opentelemetry/exporter/prometheus/init.py:288
- This PR adds
scope_info_enabledonPrometheusMetricReader, but the SDK's config-based Prometheus factory still logs thatwithout_scope_infois unsupported and ignores it (opentelemetry-sdk/src/opentelemetry/sdk/_configuration/_meter_provider.py, around_create_prometheus_metric_reader). That means "configuring scope labels" won't work for users relying on SDK configuration / opentelemetry-instrument unless that path is updated (or an alias parameter is supported).
def __init__(
self,
disable_target_info: bool = False,
prefix: str = "",
scope_info_enabled: bool = True,
*,
registry: CollectorRegistry = REGISTRY,
) -> None:
super().__init__(
preferred_temporality={
Counter: AggregationTemporality.CUMULATIVE,
UpDownCounter: AggregationTemporality.CUMULATIVE,
HistogramInstrument: AggregationTemporality.CUMULATIVE,
ObservableCounter: AggregationTemporality.CUMULATIVE,
ObservableUpDownCounter: AggregationTemporality.CUMULATIVE,
ObservableGauge: AggregationTemporality.CUMULATIVE,
},
otel_component_type=OtelComponentTypeValues.PROMETHEUS_HTTP_TEXT_METRIC_EXPORTER,
)
self._collector = _CustomCollector(
disable_target_info=disable_target_info,
prefix=prefix,
scope_info_enabled=scope_info_enabled,
)
self._registry = registry
self._registry.register(self._collector)
self._collector._callback = self.collect
self._prefix = prefix
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Adds support for configuring scope labels on the Prometheus exporter in accordance with the Prometheus spec The default behavior is to include scope labels in exported metrics.
Furthermore this PR includes changes to refactor the conversion of OTel metrics to Prometheus metrics.
Fixes #5112
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Does This PR Require a Contrib Repo Change?
Checklist: