stats/otel: Add grpc.lb.backend_service label to wrr metrics (A89)#8737
stats/otel: Add grpc.lb.backend_service label to wrr metrics (A89)#8737mbissa merged 3 commits intogrpc:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8737 +/- ##
==========================================
+ Coverage 83.22% 83.37% +0.15%
==========================================
Files 419 418 -1
Lines 32454 32377 -77
==========================================
- Hits 27009 26994 -15
+ Misses 4057 4017 -40
+ Partials 1388 1366 -22
🚀 New features to boost your workflow:
|
|
@mbissa FYI: @Pranjali-2501 |
We don't need it anymore. A75 already captures that in the gRFC itself. |
|
|
||
| // SetBackendServiceOnState stores the backendService on the resolver state so | ||
| // that it can be used later as a label in wrr metrics. | ||
| func SetBackendServiceOnState(state resolver.State, backendService string) resolver.State { |
There was a problem hiding this comment.
Nit: Can we drop the OnState suffix on this one please. I checked a whole bunch of attribute setter functions and none of them have a suffix like this. Thanks.
| "time" | ||
|
|
||
| "google.golang.org/grpc/balancer" | ||
| wrr "google.golang.org/grpc/balancer/weightedroundrobin" |
There was a problem hiding this comment.
Please avoid this unnecessary import renaming. See: go/go-style/decisions#import-renaming
| return err | ||
| } | ||
|
|
||
| newState := wrr.SetBackendServiceOnState(s.ResolverState, b.clusterName) |
There was a problem hiding this comment.
Can we eliminate this local variable and inline it on line 303?
Address: https://github.com/grpc/proposal/blob/master/A89-backend-service-metric-label.md
This PR makes available the
backend_service(cluster name) label which is decided in clusterimpl (since we are pre-A75). It is added to WRR per-call metrics.RELEASE NOTES: