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

OM 2.0: Consider relaxing the requirement for the unit to be a suffix of the metric name #286

Open
dashpole opened this issue Dec 11, 2024 · 3 comments

Comments

@dashpole
Copy link
Contributor

As described in https://docs.google.com/document/d/1skZTeiY4Lvu9ouCHQ8SCKMC5wCk7iG2VBSWR88yY3AI/edit?tab=t.0#heading=h.4mz5gmed52o5, it would be good to allow the original OpenTelemetry names unmodified when translating to OpenMetrics. This is related to #282.

Concretely, I would like to change the unit specification from MUST to SHOULD:

If [the unit] non-empty, it MUST be a suffix of the MetricFamily name separated by an underscore.

@fstab
Copy link
Member

fstab commented Dec 11, 2024

This is what we decided at the Prometheus dev summit on 2024-09-13, so yes, we should do this. Thanks @dashpole for opening an issue to follow up on this.

However, as maintainer of the Prometheus Java client library, I think we should clearly define how the naming scheme is negotiated. For example, let's say you create a counter like this:

Counter.builder()
    .name("process.cpu.time")
    .help("Total CPU seconds broken down by different states.")
    .unit(Unit.SECONDS)
    .register();

Current Behavior

If you expose that counter using the OpenTelemetry exporter, it will be named process.cpu.time with at unit s. This is because we want to enable users to implement OTel's semantic conventions with the Prometheus client libraries.

If you scrape the same metric with an Accept: application/openmetrics-text; version=1.0.0 header, the client library exposes it as process_cpu_time_seconds_total, because everything else would not be valid OpenMetrics 1.0.0.

UTF-8 Support

The new UTF-8 support we'll add an escaping=... modifier to the Accept: header. This is currently work in progress, see #922. My understanding is that if you scrape the metric above with Accept: application/openmetrics-text; version=1.0.0; escaping=allow-utf-8 the result would be {"process.cpu.time_seconds_total"}.

Making _seconds and _total optional

Now, if we make units and the _total suffix optional, how would the client library decide whether to strip the suffixes or not? Will we just strip them whenever application/openmetrics-text; version=2.0.0 is requested?

Please don't say "we use whatever name the user passes when creating the metric"

The first answer that comes to mind: For OpenMetrics 2.0.0, we simply expose the name that the user provided when creating the metric. However, I fear it's not so simple. The Prometheus Java client library is in most cases not used directly, it's used by 3rd party frameworks for exposing Prometheus format (like Micrometer, or the OTel Java SDK), and by bridges for converting metrics from other formats to Prometheus (jmx_exporter, Dropwizard metrics, ...). In most cases the user has little control over what suffixes are used when a metric name is passed to the Prometheus Java client library. I think the only way to do this without breaking too much is to decide the suffixes at scrape time, or maybe via configuration like a standard environment variable.

@dashpole
Copy link
Contributor Author

Thanks @fstab. I agree we need a better mechanism for configuring suffixes if we relax this. Given we've used content negotiation for UTF-8 support, it seems nice to have both "name changing" configurations work similarly. Ideally, unit suffix negotiation is mostly orthogonal to the protocol with exceptions where required by the protocol (OM 1.0) or to preserve backwards-compatibility (text exposition).

@fstab
Copy link
Member

fstab commented Dec 20, 2024

I started a design doc for the client library aspect of this. Feel free to edit / comment.

https://docs.google.com/document/d/1VbqxXmtZmU9MWvK936uf80ulaR7X9xU_PjrR05avKO8/edit?usp=sharing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

2 participants