Skip to content

Conversation

@ldufr
Copy link
Contributor

@ldufr ldufr commented Jan 8, 2026

This enable support for lazy deserializing of OTLP protobuf messages. By default nothing is actually changed, the added Unmarshal on a message that wasn't lazy deserializing simply return what was deserialized and can't fail.

Importantly, this PR changes the OpenTelemetry pdata package to a fork https://github.com/grafana/opentelemetry-collector, but there is an open pull request to upstream the changes to pdata, see open-telemetry/opentelemetry-collector#14371

Related: grafana/mimir#13962

Does this PR introduce a user-facing change?

NONE

Note

Introduces lazy deserialization for OTLP metrics and adopts a forked pdata to enable it.

  • Replaces go.opentelemetry.io/collector/pdata with github.com/grafana/opentelemetry-collector/pdata via go.mod replace; updates go.sum accordingly
  • In metrics_to_prw.go, reads metrics with metricSlice.Unmarshal(k, &metric) instead of At(k), adding error handling while keeping downstream processing unchanged

Written by Cursor Bugbot for commit 7bcd4da. This will update automatically on new commits. Configure here.

Copy link
Contributor

@jesusvazquez jesusvazquez left a comment

Choose a reason for hiding this comment

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

LGTM

@ldufr ldufr force-pushed the ldufresne/otlp-streaming branch from aac6008 to 70a9644 Compare January 8, 2026 12:09
@ldufr ldufr force-pushed the ldufresne/otlp-streaming branch from 70a9644 to c6ca825 Compare January 8, 2026 12:13
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

Looks good for what it is, but I don't think we want to merge this into mimir-prometheus yet, right. I think we for now want to test your Mimir branch instead?

@ldufr
Copy link
Contributor Author

ldufr commented Jan 12, 2026

@aknuds1 Just to be clear, I did run the code in a dev for a week. Should we re-run a new test before merging that and going through the normal deployment process?

@ldufr ldufr force-pushed the ldufresne/otlp-streaming branch from c6ca825 to 7bcd4da Compare January 13, 2026 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants