-
Notifications
You must be signed in to change notification settings - Fork 361
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
fix(otel): avoid crashes in tracing wrappers for streams #14477
fix(otel): avoid crashes in tracing wrappers for streams #14477
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14477 +/- ##
========================================
Coverage 93.59% 93.59%
========================================
Files 2316 2316
Lines 207222 207354 +132
========================================
+ Hits 193953 194083 +130
- Misses 13269 13271 +2 ☔ View full report in Codecov by Sentry. |
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.
Looks good. I think we need to check the early exit condition in the End()
helpers, though. Other than that, I just have nits.
Aside: I would have split this up into 3 separate PRs, even though the changes are all related / basically the same. I only looked closely at the AsyncStreamingReadWriteRpcTracing
changes. The comments probably apply to the other two wrappers.
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.
Optionally, remove the expectations in the unit tests for the generated golden tracing stubs. Or not. Then feel free to merge.
Fix the crash issue in
AsyncStreamingReadRpcTracing
AsyncStreamingWriteRpcTracing
AsyncStreamingReadWriteRpcTracing
The issue is caused by accessing metadata in
EndSpan()
before the underlying streamStart()
.Closes #14458
This change is![Reviewable](https://camo.githubusercontent.com/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)