Add primitive support for ingesting OTLP delta metrics as-is #16360

fionaliao · 2025-03-31T15:38:41Z

This PR adds primitive support for otlp delta metrics. This allows otlp metrics with delta temporality to be ingested and stored as-is, with metric type unknown. To get "increase" or "rate", sum_over_time(metric[<interval>]) (/ <interval>) can be used.

This is the first step towards implementing prometheus/proposals#48. That proposal has additional suggestions around type-aware functions and making the rate() and increase() functions work for deltas too. However, there are some questions around the best way to do querying over deltas, so having this simple implementation without changing any PromQL functions allow us to get some form of delta ingestion out there gather some feedback to decide the best way to go further.

The feature can be enabled with --enable-feature=otlp-native-delta-ingestion.

Testing

Unit tests have been written for delta sums/histograms/exp histograms.

Manual end to end testing has been done - can run with https://github.com/fionaliao/prom-otel-delta-support-testing to get some demo delta and cumualtive counters and histograms, example screenshots below.

fionaliao · 2025-03-31T18:47:21Z

@beorn7 @ArthurSens - this is a PR for simple ingestion of raw delta samples as discussed in the dev summit today (in draft - no proper testing yet!)

ArthurSens

Great start!! Let's finish tests and also add a bit of documentation on the feature flag saying that this is a very early stage implementation of your proposal. People can enable and start experimenting using deltas with existing PromQL functions, but we will collect feedback and likely build more things that improve the experience around deltas

cmd/prometheus/main.go

storage/remote/otlptranslator/prometheusremotewrite/otlp_to_openmetrics_metadata.go

storage/remote/write_handler.go

web/web.go

storage/remote/write_handler.go

fionaliao · 2025-04-03T20:10:08Z

To consider: should the flag be called otlp-native-delta-support or native-delta-support? Currently it only modifies the behaviour of the otlp endpoint, but if we start checking for the delta_counter type in functions behind the feature flag, there's nothing stopping someone from remote writing "delta" metrics, so the "otlp" part wouldn't be accurate anymore.

My preference would be to call it otlp-native-delta-support for now, and then rename to just native-delta-support if we start making type-aware functions. I'm not sure if it's okay for us to change the name of a feature flag once it's available, however.

Edit: actually I think I want two flags - one for ingesting deltas via the otlp endpoint, and one for temporality-aware functions See: #16360 (comment). I've also changed the flag name to otlp-native-delta-ingestion.

fionaliao · 2025-04-04T17:19:38Z

This is ready for review - I've added tests and did some manual e2e testing, and added a bit of documentation about the feature.

beorn7 · 2025-04-10T11:23:26Z

Currently swamped with other reviews (that have been waiting for far too long and where I am pretty much the only available reviewer). I'm happy to look at this in detail ASAP (but that might no be very soon). I know the @ArthurSens is on vacation right now. Let's see who gets to this first. Even better if we could find another reviewer for this to speed things up.

ArthurSens

Looking great, thank you for continuing the work!

Almost everything looks good, but histograms are looking a bit weird to me... could you clarify?

docs/feature_flags.md

storage/remote/otlptranslator/prometheusremotewrite/helper.go

storage/remote/write_handler.go

storage/remote/otlptranslator/prometheusremotewrite/metrics_to_prw.go

storage/remote/otlptranslator/prometheusremotewrite/metrics_to_prw_test.go

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Co-authored-by: Arthur Silva Sens <arthursens2005@gmail.com> Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

fionaliao · 2025-04-22T19:35:00Z

One interesting thing I just noticed - when attempting to use rate() on a delta native histogram, a warning annotationPromQL warning: this native histogram metric is not a counter: "test_histogram" (1:6) is added:

This is because the counter reset hint is GaugeType to avoid unnecessary chunk cutting, which is checked for in the rate() function here:

prometheus/promql/functions.go

Lines 204 to 206 in c88d0b0

    
           if isCounter && (prev.CounterResetHint == histogram.GaugeType || last.CounterResetHint == histogram.GaugeType) { 
        
           	annos.Add(annotations.NewNativeHistogramNotCounterWarning(metricName, pos)) 
        
           }

There's not a similar error for the delta counters (where the type is set to unknown for now) since there's no real type checks for counters at the moment.

I think the current PR implementation is fine as it is, this comment is just to note this behaviour.

fionaliao · 2025-04-22T19:48:15Z

@ArthurSens This is ready for re-review

ArthurSens

LGTM!

ArthurSens · 2025-04-22T20:07:24Z

Let's give @beorn7 a week to review the PR, if he doesn't fine a chance to do it then let's merge and continue the proposal :)

beorn7 · 2025-04-22T22:36:15Z

Don't wait for me. I trust you to do the right thing. And if it's not the right thing after all, it isn't the end of the world because it is behind a feature flag.

ArthurSens · 2025-04-23T12:56:58Z

Alright, thank you! Let's merge it then :)

