[receiver/prometheusreceiver] Allow for the alternate stringlabels implementation of labels to be usable
#31907
Comments
braydonk
added
enhancement
|
Pinging code owners:
See Adding Labels via Comments if you do not have permissions to add labels yourself. |
braydonk
changed the title
stringlabels implementation of stringlabels to be usablestringlabels implementation of stringlabels to be usable
braydonk
changed the title
stringlabels implementation of stringlabels to be usablestringlabels implementation of labels to be usable
Can we track that question somewhere? I have no objections to using the public methods, but would love to make sure the performance improvements land. Will that build tag be the default someday, or would we need to update the core/contrib builds to use it? @bwplotka do you know if anyone is exploring this optimization for the prometheus server? |
I can make another issue to track that question. I am not sure where I personally stand on how this should be adopted by an upstream release if at all. That's why I opted to position this only as making it possible to build with
The reason I brought up that option above is because that seems to be what upstream Prometheus does with this. For a while they distributed alternative releases with |
…#31908) **Description:** By only using the public method API for Prometheus labels (rather than assuming `labels.Labels` is an alias of a slice) it opens up the possibility to build a collector with the `stringlabels` tag, so we can use the more memory efficient labels implementation. **Link to tracking Issue:** #31907 **Testing:** I had trouble running all of the tests locally, so I think I will need some help with making that work. I did run all the tests I changed with `-tags=stringlabels` and without it.
…open-telemetry#31908) **Description:** By only using the public method API for Prometheus labels (rather than assuming `labels.Labels` is an alias of a slice) it opens up the possibility to build a collector with the `stringlabels` tag, so we can use the more memory efficient labels implementation. **Link to tracking Issue:** open-telemetry#31907 **Testing:** I had trouble running all of the tests locally, so I think I will need some help with making that work. I did run all the tests I changed with `-tags=stringlabels` and without it.
|
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 Pinging code owners:
See Adding Labels via Comments if you do not have permissions to add labels yourself. |
|
Unstale (is that possible through comments in this repo?) This is still relevant. |
|
Are there any other changes required after #31908? Or can this be closed? |
|
Oh I forgot that the PR was closed. Then this issue can probably be closed; there's probably more to look into in terms of providing a build of the collector that has this enabled but that can probably go into a new issue instead. For my use case this is good enough as I'll build it with the Closing this now, thanks! |
Component(s)
receiver/prometheus
Is your feature request related to a problem? Please describe.
In the Prometheus SDK, there is an alternate implementation of the labels.Labels type that uses a string with length encodings to represent a collection of labels instead of a slice. In my non-rigorous testing I saw some pretty reasonable improvements when stress testing a collector built with
-tags=stringlabels.However I don't mean for this issue to be an argument about whether or not this should be used/how. This issue is intended to be about how the current implementation of the Prometheus Receiver assumes
labels.Labelsto be a slice under the hood, making it so building with-tags=stringlabelsis impossible.Describe the solution you'd like
labels.Labelshas a set of public methods that allows you to interact with it in the same way regardless of which implementation it's built with. I'd like to change the spots that make the slice assumption to use the public methods to an equivalent result.This issue can be considered closed when it is possible to build a collector with this receiver using the
stringlabelsbuild tag.Describe alternatives you've considered
No response
Additional context
I learned about this alternate implementation of labels and why it improves memory usage from this OpenObservability Day 2023 talk: https://www.youtube.com/watch?v=29yKJ1312AM
The text was updated successfully, but these errors were encountered: