-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
feat(promtail-mixin): improve standalone usage #8534
base: main
Are you sure you want to change the base?
Conversation
e575c87
to
d58c7a4
Compare
d58c7a4
to
ecf6f90
Compare
Found another bug. I could not overwrite the config from the outside, so I fixed the config references and oriented myself to the loki-mixin dashboard generation, also to increase the conciseness here. |
@Zaunei this looks nice 👍 if the fixes here are still relevant to you please update to fix the merge conflicts, and I'll see if we can test deploying this quickly ourselves internally just to confirm nothing's broken |
Sure, I will update this PR. In the meantime, ne part of this PR got already upstreamed with #9684. |
ecf6f90
to
70e9ee6
Compare
Trivy scan found the following vulnerabilities:
|
70e9ee6
to
4fcdbd7
Compare
4fcdbd7
to
bb9b259
Compare
@cstyan PR is now uptodate. Happy to hear feedback :) |
@trevorwhitney does the compiled mixin here make sense? |
@Zaunei sorry for the delay in response again, Trevor is the one who's done the compiled mixins in the past but unfortunately he's out for the rest of the year AFAIK. |
@cstyan @trevorwhitney Is there anything that needs to be improved for this to be merged? |
bb9b259
to
93adc71
Compare
93adc71
to
0169618
Compare
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.
I don't think we need to pre-compile it here but I'm fine either way
0169618
to
d163387
Compare
Thanks for your approval @trevorwhitney! |
What this PR does / why we need it:
This PR improves the usage of the promtail-mixin in setups where no Loki is installed and writes metrics in the same tenant.
With the default Helm installation of promtail, the job label (
job=~"promtail-metrics"
) does not match the currently hard-coded label(job=~"$namespace/promtail"
), so the latency graphs will not work. Since the selectors will always differ slightly from setup to setup, I made them configurable.Added:
Changed:
Which issue(s) this PR fixes:
No issue created, as I consider this to be a non-breaking fix and improvement.
Special notes for your reviewer:
In order not to cause any braking changes I have taken over the values used before.
Checklist
CONTRIBUTING.md
guide (required)CHANGELOG.md
updateddocs/sources/upgrading/_index.md