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

fix: annotation scraping - scrape_port_named_metrics and metrics.grafana.com/port #50

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

adamhackl
Copy link
Contributor

@adamhackl adamhackl commented Nov 13, 2024

2 fixes in modules/kubernetes/annotations/metrics.alloy

  • In one of the first steps, __tmp_scrape is default set to false, but the next step watches for metrics.grafana.com/scrape to be blank or true and then makes the value of __tmp_scrape = true. So __tmp_scrape would never be blank. But the step that runs if the attribute scrape_port_named_metrics = true only runs if __tmp_scrape is true or blank. The point of that attribute is to scrape any port that contains the word metrics in its name, regardless of the metrics.grafana.com/scrape attribute, but it was not working that way since it could never be blank. We only want that step to not run if metrics.grafana.com/scrape = false, which it is now only set to __tmp_scrape if the attribute is set to false, not just by default.
  • The step that watches for metrics.grafana.com/port to be set to override the __address__ port had an additional source label added to it when it was copied from grafana-agent-modules, but the regex was not adjusted to account for the additional label, so the step would never work.

@adamhackl adamhackl changed the title Fix annotation scraping - scrape_port_named_metrics and metrics.grafana.com/port fix: annotation scraping - scrape_port_named_metrics and metrics.grafana.com/port Nov 13, 2024
@tim-shane tim-shane marked this pull request as ready for review December 5, 2024 14:48
regex = "^(?:;*)?(true)(;|true)*$"
replacement = "$1"
// set to false if any of the endpoint or service annotations are scrape: false
regex = "^.*(?i)(false).*$"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will defer to @bentonam for this change.

@adamhackl
Copy link
Contributor Author

I think this PR needs some more work after testing with it. The scrape_port_named_metrics works, but the regular annotation scraping becomes inconsistent for some reason.

@adamhackl adamhackl marked this pull request as draft December 31, 2024 15:06
@adamhackl adamhackl force-pushed the fix-annotation-scraping branch from bd6811f to a384ca1 Compare January 8, 2025 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants