-
Notifications
You must be signed in to change notification settings - Fork 591
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
otelgrpc: only start span if not filtered out #6695
Conversation
Have you looked into adding a test? |
@dmathieu I will look into soon, Just need to test is a bit more in a specific env and will come with tests. |
Kind of trick to add tests, but I could do it. (I hope) |
Not sure If there is something missing regarding the review, let me know. |
Could you look into the test suite failures? |
nil filter cases
avoid extra variable
dfc0e88
to
d22cc98
Compare
Sure, my mistake, fixed, I hope now it's ok |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6695 +/- ##
=====================================
Coverage 73.1% 73.1%
=====================================
Files 191 191
Lines 15858 15866 +8
=====================================
+ Hits 11598 11606 +8
Misses 3925 3925
Partials 335 335
|
Co-authored-by: Damien Mathieu <[email protected]>
instrumentation/google.golang.org/grpc/otelgrpc/test/grpc_stats_handler_test.go
Outdated
Show resolved
Hide resolved
instrumentation/google.golang.org/grpc/otelgrpc/test/grpc_stats_handler_test.go
Outdated
Show resolved
Hide resolved
Co-authored-by: Tyler Yahn <[email protected]>
…s_handler_test.go Co-authored-by: Tyler Yahn <[email protected]>
…s_handler_test.go Co-authored-by: Tyler Yahn <[email protected]>
…s_handler_test.go Co-authored-by: Tyler Yahn <[email protected]>
…s_handler_test.go Co-authored-by: Tyler Yahn <[email protected]>
@dashpole does this look good to you? |
yep |
Related to #6446
Only start a span if it's not filtered out.