feat(om1.1): relax the acceptance rules around unit and total suffixes #2745
Conversation
There was a problem hiding this comment.
Look good, added some suggestions.
Generally, to be transparent, this is a future compatibility ingestor breaking change guarded by the content negotiation that we decided (OM 2.0 WG) to proceed with this, because many implementations already do this (and some cases require for OM to be useful).
@krajorama can we add following statement to the commit/PR for clarity (if you agree with the content)
|
|
||
| The name of a MetricFamily MUST NOT result in a potential clash for sample metric names as per the ABNF with another MetricFamily in the Text Format within a MetricSet. An example would be a gauge called "foo_created" as a counter called "foo" could create a "foo_created" in the text format. | ||
|
|
||
| Ingestors MAY accept conflicting MetricFamily names, provided that there is a mechanism to distinguish the resulting MetricPoints. It follows that ingestors MAY accept MetricFamily name without the type specific suffix. |
There was a problem hiding this comment.
| Ingestors MAY accept conflicting MetricFamily names, provided that there is a mechanism to distinguish the resulting MetricPoints. It follows that ingestors MAY accept MetricFamily name without the type specific suffix. | |
| Ingestors MAY accept conflicting MetricFamily names, especially resulting from the MetricFamily names without the type or unit specific suffix (see [MetricFamily metadata](#MetricFamily-metadata)). |
|
|
||
| MetricFamily names are a string and MUST be unique within a MetricSet. Names SHOULD be in snake_case. Metric names MUST follow the restrictions in the ABNF section. | ||
|
|
||
| Ingestors MAY accept duplicate MetricFamily names, provided that either of HELP, TYPE, or UNIT is unique within the duplicated MetricFamily names. Notably Exposers MAY duplicate MetricFamilies when aggregating external metrics that have the same MetricFamily name, but come from different sources and have either different HELP, or TYPE, or UNIT. Be aware that exposing such metrics directly to end-users reduces usability due to confusion about which Metric belongs to which MetricFamily. |
There was a problem hiding this comment.
IMO we can still require metric family names to be unique within the text exposition format while being more tolerant of other naming. It properly places the burden of ensuring unique metric family names on the exposers that choose to deviate from naming recommendations (e.g. OTel SDKs), and doesn't impose an additional burden on Ingestors.
There was a problem hiding this comment.
Would you say that's the OpenTelemetry use case? To put the burden on exposer? Would that be enough for OM 1.1?
Tolerating non-unique metric family names comes from the federation, push-gateway and similar use cases. We could discard these use cases in OM 1.1 and only discuss for OM 2.0. I'm not opposed to that.
There was a problem hiding this comment.
Yes, that would meet the OpenTelemetry use-case. IMO non-unique metric family names seems more disruptive than other changes we've been discussing, so I would prefer punting that to OM 2.0. But I don't have a good sense of how important the federation, push-gateway use-cases are, so i'm open to other opinions.
There was a problem hiding this comment.
Not allowing federation/push-gateway isn't a regression, it has always been the case. I'll remove these additions from the PR.
This proposal only relaxes the rules on what is acceptable by ingestors. This allows non Prometheus systems to expose names without suffixes and still get processed. Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
krajorama
force-pushed
the
krajo/om1.1-relax-suffix-alt1
branch
from
315b71d to
5e762e6
Compare
|
On further reflection, making the relaxed rule only apply to the ingestor is a bad idea as that would mean that different ingestors would treat the same text differently . Meaning that for example if a developer exposed a metric without the _total suffix and tested it with a lenient ingestor , they would not notice that it will fail for others. Superseded by #2750 |
This proposal only relaxes the rules on what is acceptable by ingestors. This allows non Prometheus systems to expose names without suffixes and still get processed.
The reason is that Prometheus strongly recommends using the
_totaland unit suffixes for usability and to avoid conflicting metric names.Generally, to be transparent, this is a future compatibility ingestor breaking change guarded by the content negotiation that we decided (OM 2.0 WG) to proceed with this, because many implementations already do this (and some cases require for OM to be useful).