[Prometheus Receiver] Make prometheus receiver behavior compatible with Prometheus when sd_file does not exist #21680
Conversation
This file contains hidden or 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
Aneurysm9
approved these changes
May 11, 2023
There was a problem hiding this comment.
I think I would prefer to keep this as a type switch to preserve easy extensibility should validation of other SDConfig types be necessary.
There was a problem hiding this comment.
I originally kept it as type switch for type assertion - but it was giving lint error (go critic) in workflow checks as it only had one case.
rapphil
reviewed
May 23, 2023
There was a problem hiding this comment.
can you add a testcase that validates that no error happens in case the file does not exist?
564b4a7 to
6e75d0c
Compare
rapphil
approved these changes
May 24, 2023
This was referenced May 27, 2023
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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 achieves the compatibility between Prometheus Receiver and Prometheus when
sd_fileortargetgroupsare not present at the startup:sd_file_pathis not watchable.sd_file_path(and not fail with error) when whensd_fileortargetgroupsare not present at the startup.In order to achieve the above objective, this PR reverts some of the changes made in this PR to add additional config validation for
file_sdconfigs. These additional config validation creates incompatibility in the Prometheus and Prometheus Receiver behaviour as also highlighted in this issue:sd_filefails Collector in error when at the startupsd_fileis not present or thetargetgroupinside thissd_fileis not present. However, this is not the behaviour of Prometheus- Prometheus has functionality (discovery) which continues to watch thesd_file_pathprovided in the config for the updates and subsequent validation.Also, Prometheus doesnt't crash the application at runtime when
sd_fileortargetgroupare not present orsd_file_pathdoes not exist (as originally thought here). Prometheus only logs the error and continues to receive update on thissd_file_path. - so additionally ensuring the presence ofsd_fileandtagretgroupand otherwise failing with error in config validation in Collector (like here) disrupts that process.sd_file_pathwith the right naming and extension is already done in unmarshal stage in Collector (before config validation) - so providing it again in the config validation is redundant.Link to tracking Issue:
#21509
aws-observability/aws-otel-collector#2039
#5373
aws-observability/aws-otel-collector#1636
Testing:
Manual testing