-
Notifications
You must be signed in to change notification settings - Fork 183
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
Filter list of control-plane app metrics #697
Conversation
94a0516
to
dabdef5
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.
LGTM.
deploy/helm/sumologic/values.yaml
Outdated
writeRelabelConfigs: | ||
- action: keep | ||
regex: coredns | ||
sourceLabels: [job] | ||
- action: keep |
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.
Just curious on the reason to have 2 keep blocks vs something like this:
- url: http://$(CHART).$(NAMESPACE).svc.cluster.local:9888/prometheus.metrics.control-plane.coredns
writeRelabelConfigs:
- action: keep
regex: coredns;coredns_cache_(size|(hits|misses)_total)|coredns_dns_request_duration_seconds_(count|sum)|coredns_(dns_request|dns_response_rcode|forward_request)_count_total|process_(cpu_seconds_total|open_fds|resident_memory_bytes)
sourceLabels: [job,__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.
It's more clear in my personal opinion :)
I also wasn't sure how exactly works the writeRelabelConfig for such construct, but I read the docs and I don't see any risk related to that.
I can change it to be more compressed and to keep convention
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.
The only reason I brought it up was just consistency like we do the other rules. I have no problem with doing it this way either.
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.
Convention is important :D
I updated PR, need to re-test it :)
1bf687c
to
8d10bed
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.
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 👍
f95cf23
to
e485707
Compare
Description
Restrict coredns and etcd metrics
Testing performed