-
Notifications
You must be signed in to change notification settings - Fork 4.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
stats: Add optional locality label in cluster_impl picker #7434
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7434 +/- ##
==========================================
+ Coverage 81.44% 81.51% +0.06%
==========================================
Files 352 352
Lines 26919 26927 +8
==========================================
+ Hits 21924 21949 +25
+ Misses 3804 3795 -9
+ Partials 1191 1183 -8
|
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 just nits.
f105d52
to
d508527
Compare
Got to all comments; no approval yet though so assigning back to you. |
@@ -150,7 +150,12 @@ func (h *clientStatsHandler) TagRPC(ctx context.Context, info *stats.RPCTagInfo) | |||
var labels *istats.Labels | |||
if labels = istats.GetLabels(ctx); labels == nil { | |||
labels = &istats.Labels{ | |||
TelemetryLabels: make(map[string]string), | |||
// The defaults for all the per call label from a plugin that |
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.
Nits:
"labelS"
"on the call path"
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.
Done for both.
This PR adds the setting of the optional locality label in the cluster_impl picker for per call metrics. It also sets the default of "" for this label in OpenTelemetry.
RELEASE NOTES: N/A