-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[receiver/opencensus]: check for ErrListenerClosed when reporting multiplexer status #34093
Conversation
7165db8
to
c4ed6b9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two things:
- This should probably have a changelog since it's a bug fix.
- Is it possible to add a test for this? Maybe some way to make sure that shutting down isn't blocked in this specific case where the listener is closed?
9bc5d0c
to
c4ed6b9
Compare
I will try, but still testing if the actual fix resolves the issue, since I am not 100% sure yet, the PR is still in draft mode and chlog + some unit tests are still missing -> will add them ass soon as the PR will be ready for review+merging. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #34093 +/- ##
==========================================
- Coverage 82.97% 82.86% -0.12%
==========================================
Files 2131 2132 +1
Lines 156561 157389 +828
==========================================
+ Hits 129911 130421 +510
- Misses 21927 22225 +298
- Partials 4723 4743 +20 ☔ View full report in Codecov by Sentry. |
258e812
to
371ddc3
Compare
.chloggen/correctness-traces.yaml
Outdated
component: opencensusreceiver | ||
|
||
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
note: Check for ErrListenerClosed error before reporting multiplexer status. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This message should be a bit more user-friendly. What does the user of the opencensus receiver care about, and understand?
Maybe something like "Don't report an error for an expected condition during shutdown"?
…tiplexer status Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
7e623a4
to
6beb82a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this!
Fixes #33865