Introduces the `-distributor.otel-native-delta-ingestion` flag (and corresponding per-tenant setting), which enables primitive OTEL delta metrics ingestion via the OTLP endpoint. This feature was implemented in Prometheus in prometheus/prometheus#16360. This PR allows Mimir users to enable this feature too. As per the Prometheus PR: > This allows otlp metrics with delta temporality to be ingested and stored as-is, with metric type unknown. To get "increase" or "rate", `sum_over_time(metric[<interval>])` (`/ <interval>`) can be used. > This is the first step towards implementing prometheus/proposals#48. That proposal has additional suggestions around type-aware functions and making the rate() and increase() functions work for deltas too. However, there are some questions around the best way to do querying over deltas, so having this simple implementation without changing any PromQL functions allow us to get some form of delta ingestion out there gather some feedback to decide the best way to go further. --------- Co-authored-by: Taylor C <41653732+tacole02@users.noreply.github.com>

ArthurSens reviewed Mar 31, 2025

View reviewed changes

ArthurSens reviewed Apr 1, 2025

View reviewed changes

storage/remote/write_handler.go Outdated Show resolved Hide resolved

fionaliao force-pushed the fionaliao/basic-delta-support branch from 71d4da7 to 0b0bee8 Compare April 3, 2025 19:30

fionaliao changed the title ~~WIP: very primitive native otel delta ingestion~~ Add primitive support for ingesting OTLP delta metrics as-is Apr 4, 2025

fionaliao marked this pull request as ready for review April 4, 2025 17:10

fionaliao requested review from aknuds1, jesusvazquez, cstyan, bwplotka and tomwilkie as code owners April 4, 2025 17:10

fionaliao mentioned this pull request Apr 4, 2025

Proposal: OTEL delta temporality support prometheus/proposals#48

Open

ArthurSens reviewed Apr 17, 2025

View reviewed changes

fionaliao force-pushed the fionaliao/basic-delta-support branch from 912188b to 68aa01b Compare April 17, 2025 17:04

fionaliao added 13 commits April 17, 2025 19:07

Add simple delta support

e4e7e46

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Rename delta2cumulative part

5982775

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Whoops bad refactor

93a135c

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Add example yml

6caafcd

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Feature flag instead and histogram hint handling

af7f1e4

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Delete otel_delta.yml - outdated

17012bd

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Renaming to native delta support

66ca0e7

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Add more explanatory comments

f187aec

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Add more explanation to histograms

9151441

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Correct comment on d2c consumer

fb74214

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Add tests for counters and fix bug

c657eaa

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Add histogram tests

ec70758

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Add docs

5db6cc9

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

fionaliao and others added 12 commits April 17, 2025 19:08

Sort series to make test deterministic

2690678

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

More formatting

c27cc86

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Change flag name to ingestion

fb3bf12

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Explain where rate calculation can go wrong

afdefe1

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Add warning about duplicate timestamps

851ad28

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Update docs/feature_flags.md

7c8773f

Co-authored-by: Arthur Silva Sens <arthursens2005@gmail.com> Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Fix tests

0c25109

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Remove unnecessary if

420e36b

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Add warning to d2c section

49bcf4a

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Make unknown type error when getting temporality

1c47ff5

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Correct type comment - not planning to add delta metric metadata type

1e7f221

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

Remove unused param for empty type

a48bee8

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

fionaliao force-pushed the fionaliao/basic-delta-support branch from 18e34fb to a48bee8 Compare April 17, 2025 18:08

Rewrite temporality logic to be clearer

d5c4cdf

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

fionaliao mentioned this pull request Apr 18, 2025

Support OTel delta temporality #12763

Open

irisgve mentioned this pull request Apr 18, 2025

Support for delta temporality in the OTLP ingest grafana/mimir#10439

Open

Change spurious to unnecessary - better description

bead944

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>

ArthurSens approved these changes Apr 22, 2025

View reviewed changes

ArthurSens merged commit 7ec63b1 into main Apr 23, 2025
45 checks passed

ArthurSens deleted the fionaliao/basic-delta-support branch April 23, 2025 12:58

charleskorn mentioned this pull request May 1, 2025

Sync upstream at 0ce6022 grafana/mimir-prometheus#869

Merged

fionaliao mentioned this pull request Jun 4, 2025

Allow primitive OTEL delta ingestion to be enabled grafana/mimir#11631

Merged

4 tasks

yeya24 mentioned this pull request Jul 10, 2025

Allow primitive OTEL delta ingestion in Cortex cortexproject/cortex#6874

Closed

Add primitive support for ingesting OTLP delta metrics as-is #16360

Add primitive support for ingesting OTLP delta metrics as-is #16360

Uh oh!

Conversation

fionaliao commented Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

Uh oh!

fionaliao commented Mar 31, 2025

Uh oh!

ArthurSens left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fionaliao commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fionaliao commented Apr 4, 2025

Uh oh!

beorn7 commented Apr 10, 2025

Uh oh!

ArthurSens left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fionaliao commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fionaliao commented Apr 22, 2025

Uh oh!

ArthurSens left a comment

Choose a reason for hiding this comment

Uh oh!

ArthurSens commented Apr 22, 2025

Uh oh!

beorn7 commented Apr 22, 2025

Uh oh!

ArthurSens commented Apr 23, 2025

Uh oh!

Uh oh!

Uh oh!

fionaliao commented Mar 31, 2025 •

edited

Loading

fionaliao commented Apr 3, 2025 •

edited

Loading

fionaliao commented Apr 22, 2025 •

edited

Loading