Skip to content
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: support more specific prefixes in ?collect parameter #387

Conversation

antoinedeschenes
Copy link
Contributor

@antoinedeschenes antoinedeschenes commented Nov 14, 2024

This changes the ?collect parameter filter to allow more specific prefixes than those setup in the metrics-prefixes.

Example use cases:

  1. I have the redis.googleapis.com/stats/memory/usage_ratio prefix set in the ?collect config of dozens of projects.

Now, I'd like to enable the redis.googleapis.com/stats/memory/usage metric in the stackdriver_exporter metric prefixes, but since it conflicts with the more-specific redis.googleapis.com/stats/memory/usage_ratio metric name, the exporter will stop accepting the ?collect param I've set in other configurations.

  1. I want to whitelist all redis.googleapis.com metrics, but only collect 1 or 2 metrics with ?collect. Currently, the prefixes have to be exact matches so this is not possible without the proposed change.

To test:

$ go run . --google.project-id=<gcp-project-name> --monitoring.metrics-prefixes=redis.googleapis.com/stats/
$ curl http://localhost:9255/metrics?collect=redis.googleapis.com/stats/memory/usage/ | grep stackdriver_redis

Other question: is there a technical reason that would prevent from having a flag disabling prefix filtering in ?collect?

@antoinedeschenes antoinedeschenes force-pushed the feat/collect-specific-prefixes branch from 594fcb3 to 3de3928 Compare November 14, 2024 18:38
@antoinedeschenes
Copy link
Contributor Author

antoinedeschenes commented Nov 14, 2024

cc: @SuperQ @kgeckhart

@antoinedeschenes
Copy link
Contributor Author

antoinedeschenes commented Nov 14, 2024

Looks like that feature was already implemented along another change and then reverted through PR #316

though the reason for the revert seems to be that the main handler was removed, which caused a regression to aggregate-deltas #315

@kgeckhart kgeckhart self-requested a review November 18, 2024 16:36
Copy link
Contributor

@kgeckhart kgeckhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other question: is there a technical reason that would prevent from having a flag disabling prefix filtering in ?collect?

The configuration could be a bit confusing if you have to provide metric prefixes that are completely ignored when calling collect.

Looks like that feature was already implemented along another change and then reverted through PR #316

though the reason for the revert seems to be that the main handler was removed, which caused a regression to aggregate-deltas #315

You're right, this implementation should be okay because it's not changing any of the handler functionality directly.

Thanks for doing this! I'll see if I can resolve ?collect being incompatible with aggregate deltas separately.

@antoinedeschenes
Copy link
Contributor Author

antoinedeschenes commented Nov 18, 2024

The configuration could be a bit confusing if you have to provide metric prefixes that are completely ignored when calling collect.

Oh, by that I mean that ?collect could allow to specify metric prefixes that aren't specified in the exporter command-line arguments

@kgeckhart kgeckhart merged commit 231987c into prometheus-community:master Dec 2, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants