-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Add filtering option for prometheus collector #16420
Add filtering option for prometheus collector #16420
Conversation
Signed-off-by: chrismark <[email protected]>
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.
This approach LGTM, I agree with limiting it to filtering by now, and we can consider adding mapping in the future.
I think we could accept using include_metrics
and ignore_metrics
at the same time, I added a comment about that.
I preferred to add the processing on collector layer to avoid touching GetFamilies since it is used by many other places like the state_* metricsets of k8s module. Not sure if we would like to have it in there (in the helper method).
👍 we can revisit this later if needed.
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
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.
A couple of comments more, and I'd be ok with continuing with this proposed implementation. Thanks!
Thanks for reviewing! I will work on the comments and I also plan to add some unit-tests maybe for the helper functions so as to secure the logic. |
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
Giving a heads-up on this:
Still need to update the docs with these new settings description. |
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
(cherry picked from commit db85050)
Drafting what was discussed on #15493 to start implementation specific discussions.For the time being only filters are implemented, but mappings will be added if we agree on this.
@jsoriano I preferred to add the processing on collector layer to avoid touching
GetFamilies
since it is used by many other places like thestate_*
metricsets of k8s module. Not sure if we would like to have it in there (in the helper method).What does this PR do?
This PR introduces filtering options for prometheus collector metricset.
Why is it important?
This is important when someone want to filter out metrics that come in a single response for instance in case of prometheus exporters.
This will be useful in light-modules like
ibmmq
module where currently we use script processor to filter out metrics as well as in OpenMetrics module where a user may want to explicitly define which metrics want to keep.How to test this PR locally
docker-compose up
. Then metrics should be exported athttp://localhost:9100/metrics
.And expect to see only events that match
node_filesystem_*
but also notnode_filesystem_device_*
and are notnode_filesystem_readonly
.3. Try to remove
^node_filesystem_readonly$
fromexclude
filters and see that this metric is included in events. One can try different combinations.Related issues