[pkg/translator/prometheus] fix panic for duplicated feature gate registration in otel-collector and prometheus packages #32334
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
dashpole
reviewed
Apr 12, 2024
dashpole
approved these changes
Apr 12, 2024
|
I don't think this is the way we should go with this. The feature gate registrations will be removed from Prometheus and they're planning a hard fork so there's little likelihood of the conflict being reintroduced. I don't believe there's any urgent need to work around this here at this time. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
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 mitigates the bug that causes the otelcollector to crash when a project depends on both opentelemetry-collector-contrib and prometheus packages. This is a fix until the code is maintained in a single package and MustRegister() could be used again.
The variable here is initialized by a function that attempts registration. If that errors, search the global registry for a gate that already exist with that ID. If none is still found, then panic.
The same changes will need to be copied over to the duplicate files in the prometheus repository so that both are not calling MustRegister().
Link to tracking Issue:
prometheus/prometheus#13842
Testing:
Added tests that explicitly re-register the flag to ensure there's no panic and the existing gate returned is expected.