From 89703d5caee44ec5df310119862358750a3af2b9 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Tue, 17 Feb 2026 21:00:55 -0500 Subject: [PATCH 01/10] chore: introduce PeerInfo & MetadataExtractor Centralize sideband metadata collection using a new interceptor. Which gets injected into the GrpcCallContext channel. This provides the following benefits: - it works even if the end user sets their own channel provider - centralizes fetching of sideband metadata - removes the need for fetching directpath signals from grpc internals Change-Id: I42917074d65ccd7b8680f4a2a10c904b7646e4b6 --- google-cloud-bigtable/pom.xml | 1 - .../v2/stub/EnhancedBigtableStubSettings.java | 3 +- .../v2/stub/MetadataExtractorInterceptor.java | 192 ++++++++++++++++++ .../metrics/BigtableGrpcStreamTracer.java | 31 +-- .../data/v2/stub/metrics/BigtableTracer.java | 16 +- .../BigtableTracerStreamingCallable.java | 36 ++-- .../metrics/BigtableTracerUnaryCallable.java | 33 +-- .../v2/stub/metrics/BuiltinMetricsTracer.java | 70 +++---- .../data/v2/stub/metrics/CompositeTracer.java | 8 + .../data/v2/stub/metrics/MetricsTracer.java | 26 ++- .../bigtable/data/v2/stub/metrics/Util.java | 128 +++--------- .../metrics/BuiltinMetricsTracerTest.java | 6 +- .../data/v2/stub/metrics/UtilTest.java | 20 -- 13 files changed, 322 insertions(+), 248 deletions(-) create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java diff --git a/google-cloud-bigtable/pom.xml b/google-cloud-bigtable/pom.xml index f353a8c9af..b84d5b6c7b 100644 --- a/google-cloud-bigtable/pom.xml +++ b/google-cloud-bigtable/pom.xml @@ -147,7 +147,6 @@ io.grpc grpc-alts - runtime org.checkerframework diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java index d1fe259ea1..6a9dcdfbec 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java @@ -922,7 +922,8 @@ private Builder() { .setReverseScans(true) .setLastScannedRowResponses(true) .setDirectAccessRequested(DIRECT_PATH_ENABLED) - .setTrafficDirectorEnabled(DIRECT_PATH_ENABLED); + .setTrafficDirectorEnabled(DIRECT_PATH_ENABLED) + .setPeerInfo(true); } private Builder(EnhancedBigtableStubSettings settings) { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java new file mode 100644 index 0000000000..81141b4732 --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java @@ -0,0 +1,192 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub; + +import com.google.api.core.InternalApi; +import com.google.api.gax.grpc.GrpcCallContext; +import com.google.bigtable.v2.PeerInfo; +import com.google.bigtable.v2.ResponseParams; +import com.google.common.base.Strings; +import com.google.protobuf.InvalidProtocolBufferException; +import io.grpc.Attributes; +import io.grpc.CallOptions; +import io.grpc.Channel; +import io.grpc.ClientCall; +import io.grpc.ClientInterceptor; +import io.grpc.ClientInterceptors; +import io.grpc.ForwardingClientCall; +import io.grpc.ForwardingClientCallListener; +import io.grpc.Metadata; +import io.grpc.MethodDescriptor; +import io.grpc.Status; +import io.grpc.alts.AltsContextUtil; + +import javax.annotation.Nullable; +import java.util.Base64; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +@InternalApi +public class MetadataExtractorInterceptor implements ClientInterceptor { + private final SidebandData sidebandData = new SidebandData(); + + public GrpcCallContext injectInto(GrpcCallContext ctx) { + return ctx + .withChannel(ClientInterceptors.intercept(ctx.getChannel(), this)) + .withCallOptions(ctx.getCallOptions().withOption(SidebandData.KEY, sidebandData)); + } + + @Override + public ClientCall interceptCall(MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { + return new ForwardingClientCall.SimpleForwardingClientCall(channel.newCall(methodDescriptor, callOptions)) { + @Override + public void start(Listener responseListener, Metadata headers) { + sidebandData.reset(); + + super.start(new ForwardingClientCallListener.SimpleForwardingClientCallListener(responseListener) { + @Override + public void onHeaders(Metadata headers) { + sidebandData.onResponseHeaders(headers, getAttributes()); + super.onHeaders(headers); + } + + @Override + public void onClose(Status status, Metadata trailers) { + sidebandData.onClose(status, trailers); + super.onClose(status, trailers); + } + }, headers); + } + }; + } + + public SidebandData getSidebandData() { + return sidebandData; + } + + public static class SidebandData { + private static final CallOptions.Key KEY = CallOptions.Key.create("bigtable-sideband"); + + private static final Metadata.Key SERVER_TIMING_HEADER_KEY = + Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER); + private static final Pattern SERVER_TIMING_HEADER_PATTERN = Pattern.compile(".*dur=(?\\d+)"); + private static final Metadata.Key LOCATION_METADATA_KEY = + Metadata.Key.of("x-goog-ext-425905942-bin", Metadata.BINARY_BYTE_MARSHALLER); + private static final Metadata.Key PEER_INFO_KEY = + Metadata.Key.of("bigtable-peer-info", Metadata.ASCII_STRING_MARSHALLER); + + @Nullable + private volatile ResponseParams responseParams; + @Nullable + private volatile PeerInfo peerInfo; + @Nullable + private volatile Long gfeTiming; + + @Nullable + public ResponseParams getResponseParams() { + return responseParams; + } + + @Nullable + public PeerInfo getPeerInfo() { + return peerInfo; + } + + @Nullable + public Long getGfeTiming() { + return gfeTiming; + } + + + private void reset() { + responseParams = null; + peerInfo = null; + gfeTiming = null; + } + void onResponseHeaders(Metadata md, Attributes attributes) { + responseParams = extractResponseParams(md); + gfeTiming = extractGfeLatency(md); + peerInfo = extractPeerInfo(md, gfeTiming, attributes); + } + void onClose(Status status, Metadata trailers) { + if (responseParams == null) { + responseParams = extractResponseParams(trailers); + } + } + + @Nullable + private static Long extractGfeLatency(Metadata metadata) { + String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY); + if (serverTiming == null) { + return null; + } + Matcher matcher = SERVER_TIMING_HEADER_PATTERN.matcher(serverTiming); + // this should always be true + if (matcher.find()) { + return Long.parseLong(matcher.group("dur")); + } + return null; + } + + @Nullable + private static PeerInfo extractPeerInfo(Metadata metadata, Long gfeTiming, Attributes attributes) { + String encodedStr = metadata.get(PEER_INFO_KEY); + if (Strings.isNullOrEmpty(encodedStr)) { + return null; + } + + try { + byte[] decoded = Base64.getUrlDecoder().decode(encodedStr); + PeerInfo peerInfo = PeerInfo.parseFrom(decoded); + PeerInfo.TransportType effectiveTransport = peerInfo.getTransportType(); + + if (effectiveTransport == PeerInfo.TransportType.TRANSPORT_TYPE_UNKNOWN) { + boolean isAlts = AltsContextUtil.check(attributes); + if (isAlts) { + effectiveTransport = PeerInfo.TransportType.TRANSPORT_TYPE_DIRECT_ACCESS; + } else if (gfeTiming != null) { + effectiveTransport = PeerInfo.TransportType.TRANSPORT_TYPE_CLOUD_PATH; + } + } + if (effectiveTransport != PeerInfo.TransportType.TRANSPORT_TYPE_UNKNOWN) { + peerInfo = peerInfo.toBuilder() + .setTransportType(effectiveTransport) + .build(); + } + return peerInfo; + } catch (Exception e) { + throw new IllegalArgumentException( + "Failed to parse " + + PEER_INFO_KEY.name() + + " from the response header value: " + + encodedStr); + } + } + + + @Nullable + private static ResponseParams extractResponseParams(Metadata metadata) { + byte[] responseParams = metadata.get(LOCATION_METADATA_KEY); + if (responseParams != null) { + try { + return ResponseParams.parseFrom(responseParams); + } catch (InvalidProtocolBufferException e) { + } + } + return null; + } + } +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java index a364adbc46..6c613fb7cc 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java @@ -26,16 +26,9 @@ * asking gRPC to start an RPC and gRPC actually serializing that RPC. */ class BigtableGrpcStreamTracer extends ClientStreamTracer { - private static final String GRPC_LB_LOCALITY_KEY = "grpc.lb.locality"; - private static final String GRPC_LB_BACKEND_SERVICE_KEY = "grpc.lb.backend_service"; - - private final StreamInfo info; private final BigtableTracer tracer; - private volatile String backendService = null; - private volatile String locality = null; - public BigtableGrpcStreamTracer(StreamInfo info, BigtableTracer tracer) { - this.info = info; + public BigtableGrpcStreamTracer(BigtableTracer tracer) { this.tracer = tracer; } @@ -44,26 +37,6 @@ public void outboundMessageSent(int seqNo, long optionalWireSize, long optionalU tracer.grpcMessageSent(); } - @Override - public void addOptionalLabel(String key, String value) { - switch (key) { - case GRPC_LB_LOCALITY_KEY: - this.locality = value; - break; - case GRPC_LB_BACKEND_SERVICE_KEY: - this.backendService = value; - break; - } - - super.addOptionalLabel(key, value); - } - - @Override - public void streamClosed(Status status) { - tracer.setTransportAttrs(TransportAttrs.create(locality, backendService)); - super.streamClosed(status); - } - static class Factory extends ClientStreamTracer.Factory { private final BigtableTracer tracer; @@ -75,7 +48,7 @@ static class Factory extends ClientStreamTracer.Factory { @Override public ClientStreamTracer newClientStreamTracer( ClientStreamTracer.StreamInfo info, Metadata headers) { - return new BigtableGrpcStreamTracer(info, tracer); + return new BigtableGrpcStreamTracer(tracer); } } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java index 898d743cd9..a46b9de4bb 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java @@ -20,6 +20,8 @@ import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.tracing.ApiTracer; import com.google.api.gax.tracing.BaseApiTracer; +import com.google.cloud.bigtable.data.v2.stub.MetadataExtractorInterceptor; + import java.time.Duration; import javax.annotation.Nullable; @@ -74,7 +76,9 @@ public int getAttempt() { * Record the latency between Google's network receives the RPC and reads back the first byte of * the response from server-timing header. If server-timing header is missing, increment the * missing header count. + * @deprecated Use {@link #setSidebandData(MetadataExtractorInterceptor.SidebandData)} */ + @Deprecated public void recordGfeMetadata(@Nullable Long latency, @Nullable Throwable throwable) { // noop } @@ -84,15 +88,23 @@ public void batchRequestThrottled(long throttledTimeMs) { // noop } + public void setSidebandData(MetadataExtractorInterceptor.SidebandData sidebandData) { + // noop + } /** * Set the Bigtable zone and cluster so metrics can be tagged with location information. This will * be called in BuiltinMetricsTracer. + * @deprecated Use {@link #setSidebandData(MetadataExtractorInterceptor.SidebandData)} */ + @Deprecated public void setLocations(String zone, String cluster) { - // noop } - /** Set the underlying transport used to process the attempt */ + /** + * Set the underlying transport used to process the attempt + * @deprecated Use {@link #setSidebandData(MetadataExtractorInterceptor.SidebandData)} + */ + @Deprecated public void setTransportAttrs(BuiltinMetricsTracer.TransportAttrs attrs) {} @Deprecated diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java index 13b832b8b1..14333824ab 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java @@ -16,14 +16,18 @@ package com.google.cloud.bigtable.data.v2.stub.metrics; import com.google.api.core.InternalApi; +import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.GrpcResponseMetadata; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ResponseObserver; import com.google.api.gax.rpc.ServerStreamingCallable; import com.google.api.gax.rpc.StreamController; +import com.google.cloud.bigtable.data.v2.stub.MetadataExtractorInterceptor; import com.google.cloud.bigtable.data.v2.stub.SafeResponseObserver; import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; +import io.grpc.ClientInterceptors; + import java.util.concurrent.TimeUnit; import javax.annotation.Nonnull; @@ -37,8 +41,6 @@ *
  • -This class will also access trailers from {@link GrpcResponseMetadata} to record zone and * cluster ids. *
  • -Call {@link BigtableTracer#onRequest(int)} to record the request events in a stream. - *
  • -This class will also inject a {@link BigtableGrpcStreamTracer} that'll record the time an - * RPC spent in a grpc channel queue. *
  • This class is considered an internal implementation detail and not meant to be used by * applications. */ @@ -56,22 +58,26 @@ public BigtableTracerStreamingCallable( @Override public void call( RequestT request, ResponseObserver responseObserver, ApiCallContext context) { - final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); + GrpcCallContext grpcCtx = (GrpcCallContext) context; + + MetadataExtractorInterceptor metadataExtractor = new MetadataExtractorInterceptor(); + grpcCtx = metadataExtractor.injectInto(grpcCtx); + + // tracer should always be an instance of bigtable tracer if (context.getTracer() instanceof BigtableTracer) { BigtableTracer tracer = (BigtableTracer) context.getTracer(); + grpcCtx.withCallOptions( + grpcCtx.getCallOptions().withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer))); + BigtableTracerResponseObserver innerObserver = - new BigtableTracerResponseObserver<>(responseObserver, tracer, responseMetadata); + new BigtableTracerResponseObserver<>(responseObserver, tracer, metadataExtractor.getSidebandData()); if (context.getRetrySettings() != null) { tracer.setTotalTimeoutDuration(context.getRetrySettings().getTotalTimeoutDuration()); } - innerCallable.call( - request, - innerObserver, - Util.injectBigtableStreamTracer( - context, responseMetadata, (BigtableTracer) context.getTracer())); + innerCallable.call(request, innerObserver, grpcCtx); } else { - innerCallable.call(request, responseObserver, context); + innerCallable.call(request, responseObserver, grpcCtx); } } @@ -79,17 +85,17 @@ private class BigtableTracerResponseObserver extends SafeResponseObse private final BigtableTracer tracer; private final ResponseObserver outerObserver; - private final GrpcResponseMetadata responseMetadata; + private final MetadataExtractorInterceptor.SidebandData sidebandData; BigtableTracerResponseObserver( ResponseObserver observer, BigtableTracer tracer, - GrpcResponseMetadata metadata) { + MetadataExtractorInterceptor.SidebandData sidebandData) { super(observer); this.tracer = tracer; this.outerObserver = observer; - this.responseMetadata = metadata; + this.sidebandData = sidebandData; } @Override @@ -107,13 +113,13 @@ protected void onResponseImpl(ResponseT response) { @Override protected void onErrorImpl(Throwable t) { - Util.recordMetricsFromMetadata(responseMetadata, tracer, t); + tracer.setSidebandData(sidebandData); outerObserver.onError(t); } @Override protected void onCompleteImpl() { - Util.recordMetricsFromMetadata(responseMetadata, tracer, null); + tracer.setSidebandData(sidebandData); outerObserver.onComplete(); } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java index 37ba74bfdb..abd4ebf1da 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java @@ -19,9 +19,11 @@ import com.google.api.core.ApiFutureCallback; import com.google.api.core.ApiFutures; import com.google.api.core.InternalApi; +import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.GrpcResponseMetadata; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.UnaryCallable; +import com.google.cloud.bigtable.data.v2.stub.MetadataExtractorInterceptor; import com.google.common.base.Preconditions; import com.google.common.util.concurrent.MoreExecutors; import javax.annotation.Nonnull; @@ -35,8 +37,6 @@ * the gfe_header_missing_counter in this case. *
  • -This class will also access trailers from {@link GrpcResponseMetadata} to record zone and * cluster ids. - *
  • -This class will also inject a {@link BigtableGrpcStreamTracer} that'll record the time an - * RPC spent in a grpc channel queue. *
  • This class is considered an internal implementation detail and not meant to be used by * applications. */ @@ -52,46 +52,49 @@ public BigtableTracerUnaryCallable(@Nonnull UnaryCallable i @Override public ApiFuture futureCall(RequestT request, ApiCallContext context) { + MetadataExtractorInterceptor interceptor = new MetadataExtractorInterceptor(); + GrpcCallContext grpcCtx = interceptor.injectInto((GrpcCallContext) context); + // tracer should always be an instance of BigtableTracer if (context.getTracer() instanceof BigtableTracer) { BigtableTracer tracer = (BigtableTracer) context.getTracer(); - final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); + + grpcCtx.withCallOptions( + grpcCtx.getCallOptions().withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer))); + BigtableTracerUnaryCallback callback = - new BigtableTracerUnaryCallback( - (BigtableTracer) context.getTracer(), responseMetadata); + new BigtableTracerUnaryCallback<>( + (BigtableTracer) context.getTracer(), interceptor.getSidebandData()); if (context.getRetrySettings() != null) { tracer.setTotalTimeoutDuration(context.getRetrySettings().getTotalTimeoutDuration()); } ApiFuture future = - innerCallable.futureCall( - request, - Util.injectBigtableStreamTracer( - context, responseMetadata, (BigtableTracer) context.getTracer())); + innerCallable.futureCall(request, grpcCtx); ApiFutures.addCallback(future, callback, MoreExecutors.directExecutor()); return future; } else { - return innerCallable.futureCall(request, context); + return innerCallable.futureCall(request, grpcCtx); } } private class BigtableTracerUnaryCallback implements ApiFutureCallback { private final BigtableTracer tracer; - private final GrpcResponseMetadata responseMetadata; + private final MetadataExtractorInterceptor.SidebandData sidebandData; - BigtableTracerUnaryCallback(BigtableTracer tracer, GrpcResponseMetadata responseMetadata) { + BigtableTracerUnaryCallback(BigtableTracer tracer, MetadataExtractorInterceptor.SidebandData sidebandData) { this.tracer = tracer; - this.responseMetadata = responseMetadata; + this.sidebandData = sidebandData; } @Override public void onFailure(Throwable throwable) { - Util.recordMetricsFromMetadata(responseMetadata, tracer, throwable); + tracer.setSidebandData(sidebandData); } @Override public void onSuccess(ResponseT response) { - Util.recordMetricsFromMetadata(responseMetadata, tracer, null); + tracer.setSidebandData(sidebandData); } } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index e6ebad367a..9523a18bd7 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -35,11 +35,11 @@ import com.google.api.gax.retrying.ServerStreamingAttemptException; import com.google.api.gax.tracing.SpanName; import com.google.auto.value.AutoValue; +import com.google.bigtable.v2.PeerInfo; import com.google.cloud.bigtable.Version; +import com.google.cloud.bigtable.data.v2.stub.MetadataExtractorInterceptor; import com.google.common.base.Stopwatch; -import com.google.common.base.Strings; import com.google.common.math.IntMath; -import com.google.gson.Gson; import com.google.gson.reflect.TypeToken; import io.grpc.Deadline; import io.opentelemetry.api.common.Attributes; @@ -48,13 +48,12 @@ import io.opentelemetry.api.metrics.LongCounter; import java.time.Duration; import java.util.Map; +import java.util.Optional; import java.util.concurrent.CancellationException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; -import java.util.logging.Level; -import java.util.logging.Logger; import javax.annotation.Nullable; /** @@ -75,8 +74,6 @@ static TransportAttrs create(@Nullable String locality, @Nullable String backend } } - private static final Logger logger = Logger.getLogger(BuiltinMetricsTracer.class.getName()); - private static final Gson GSON = new Gson(); private static final TypeToken> LOCALITY_TYPE = new TypeToken>() {}; @@ -108,22 +105,18 @@ static TransportAttrs create(@Nullable String locality, @Nullable String backend private final AtomicInteger requestLeft = new AtomicInteger(0); - // Monitored resource labels private String tableId = ""; - private String zone = "global"; - private String cluster = ""; private final AtomicLong totalClientBlockingTime = new AtomicLong(0); private final Attributes baseAttributes; - private Long serverLatencies = null; private final AtomicLong grpcMessageSentDelay = new AtomicLong(0); private Deadline operationDeadline = null; private volatile long remainingDeadlineAtAttemptStart = 0; - private TransportAttrs transportAttrs = null; + private MetadataExtractorInterceptor.SidebandData sidebandData = null; // OpenCensus (and server) histogram buckets use [start, end), however OpenTelemetry uses (start, // end]. To work around this, we measure all the latencies in nanoseconds and convert them @@ -328,21 +321,8 @@ public int getAttempt() { } @Override - public void recordGfeMetadata(@Nullable Long latency, @Nullable Throwable throwable) { - if (latency != null) { - serverLatencies = latency; - } - } - - @Override - public void setLocations(String zone, String cluster) { - this.zone = zone; - this.cluster = cluster; - } - - @Override - public void setTransportAttrs(TransportAttrs attrs) { - this.transportAttrs = attrs; + public void setSidebandData(MetadataExtractorInterceptor.SidebandData sidebandData) { + this.sidebandData = sidebandData; } @Override @@ -390,8 +370,8 @@ private void recordOperationCompletion(@Nullable Throwable status) { Attributes attributes = baseAttributes.toBuilder() .put(TABLE_ID_KEY, tableId) - .put(CLUSTER_ID_KEY, cluster) - .put(ZONE_ID_KEY, zone) + .put(CLUSTER_ID_KEY, Util.formatClusterIdMetricLabel(sidebandData.getResponseParams())) + .put(ZONE_ID_KEY, Util.formatZoneIdMetricLabel(sidebandData.getResponseParams())) .put(METHOD_KEY, spanName.toString()) .put(CLIENT_NAME_KEY, NAME) .put(STREAMING_KEY, isStreaming) @@ -445,8 +425,8 @@ private void recordAttemptCompletion(@Nullable Throwable status) { Attributes attributes = baseAttributes.toBuilder() .put(TABLE_ID_KEY, tableId) - .put(CLUSTER_ID_KEY, cluster) - .put(ZONE_ID_KEY, zone) + .put(CLUSTER_ID_KEY, Util.formatClusterIdMetricLabel(sidebandData.getResponseParams())) + .put(ZONE_ID_KEY, Util.formatZoneIdMetricLabel(sidebandData.getResponseParams())) .put(METHOD_KEY, spanName.toString()) .put(CLIENT_NAME_KEY, NAME) .put(STREAMING_KEY, isStreaming) @@ -459,29 +439,25 @@ private void recordAttemptCompletion(@Nullable Throwable status) { attemptLatenciesHistogram.record( convertToMs(attemptTimer.elapsed(TimeUnit.NANOSECONDS)), attributes); - String transportType = "cloudpath"; + String transportTypeStr = "cloudpath"; String transportRegion = ""; String transportZone = ""; String transportSubzone = ""; - try { - if (transportAttrs != null && !Strings.isNullOrEmpty(transportAttrs.getLocality())) { - // only directpath has locality - transportType = "directpath"; - Map localityMap = - GSON.fromJson(transportAttrs.getLocality(), LOCALITY_TYPE); - transportRegion = localityMap.getOrDefault("region", ""); - transportZone = localityMap.getOrDefault("zone", ""); - transportSubzone = localityMap.getOrDefault("sub_zone", ""); - } - } catch (RuntimeException e) { - logger.log( - Level.WARNING, "Failed to parse transport locality: " + transportAttrs.getLocality(), e); + if (sidebandData != null) { + transportTypeStr = Util.formatTransportTypeMetricLabel(sidebandData.getPeerInfo()); + transportZone = Optional.ofNullable(sidebandData.getPeerInfo()) + .map(PeerInfo::getApplicationFrontendZone) + .orElse(""); + transportSubzone = Optional.ofNullable(sidebandData.getPeerInfo()) + .map(PeerInfo::getApplicationFrontendSubzone) + .orElse(""); } + attemptLatencies2Histogram.record( convertToMs(attemptTimer.elapsed(TimeUnit.NANOSECONDS)), attributes.toBuilder() - .put(TRANSPORT_TYPE, transportType) + .put(TRANSPORT_TYPE, transportTypeStr) .put(TRANSPORT_REGION, transportRegion) .put(TRANSPORT_ZONE, transportZone) .put(TRANSPORT_SUBZONE, transportSubzone) @@ -493,8 +469,8 @@ private void recordAttemptCompletion(@Nullable Throwable status) { remainingDeadlineHistogram.record(Math.max(0, remainingDeadlineAtAttemptStart), attributes); } - if (serverLatencies != null) { - serverLatenciesHistogram.record(serverLatencies, attributes); + if (sidebandData.getGfeTiming() != null) { + serverLatenciesHistogram.record(sidebandData.getGfeTiming(), attributes); connectivityErrorCounter.add(0, attributes); } else { connectivityErrorCounter.add(1, attributes); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java index f6d0858459..8c1a58c152 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java @@ -19,6 +19,7 @@ import com.google.api.core.ObsoleteApi; import com.google.api.gax.tracing.ApiTracer; +import com.google.cloud.bigtable.data.v2.stub.MetadataExtractorInterceptor; import com.google.common.collect.ImmutableList; import java.util.ArrayList; import java.util.List; @@ -225,6 +226,13 @@ public void setTransportAttrs(BuiltinMetricsTracer.TransportAttrs attrs) { } } + @Override + public void setSidebandData(MetadataExtractorInterceptor.SidebandData sidebandData) { + for (BigtableTracer bigtableTracer : bigtableTracers) { + bigtableTracer.setSidebandData(sidebandData); + } + } + @Override public void onRequest(int requestCount) { for (BigtableTracer tracer : bigtableTracers) { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java index c322b75df8..c32d4c2ba2 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java @@ -21,6 +21,7 @@ import com.google.api.gax.retrying.ServerStreamingAttemptException; import com.google.api.gax.tracing.ApiTracerFactory.OperationType; import com.google.api.gax.tracing.SpanName; +import com.google.cloud.bigtable.data.v2.stub.MetadataExtractorInterceptor; import com.google.common.base.Stopwatch; import io.opencensus.stats.MeasureMap; import io.opencensus.stats.StatsRecorder; @@ -63,6 +64,7 @@ class MetricsTracer extends BigtableTracer { private volatile boolean reportBatchingLatency = false; private volatile long batchThrottledLatency = 0; + private MetadataExtractorInterceptor.SidebandData sidebandData; MetricsTracer( OperationType operationType, @@ -187,6 +189,14 @@ private void recordAttemptCompletion(@Nullable Throwable throwable) { RpcMeasureConstants.BIGTABLE_ATTEMPT_LATENCY, attemptTimer.elapsed(TimeUnit.MILLISECONDS)); + if (sidebandData != null && sidebandData.getGfeTiming() != null) { + measures + .put(RpcMeasureConstants.BIGTABLE_GFE_LATENCY, sidebandData.getGfeTiming()) + .put(RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT, 0L); + } else { + measures.put(RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT, 1L); + } + if (reportBatchingLatency) { measures.put(RpcMeasureConstants.BIGTABLE_BATCH_THROTTLED_TIME, batchThrottledLatency); @@ -226,20 +236,8 @@ public int getAttempt() { } @Override - public void recordGfeMetadata(@Nullable Long latency, @Nullable Throwable throwable) { - MeasureMap measures = stats.newMeasureMap(); - if (latency != null) { - measures - .put(RpcMeasureConstants.BIGTABLE_GFE_LATENCY, latency) - .put(RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT, 0L); - } else { - measures.put(RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT, 1L); - } - measures.record( - newTagCtxBuilder() - .putLocal( - RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create(Util.extractStatus(throwable))) - .build()); + public void setSidebandData(MetadataExtractorInterceptor.SidebandData sidebandData) { + this.sidebandData = sidebandData; } @Override diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index 9ba2d39c49..fc271fb19e 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -29,6 +29,7 @@ import com.google.bigtable.v2.MaterializedViewName; import com.google.bigtable.v2.MutateRowRequest; import com.google.bigtable.v2.MutateRowsRequest; +import com.google.bigtable.v2.PeerInfo; import com.google.bigtable.v2.ReadChangeStreamRequest; import com.google.bigtable.v2.ReadModifyWriteRowRequest; import com.google.bigtable.v2.ReadRowsRequest; @@ -36,6 +37,7 @@ import com.google.bigtable.v2.SampleRowKeysRequest; import com.google.bigtable.v2.TableName; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; +import com.google.common.base.Strings; import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableMap; import com.google.protobuf.InvalidProtocolBufferException; @@ -57,7 +59,9 @@ import java.time.temporal.ChronoUnit; import java.util.Arrays; import java.util.List; +import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; @@ -74,12 +78,6 @@ public class Util { static final Metadata.Key ATTEMPT_EPOCH_KEY = Metadata.Key.of("bigtable-client-attempt-epoch-usec", Metadata.ASCII_STRING_MARSHALLER); - private static final Metadata.Key SERVER_TIMING_HEADER_KEY = - Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER); - private static final Pattern SERVER_TIMING_HEADER_PATTERN = Pattern.compile(".*dur=(?\\d+)"); - static final Metadata.Key LOCATION_METADATA_KEY = - Metadata.Key.of("x-goog-ext-425905942-bin", Metadata.BINARY_BYTE_MARSHALLER); - /** Convert an exception into a value that can be used to create an OpenCensus tag value. */ public static String extractStatus(@Nullable Throwable error) { final String statusString; @@ -101,25 +99,6 @@ public static String extractStatus(@Nullable Throwable error) { return statusString; } - /** - * Await the result of the future and convert it into a value that can be used as an OpenCensus - * tag value. - */ - static TagValue extractStatusFromFuture(Future future) { - Throwable error = null; - - try { - future.get(); - } catch (InterruptedException e) { - error = e; - Thread.currentThread().interrupt(); - } catch (ExecutionException e) { - error = e.getCause(); - } catch (RuntimeException e) { - error = e; - } - return TagValue.create(extractStatus(error)); - } static String extractTableId(Object request) { String tableName = null; @@ -179,84 +158,6 @@ static Map> createStatsHeaders(ApiCallContext apiCallContex return headers.build(); } - private static Long getGfeLatency(@Nullable Metadata metadata) { - if (metadata == null) { - return null; - } - String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY); - if (serverTiming == null) { - return null; - } - Matcher matcher = SERVER_TIMING_HEADER_PATTERN.matcher(serverTiming); - // this should always be true - if (matcher.find()) { - long latency = Long.valueOf(matcher.group("dur")); - return latency; - } - return null; - } - - private static ResponseParams getResponseParams(@Nullable Metadata metadata) { - if (metadata == null) { - return null; - } - byte[] responseParams = metadata.get(Util.LOCATION_METADATA_KEY); - if (responseParams != null) { - try { - return ResponseParams.parseFrom(responseParams); - } catch (InvalidProtocolBufferException e) { - } - } - return null; - } - - static void recordMetricsFromMetadata( - GrpcResponseMetadata responseMetadata, BigtableTracer tracer, Throwable throwable) { - Metadata metadata = responseMetadata.getMetadata(); - - // Get the response params from the metadata. Check both headers and trailers - // because in different environments the metadata could be returned in headers or trailers - @Nullable ResponseParams responseParams = getResponseParams(responseMetadata.getMetadata()); - if (responseParams == null) { - responseParams = getResponseParams(responseMetadata.getTrailingMetadata()); - } - // Set tracer locations if response params is not null - if (responseParams != null) { - tracer.setLocations(responseParams.getZoneId(), responseParams.getClusterId()); - } - - // server-timing metric will be added through GrpcResponseMetadata#onHeaders(Metadata), - // so it's not checking trailing metadata here. - @Nullable Long latency = getGfeLatency(metadata); - // For direct path, we won't see GFE server-timing header. However, if we received the - // location info, we know that there isn't a connectivity issue. Set the latency to - // 0 so gfe missing header won't get incremented. - if (responseParams != null && latency == null) { - latency = 0L; - } - // Record gfe metrics - tracer.recordGfeMetadata(latency, throwable); - } - - /** - * This method bridges gRPC stream tracing to bigtable tracing by adding a {@link - * io.grpc.ClientStreamTracer} to the callContext. - */ - static GrpcCallContext injectBigtableStreamTracer( - ApiCallContext context, GrpcResponseMetadata responseMetadata, BigtableTracer tracer) { - if (context instanceof GrpcCallContext) { - GrpcCallContext callContext = (GrpcCallContext) context; - CallOptions callOptions = callContext.getCallOptions(); - return responseMetadata.addHandlers( - callContext.withCallOptions( - callOptions.withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer)))); - } else { - // context should always be an instance of GrpcCallContext. If not throw an exception - // so we can see what class context is. - throw new RuntimeException("Unexpected context class: " + context.getClass().getName()); - } - } - public static OpenTelemetrySdk newInternalOpentelemetry( EnhancedBigtableStubSettings settings, Credentials credentials, @@ -285,4 +186,25 @@ public static OpenTelemetrySdk newInternalOpentelemetry( .build()); return OpenTelemetrySdk.builder().setMeterProvider(meterProviderBuilder.build()).build(); } + + public static String formatTransportTypeMetricLabel(PeerInfo peerInfo) { + return Optional.ofNullable(peerInfo) + .map(PeerInfo::getTransportType) + .orElse(PeerInfo.TransportType.TRANSPORT_TYPE_UNKNOWN) + .name().replace("TRANSPORT_TYPE_", "").toLowerCase(Locale.ENGLISH); + } + + public static String formatClusterIdMetricLabel(ResponseParams params) { + if (params == null || Strings.isNullOrEmpty(params.getClusterId())) { + return ""; + } + return params.getClusterId(); + } + + public static String formatZoneIdMetricLabel(ResponseParams params) { + if (params == null || Strings.isNullOrEmpty(params.getZoneId())) { + return "global"; + } + return params.getZoneId(); + } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java index df63ff8019..73c2e3db1b 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java @@ -73,6 +73,7 @@ import com.google.cloud.bigtable.data.v2.stub.BigtableClientContext; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStub; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; +import com.google.cloud.bigtable.data.v2.stub.MetadataExtractorInterceptor; import com.google.common.base.Stopwatch; import com.google.common.collect.Comparators; import com.google.common.collect.Range; @@ -136,6 +137,9 @@ @RunWith(JUnit4.class) public class BuiltinMetricsTracerTest { + private static final Metadata.Key LOCATION_METADATA_KEY = + Metadata.Key.of("x-goog-ext-425905942-bin", Metadata.BINARY_BYTE_MARSHALLER); + private static final String PROJECT_ID = "fake-project"; private static final String INSTANCE_ID = "fake-instance"; private static final String APP_PROFILE_ID = "default"; @@ -211,7 +215,7 @@ public void sendHeaders(Metadata headers) { ResponseParams params = ResponseParams.newBuilder().setZoneId(ZONE).setClusterId(CLUSTER).build(); byte[] byteArray = params.toByteArray(); - headers.put(Util.LOCATION_METADATA_KEY, byteArray); + headers.put(LOCATION_METADATA_KEY, byteArray); super.sendHeaders(headers); } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/UtilTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/UtilTest.java index 3c0fb4e617..9e46b3c7f5 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/UtilTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/UtilTest.java @@ -34,11 +34,6 @@ public void testOk() { assertThat(tagValue.asString()).isEqualTo("OK"); } - @Test - public void testOkFuture() { - TagValue tagValue = Util.extractStatusFromFuture(Futures.immediateFuture(null)); - assertThat(tagValue.asString()).isEqualTo("OK"); - } @Test public void testError() { @@ -48,19 +43,4 @@ public void testError() { TagValue tagValue = TagValue.create(Util.extractStatus(error)); assertThat(tagValue.asString()).isEqualTo("DEADLINE_EXCEEDED"); } - - @Test - public void testErrorFuture() { - DeadlineExceededException error = - new DeadlineExceededException( - "Deadline exceeded", null, GrpcStatusCode.of(Status.Code.DEADLINE_EXCEEDED), true); - TagValue tagValue = Util.extractStatusFromFuture(Futures.immediateFailedFuture(error)); - assertThat(tagValue.asString()).isEqualTo("DEADLINE_EXCEEDED"); - } - - @Test - public void testCancelledFuture() { - TagValue tagValue = Util.extractStatusFromFuture(Futures.immediateCancelledFuture()); - assertThat(tagValue.asString()).isEqualTo("CANCELLED"); - } } From 06f15f71e3fdda1c2512e01962e8d504984d3bc6 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Tue, 17 Feb 2026 21:01:39 -0500 Subject: [PATCH 02/10] format Change-Id: Id9dfd28ca4a3f9e98474b33c98895e05b830b410 --- .../v2/stub/MetadataExtractorInterceptor.java | 271 +++++++++--------- .../metrics/BigtableGrpcStreamTracer.java | 2 - .../data/v2/stub/metrics/BigtableTracer.java | 8 +- .../BigtableTracerStreamingCallable.java | 10 +- .../metrics/BigtableTracerUnaryCallable.java | 10 +- .../v2/stub/metrics/BuiltinMetricsTracer.java | 6 +- .../data/v2/stub/metrics/MetricsTracer.java | 4 +- .../bigtable/data/v2/stub/metrics/Util.java | 18 +- .../metrics/BuiltinMetricsTracerTest.java | 3 +- .../data/v2/stub/metrics/UtilTest.java | 2 - 10 files changed, 164 insertions(+), 170 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java index 81141b4732..f126700dc8 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java @@ -33,160 +33,161 @@ import io.grpc.MethodDescriptor; import io.grpc.Status; import io.grpc.alts.AltsContextUtil; - -import javax.annotation.Nullable; import java.util.Base64; import java.util.regex.Matcher; import java.util.regex.Pattern; +import javax.annotation.Nullable; @InternalApi public class MetadataExtractorInterceptor implements ClientInterceptor { - private final SidebandData sidebandData = new SidebandData(); - - public GrpcCallContext injectInto(GrpcCallContext ctx) { - return ctx - .withChannel(ClientInterceptors.intercept(ctx.getChannel(), this)) - .withCallOptions(ctx.getCallOptions().withOption(SidebandData.KEY, sidebandData)); + private final SidebandData sidebandData = new SidebandData(); + + public GrpcCallContext injectInto(GrpcCallContext ctx) { + return ctx.withChannel(ClientInterceptors.intercept(ctx.getChannel(), this)) + .withCallOptions(ctx.getCallOptions().withOption(SidebandData.KEY, sidebandData)); + } + + @Override + public ClientCall interceptCall( + MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { + return new ForwardingClientCall.SimpleForwardingClientCall( + channel.newCall(methodDescriptor, callOptions)) { + @Override + public void start(Listener responseListener, Metadata headers) { + sidebandData.reset(); + + super.start( + new ForwardingClientCallListener.SimpleForwardingClientCallListener( + responseListener) { + @Override + public void onHeaders(Metadata headers) { + sidebandData.onResponseHeaders(headers, getAttributes()); + super.onHeaders(headers); + } + + @Override + public void onClose(Status status, Metadata trailers) { + sidebandData.onClose(status, trailers); + super.onClose(status, trailers); + } + }, + headers); + } + }; + } + + public SidebandData getSidebandData() { + return sidebandData; + } + + public static class SidebandData { + private static final CallOptions.Key KEY = + CallOptions.Key.create("bigtable-sideband"); + + private static final Metadata.Key SERVER_TIMING_HEADER_KEY = + Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER); + private static final Pattern SERVER_TIMING_HEADER_PATTERN = + Pattern.compile(".*dur=(?\\d+)"); + private static final Metadata.Key LOCATION_METADATA_KEY = + Metadata.Key.of("x-goog-ext-425905942-bin", Metadata.BINARY_BYTE_MARSHALLER); + private static final Metadata.Key PEER_INFO_KEY = + Metadata.Key.of("bigtable-peer-info", Metadata.ASCII_STRING_MARSHALLER); + + @Nullable private volatile ResponseParams responseParams; + @Nullable private volatile PeerInfo peerInfo; + @Nullable private volatile Long gfeTiming; + + @Nullable + public ResponseParams getResponseParams() { + return responseParams; } - @Override - public ClientCall interceptCall(MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { - return new ForwardingClientCall.SimpleForwardingClientCall(channel.newCall(methodDescriptor, callOptions)) { - @Override - public void start(Listener responseListener, Metadata headers) { - sidebandData.reset(); - - super.start(new ForwardingClientCallListener.SimpleForwardingClientCallListener(responseListener) { - @Override - public void onHeaders(Metadata headers) { - sidebandData.onResponseHeaders(headers, getAttributes()); - super.onHeaders(headers); - } - - @Override - public void onClose(Status status, Metadata trailers) { - sidebandData.onClose(status, trailers); - super.onClose(status, trailers); - } - }, headers); - } - }; + @Nullable + public PeerInfo getPeerInfo() { + return peerInfo; } - public SidebandData getSidebandData() { - return sidebandData; + @Nullable + public Long getGfeTiming() { + return gfeTiming; } - public static class SidebandData { - private static final CallOptions.Key KEY = CallOptions.Key.create("bigtable-sideband"); - - private static final Metadata.Key SERVER_TIMING_HEADER_KEY = - Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER); - private static final Pattern SERVER_TIMING_HEADER_PATTERN = Pattern.compile(".*dur=(?\\d+)"); - private static final Metadata.Key LOCATION_METADATA_KEY = - Metadata.Key.of("x-goog-ext-425905942-bin", Metadata.BINARY_BYTE_MARSHALLER); - private static final Metadata.Key PEER_INFO_KEY = - Metadata.Key.of("bigtable-peer-info", Metadata.ASCII_STRING_MARSHALLER); - - @Nullable - private volatile ResponseParams responseParams; - @Nullable - private volatile PeerInfo peerInfo; - @Nullable - private volatile Long gfeTiming; - - @Nullable - public ResponseParams getResponseParams() { - return responseParams; - } - - @Nullable - public PeerInfo getPeerInfo() { - return peerInfo; - } + private void reset() { + responseParams = null; + peerInfo = null; + gfeTiming = null; + } - @Nullable - public Long getGfeTiming() { - return gfeTiming; - } + void onResponseHeaders(Metadata md, Attributes attributes) { + responseParams = extractResponseParams(md); + gfeTiming = extractGfeLatency(md); + peerInfo = extractPeerInfo(md, gfeTiming, attributes); + } + void onClose(Status status, Metadata trailers) { + if (responseParams == null) { + responseParams = extractResponseParams(trailers); + } + } - private void reset() { - responseParams = null; - peerInfo = null; - gfeTiming = null; - } - void onResponseHeaders(Metadata md, Attributes attributes) { - responseParams = extractResponseParams(md); - gfeTiming = extractGfeLatency(md); - peerInfo = extractPeerInfo(md, gfeTiming, attributes); - } - void onClose(Status status, Metadata trailers) { - if (responseParams == null) { - responseParams = extractResponseParams(trailers); - } - } + @Nullable + private static Long extractGfeLatency(Metadata metadata) { + String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY); + if (serverTiming == null) { + return null; + } + Matcher matcher = SERVER_TIMING_HEADER_PATTERN.matcher(serverTiming); + // this should always be true + if (matcher.find()) { + return Long.parseLong(matcher.group("dur")); + } + return null; + } - @Nullable - private static Long extractGfeLatency(Metadata metadata) { - String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY); - if (serverTiming == null) { - return null; - } - Matcher matcher = SERVER_TIMING_HEADER_PATTERN.matcher(serverTiming); - // this should always be true - if (matcher.find()) { - return Long.parseLong(matcher.group("dur")); - } - return null; + @Nullable + private static PeerInfo extractPeerInfo( + Metadata metadata, Long gfeTiming, Attributes attributes) { + String encodedStr = metadata.get(PEER_INFO_KEY); + if (Strings.isNullOrEmpty(encodedStr)) { + return null; + } + + try { + byte[] decoded = Base64.getUrlDecoder().decode(encodedStr); + PeerInfo peerInfo = PeerInfo.parseFrom(decoded); + PeerInfo.TransportType effectiveTransport = peerInfo.getTransportType(); + + if (effectiveTransport == PeerInfo.TransportType.TRANSPORT_TYPE_UNKNOWN) { + boolean isAlts = AltsContextUtil.check(attributes); + if (isAlts) { + effectiveTransport = PeerInfo.TransportType.TRANSPORT_TYPE_DIRECT_ACCESS; + } else if (gfeTiming != null) { + effectiveTransport = PeerInfo.TransportType.TRANSPORT_TYPE_CLOUD_PATH; + } } - - @Nullable - private static PeerInfo extractPeerInfo(Metadata metadata, Long gfeTiming, Attributes attributes) { - String encodedStr = metadata.get(PEER_INFO_KEY); - if (Strings.isNullOrEmpty(encodedStr)) { - return null; - } - - try { - byte[] decoded = Base64.getUrlDecoder().decode(encodedStr); - PeerInfo peerInfo = PeerInfo.parseFrom(decoded); - PeerInfo.TransportType effectiveTransport = peerInfo.getTransportType(); - - if (effectiveTransport == PeerInfo.TransportType.TRANSPORT_TYPE_UNKNOWN) { - boolean isAlts = AltsContextUtil.check(attributes); - if (isAlts) { - effectiveTransport = PeerInfo.TransportType.TRANSPORT_TYPE_DIRECT_ACCESS; - } else if (gfeTiming != null) { - effectiveTransport = PeerInfo.TransportType.TRANSPORT_TYPE_CLOUD_PATH; - } - } - if (effectiveTransport != PeerInfo.TransportType.TRANSPORT_TYPE_UNKNOWN) { - peerInfo = peerInfo.toBuilder() - .setTransportType(effectiveTransport) - .build(); - } - return peerInfo; - } catch (Exception e) { - throw new IllegalArgumentException( - "Failed to parse " - + PEER_INFO_KEY.name() - + " from the response header value: " - + encodedStr); - } + if (effectiveTransport != PeerInfo.TransportType.TRANSPORT_TYPE_UNKNOWN) { + peerInfo = peerInfo.toBuilder().setTransportType(effectiveTransport).build(); } + return peerInfo; + } catch (Exception e) { + throw new IllegalArgumentException( + "Failed to parse " + + PEER_INFO_KEY.name() + + " from the response header value: " + + encodedStr); + } + } - - @Nullable - private static ResponseParams extractResponseParams(Metadata metadata) { - byte[] responseParams = metadata.get(LOCATION_METADATA_KEY); - if (responseParams != null) { - try { - return ResponseParams.parseFrom(responseParams); - } catch (InvalidProtocolBufferException e) { - } - } - return null; + @Nullable + private static ResponseParams extractResponseParams(Metadata metadata) { + byte[] responseParams = metadata.get(LOCATION_METADATA_KEY); + if (responseParams != null) { + try { + return ResponseParams.parseFrom(responseParams); + } catch (InvalidProtocolBufferException e) { } + } + return null; } + } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java index 6c613fb7cc..9b220c1de3 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java @@ -15,10 +15,8 @@ */ package com.google.cloud.bigtable.data.v2.stub.metrics; -import com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsTracer.TransportAttrs; import io.grpc.ClientStreamTracer; import io.grpc.Metadata; -import io.grpc.Status; /** * Records the time a request is enqueued in a grpc channel queue. This a bridge between gRPC stream diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java index a46b9de4bb..0c00495eaa 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java @@ -21,7 +21,6 @@ import com.google.api.gax.tracing.ApiTracer; import com.google.api.gax.tracing.BaseApiTracer; import com.google.cloud.bigtable.data.v2.stub.MetadataExtractorInterceptor; - import java.time.Duration; import javax.annotation.Nullable; @@ -76,6 +75,7 @@ public int getAttempt() { * Record the latency between Google's network receives the RPC and reads back the first byte of * the response from server-timing header. If server-timing header is missing, increment the * missing header count. + * * @deprecated Use {@link #setSidebandData(MetadataExtractorInterceptor.SidebandData)} */ @Deprecated @@ -91,17 +91,19 @@ public void batchRequestThrottled(long throttledTimeMs) { public void setSidebandData(MetadataExtractorInterceptor.SidebandData sidebandData) { // noop } + /** * Set the Bigtable zone and cluster so metrics can be tagged with location information. This will * be called in BuiltinMetricsTracer. + * * @deprecated Use {@link #setSidebandData(MetadataExtractorInterceptor.SidebandData)} */ @Deprecated - public void setLocations(String zone, String cluster) { - } + public void setLocations(String zone, String cluster) {} /** * Set the underlying transport used to process the attempt + * * @deprecated Use {@link #setSidebandData(MetadataExtractorInterceptor.SidebandData)} */ @Deprecated diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java index 14333824ab..ef60cee906 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java @@ -26,8 +26,6 @@ import com.google.cloud.bigtable.data.v2.stub.SafeResponseObserver; import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; -import io.grpc.ClientInterceptors; - import java.util.concurrent.TimeUnit; import javax.annotation.Nonnull; @@ -63,15 +61,17 @@ public void call( MetadataExtractorInterceptor metadataExtractor = new MetadataExtractorInterceptor(); grpcCtx = metadataExtractor.injectInto(grpcCtx); - // tracer should always be an instance of bigtable tracer if (context.getTracer() instanceof BigtableTracer) { BigtableTracer tracer = (BigtableTracer) context.getTracer(); grpcCtx.withCallOptions( - grpcCtx.getCallOptions().withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer))); + grpcCtx + .getCallOptions() + .withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer))); BigtableTracerResponseObserver innerObserver = - new BigtableTracerResponseObserver<>(responseObserver, tracer, metadataExtractor.getSidebandData()); + new BigtableTracerResponseObserver<>( + responseObserver, tracer, metadataExtractor.getSidebandData()); if (context.getRetrySettings() != null) { tracer.setTotalTimeoutDuration(context.getRetrySettings().getTotalTimeoutDuration()); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java index abd4ebf1da..1e96fbe58f 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java @@ -60,7 +60,9 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) BigtableTracer tracer = (BigtableTracer) context.getTracer(); grpcCtx.withCallOptions( - grpcCtx.getCallOptions().withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer))); + grpcCtx + .getCallOptions() + .withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer))); BigtableTracerUnaryCallback callback = new BigtableTracerUnaryCallback<>( @@ -68,8 +70,7 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) if (context.getRetrySettings() != null) { tracer.setTotalTimeoutDuration(context.getRetrySettings().getTotalTimeoutDuration()); } - ApiFuture future = - innerCallable.futureCall(request, grpcCtx); + ApiFuture future = innerCallable.futureCall(request, grpcCtx); ApiFutures.addCallback(future, callback, MoreExecutors.directExecutor()); return future; } else { @@ -82,7 +83,8 @@ private class BigtableTracerUnaryCallback implements ApiFutureCallbac private final BigtableTracer tracer; private final MetadataExtractorInterceptor.SidebandData sidebandData; - BigtableTracerUnaryCallback(BigtableTracer tracer, MetadataExtractorInterceptor.SidebandData sidebandData) { + BigtableTracerUnaryCallback( + BigtableTracer tracer, MetadataExtractorInterceptor.SidebandData sidebandData) { this.tracer = tracer; this.sidebandData = sidebandData; } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 9523a18bd7..6bdc063177 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -446,10 +446,12 @@ private void recordAttemptCompletion(@Nullable Throwable status) { if (sidebandData != null) { transportTypeStr = Util.formatTransportTypeMetricLabel(sidebandData.getPeerInfo()); - transportZone = Optional.ofNullable(sidebandData.getPeerInfo()) + transportZone = + Optional.ofNullable(sidebandData.getPeerInfo()) .map(PeerInfo::getApplicationFrontendZone) .orElse(""); - transportSubzone = Optional.ofNullable(sidebandData.getPeerInfo()) + transportSubzone = + Optional.ofNullable(sidebandData.getPeerInfo()) .map(PeerInfo::getApplicationFrontendSubzone) .orElse(""); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java index c32d4c2ba2..53b4ca87a8 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java @@ -191,8 +191,8 @@ private void recordAttemptCompletion(@Nullable Throwable throwable) { if (sidebandData != null && sidebandData.getGfeTiming() != null) { measures - .put(RpcMeasureConstants.BIGTABLE_GFE_LATENCY, sidebandData.getGfeTiming()) - .put(RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT, 0L); + .put(RpcMeasureConstants.BIGTABLE_GFE_LATENCY, sidebandData.getGfeTiming()) + .put(RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT, 0L); } else { measures.put(RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT, 1L); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index fc271fb19e..ce16979abc 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -16,8 +16,6 @@ package com.google.cloud.bigtable.data.v2.stub.metrics; import com.google.api.core.InternalApi; -import com.google.api.gax.grpc.GrpcCallContext; -import com.google.api.gax.grpc.GrpcResponseMetadata; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ApiException; import com.google.api.gax.rpc.StatusCode; @@ -40,13 +38,10 @@ import com.google.common.base.Strings; import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableMap; -import com.google.protobuf.InvalidProtocolBufferException; -import io.grpc.CallOptions; import io.grpc.Metadata; import io.grpc.Status; import io.grpc.StatusException; import io.grpc.StatusRuntimeException; -import io.opencensus.tags.TagValue; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.metrics.InstrumentSelector; import io.opentelemetry.sdk.metrics.SdkMeterProvider; @@ -63,11 +58,7 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.CancellationException; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import javax.annotation.Nullable; /** Utilities to help integrating with OpenCensus. */ @@ -99,7 +90,6 @@ public static String extractStatus(@Nullable Throwable error) { return statusString; } - static String extractTableId(Object request) { String tableName = null; String authorizedViewName = null; @@ -189,9 +179,11 @@ public static OpenTelemetrySdk newInternalOpentelemetry( public static String formatTransportTypeMetricLabel(PeerInfo peerInfo) { return Optional.ofNullable(peerInfo) - .map(PeerInfo::getTransportType) - .orElse(PeerInfo.TransportType.TRANSPORT_TYPE_UNKNOWN) - .name().replace("TRANSPORT_TYPE_", "").toLowerCase(Locale.ENGLISH); + .map(PeerInfo::getTransportType) + .orElse(PeerInfo.TransportType.TRANSPORT_TYPE_UNKNOWN) + .name() + .replace("TRANSPORT_TYPE_", "") + .toLowerCase(Locale.ENGLISH); } public static String formatClusterIdMetricLabel(ResponseParams params) { diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java index 73c2e3db1b..3d0e6425d9 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java @@ -73,7 +73,6 @@ import com.google.cloud.bigtable.data.v2.stub.BigtableClientContext; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStub; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; -import com.google.cloud.bigtable.data.v2.stub.MetadataExtractorInterceptor; import com.google.common.base.Stopwatch; import com.google.common.collect.Comparators; import com.google.common.collect.Range; @@ -138,7 +137,7 @@ @RunWith(JUnit4.class) public class BuiltinMetricsTracerTest { private static final Metadata.Key LOCATION_METADATA_KEY = - Metadata.Key.of("x-goog-ext-425905942-bin", Metadata.BINARY_BYTE_MARSHALLER); + Metadata.Key.of("x-goog-ext-425905942-bin", Metadata.BINARY_BYTE_MARSHALLER); private static final String PROJECT_ID = "fake-project"; private static final String INSTANCE_ID = "fake-instance"; diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/UtilTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/UtilTest.java index 9e46b3c7f5..824d8be307 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/UtilTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/UtilTest.java @@ -19,7 +19,6 @@ import com.google.api.gax.grpc.GrpcStatusCode; import com.google.api.gax.rpc.DeadlineExceededException; -import com.google.common.util.concurrent.Futures; import io.grpc.Status; import io.opencensus.tags.TagValue; import org.junit.Test; @@ -34,7 +33,6 @@ public void testOk() { assertThat(tagValue.asString()).isEqualTo("OK"); } - @Test public void testError() { DeadlineExceededException error = From 9b5dd311b9d54a6d3ce5b039a4fd1b9d041d8d9f Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Tue, 17 Feb 2026 21:11:26 -0500 Subject: [PATCH 03/10] oops Change-Id: I87f110743cd6261e07b59bdcf2bc005af0916d35 --- .../data/v2/stub/metrics/BigtableTracerStreamingCallable.java | 2 +- .../data/v2/stub/metrics/BigtableTracerUnaryCallable.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java index ef60cee906..73b8a268bf 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java @@ -64,7 +64,7 @@ public void call( // tracer should always be an instance of bigtable tracer if (context.getTracer() instanceof BigtableTracer) { BigtableTracer tracer = (BigtableTracer) context.getTracer(); - grpcCtx.withCallOptions( + grpcCtx = grpcCtx.withCallOptions( grpcCtx .getCallOptions() .withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer))); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java index 1e96fbe58f..7e2aaf4f00 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java @@ -59,7 +59,7 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) if (context.getTracer() instanceof BigtableTracer) { BigtableTracer tracer = (BigtableTracer) context.getTracer(); - grpcCtx.withCallOptions( + grpcCtx = grpcCtx.withCallOptions( grpcCtx .getCallOptions() .withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer))); From 729f9be781fbd287f0f6ac1d1f4485ccae62d905 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Tue, 17 Feb 2026 21:11:56 -0500 Subject: [PATCH 04/10] format Change-Id: Ide93ae0406012e8e5779d65cb9770dbda5d0562e --- .../v2/stub/metrics/BigtableTracerStreamingCallable.java | 9 +++++---- .../v2/stub/metrics/BigtableTracerUnaryCallable.java | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java index 73b8a268bf..8518cb6be5 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java @@ -64,10 +64,11 @@ public void call( // tracer should always be an instance of bigtable tracer if (context.getTracer() instanceof BigtableTracer) { BigtableTracer tracer = (BigtableTracer) context.getTracer(); - grpcCtx = grpcCtx.withCallOptions( - grpcCtx - .getCallOptions() - .withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer))); + grpcCtx = + grpcCtx.withCallOptions( + grpcCtx + .getCallOptions() + .withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer))); BigtableTracerResponseObserver innerObserver = new BigtableTracerResponseObserver<>( diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java index 7e2aaf4f00..f10e46059d 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java @@ -59,10 +59,11 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) if (context.getTracer() instanceof BigtableTracer) { BigtableTracer tracer = (BigtableTracer) context.getTracer(); - grpcCtx = grpcCtx.withCallOptions( - grpcCtx - .getCallOptions() - .withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer))); + grpcCtx = + grpcCtx.withCallOptions( + grpcCtx + .getCallOptions() + .withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer))); BigtableTracerUnaryCallback callback = new BigtableTracerUnaryCallback<>( From 8a35ede22bab603f7858b7d0bb172ac68bc455ad Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Tue, 17 Feb 2026 21:18:46 -0500 Subject: [PATCH 05/10] remove replaced location & gfe methods Change-Id: I2aff3f13f2f07b400d6f2099b75c9f1462077e44 --- .../data/v2/stub/metrics/BigtableTracer.java | 37 ------------------- .../data/v2/stub/metrics/CompositeTracer.java | 28 -------------- .../v2/stub/metrics/CompositeTracerTest.java | 21 ++++------- 3 files changed, 7 insertions(+), 79 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java index 0c00495eaa..a1a53b6089 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java @@ -71,18 +71,6 @@ public int getAttempt() { return attempt; } - /** - * Record the latency between Google's network receives the RPC and reads back the first byte of - * the response from server-timing header. If server-timing header is missing, increment the - * missing header count. - * - * @deprecated Use {@link #setSidebandData(MetadataExtractorInterceptor.SidebandData)} - */ - @Deprecated - public void recordGfeMetadata(@Nullable Long latency, @Nullable Throwable throwable) { - // noop - } - /** Adds an annotation of the total throttled time of a batch. */ public void batchRequestThrottled(long throttledTimeMs) { // noop @@ -92,31 +80,6 @@ public void setSidebandData(MetadataExtractorInterceptor.SidebandData sidebandDa // noop } - /** - * Set the Bigtable zone and cluster so metrics can be tagged with location information. This will - * be called in BuiltinMetricsTracer. - * - * @deprecated Use {@link #setSidebandData(MetadataExtractorInterceptor.SidebandData)} - */ - @Deprecated - public void setLocations(String zone, String cluster) {} - - /** - * Set the underlying transport used to process the attempt - * - * @deprecated Use {@link #setSidebandData(MetadataExtractorInterceptor.SidebandData)} - */ - @Deprecated - public void setTransportAttrs(BuiltinMetricsTracer.TransportAttrs attrs) {} - - @Deprecated - /** - * @deprecated {@link #grpcMessageSent()} is called instead. - */ - public void grpcChannelQueuedLatencies(long queuedTimeMs) { - // noop - } - /** Called when the message is sent on a grpc channel. */ public void grpcMessageSent() { // noop diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java index 8c1a58c152..fad00a6d91 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java @@ -198,13 +198,6 @@ public int getAttempt() { return attempt; } - @Override - public void recordGfeMetadata(@Nullable Long latency, @Nullable Throwable throwable) { - for (BigtableTracer tracer : bigtableTracers) { - tracer.recordGfeMetadata(latency, throwable); - } - } - @Override public void batchRequestThrottled(long throttledTimeMs) { for (BigtableTracer tracer : bigtableTracers) { @@ -212,20 +205,6 @@ public void batchRequestThrottled(long throttledTimeMs) { } } - @Override - public void setLocations(String zone, String cluster) { - for (BigtableTracer tracer : bigtableTracers) { - tracer.setLocations(zone, cluster); - } - } - - @Override - public void setTransportAttrs(BuiltinMetricsTracer.TransportAttrs attrs) { - for (BigtableTracer tracer : bigtableTracers) { - tracer.setTransportAttrs(attrs); - } - } - @Override public void setSidebandData(MetadataExtractorInterceptor.SidebandData sidebandData) { for (BigtableTracer bigtableTracer : bigtableTracers) { @@ -254,13 +233,6 @@ public void afterResponse(long applicationLatency) { } } - @Override - public void grpcChannelQueuedLatencies(long queuedTimeMs) { - for (BigtableTracer tracer : bigtableTracers) { - tracer.grpcChannelQueuedLatencies(queuedTimeMs); - } - } - @Override public void grpcMessageSent() { for (BigtableTracer tracer : bigtableTracers) { diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracerTest.java index 71a4728f9f..62c343f16c 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracerTest.java @@ -25,10 +25,9 @@ import com.google.api.gax.tracing.ApiTracer; import com.google.api.gax.tracing.ApiTracer.Scope; import com.google.bigtable.v2.ReadRowsRequest; +import com.google.cloud.bigtable.data.v2.stub.MetadataExtractorInterceptor; import com.google.cloud.bigtable.misc_utilities.MethodComparator; import com.google.common.collect.ImmutableList; -import io.grpc.Status; -import io.grpc.StatusRuntimeException; import java.lang.reflect.Method; import java.util.Arrays; import org.junit.Assert; @@ -241,11 +240,12 @@ public void testGetAttempt() { } @Test - public void testRecordGfeLatency() { - Throwable t = new StatusRuntimeException(Status.UNAVAILABLE); - compositeTracer.recordGfeMetadata(20L, t); - verify(child3, times(1)).recordGfeMetadata(20L, t); - verify(child4, times(1)).recordGfeMetadata(20L, t); + public void testSidebandData() { + MetadataExtractorInterceptor.SidebandData sidebandData = + new MetadataExtractorInterceptor.SidebandData(); + compositeTracer.setSidebandData(sidebandData); + verify(child3, times(1)).setSidebandData(sidebandData); + verify(child4, times(1)).setSidebandData(sidebandData); } @Test @@ -264,13 +264,6 @@ public void testMethodsOverride() { .containsAtLeastElementsIn(baseMethods); } - @Test - public void testRequestBlockedOnChannel() { - compositeTracer.grpcChannelQueuedLatencies(5L); - verify(child3, times(1)).grpcChannelQueuedLatencies(5L); - verify(child4, times(1)).grpcChannelQueuedLatencies(5L); - } - @Test public void testGrpcMessageSent() { compositeTracer.grpcMessageSent(); From a1cc2f8472d385911eecd9bf53cab000b44504a9 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 18 Feb 2026 08:25:35 -0500 Subject: [PATCH 06/10] add todo Change-Id: If07939e62e04e9c7a27862bf87ca5b8731711b75 --- .../bigtable/data/v2/stub/MetadataExtractorInterceptor.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java index f126700dc8..cdaafcab19 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java @@ -17,6 +17,7 @@ import com.google.api.core.InternalApi; import com.google.api.gax.grpc.GrpcCallContext; +import com.google.api.gax.grpc.GrpcTransportChannel; import com.google.bigtable.v2.PeerInfo; import com.google.bigtable.v2.ResponseParams; import com.google.common.base.Strings; @@ -43,6 +44,9 @@ public class MetadataExtractorInterceptor implements ClientInterceptor { private final SidebandData sidebandData = new SidebandData(); public GrpcCallContext injectInto(GrpcCallContext ctx) { + // TODO: migrate to using .withTransportChannel + // This will require a change on gax's side to expose the underlying ManagedChannel in + // GrpcTransportChannel (its currently package private). return ctx.withChannel(ClientInterceptors.intercept(ctx.getChannel(), this)) .withCallOptions(ctx.getCallOptions().withOption(SidebandData.KEY, sidebandData)); } From 025550487911aad10f7fd0b734b4de8f59da99f1 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 18 Feb 2026 08:36:41 -0500 Subject: [PATCH 07/10] fix null handling of sideband data formating and remove stale code Change-Id: Icf1b8ed5d020c9bf5386173b817a89f1679369b4 --- .../v2/stub/MetadataExtractorInterceptor.java | 1 - .../v2/stub/metrics/BuiltinMetricsTracer.java | 29 +++-------------- .../bigtable/data/v2/stub/metrics/Util.java | 32 +++++++++++-------- 3 files changed, 24 insertions(+), 38 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java index cdaafcab19..741ad37b38 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java @@ -17,7 +17,6 @@ import com.google.api.core.InternalApi; import com.google.api.gax.grpc.GrpcCallContext; -import com.google.api.gax.grpc.GrpcTransportChannel; import com.google.bigtable.v2.PeerInfo; import com.google.bigtable.v2.ResponseParams; import com.google.common.base.Strings; diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 6bdc063177..6597df39da 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -34,20 +34,17 @@ import com.google.api.core.ObsoleteApi; import com.google.api.gax.retrying.ServerStreamingAttemptException; import com.google.api.gax.tracing.SpanName; -import com.google.auto.value.AutoValue; import com.google.bigtable.v2.PeerInfo; import com.google.cloud.bigtable.Version; import com.google.cloud.bigtable.data.v2.stub.MetadataExtractorInterceptor; import com.google.common.base.Stopwatch; import com.google.common.math.IntMath; -import com.google.gson.reflect.TypeToken; import io.grpc.Deadline; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.DoubleGauge; import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.LongCounter; import java.time.Duration; -import java.util.Map; import java.util.Optional; import java.util.concurrent.CancellationException; import java.util.concurrent.TimeUnit; @@ -61,22 +58,6 @@ * bigtable.googleapis.com/client namespace */ class BuiltinMetricsTracer extends BigtableTracer { - @AutoValue - abstract static class TransportAttrs { - @Nullable - abstract String getLocality(); - - @Nullable - abstract String getBackendService(); - - static TransportAttrs create(@Nullable String locality, @Nullable String backendService) { - return new AutoValue_BuiltinMetricsTracer_TransportAttrs(locality, backendService); - } - } - - private static final TypeToken> LOCALITY_TYPE = - new TypeToken>() {}; - private static final String NAME = "java-bigtable/" + Version.VERSION; private final OperationType operationType; private final SpanName spanName; @@ -370,8 +351,8 @@ private void recordOperationCompletion(@Nullable Throwable status) { Attributes attributes = baseAttributes.toBuilder() .put(TABLE_ID_KEY, tableId) - .put(CLUSTER_ID_KEY, Util.formatClusterIdMetricLabel(sidebandData.getResponseParams())) - .put(ZONE_ID_KEY, Util.formatZoneIdMetricLabel(sidebandData.getResponseParams())) + .put(CLUSTER_ID_KEY, Util.formatClusterIdMetricLabel(sidebandData)) + .put(ZONE_ID_KEY, Util.formatZoneIdMetricLabel(sidebandData)) .put(METHOD_KEY, spanName.toString()) .put(CLIENT_NAME_KEY, NAME) .put(STREAMING_KEY, isStreaming) @@ -425,8 +406,8 @@ private void recordAttemptCompletion(@Nullable Throwable status) { Attributes attributes = baseAttributes.toBuilder() .put(TABLE_ID_KEY, tableId) - .put(CLUSTER_ID_KEY, Util.formatClusterIdMetricLabel(sidebandData.getResponseParams())) - .put(ZONE_ID_KEY, Util.formatZoneIdMetricLabel(sidebandData.getResponseParams())) + .put(CLUSTER_ID_KEY, Util.formatClusterIdMetricLabel(sidebandData)) + .put(ZONE_ID_KEY, Util.formatZoneIdMetricLabel(sidebandData)) .put(METHOD_KEY, spanName.toString()) .put(CLIENT_NAME_KEY, NAME) .put(STREAMING_KEY, isStreaming) @@ -445,7 +426,7 @@ private void recordAttemptCompletion(@Nullable Throwable status) { String transportSubzone = ""; if (sidebandData != null) { - transportTypeStr = Util.formatTransportTypeMetricLabel(sidebandData.getPeerInfo()); + transportTypeStr = Util.formatTransportTypeMetricLabel(sidebandData); transportZone = Optional.ofNullable(sidebandData.getPeerInfo()) .map(PeerInfo::getApplicationFrontendZone) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index ce16979abc..da7de371c3 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -35,7 +35,7 @@ import com.google.bigtable.v2.SampleRowKeysRequest; import com.google.bigtable.v2.TableName; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; -import com.google.common.base.Strings; +import com.google.cloud.bigtable.data.v2.stub.MetadataExtractorInterceptor; import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableMap; import io.grpc.Metadata; @@ -177,8 +177,10 @@ public static OpenTelemetrySdk newInternalOpentelemetry( return OpenTelemetrySdk.builder().setMeterProvider(meterProviderBuilder.build()).build(); } - public static String formatTransportTypeMetricLabel(PeerInfo peerInfo) { - return Optional.ofNullable(peerInfo) + public static String formatTransportTypeMetricLabel( + MetadataExtractorInterceptor.SidebandData sidebandData) { + return Optional.ofNullable(sidebandData) + .flatMap(s -> Optional.ofNullable(s.getPeerInfo())) .map(PeerInfo::getTransportType) .orElse(PeerInfo.TransportType.TRANSPORT_TYPE_UNKNOWN) .name() @@ -186,17 +188,21 @@ public static String formatTransportTypeMetricLabel(PeerInfo peerInfo) { .toLowerCase(Locale.ENGLISH); } - public static String formatClusterIdMetricLabel(ResponseParams params) { - if (params == null || Strings.isNullOrEmpty(params.getClusterId())) { - return ""; - } - return params.getClusterId(); + public static String formatClusterIdMetricLabel( + @Nullable MetadataExtractorInterceptor.SidebandData sidebandData) { + return Optional.ofNullable(sidebandData) + .flatMap(d -> Optional.ofNullable(d.getResponseParams())) + .map(ResponseParams::getClusterId) + .filter(s -> !s.isEmpty()) + .orElse(""); } - public static String formatZoneIdMetricLabel(ResponseParams params) { - if (params == null || Strings.isNullOrEmpty(params.getZoneId())) { - return "global"; - } - return params.getZoneId(); + public static String formatZoneIdMetricLabel( + @Nullable MetadataExtractorInterceptor.SidebandData sidebandData) { + return Optional.ofNullable(sidebandData) + .flatMap(d -> Optional.ofNullable(d.getResponseParams())) + .map(ResponseParams::getZoneId) + .filter(s -> !s.isEmpty()) + .orElse("global"); } } From eca71a806fa159d6f8a6c49b20efab248663f0e2 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 18 Feb 2026 11:26:16 -0500 Subject: [PATCH 08/10] todo Change-Id: I99e5dd3b4b2397d32fbd41ac7c4e697f6788cd4f --- .../bigtable/data/v2/stub/MetadataExtractorInterceptor.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java index 741ad37b38..5b43f57527 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MetadataExtractorInterceptor.java @@ -160,6 +160,8 @@ private static PeerInfo extractPeerInfo( PeerInfo peerInfo = PeerInfo.parseFrom(decoded); PeerInfo.TransportType effectiveTransport = peerInfo.getTransportType(); + // TODO: remove this once transport_type is being sent by the server + // This is a temporary workaround to detect directpath until its available from the server if (effectiveTransport == PeerInfo.TransportType.TRANSPORT_TYPE_UNKNOWN) { boolean isAlts = AltsContextUtil.check(attributes); if (isAlts) { From 0a353dc4dcb6f23990e8a92a2533c152857ce635 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 18 Feb 2026 11:38:45 -0500 Subject: [PATCH 09/10] remove stale dep Change-Id: I3c98eef573aea3364f2788135407acbba991d7c7 --- google-cloud-bigtable/pom.xml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/google-cloud-bigtable/pom.xml b/google-cloud-bigtable/pom.xml index 703bac2116..bd4c6f0b63 100644 --- a/google-cloud-bigtable/pom.xml +++ b/google-cloud-bigtable/pom.xml @@ -136,10 +136,6 @@ com.google.protobuf protobuf-java-util - - com.google.code.gson - gson - io.opencensus opencensus-api From 9f1da68e0baa28451e0fb76752db06978cc6e37a Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 18 Feb 2026 15:43:17 -0500 Subject: [PATCH 10/10] Eagerly set sideband data instead of deferring until onClose Also defensively add null checks for it and a todo to remove them Change-Id: Ie8237bfcc8c5a0886735ca5b93c0f03f5373e24b --- .../BigtableTracerStreamingCallable.java | 28 ++--------- .../metrics/BigtableTracerUnaryCallable.java | 48 ++----------------- .../v2/stub/metrics/BuiltinMetricsTracer.java | 7 ++- 3 files changed, 14 insertions(+), 69 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java index 8518cb6be5..3cdcdc374e 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java @@ -17,7 +17,6 @@ import com.google.api.core.InternalApi; import com.google.api.gax.grpc.GrpcCallContext; -import com.google.api.gax.grpc.GrpcResponseMetadata; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ResponseObserver; import com.google.api.gax.rpc.ServerStreamingCallable; @@ -30,17 +29,8 @@ import javax.annotation.Nonnull; /** - * This callable will - *
  • -Inject a {@link GrpcResponseMetadata} to access the headers returned by gRPC methods upon - * completion. The {@link BigtableTracer} will process metrics that were injected in the - * header/trailer and publish them to OpenCensus. If {@link GrpcResponseMetadata#getMetadata()} - * returned null, it probably means that the request has never reached GFE, and it'll increment - * the gfe_header_missing_counter in this case. - *
  • -This class will also access trailers from {@link GrpcResponseMetadata} to record zone and - * cluster ids. - *
  • -Call {@link BigtableTracer#onRequest(int)} to record the request events in a stream. - *
  • This class is considered an internal implementation detail and not meant to be used by - * applications. + * This class is considered an internal implementation detail and not meant to be used by + * applications. */ @InternalApi public class BigtableTracerStreamingCallable @@ -64,6 +54,7 @@ public void call( // tracer should always be an instance of bigtable tracer if (context.getTracer() instanceof BigtableTracer) { BigtableTracer tracer = (BigtableTracer) context.getTracer(); + tracer.setSidebandData(metadataExtractor.getSidebandData()); grpcCtx = grpcCtx.withCallOptions( grpcCtx @@ -71,8 +62,7 @@ public void call( .withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer))); BigtableTracerResponseObserver innerObserver = - new BigtableTracerResponseObserver<>( - responseObserver, tracer, metadataExtractor.getSidebandData()); + new BigtableTracerResponseObserver<>(responseObserver, tracer); if (context.getRetrySettings() != null) { tracer.setTotalTimeoutDuration(context.getRetrySettings().getTotalTimeoutDuration()); } @@ -83,20 +73,14 @@ public void call( } private class BigtableTracerResponseObserver extends SafeResponseObserver { - private final BigtableTracer tracer; private final ResponseObserver outerObserver; - private final MetadataExtractorInterceptor.SidebandData sidebandData; - BigtableTracerResponseObserver( - ResponseObserver observer, - BigtableTracer tracer, - MetadataExtractorInterceptor.SidebandData sidebandData) { + BigtableTracerResponseObserver(ResponseObserver observer, BigtableTracer tracer) { super(observer); this.tracer = tracer; this.outerObserver = observer; - this.sidebandData = sidebandData; } @Override @@ -114,13 +98,11 @@ protected void onResponseImpl(ResponseT response) { @Override protected void onErrorImpl(Throwable t) { - tracer.setSidebandData(sidebandData); outerObserver.onError(t); } @Override protected void onCompleteImpl() { - tracer.setSidebandData(sidebandData); outerObserver.onComplete(); } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java index f10e46059d..363a69af3d 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java @@ -16,29 +16,17 @@ package com.google.cloud.bigtable.data.v2.stub.metrics; import com.google.api.core.ApiFuture; -import com.google.api.core.ApiFutureCallback; -import com.google.api.core.ApiFutures; import com.google.api.core.InternalApi; import com.google.api.gax.grpc.GrpcCallContext; -import com.google.api.gax.grpc.GrpcResponseMetadata; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.UnaryCallable; import com.google.cloud.bigtable.data.v2.stub.MetadataExtractorInterceptor; import com.google.common.base.Preconditions; -import com.google.common.util.concurrent.MoreExecutors; import javax.annotation.Nonnull; /** - * This callable will: - *
  • - Inject a {@link GrpcResponseMetadata} to access the headers returned by gRPC methods upon - * completion. The {@link BigtableTracer} will process metrics that were injected in the - * header/trailer and publish them to OpenCensus. If {@link GrpcResponseMetadata#getMetadata()} - * returned null, it probably means that the request has never reached GFE, and it'll increment - * the gfe_header_missing_counter in this case. - *
  • -This class will also access trailers from {@link GrpcResponseMetadata} to record zone and - * cluster ids. - *
  • This class is considered an internal implementation detail and not meant to be used by - * applications. + * This class is considered an internal implementation detail and not meant to be used by + * applications. */ @InternalApi public class BigtableTracerUnaryCallable @@ -58,6 +46,7 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) // tracer should always be an instance of BigtableTracer if (context.getTracer() instanceof BigtableTracer) { BigtableTracer tracer = (BigtableTracer) context.getTracer(); + tracer.setSidebandData(interceptor.getSidebandData()); grpcCtx = grpcCtx.withCallOptions( @@ -65,39 +54,10 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) .getCallOptions() .withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer))); - BigtableTracerUnaryCallback callback = - new BigtableTracerUnaryCallback<>( - (BigtableTracer) context.getTracer(), interceptor.getSidebandData()); if (context.getRetrySettings() != null) { tracer.setTotalTimeoutDuration(context.getRetrySettings().getTotalTimeoutDuration()); } - ApiFuture future = innerCallable.futureCall(request, grpcCtx); - ApiFutures.addCallback(future, callback, MoreExecutors.directExecutor()); - return future; - } else { - return innerCallable.futureCall(request, grpcCtx); - } - } - - private class BigtableTracerUnaryCallback implements ApiFutureCallback { - - private final BigtableTracer tracer; - private final MetadataExtractorInterceptor.SidebandData sidebandData; - - BigtableTracerUnaryCallback( - BigtableTracer tracer, MetadataExtractorInterceptor.SidebandData sidebandData) { - this.tracer = tracer; - this.sidebandData = sidebandData; - } - - @Override - public void onFailure(Throwable throwable) { - tracer.setSidebandData(sidebandData); - } - - @Override - public void onSuccess(ResponseT response) { - tracer.setSidebandData(sidebandData); } + return innerCallable.futureCall(request, grpcCtx); } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 6597df39da..546ea41c9f 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -97,7 +97,10 @@ class BuiltinMetricsTracer extends BigtableTracer { private Deadline operationDeadline = null; private volatile long remainingDeadlineAtAttemptStart = 0; - private MetadataExtractorInterceptor.SidebandData sidebandData = null; + // TODO: ensure that this is never null and remove all of the checks + // Sideband data wrapper itself should never be null unless a callable chain forgets to + // add BigtableTracer{Streaming,Unary}Callable. Which would be considered a bug. + @Nullable private volatile MetadataExtractorInterceptor.SidebandData sidebandData = null; // OpenCensus (and server) histogram buckets use [start, end), however OpenTelemetry uses (start, // end]. To work around this, we measure all the latencies in nanoseconds and convert them @@ -452,7 +455,7 @@ private void recordAttemptCompletion(@Nullable Throwable status) { remainingDeadlineHistogram.record(Math.max(0, remainingDeadlineAtAttemptStart), attributes); } - if (sidebandData.getGfeTiming() != null) { + if (sidebandData != null && sidebandData.getGfeTiming() != null) { serverLatenciesHistogram.record(sidebandData.getGfeTiming(), attributes); connectivityErrorCounter.add(0, attributes); } else {