-
Notifications
You must be signed in to change notification settings - Fork 4.6k
stats/opentelemetry: Add support for optional label grpc.lb.backend_service in per-call metrics
#8637
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8637 +/- ##
==========================================
- Coverage 82.17% 81.96% -0.22%
==========================================
Files 415 415
Lines 40709 40759 +50
==========================================
- Hits 33453 33408 -45
- Misses 5883 5992 +109
+ Partials 1373 1359 -14
🚀 New features to boost your workflow:
|
dfawley
left a comment
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.
LGTM thanks! Assigning @easwars for visibility
| // Nothing to assert for the other stats.Handler callouts. | ||
| } | ||
|
|
||
| } |
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.
Similar here.
grpc.lb.backend_service in per-call metrics
|
One small comment/question on the implementation given the dependency on |
You are correct that a custom statsHandler would not have access to these labels. But if you have a reason to want access to these labels, we can have a discussion about it, and we could move this out of Please let us know what you think. Thanks. |
|
@easwars yeah that works for me, I can add integration tests in my implementation and check before upgrades. As long as there is a method breaking changes are totally fine |
|
@davinci26 : Please feel free to send us a PR. Thanks. |
Addresses A89
RELEASE NOTES:
grpc.lb.backend_servicein per-call metrics