-
Notifications
You must be signed in to change notification settings - Fork 2.9k
[receiver/prometheusreceiver] fix prom receiver config reload #40780
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
Conversation
Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just some very small comments to make tests smaller. They are non-blocking of course!
I'd like to see another review from David or Krajo about the strategy as well, maybe they see something that I couldn't
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I'm kicking myself for not finding this solution even though I thought of it and did some search on the current lib :( Nice job!
I've tested this locally with a simple target that uses basic auth.
Description
Fixing a bug where reloading the Prometheus config, caused config entries of type Secret to be replaced with the literal value of
<secret>
.Link to tracking issue
Fixes #40520, #40916
Testing
Reload()
method doesn't mask secrets for existingscrape_configs
target_allocator
config, which was the original reason for theReload()
method, which introduced the bug (https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/40103/files#r2097520578)Documentation
N/A