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

Inconsistencies between PrometheusNaming.sanitizeMetricName() methods #974

Closed
mimaison opened this issue Jul 30, 2024 · 0 comments · Fixed by #975
Closed

Inconsistencies between PrometheusNaming.sanitizeMetricName() methods #974

mimaison opened this issue Jul 30, 2024 · 0 comments · Fixed by #975
Assignees

Comments

@mimaison
Copy link
Contributor

We have 2 methods to sanitize metric name:

These behave slightly differently and I think both are incorrect:

1. Handling of colons

String metricName = "some.metric:name";
System.out.println(PrometheusNaming.sanitizeMetricName(metricName));
System.out.println(PrometheusNaming.sanitizeMetricName(metricName, Unit.SECONDS));

This prints:

some.metric:name
some.metric_name_seconds

This is because the variant with the unit argument calls sanitizeLabelName() on the metric name before appending the unit suffix instead of calling sanitizeMetricName(): https://github.com/prometheus/client_java/blob/main/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/PrometheusNaming.java#L160C25-L160C42

While colons are allowed in Prometheus metric names (https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels), they should only be used in recording rules. So sanitizeMetricName(String metricName) should replace colons with underscores.

2. Handling of underscore prefix

String metricName = "__some_metric_name";
System.out.println(PrometheusNaming.sanitizeMetricName(metricName));
System.out.println(PrometheusNaming.sanitizeMetricName(metricName, Unit.SECONDS));

This prints:

__some_metric_name
_some_metric_name_seconds

Metric names should be allowed to start with __, this is only disallowed for labels. So sanitizeMetricName(String metricName, Unit unit) should not call sanitizeLabelName() and allow metric names starting with __.

jonatan-ivanov added a commit to micrometer-metrics/micrometer that referenced this issue Nov 7, 2024
The Prometheus Java Client used to let users to use colons (":")
in the metric names. Apparently, this should have not been allowed.

See the docs: https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels
"Metric names may contain ASCII letters, digits, underscores,
and colons. It must match the regex [a-zA-Z_:][a-zA-Z0-9_:]*.
Note: The colons are reserved for user defined recording rules.
They should not be used by exporters or direct instrumentation."

So even if metric names can contain colons (":"),
exporters (like the Prometheus Java Client or Micrometer)
should not use it. The Prometheus Java Client replaces colons (":")
to underscores ("_) starting from 1.3.3.

See prometheus/client_java#974
See prometheus/client_java#975
jonatan-ivanov added a commit to micrometer-metrics/micrometer that referenced this issue Nov 7, 2024
The Prometheus Java Client used to let users to use colons (":")
in the metric names. Apparently, this should have not been allowed.

See the docs: https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels
"Metric names may contain ASCII letters, digits, underscores,
and colons. It must match the regex [a-zA-Z_:][a-zA-Z0-9_:]*.
Note: The colons are reserved for user defined recording rules.
They should not be used by exporters or direct instrumentation."

So even if metric names can contain colons (":"),
exporters (like the Prometheus Java Client or Micrometer)
should not use it. The Prometheus Java Client replaces colons (":")
to underscores ("_") starting from 1.3.3.

See prometheus/client_java#974
See prometheus/client_java#975
jonatan-ivanov added a commit to micrometer-metrics/micrometer that referenced this issue Nov 7, 2024
The Prometheus Java Client used to let users to use colons (":")
in the metric names. Apparently, this should have not been allowed.

See the docs: https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels
"Metric names may contain ASCII letters, digits, underscores,
and colons. It must match the regex [a-zA-Z_:][a-zA-Z0-9_:]*.
Note: The colons are reserved for user defined recording rules.
They should not be used by exporters or direct instrumentation."

So even if metric names can contain colons (":"),
exporters (like the Prometheus Java Client or Micrometer)
should not use it. The Prometheus Java Client replaces colons (":")
to underscores ("_") starting from 1.3.3.

See prometheus/client_java#974
See prometheus/client_java#975
See #5649
jonatan-ivanov added a commit to micrometer-metrics/micrometer that referenced this issue Nov 7, 2024
The Prometheus Java Client used to let users to use colons (":")
in the metric names. Apparently, this should have not been allowed.

See the docs: https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels
"Metric names may contain ASCII letters, digits, underscores,
and colons. It must match the regex [a-zA-Z_:][a-zA-Z0-9_:]*.
Note: The colons are reserved for user defined recording rules.
They should not be used by exporters or direct instrumentation."

So even if metric names can contain colons (":"),
exporters (like the Prometheus Java Client or Micrometer)
should not use it. The Prometheus Java Client replaces colons (":")
to underscores ("_") starting from 1.3.3.

See prometheus/client_java#974
See prometheus/client_java#975
See #5649
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants