Skip to content

Implement grpc.lb.backend_service optional label #11990

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

Merged
merged 2 commits into from
Apr 21, 2025

Conversation

ejona86
Copy link
Member

@ejona86 ejona86 commented Apr 2, 2025

This completes gRFC A89. 7162d2d and fc86084 had already implemented the LB plumbing for the optional label on RPC metrics. This observes the value in OpenTelemetry and adds it to WRR metrics as well.

https://github.com/grpc/proposal/blob/master/A89-backend-service-metric-label.md

This completes gRFC A89. 7162d2d and fc86084 had already implemented
the LB plumbing for the optional label on RPC metrics. This observes the
value in OpenTelemetry and adds it to WRR metrics as well.

https://github.com/grpc/proposal/blob/master/A89-backend-service-metric-label.md
@ejona86
Copy link
Member Author

ejona86 commented Apr 9, 2025

@AgraVator, can you take a look?

Comment on lines +183 to +189
String backendService
= resolvedAddresses.getAttributes().get(NameResolver.ATTR_BACKEND_SERVICE);
if (backendService != null) {
this.backendService = backendService;
} else {
this.backendService = "";
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can think of exposing a getOrDefault() from the Attributes class

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically null could be stored as a value, and then we'd still get null here even with getOrDefault(). Some of our Key classes (but not Attributes) allow choosing a "default" value when it isn't present. That'd probably be better in general than a getOrDefault(). Even if I wanted to do that now, I wouldn't mix it into this PR.

Tests shouldn't share constants with code-under-test unless it is the
API contract.
@ejona86 ejona86 merged commit 9619453 into grpc:master Apr 21, 2025
16 checks passed
@ejona86 ejona86 deleted the backend-service-label branch April 21, 2025 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants