[receiver/prometheus] include scrape configs provided via scrape_config_files
#34897
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… up receiver Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
bacherfl
changed the title
scrape_config_files
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
bacherfl
commented
Aug 28, 2024
| if !enableNativeHistogramsGate.IsEnabled() { | ||
| // Enforce scraping classic histograms to avoid dropping them. | ||
| for _, scrapeConfig := range cfg.ScrapeConfigs { | ||
| for _, scrapeConfig := range scrapeConfigs { |
There was a problem hiding this comment.
Here we have the limitation that the scrapeConfig.ScrapeClassigHistograms property is only applied to the scrapeconfigs provided directly in the config, but not the ones via scrape_config_files, as the scrapeManager internally calls GetScrapeConfigs(), which leads to the scrapeConfigs being read from the file system again.
One potential solution could be to set cfg.ScrapeConfigs here to the result of GetScrapeConfigs(), and clear the ScrapeConfigFiles property, but this feels a bit hacky. What do you think @dashpole ?
# Conflicts: # receiver/prometheusreceiver/config.go # receiver/prometheusreceiver/metrics_receiver.go
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
atoulme
reviewed
Sep 4, 2024
Co-authored-by: Antoine Toulme <antoine@toulme.name>
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
iblancasa
approved these changes
Sep 9, 2024
dashpole
approved these changes
Sep 9, 2024
dashpole
added
the
ready to merge
jriguera
pushed a commit
to springernature/opentelemetry-collector-contrib
that referenced
this pull request
Oct 4, 2024
…fig_files` (open-telemetry#34897) **Description:** This PR fixes a bug in the prometheus receiver where the scrape configs provided via `scrape_config_files` were not applied. Using `scrape_config_files` instead of providing the scrape configs directly does come with some limitations regarding the use of env vars, as also mentioned in open-telemetry#34786 (comment). **Link to tracking Issue:** open-telemetry#34786 **Testing:** Added unit tests --------- Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com> Co-authored-by: Antoine Toulme <antoine@toulme.name> Co-authored-by: David Ashpole <dashpole@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description: This PR fixes a bug in the prometheus receiver where the scrape configs provided via
scrape_config_fileswere not applied. Usingscrape_config_filesinstead of providing the scrape configs directly does come with some limitations regarding the use of env vars, as also mentioned in #34786 (comment).Link to tracking Issue: #34786
Testing: Added unit tests