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

Support full metric name normalisation in otelcol.exporter.prometheus #436

Open
ptodev opened this issue Jun 6, 2023 · 3 comments
Open
Labels
enhancement New feature or request

Comments

@ptodev
Copy link
Contributor

ptodev commented Jun 6, 2023

Request

Prometheus and Otlp have different conventions and requirements for metric names and labels. Currently, otelcol.exporter.prometheus only does the bare minimum to make names Prometheus compliant (simple normalisation). However, Otel Collector was recently updated to do a full normalisation by default.

Most users will expect the Agent's otelcol.exporter.prometheus component to work in the same way as the Collector's prometheus exporter. Hence it'd be good to bring the behaviour in line with the Collector by doing full normalisation by default and letting the users to opt out of it if they prefer.

The bits of code which currently do the conversion are the calls to prometheus.BuildPromCompliantName here:
component/otelcol/exporter/prometheus/internal/convert/convert.go

In order to resolve this, we need to somehow enable the pkg.translator.prometheus.NormalizeName feature gate. There are two ways to enable it:

  • Upgrade to a version of the Prometheus translator where the feature gate is in beta (which means it's enabled by default)
  • Keep using the old version of the translator as we do now, but enable the feature gate in the code. Also we should write tests that will fail once we upgrade the translator so we remember to switch off the gate.

The big question we need to answer is whether this should be configurable. If it's configurable, then the config entry will apply for all instances of otelcol.exporter.prometheus, and potentially also otelcol.receiver.prometheus. The configuration would have to go in the root of the agent config file (like the logging block), or it should be a command line argument.

Either way this would be a breaking change, since at the very least the default behaviour will change.

Also check if this change impacts otelcol.receiver.prometheus.

Use case

The Agent's behaviour will be more in line with the Collector's. At the moment sometimes different pipelines produce different metrics.

@ptodev ptodev added the enhancement New feature or request label Jun 6, 2023
@ptodev ptodev changed the title Support full metric normalisation in otelcol.exporter.prometheus Support full metric name normalisation in otelcol.exporter.prometheus Jun 6, 2023
@gouthamve
Copy link
Member

Hi, looks like the Collector is rolling back enabling the gate by default. So it reduces urgency but we should try to support both normalisations in config if possible.

open-telemetry/opentelemetry-collector-contrib#23229

@zeitlinger
Copy link
Member

support both

that's fine as long as we align on the standard config - so that we know which metric names to use in the app olly plugin and our recommended dashboards.

@ptodev
Copy link
Contributor Author

ptodev commented Jun 12, 2023

Hi @gouthamve, how does the priority of this item compare to other otel items, such as these:

I'm asking just to make sure we work on the more important things first.

@rfratto rfratto transferred this issue from grafana/agent Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Development

No branches or pull requests

3 participants