Open 08d2 opened 2 years ago
It is somewhat surprising that the metric name provided to counter constructors will have _total
appended to it automatically. It would be cool if that were better documented, and ideally only happened if the provided metric name didn't already end in _total
.
The
HELP
andTYPE
lines should usemy_counter_total
, notmy_counter
.
Not according to the OpenMetrics specification:
https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#counter-1
It is somewhat surprising that the metric name provided to counter constructors will have
_total
appended to it automatically. It would be cool if that were better documented, and ideally only happened if the provided metric name didn't already end in_total
.
Good idea. For the former, would you mind extending the documentation @08d2? For the latter, let's move the discussion to https://github.com/prometheus/client_rust/issues/52.
Not according to the OpenMetrics specification
I don't know anything about OpenMetrics, really, and I don't consider it any kind of authority. I can observe that the "_created" thing described in your link is not valid for Prometheus, though.
edit: Ah, reading more carefully....
# TYPE foo counter
foo_total 17.0
AFAIK, this is not compatible with the Prometheus exposition format. For Prometheus, the identifier after e.g. TYPE
and HELP
tokens must be the actual name of the metric. And HELP
is mandatory.
# TYPE foo_total counter
# HELP foo_total Total number of foo events.
foo_total 17.0
AFAIK, this is not compatible with the Prometheus exposition format.
Note that you need to set the right content type in order for Prometheus parse the output of a scrape in the Open Metrics format.
We have ~100 unsuffixed metrics at work and we are trying to move away from prometheus
and to switch to prometheus-client
, this automatic suffixing is an issue, we don't want to change dozens of dashboards because a library forces counter renames on us. Would you accept a PR adding a LegacyUnsuffixedCounter<A>
type to this crate?
Would you accept a PR adding a
LegacyUnsuffixedCounter<A>
type to this crate?
That sounds reasonable. I would suggest we add the above type under a feature flag (e.g. prometheus-legacy-format
).
Note that I would see this as a small helper to enable folks to migrate to the OpenMetrics format eventually. I don't see this as a long term solution for downstream users. Do you agree with that mind set @nox?
//CC @RichiH in case you have opinions.
I can understand the resistance to changing names, but also note that following best current practices earlier rather than later is usually Least Bad, long term.
The proposed seems like a partial undoing of one specific thing, not a full Prometheus exposition format 0.0.4; that's arguably the worst of both worlds globally, even though it may appear to make sense locally.
All that being said, no strong opinions on if this is implemented or not.
Note that I would see this as a small helper to enable folks to migrate to the OpenMetrics format eventually.
OpenMetrics remains an experiment, not a standard. Prometheus client libraries should default to the core exposition formats, and leave OpenMetrics as an opt-in alternative.
The problem is demonstrated by the following comment —
The
HELP
andTYPE
lines should usemy_counter_total
, notmy_counter
.