Skip to content
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.

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

[receiver/prometheusreceiver] Use Prometheus Labels public method API #31908

Merged
merged 1 commit into from
Mar 27, 2024

Conversation

braydonk
Copy link
Contributor

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.

Documentation:
Should not need any changes.

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.
@braydonk braydonk requested review from Aneurysm9, dashpole and a team as code owners March 22, 2024 15:29
@github-actions github-actions bot added the receiver/prometheus Prometheus receiver label Mar 22, 2024
@braydonk braydonk changed the title Use Prometheus Labels public method API [receiver/prometheusreceiver] Use Prometheus Labels public method API Mar 22, 2024
Copy link
Contributor

@dashpole dashpole left a comment

Choose a reason for hiding this comment

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

Is there any way for us to prevent us from using the fields of the labels, and breaking the build with the new tag?

@braydonk
Copy link
Contributor Author

I don't think there is. If there was an interface type or something, it would mean we could use that instead of labels.Labels directly. But I don't see anything like that.

The way it's implemented upstream, these public methods are sort of an unstated interface (if any methods don't exist in one version of the Labels implementation then the build wouldn't work anyway) so I think the best we can do at the moment is ensure that any interactions with the labels.Labels type makes no assumptions about the underlying type it aliases.

@dashpole dashpole added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Mar 22, 2024
@dmitryax dmitryax merged commit 89b20d5 into open-telemetry:main Mar 27, 2024
147 of 148 checks passed
@github-actions github-actions bot added this to the next release milestone Mar 27, 2024
krajorama added a commit to krajorama/opentelemetry-collector-contrib that referenced this pull request Mar 27, 2024
Adopt open-telemetry#31908

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
receiver/prometheus Prometheus receiver Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants