-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
[promtail] Refactor promtail client metrics so that we can't have duplicate metrics collected for the lag metric. #5521
Conversation
collected for the lag metric. Signed-off-by: Callum Styan <callumstyan@gmail.com>
Signed-off-by: Callum Styan <callumstyan@gmail.com>
Signed-off-by: Callum Styan <callumstyan@gmail.com>
lblSet[HostLabel] = c.cfg.URL.Host | ||
// also set client name since if we have multiple promtail clients configured we will run into a | ||
// duplicate metric collected with same labels error when trying to hit the /metrics endpoint | ||
lblSet[ClientLabel] = c.name |
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.
id or config id would have work better IMO.
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.
for the configuration field name?
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
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
configs. Signed-off-by: Callum Styan <callumstyan@gmail.com>
Still some things I need to fix, but some quick reivews/sanity checks would be good.
So, this should solve #5494
The cause of the issue is relatively simple, there was nothing in our code ensuring that for multiple promtail clients in the same binary that this metric would have a unique labelset per client. External labels are not applied to the metrics on
/metrics
. I've done a few things in this PR:main.go
, and the metrics struct is passed in to client constructors from thereSigned-off-by: Callum Styan callumstyan@gmail.com