8000 Migrate OTLP endpoint to PRW 2.0 for ingestion by dashpole · Pull Request #16784 · prometheus/prometheus · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Migrate OTLP endpoint to PRW 2.0 for ingestion #16784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 32 commits into
base: main
Choose a base branch
from
Open

Migrate OTLP endpoint to PRW 2.0 for ingestion #16784

wants to merge 32 commits into from

Conversation

dashpole
Copy link
Contributor
@dashpole dashpole commented Jun 26, 2025 •
edited
Loading

Part of #16610. See #16610 (comment) for alternatives.

The current OTLP endpoint translates OTLP to PRW 1.0, and then appends the metrics to the TSDB using the PRW 1.0 ingestion path. This PR migrates this translation logic to instead translate to PRW 2.0 and use the PRW 2.0 ingestion path.

Currently, the PRW 1.0 endpoint just ignores metadata, so we are dropping type, help, and unit information. Migrating to 2.0 will fix that. It will also allow the OTLP endpoint to use the type and unit feature implementation without re-implementing it in the translation layer.

This PR can be largely summarized as:

  • Rename prompb types to writev2 types. Those are split into their own commits.
  • Migrate from prompb.Label with labels.Label, and adding support for symbolization.
  • Metadata is attached to each TimeSeries, instead of being stored separately. Metadata is added to all unit tests.
  • Migrate the OTLP write handler to use writeV2 instead of write
  • Small fixes

The existing e2e tests in write_test.go didn't need any changes, which helps to show that this doesn't change any behavior.

dashpole added 23 commits June 26, 2025 14:16
@dashpole
prompb.TimeSeries -> writev2.TimeSeries
8b0b616 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
prompb.MetricMetadata -> writev2.Metadata
a46df75 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
prompb.Sample -> writev2.Sample
3763bd8 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
prompb.Exemplar -> writev2.Exemplar
2423003 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
prompb.Histogram -> writev2.Histogram
6935d07 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
prompb.BucketSpan -> writev2.BucketSpan
52d3bf3 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
add symbol table
cc6532e 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
prompb.Label -> labels.Label
e13df88 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
replace timeSeriesSignature with labels.Labels.Hash()
438bcfe 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
replace labels.Labels{ with labels.New
008901a 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
use labels.Builder to build labels
4d00b6d 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
use labels builder for exemplar and metric labels
90fa6cb 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
iterate over labels.Labels using Range
7ac34cf 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
use labels.ScratchBuilder to build promoted resource labels
b4c3bb4 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
use labels.Equal in isSameMetric
cdbd50b 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
use labels.Len() instead of len
e099cce 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
use labels.Builder to implement created timeseries
f77e277 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
replace isSameMetric with labels.Equal
79a85ff 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
fix unit tests
5eccb76 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
migrate OTLP write handler to use PRW 2.0
e67e9a3 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
add Metadata directly to TimeSeries
abaf3e3 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
re-use labels.ScratchBuilder and labels.Builder
62e8edd 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
fixes after rebase
dc834df 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
fix lint
f79b1b4 
Signed-off-by: David Ashpole <dashpole@google.com>
dashpole added 2 commits June 26, 2025 17:39
@dashpole
ensure labels are sorted before merging values
9ab8496 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
use different metric name for OTLP endpoint metrics
9d1ba67 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole dashpole marked this pull request as ready for review June 26, 2025 17:54
@dashpole dashpole requested a review from aknuds1 as a code owner June 26, 2025 17:54
@dashpole
Copy link
Contributor Author

cc @ArthurSens @jmichalek132 @aknuds1 @bwplotka

bwplotka
bwplotka approved these changes Jun 27, 2025
View reviewed changes
Copy link
Member
@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Epic work, thanks!

Looks good in general, some minor suggestions only 👍🏽

storage/remote/otlptranslator/prometheusremotewrite/helper.go
}

return labels
return c.builder.Labels()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why we return built labels and not builder? Looks like we expect to still add at least name after this. By returning builder we make it a bit more efficient in theory. (could be done in separate PR)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, so it made sense in the past as we were creating proto labels later. But now we operate on labels.Labels, so maybe it's viable to pass builders/scratchbuilders?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to take a single set of base attributes, and add one or two labels (e.g. for name with _sum, or _bucket suffix) to multiple different label sets. What i've done is use a ScratchBuilder to build the base attributes, and then use a Builder to append the special ones. There isn't a copy method I can see for ScratchBuilder, otherwise I could use that to build multiple variations of the same set of labels. Let me know if there is a better pattern.

dashpole and others added 6 commits June 27, 2025 10:39
@dashpole @bwplotka
Apply suggestions from code review
9e3e9f8 
Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com>
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
add periods at the end of comments
0debc7e 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
add TODOs for optimization
c80d2f8 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
rename createLables to addLabels
4788a53 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
pass builder to addPromotedAttributes
6f502eb 
Signed-off-by: David Ashpole <dashpole@google.com>
@dashpole
replace labels.New with labels.FromStrings in tests
d0464bd 
Signed-off-by: David Ashpole <dashpole@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ArthurSens ArthurSens ArthurSens left review comments

@bwplotka bwplotka bwplotka approved these changes

@aknuds1 aknuds1 Awaiting requested review from aknuds1 aknuds1 is a code owner

@jesusvazquez jesusvazquez Awaiting requested review from jesusvazquez jesusvazquez is a code owner

@cstyan cstyan Awaiting requested review from cstyan cstyan is a code owner

@tomwilkie tomwilkie Awaiting requested review from tomwilkie tomwilkie is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dashpole @bwplotka @ArthurSens
0