Repository navigation
Conversation
…flag Signed-off-by: Raphael Bizos <r.bizos@criteo.com>
Signed-off-by: Raphael Bizos <r.bizos@criteo.com>
| def generate_latest(registry, escaping=UNDERSCORES, version="1.0.0"): | ||
| '''Returns the metrics from the registry in latest text format as a string.''' | ||
| if _openmetrics_2_enabled and parse_version(version) == (2, 0, 0): | ||
| return _generate_latest_2_0(registry, escaping) |
There was a problem hiding this comment.
OpenMetrics 2.0 natively supports UTF-8 names. Should generate_latest (and choose_encoder once 2.0 is enabled) default escaping to ALLOWUTF8 when version is 2.0.0 and no explicit escaping is passed?
There was a problem hiding this comment.
Why not, this would need to be documented as breaking somehow as without changing the escaping param this could break alerts/dashboards. I do like the idea though, maybe on a separate change ?
There was a problem hiding this comment.
This made me think of another similar issue, on OM2. From https://prometheus.io/docs/guides/open_metrics_2_0_migration/#counter-suffix-rules
In OM 2.0, counter Metric names should end in _total. It is no longer mandatory.
Currently it gets added automatically and the users have no control. The way it works is that the suffix is stripped before hand and added while serializing the metrics. This would require to be reworked totally to allow for user choice. let me add that to the issue and do that separately from the implem of the actual metrics :)
| escaped_name = escape_metric_name(family_name, escaping) | ||
| output = [f'# TYPE {escaped_name} {metric.type}\n'] | ||
| if metric.unit: | ||
| output.append(f'# UNIT {escaped_name} {metric.unit}\n') |
There was a problem hiding this comment.
In the OpenMetrics 2.0 ABNF (# UNIT <name> *metricname-char LF), units still only allow [a-zA-Z0-9_:] characters even when UTF-8 metric names are enabled. Because Metric.__init__ validates unit via _validate_metric_name(name) (which allows arbitrary UTF-8 by default), something like Gauge('foo', 'help', unit='req/s') succeeds at construction time and emits # UNIT "foo_req/s" req/s here. Should we validate or escape metric.unit so it conforms to *metricname-char?
|
|
||
| def _format_sample_2_0(family_name, sample, escaping): | ||
| """Formats a sample as ``name{labels} value [timestamp]``, named after its family as OpenMetrics 2.0 requires.""" | ||
| line = f'{_format_name_and_labels_2_0(family_name, sample.labels, escaping)} {floatToGoString(sample.value)}' |
There was a problem hiding this comment.
If a custom collector constructs a Metric and adds a sample whose sample.name does not match family_name, this will silently rewrite the sample name to family_name. Should we validate that sample.name == family_name and raise a ValueError on mismatch?
There was a problem hiding this comment.
That's a good point, here I'm a bit greedy as I force it to the family_name so that its always a OM2 valid output. I'm very hesitant to failing the full scrape for a sample name mismatching.
Part of #1210.
Preparing for the OpenMetrics 2.0 exposition by making room for a separate exposition function.
Until every metric type is supported, version 2.0.0 is still served by the 1.0 writer, so nothing
changes for users.
The 2.0 exposition supports only
gauge,infoandunknownmetric types.Follow-ups
chosen by content negotiation alone. The current native histogram implementation will be moved to OM2.0 exposition branch.
@csmarchbanks