feat(spanner): add optional endpoint attribute to client metrics for location-aware routing#13740
feat(spanner): add optional endpoint attribute to client metrics for location-aware routing#13740rahul2393 wants to merge 1 commit into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request introduces support for exporting Cloud Spanner client-side metrics to a caller-owned OpenTelemetry destination. It adds the MetricsProvider interface and its implementations (DefaultMetricsProvider, NoopMetricsProvider, and CustomOpenTelemetryMetricsProvider), allowing custom metrics pipelines to be configured independently of the default Cloud Monitoring export. Additionally, it integrates endpoint routing labels for Spanner Omni and includes comprehensive test coverage. The reviewer feedback suggests a performance optimization in BuiltInMetricsConstant.java to pre-compute the common attribute keys as a static final field rather than streaming and mapping them on every call to baseAttributesFilter.
| private static Set<String> baseAttributesFilter(boolean includeEndpointAttribute) { | ||
| Set<String> attributesFilter = | ||
| BuiltInMetricsConstant.COMMON_ATTRIBUTES.stream() | ||
| .map(AttributeKey::getKey) | ||
| .collect(Collectors.toCollection(HashSet::new)); | ||
| if (includeEndpointAttribute) { | ||
| attributesFilter.add(ENDPOINT_KEY.getKey()); | ||
| } | ||
| return attributesFilter; | ||
| } |
There was a problem hiding this comment.
The set of common attribute keys is derived from COMMON_ATTRIBUTES which is a static constant. Instead of streaming, mapping, and collecting COMMON_ATTRIBUTES on every single call to baseAttributesFilter, we can pre-compute this set of keys once as a static final field and reuse it. This improves both readability and performance during initialization.
private static final Set<String> COMMON_ATTRIBUTE_KEYS =
BuiltInMetricsConstant.COMMON_ATTRIBUTES.stream()
.map(AttributeKey::getKey)
.collect(Collectors.toSet());
private static Set<String> baseAttributesFilter(boolean includeEndpointAttribute) {
Set<String> attributesFilter = new HashSet<>(COMMON_ATTRIBUTE_KEYS);
if (includeEndpointAttribute) {
attributesFilter.add(ENDPOINT_KEY.getKey());
}
return attributesFilter;
}…location-aware routing Adds the ENDPOINT_KEY metric attribute: KeyAwareChannel labels each attempt with the endpoint the location-aware fastpath routed it to, and the custom-export attempt views allow the label through their attribute filter. Cloud Monitoring views never include it. Stacked follow-up to the client metrics export change (googleapis#13679); until that merges this commit also carries the client metrics export work. Merge after googleapis#13679 lands.
a8b2e77 to
307e31a
Compare
Summary
Adds an optional
endpointattribute to Spanner client metrics attempt-level instruments. When Spanner Omni location-aware routing selects a specific endpoint for a request, that endpoint is recorded as a metric attribute, so operators can attribute client-side attempt latency and attempt counts to the routed endpoint.Scope
Stacked on top of #13741 (Client Metrics export). This PR adds only the
endpointattribute:ENDPOINT_KEYattribute plumbed through the custom-export views — attempt latency and attempt count only. Operation-level views and the Cloud Monitoring views deliberately drop it.KeyAwareChannelrecords the routed endpoint on the per-attempt tracer at endpoint-selection time.Stacking note
Until #13741 merges, this PR's diff also includes #13741's commit. Once #13741 lands, this branch will be rebased onto
mainfor a clean endpoint-only diff. Do not merge before #13741.