envoyproxy / envoyproxy/envoy

prometheus metric exporter ignores units

Open
#15,768 2 comments 0 reactions 0 assignees View on GitHub
area/stats help wanted
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 22h
Merged PRs (30d)
430

Description

The [current exporter](https://github.com/envoyproxy/envoy/blob/1d1b708c7bf6efa02c41d9ce22cbf1e4a1aeec2c/source/server/admin/prometheus_stats.cc#L158)
```
double bucket = supported_buckets[i];
uint64_t value = computed_buckets[i];
...
output.append(fmt::format("{0}_bucket{{{1}le=\"{2:.32g}\"}} {3}\n", prefixed_tag_extracted_name,
hist_tags, bucket, value));
```

outputs the bucket name without consideration for the units. However Prometheus has [strong conventions for metrics](https://prometheus.io/docs/practices/naming/) which require the metric to be in base units. That is a time metric should have values comprised of seconds, not milliseconds or microseconds.

This is important as it allows timeseries operations between metrics from disparate sources without normalisation transformations scattered all over the place.

Prometheus should be normalising the `sum` and `le` fields for `Microseconds` and `Milliseconds` unit histograms into `Seconds` on output in the Prometheus exporter (and I think I read that the GRPC exporter is a separate codepath, so there too).

*Repro steps*:

Request a prometheus stats page from Envoy, and an internal stats page from a microservice, and try to compare time statistics between them.

I recognise that there are backwards compatibility considerations here, so perhaps an opt-out to allow organisations without other datasources and a strong investment solely in Envoy metrics would be a good idea.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.