Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle conflicting metric descriptions and types in prometheus exporter #2869

Closed
jack-berg opened this issue Oct 11, 2022 · 2 comments · Fixed by #2890
Closed

Handle conflicting metric descriptions and types in prometheus exporter #2869

jack-berg opened this issue Oct 11, 2022 · 2 comments · Fixed by #2890
Assignees
Labels
area:sdk Related to the SDK [label deprecated] triaged-accepted [label deprecated] Issue triaged and accepted by OTel community, can proceed with creating a PR spec:metrics Related to the specification/metrics directory

Comments

@jack-berg
Copy link
Member

With #2703 instrumentation scope name and version are included as attributes in the prometheus exporter, which helps to accommodate OpenTelemetry metric's ability to have instruments with the same name under different scopes.

However, these instruments can still have conflicting types and descriptions, which present problems in prometheus scrapers as discussed in open-telemetry/opentelemetry-java#4834.

The following scenario illustrates the problem:

  • Scope name: scope1, instrument name: my_instrument, instrument type: counter, description: description1
  • Scope name: scope2, instrument name: my_instrument, instrument type: gauge, description: description2

As of #2703, this will be serialized in the prometheus exporter as follows:

# HELP my_instrument description1
# TYPE my_instrument counter
my_instrument{otel_scope_name="scope1"} 1
# HELP my_instrument description2
# TYPE my_instrument gauge
my_instrument{otel_scope_name="scope2"} 1

One possible solution is to detect conflicting descriptions / types in the prometheus exporter and log an error and / or skip the offending metric. Views can be used to change the name of one of the offending instruments to avoid the conflict.

@jack-berg jack-berg added the spec:metrics Related to the specification/metrics directory label Oct 11, 2022
@dashpole
Copy link
Contributor

+1

OpenMetrics MetricFamily names (i.e. what type, description, unit, are associated with) MUST be unique within the metrics served on an endpoint:

Each MetricFamily name MUST be unique.

@rbailey7210 rbailey7210 added the [label deprecated] triaged-accepted [label deprecated] Issue triaged and accepted by OTel community, can proceed with creating a PR label Oct 14, 2022
@reyang reyang added the area:sdk Related to the SDK label Oct 14, 2022
@reyang
Copy link
Member

reyang commented Oct 14, 2022

Here is what I'm thinking:

  1. This is not a unique problem in the exporter, we also have the same challenge when the Collector is trying to convert OTLP to Prometheus, and this is something @dashpole was trying to cover here
  2. For exporter, one approach is to make sure SDK -> PrometheusExporter -> Prometheus and SDK -> OTLP Exporter -> Collector -> PromethuesExporter -> Prometheus will end up having the same result, which sounds nice but I don't feel it is necessary.
  3. The alternative approach is to be a bit conservative and only provide a subset of features, one extreme case is "if the same name is used by multiple streams, the behavior is undefined (it could cause Prometheus to completely drop the entire scraped results), and the expectation is that the user will use View API to rename things properly to avoid conflicts".

I would vote for something simple/conservative, rather than trying to introduce a perfect escape rule/sequence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:sdk Related to the SDK [label deprecated] triaged-accepted [label deprecated] Issue triaged and accepted by OTel community, can proceed with creating a PR spec:metrics Related to the specification/metrics directory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants