Add closing support to otel metrics instrumentation #3444
Merged
+81
−44
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
|
Hello @rokn, thank you for the contribution. Will review the PR shortly. Pinging @monkey92t if he has interest to check this one as well. |
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.
Hey there, I'm opening this PR to try to address the problem from #3424.
This adds a new option when setting up the OpenTelemetry metric hooks which would allow the users to pass a close channel so they can notify us when we need to stop the metrics reporting and unregister all the needed callbacks.
The approach with a close channel through an option was chosen as to not introduce any breaking changes. Another possibility is to introduce a new function which receives a context as first param and waits for the context to close. I think both can work so it's up to you guys to decide what would be best.
I'll add some tests if you think this approach is fine