Skip to content

Export remote.LabelsToLabelsProto() and remote.LabelProtosToLabels()#14316

Merged
krajorama merged 2 commits intoprometheus:mainfrom
pracucci:export-labelsToLabelsProto
Jun 20, 2024
Merged

Export remote.LabelsToLabelsProto() and remote.LabelProtosToLabels()#14316
krajorama merged 2 commits intoprometheus:mainfrom
pracucci:export-labelsToLabelsProto

Conversation

@pracucci
Copy link
Contributor

Similar to #14306, in this PR I'm asking you if we could export remote.LabelsToLabelsProto() and remote.LabelProtosToLabels() too. The rationale is that remote read endpoints are typically implemented by 3rd party systems (Grafana Mimir in our case) and having the utilities to convert data types to/from the remote protobuf would be helpful to avoid code duplication.

Signed-off-by: Marco Pracucci <marco@pracucci.com>
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.

We are switching to v2 very soon (or at least adding it and slowly deprecating v1), and there are no labels there, so I think we will all operate on labels.Labels going forward. (see https://github.com/prometheus/prometheus/blob/remote-write-2.0/prompb/io/prometheus/write/v2/types.pb.go)

Do you think those helpers will be still relevant? If yes, LGTM (:

@cstyan
Copy link
Member

cstyan commented Jun 20, 2024

I think they'll still need the helpers, it's going to be a long time before we remove support for 1.0 completely.

@pracucci
Copy link
Contributor Author

We are switching to v2 very soon (or at least adding it and slowly deprecating v1), and there are no labels there, so I think we will all operate on labels.Labels going forward. (see https://github.com/prometheus/prometheus/blob/remote-write-2.0/prompb/io/prometheus/write/v2/types.pb.go)

I would need these helpers for the remote read implementation in Mimir. Does the remote write v2 protocol affects remote read too?

That being said, I think v1 will still be support for long time.

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.

4 participants