open-telemetry / opentelemetry-collector-contrib

Contrib repository for the OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
2.91k stars 2.27k forks source link

[exporter/prometheusremotewrite] Support for delta Exponential Histograms #22806

Open mx-psi opened 1 year ago

mx-psi commented 1 year ago

Component(s)

exporter/prometheusremotewrite

Is your feature request related to a problem? Please describe.

This issue is similar to #19153; currently the Prometheus Remote Write exporter only supports cumulative exponential histograms and drops delta exponential histograms (see #17370 and the related spec guidance added on open-telemetry/opentelemetry-specification/pull/3079).

Describe the solution you'd like

Delta exponential histograms are accumulated into a cumulative exponential histogram and then the cumulative exponential histogram to Prometheus native histogram conversion logic is applied. To merge the exponential histograms, the lowest scale among them should be used.

Describe alternatives you've considered

We could have this functionality on a separate processor. The proposed solution for #19153 seems to add functionality in the exporter itself, so I went with that, but I also see advantages on having a separate processor for this.

Additional context

This needs a change in the specification, since it currently says:

OpenTelemetry Exponential Histogram metrics with the delta aggregation temporality are dropped.

It would have to be changed to say

OpenTelemetry Exponential Histograms with Delta aggregation temporality SHOULD be aggregated into a Cumulative aggregation temporality and follow the logic above, or MUST be dropped.

similar to what we say for OpenTelemetry Histograms with Delta aggregation temporality.


This issue is specific to the PRW exporter since the Prometheus exporter does not support exponential histograms yet, but I tagged it with exporter/prometheus since we should take this exporter also into account.

github-actions[bot] commented 1 year ago

Pinging code owners for exporter/prometheus: @Aneurysm9. See Adding Labels via Comments if you do not have permissions to add labels yourself.

github-actions[bot] commented 1 year ago

Pinging code owners for exporter/prometheusremotewrite: @Aneurysm9 @rapphil. See Adding Labels via Comments if you do not have permissions to add labels yourself.

github-actions[bot] commented 1 year ago

Pinging code owners for pkg/translator/prometheusremotewrite: @Aneurysm9 @kovrus. See Adding Labels via Comments if you do not have permissions to add labels yourself.

github-actions[bot] commented 1 year ago

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

MovieStoreGuy commented 1 year ago

@mx-psi do you know if anyone is interested on working on this?

mx-psi commented 1 year ago

@mx-psi do you know if anyone is interested on working on this?

No, I opened this on behalf of a customer, mostly to gauge interest and see if other folks were interested. I am not planning to work on this at the moment

github-actions[bot] commented 1 year ago

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

github-actions[bot] commented 10 months ago

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

github-actions[bot] commented 8 months ago

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

jakegut commented 1 month ago

Rather than converting from delta to cumulative would it be possible to send them as gauge histograms instead?

Looking at the original design doc and the protobuf spec it seems like gauge histograms are just delta histograms.