From 2ae69f7227fbd7aa26a8ac0f85ebd1b7dc52de88 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Sun, 25 Jun 2023 18:29:44 -0400 Subject: [PATCH 01/26] OTEL POC with attempt latency. --- gax-java/gax/pom.xml | 12 ++ .../com/google/api/gax/tracing/ApiTracer.java | 3 + .../tracing/OpenTelemetryMetricsTracer.java | 150 ++++++++++++++++++ .../tracing/OpenTelemetryTracerFactory.java | 70 ++++++++ gax-java/pom.xml | 7 + 5 files changed, 242 insertions(+) create mode 100644 gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java create mode 100644 gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracerFactory.java diff --git a/gax-java/gax/pom.xml b/gax-java/gax/pom.xml index 8fb0bf47e7..9c36010c63 100644 --- a/gax-java/gax/pom.xml +++ b/gax-java/gax/pom.xml @@ -57,6 +57,18 @@ graal-sdk provided + + io.opentelemetry + opentelemetry-api + + + io.opentelemetry + opentelemetry-sdk + + + io.opentelemetry + opentelemetry-exporter-otlp + diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java index 3176be4b92..8bdee76ea9 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java @@ -100,6 +100,9 @@ public interface ApiTracer { /** Adds an annotation that the attempt succeeded. */ void attemptSucceeded(); + default String attemptLatencyName() { + return ""; + }; /** Add an annotation that the attempt was cancelled by the user. */ void attemptCancelled(); diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java new file mode 100644 index 0000000000..e41368beef --- /dev/null +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java @@ -0,0 +1,150 @@ +/* + * Copyright 2023 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.google.api.gax.tracing; + +import com.google.common.base.Stopwatch; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.DoubleHistogram; +import io.opentelemetry.api.metrics.LongCounter; +import io.opentelemetry.api.metrics.Meter; +import org.threeten.bp.Duration; + +import java.util.concurrent.TimeUnit; + +import static io.opentelemetry.api.common.AttributeKey.stringKey; + +public class OpenTelemetryMetricsTracer implements ApiTracer { + protected Meter meter; + + private Stopwatch attemptTimer; + + private SpanName spanName; + + public OpenTelemetryMetricsTracer(Meter meter, SpanName spanName) { + this.meter = meter; + this.spanName = spanName; + } + + @Override + public Scope inScope() { + return () -> {}; + } + + @Override + public void operationSucceeded() { + + } + + @Override + public void operationCancelled() { + + } + + @Override + public void operationFailed(Throwable error) { + + } + + @Override + public void connectionSelected(String id) { + + } + + @Override + public void attemptStarted(int attemptNumber) { + + } + + @Override + public void attemptStarted(Object request, int attemptNumber) { + attemptTimer = Stopwatch.createStarted(); + LongCounter longCounter = meter.counterBuilder("attempt_count") + .setDescription("Attempt Count") + .setUnit("1") + .build(); + longCounter.add(1); + } + + @Override + public void attemptSucceeded() { + DoubleHistogram doubleHistogram = meter.histogramBuilder(attemptLatencyName() + "-attempt_latency") + .setDescription("Duration of an individual operation attempt") + .setUnit("ms") + .build(); + Attributes attributes = Attributes.of(stringKey("method_name"), spanName.toString()); + doubleHistogram.record(attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); + } + + @Override + public void attemptCancelled() { + + } + + @Override + public void attemptFailed(Throwable error, Duration delay) { + + } + + @Override + public void attemptFailedRetriesExhausted(Throwable error) { + + } + + @Override + public void attemptPermanentFailure(Throwable error) { + + } + + @Override + public void lroStartFailed(Throwable error) { + + } + + @Override + public void lroStartSucceeded() { + + } + + @Override + public void responseReceived() { + + } + + @Override + public void requestSent() { + + } + + @Override + public void batchRequestSent(long elementCount, long requestSize) { + + } +} diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracerFactory.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracerFactory.java new file mode 100644 index 0000000000..a7da78da69 --- /dev/null +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracerFactory.java @@ -0,0 +1,70 @@ +/* + * Copyright 2023 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.tracing; + +import com.google.api.core.InternalApi; +import io.opencensus.trace.Tracer; +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.metrics.Meter; + +/** + * A {@link ApiTracerFactory} to build instances of {@link OpencensusTracer}. + * + *

This class wraps the {@link Tracer} provided by Opencensus in {@code Tracing.getTracer()}. It + * will be used to create new spans and wrap them in {@link OpencensusTracer} defined in gax. + * + *

This class is thread safe. + */ +@InternalApi("For google-cloud-java client use only") +public class OpenTelemetryTracerFactory extends BaseApiTracerFactory { + protected Meter meter; + public OpenTelemetryTracerFactory(OpenTelemetry openTelemetry, String libraryName, String libraryVersion) { + meter = openTelemetry.meterBuilder(libraryName) + .setInstrumentationVersion(libraryVersion) + .build(); + + +// +// // Build an asynchronous instrument, e.g. Gauge +// ObservableDoubleGauge observableDoubleGauge = meter +// .gaugeBuilder("cpu_usage") +// .setDescription("CPU Usage") +// .setUnit("ms") +// +// .buildWithCallback(measurement -> { +// measurement.record(0.3, Attributes.of(stringKey("Key"), "SomeWork")); +// }); + } + + @Override + public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) { + return new OpenTelemetryMetricsTracer(meter, spanName); + } +} diff --git a/gax-java/pom.xml b/gax-java/pom.xml index 888f166b30..245df80f32 100644 --- a/gax-java/pom.xml +++ b/gax-java/pom.xml @@ -159,6 +159,13 @@ pom import + + io.opentelemetry + opentelemetry-bom + 1.27.0 + pom + import + From 6671379ed084889755401d5ab1ef7daa9a428728 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Tue, 27 Jun 2023 14:06:57 -0400 Subject: [PATCH 02/26] Add ClientMetricsTracer. Add channel size metric. --- .../com/google/api/gax/grpc/ChannelPool.java | 15 ++++++++++++- .../InstantiatingGrpcChannelProvider.java | 10 +++++++-- .../com/google/api/gax/rpc/ClientContext.java | 4 ++++ .../api/gax/rpc/TransportChannelProvider.java | 3 +++ .../com/google/api/gax/tracing/ApiTracer.java | 4 +++- .../api/gax/tracing/ApiTracerFactory.java | 5 +++++ .../api/gax/tracing/ClientMetricsTracer.java | 10 +++++++++ .../OpenTelemetryClientMetricsTracer.java | 21 +++++++++++++++++++ .../tracing/OpenTelemetryMetricsTracer.java | 2 +- 9 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 gax-java/gax/src/main/java/com/google/api/gax/tracing/ClientMetricsTracer.java create mode 100644 gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryClientMetricsTracer.java diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPool.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPool.java index 7f045cc44d..6bb6e9eaaf 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPool.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPool.java @@ -30,6 +30,7 @@ package com.google.api.gax.grpc; import com.google.api.core.InternalApi; +import com.google.api.gax.tracing.ClientMetricsTracer; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -80,11 +81,22 @@ class ChannelPool extends ManagedChannel { private final AtomicInteger indexTicker = new AtomicInteger(); private final String authority; + private ClientMetricsTracer clientMetricsTracer; static ChannelPool create(ChannelPoolSettings settings, ChannelFactory channelFactory) - throws IOException { + throws IOException { return new ChannelPool(settings, channelFactory, Executors.newSingleThreadScheduledExecutor()); } + static ChannelPool create(ChannelPoolSettings settings, ChannelFactory channelFactory, ClientMetricsTracer clientMetricsTracer) + throws IOException { + ChannelPool channelPool = new ChannelPool(settings, channelFactory, Executors.newSingleThreadScheduledExecutor()); + channelPool.clientMetricsTracer = clientMetricsTracer; + if (channelPool.clientMetricsTracer != null) { + channelPool.clientMetricsTracer.recordCurrentChannelSize(1); + } + return channelPool; + } + /** * Initializes the channel pool. Assumes that all channels have the same authority. * @@ -298,6 +310,7 @@ void resize() { shrink(dampenedTarget); } + clientMetricsTracer.recordCurrentChannelSize(localEntries.size()); } /** Not threadsafe, must be called under the entryWriteLock monitor */ diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 06d0c18e51..26294c706b 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -40,6 +40,7 @@ import com.google.api.gax.rpc.TransportChannelProvider; import com.google.api.gax.rpc.internal.EnvironmentProvider; import com.google.api.gax.rpc.mtls.MtlsProvider; +import com.google.api.gax.tracing.ClientMetricsTracer; import com.google.auth.Credentials; import com.google.auth.oauth2.ComputeEngineCredentials; import com.google.common.annotations.VisibleForTesting; @@ -112,7 +113,7 @@ public final class InstantiatingGrpcChannelProvider implements TransportChannelP @Nullable private final Boolean allowNonDefaultServiceAccount; @VisibleForTesting final ImmutableMap directPathServiceConfig; @Nullable private final MtlsProvider mtlsProvider; - + @Nullable private ClientMetricsTracer clientMetricsTracer; @Nullable private final ApiFunction channelConfigurator; @@ -178,6 +179,11 @@ public String getTransportName() { return GrpcTransportChannel.getGrpcTransportName(); } + @Override + public void setClientMetricsTracer(ClientMetricsTracer clientMetricsTracer) { + this.clientMetricsTracer = clientMetricsTracer; + } + @Override public boolean needsEndpoint() { return endpoint == null; @@ -235,7 +241,7 @@ public TransportChannel getTransportChannel() throws IOException { private TransportChannel createChannel() throws IOException { return GrpcTransportChannel.create( ChannelPool.create( - channelPoolSettings, InstantiatingGrpcChannelProvider.this::createSingleChannel)); + channelPoolSettings, InstantiatingGrpcChannelProvider.this::createSingleChannel, clientMetricsTracer)); } private boolean isDirectPathEnabled() { diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index 91e10aa7bf..e309bb5bfa 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -39,6 +39,7 @@ import com.google.api.gax.rpc.mtls.MtlsProvider; import com.google.api.gax.tracing.ApiTracerFactory; import com.google.api.gax.tracing.BaseApiTracerFactory; +import com.google.api.gax.tracing.ClientMetricsTracer; import com.google.auth.Credentials; import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; @@ -198,6 +199,9 @@ public static ClientContext create(StubSettings settings) throws IOException { if (transportChannelProvider.needsEndpoint()) { transportChannelProvider = transportChannelProvider.withEndpoint(endpoint); } + ClientMetricsTracer clientMetricsTracer = settings.getTracerFactory().newClientMetricsTracer(); + transportChannelProvider.setClientMetricsTracer(clientMetricsTracer); + TransportChannel transportChannel = transportChannelProvider.getTransportChannel(); ApiCallContext defaultCallContext = diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/TransportChannelProvider.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/TransportChannelProvider.java index b2acd458f0..fc25cdbd2d 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/TransportChannelProvider.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/TransportChannelProvider.java @@ -31,6 +31,7 @@ import com.google.api.core.BetaApi; import com.google.api.core.InternalExtensionOnly; +import com.google.api.gax.tracing.ClientMetricsTracer; import com.google.auth.Credentials; import java.io.IOException; import java.util.Map; @@ -142,4 +143,6 @@ public interface TransportChannelProvider { *

This string can be used for identifying transports for switching logic. */ String getTransportName(); + + default void setClientMetricsTracer(ClientMetricsTracer clientMetricsTracer) {}; } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java index 8bdee76ea9..6e6311f8db 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java @@ -100,8 +100,10 @@ public interface ApiTracer { /** Adds an annotation that the attempt succeeded. */ void attemptSucceeded(); + + //This is for libraries to override to intended name default String attemptLatencyName() { - return ""; + return "attempt_latency"; }; /** Add an annotation that the attempt was cancelled by the user. */ diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java index bb8345b88c..4e3e091e7a 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java @@ -61,4 +61,9 @@ enum OperationType { * @param operationType the type of operation that the tracer will trace */ ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType); + + //This probably needs to be moved to a new factory + default ClientMetricsTracer newClientMetricsTracer() { + return null; + }; } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ClientMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ClientMetricsTracer.java new file mode 100644 index 0000000000..03ad5eef54 --- /dev/null +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ClientMetricsTracer.java @@ -0,0 +1,10 @@ +package com.google.api.gax.tracing; + +public interface ClientMetricsTracer { + + void recordCurrentChannelSize(int channelSize); + + default String channelSizeName() { + return "channel_size"; + }; +} diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryClientMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryClientMetricsTracer.java new file mode 100644 index 0000000000..0cbe0c3590 --- /dev/null +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryClientMetricsTracer.java @@ -0,0 +1,21 @@ +package com.google.api.gax.tracing; + +import io.opentelemetry.api.metrics.Meter; + +public class OpenTelemetryClientMetricsTracer implements ClientMetricsTracer { + + private Meter meter; + public OpenTelemetryClientMetricsTracer(Meter meter) { + this.meter = meter; + } + + @Override + public void recordCurrentChannelSize(int channelSize) { + meter + .gaugeBuilder(channelSizeName()) + .setDescription("Channel Size") + .setUnit("1") + .ofLongs() + .buildWithCallback(measurement -> measurement.record(channelSize)); + } +} diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java index e41368beef..9dd4329e05 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java @@ -95,7 +95,7 @@ public void attemptStarted(Object request, int attemptNumber) { @Override public void attemptSucceeded() { - DoubleHistogram doubleHistogram = meter.histogramBuilder(attemptLatencyName() + "-attempt_latency") + DoubleHistogram doubleHistogram = meter.histogramBuilder(attemptLatencyName()) .setDescription("Duration of an individual operation attempt") .setUnit("ms") .build(); From cd9f38770a253f46035b41419f283d6ee2927d94 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Thu, 29 Jun 2023 17:39:36 -0400 Subject: [PATCH 03/26] Add response to OperationSucceeded. Add a way to add attributes to metrics. --- .../api/gax/retrying/BasicRetryingFuture.java | 2 +- .../com/google/api/gax/tracing/ApiTracer.java | 3 ++ .../tracing/OpenTelemetryMetricsTracer.java | 48 +++++++++++++++++-- .../google/api/gax/tracing/TraceFinisher.java | 2 +- .../gax/tracing/TracedResponseObserver.java | 2 +- 5 files changed, 49 insertions(+), 8 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java b/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java index de7b5b5acb..df66ba35b2 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java @@ -199,7 +199,7 @@ void handleAttempt(Throwable throwable, ResponseT response) { } super.setException(throwable); } else { - tracer.attemptSucceeded(); + tracer.attemptSucceeded(response); super.set(response); } } catch (CancellationException e) { diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java index 6e6311f8db..48292ac98e 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java @@ -57,6 +57,8 @@ public interface ApiTracer { */ void operationSucceeded(); + default void operationSucceeded(Object response){}; + /** * Signals that the operation was cancelled by the user. The tracer is now considered closed and * should no longer be used. @@ -100,6 +102,7 @@ public interface ApiTracer { /** Adds an annotation that the attempt succeeded. */ void attemptSucceeded(); + default void attemptSucceeded(Object response) {}; //This is for libraries to override to intended name default String attemptLatencyName() { diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java index 9dd4329e05..cc32d79cff 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java @@ -32,11 +32,14 @@ import com.google.common.base.Stopwatch; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.LongCounter; import io.opentelemetry.api.metrics.Meter; import org.threeten.bp.Duration; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.TimeUnit; import static io.opentelemetry.api.common.AttributeKey.stringKey; @@ -46,11 +49,27 @@ public class OpenTelemetryMetricsTracer implements ApiTracer { private Stopwatch attemptTimer; + private final Stopwatch operationTimer = Stopwatch.createStarted(); + private SpanName spanName; + private DoubleHistogram attemptLatencyRecorder; + + private DoubleHistogram operationLatencyRecorder; + + Map operationLatencyLabels = new HashMap<>(); + public OpenTelemetryMetricsTracer(Meter meter, SpanName spanName) { this.meter = meter; this.spanName = spanName; + this.attemptLatencyRecorder = meter.histogramBuilder(attemptLatencyName()) + .setDescription("Duration of an individual operation attempt") + .setUnit("ms") + .build(); + this.operationLatencyRecorder = meter.histogramBuilder("operation_latency") + .setDescription("Total time until final operation success or failure, including retries and backoff.") + .setUnit("ms") + .build(); } @Override @@ -63,6 +82,24 @@ public void operationSucceeded() { } + //This is just one way to add labels, we could have another layer of abstraction(a separate class) just for get/set labels because the logic is generic. + public void addOperationLatencyLabels(String key, String value) { + operationLatencyLabels.put(key, value); + } + + public Map getOperationLatencyLabels() { + return operationLatencyLabels; + } + + @Override + public void operationSucceeded(Object response) { + AttributesBuilder attributesBuilder = Attributes.builder(); + getOperationLatencyLabels().forEach((key, value) -> { + attributesBuilder.put(stringKey(key), value); + }); + operationLatencyRecorder.record(operationTimer.elapsed(TimeUnit.MILLISECONDS), attributesBuilder.build()); + } + @Override public void operationCancelled() { @@ -95,12 +132,13 @@ public void attemptStarted(Object request, int attemptNumber) { @Override public void attemptSucceeded() { - DoubleHistogram doubleHistogram = meter.histogramBuilder(attemptLatencyName()) - .setDescription("Duration of an individual operation attempt") - .setUnit("ms") - .build(); + + } + + @Override + public void attemptSucceeded(Object response) { Attributes attributes = Attributes.of(stringKey("method_name"), spanName.toString()); - doubleHistogram.record(attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); + attemptLatencyRecorder.record(attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); } @Override diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/TraceFinisher.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/TraceFinisher.java index 292a827759..1a517dfc7a 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/TraceFinisher.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/TraceFinisher.java @@ -53,6 +53,6 @@ public void onFailure(Throwable throwable) { @Override public void onSuccess(T responseT) { - tracer.operationSucceeded(); + tracer.operationSucceeded(responseT); } } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedResponseObserver.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedResponseObserver.java index ba72d2f5b7..768de72f12 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedResponseObserver.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedResponseObserver.java @@ -106,7 +106,7 @@ public void onError(Throwable t) { @Override public void onComplete() { - tracer.operationSucceeded(); + tracer.operationSucceeded(null); innerObserver.onComplete(); } } From 5eccd0904db8344c489ae9fc98f70b1d67f46f55 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Thu, 6 Jul 2023 14:42:30 -0400 Subject: [PATCH 04/26] Add metrics for retryCount. --- .../com/google/api/gax/grpc/ChannelPool.java | 15 +- .../InstantiatingGrpcChannelProvider.java | 5 +- .../api/gax/retrying/BasicRetryingFuture.java | 1 + .../com/google/api/gax/tracing/ApiTracer.java | 6 +- .../api/gax/tracing/ApiTracerFactory.java | 2 +- .../api/gax/tracing/ClientMetricsTracer.java | 8 +- .../OpenTelemetryClientMetricsTracer.java | 27 +- .../tracing/OpenTelemetryMetricsTracer.java | 268 ++++++++---------- .../tracing/OpenTelemetryTracerFactory.java | 19 +- 9 files changed, 165 insertions(+), 186 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPool.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPool.java index 6bb6e9eaaf..5f7502d307 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPool.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPool.java @@ -82,17 +82,22 @@ class ChannelPool extends ManagedChannel { private final String authority; private ClientMetricsTracer clientMetricsTracer; + static ChannelPool create(ChannelPoolSettings settings, ChannelFactory channelFactory) - throws IOException { + throws IOException { return new ChannelPool(settings, channelFactory, Executors.newSingleThreadScheduledExecutor()); } - static ChannelPool create(ChannelPoolSettings settings, ChannelFactory channelFactory, ClientMetricsTracer clientMetricsTracer) + static ChannelPool create( + ChannelPoolSettings settings, + ChannelFactory channelFactory, + ClientMetricsTracer clientMetricsTracer) throws IOException { - ChannelPool channelPool = new ChannelPool(settings, channelFactory, Executors.newSingleThreadScheduledExecutor()); + ChannelPool channelPool = + new ChannelPool(settings, channelFactory, Executors.newSingleThreadScheduledExecutor()); channelPool.clientMetricsTracer = clientMetricsTracer; if (channelPool.clientMetricsTracer != null) { - channelPool.clientMetricsTracer.recordCurrentChannelSize(1); + channelPool.clientMetricsTracer.recordCurrentChannelSize(settings.getInitialChannelCount()); } return channelPool; } @@ -310,7 +315,7 @@ void resize() { shrink(dampenedTarget); } - clientMetricsTracer.recordCurrentChannelSize(localEntries.size()); + clientMetricsTracer.recordCurrentChannelSize(entries.get().size()); } /** Not threadsafe, must be called under the entryWriteLock monitor */ diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 26294c706b..da0e596b70 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -114,6 +114,7 @@ public final class InstantiatingGrpcChannelProvider implements TransportChannelP @VisibleForTesting final ImmutableMap directPathServiceConfig; @Nullable private final MtlsProvider mtlsProvider; @Nullable private ClientMetricsTracer clientMetricsTracer; + @Nullable private final ApiFunction channelConfigurator; @@ -241,7 +242,9 @@ public TransportChannel getTransportChannel() throws IOException { private TransportChannel createChannel() throws IOException { return GrpcTransportChannel.create( ChannelPool.create( - channelPoolSettings, InstantiatingGrpcChannelProvider.this::createSingleChannel, clientMetricsTracer)); + channelPoolSettings, + InstantiatingGrpcChannelProvider.this::createSingleChannel, + clientMetricsTracer)); } private boolean isDirectPathEnabled() { diff --git a/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java b/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java index df66ba35b2..1a46409ab0 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java @@ -200,6 +200,7 @@ void handleAttempt(Throwable throwable, ResponseT response) { super.setException(throwable); } else { tracer.attemptSucceeded(response); + tracer.retryCount(attemptSettings.getAttemptCount()); super.set(response); } } catch (CancellationException e) { diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java index 48292ac98e..73012d1bfd 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java @@ -57,7 +57,7 @@ public interface ApiTracer { */ void operationSucceeded(); - default void operationSucceeded(Object response){}; + default void operationSucceeded(Object response) {}; /** * Signals that the operation was cancelled by the user. The tracer is now considered closed and @@ -102,9 +102,10 @@ public interface ApiTracer { /** Adds an annotation that the attempt succeeded. */ void attemptSucceeded(); + default void attemptSucceeded(Object response) {}; - //This is for libraries to override to intended name + // This is for libraries to override to intended name default String attemptLatencyName() { return "attempt_latency"; }; @@ -136,6 +137,7 @@ default String attemptLatencyName() { */ void attemptPermanentFailure(Throwable error); + default void retryCount(int count) {}; /** * Signals that the initial RPC for the long running operation failed. * diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java index 4e3e091e7a..d8cbef51bf 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java @@ -62,7 +62,7 @@ enum OperationType { */ ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType); - //This probably needs to be moved to a new factory + // This probably needs to be moved to a new factory default ClientMetricsTracer newClientMetricsTracer() { return null; }; diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ClientMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ClientMetricsTracer.java index 03ad5eef54..12b7f534c5 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ClientMetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ClientMetricsTracer.java @@ -2,9 +2,9 @@ public interface ClientMetricsTracer { - void recordCurrentChannelSize(int channelSize); + void recordCurrentChannelSize(int channelSize); - default String channelSizeName() { - return "channel_size"; - }; + default String channelSizeName() { + return "channel_size"; + }; } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryClientMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryClientMetricsTracer.java index 0cbe0c3590..61437760af 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryClientMetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryClientMetricsTracer.java @@ -1,21 +1,22 @@ package com.google.api.gax.tracing; +import io.opentelemetry.api.metrics.LongGaugeBuilder; import io.opentelemetry.api.metrics.Meter; public class OpenTelemetryClientMetricsTracer implements ClientMetricsTracer { - private Meter meter; - public OpenTelemetryClientMetricsTracer(Meter meter) { - this.meter = meter; - } + private final LongGaugeBuilder longGaugeBuilder; + private Meter meter; - @Override - public void recordCurrentChannelSize(int channelSize) { - meter - .gaugeBuilder(channelSizeName()) - .setDescription("Channel Size") - .setUnit("1") - .ofLongs() - .buildWithCallback(measurement -> measurement.record(channelSize)); - } + public OpenTelemetryClientMetricsTracer(Meter meter) { + this.meter = meter; + longGaugeBuilder = + meter.gaugeBuilder(channelSizeName()).setDescription("Channel Size").setUnit("1").ofLongs(); + } + + @Override + public void recordCurrentChannelSize(int channelSize) { + longGaugeBuilder.buildWithCallback( + observableLongMeasurement -> observableLongMeasurement.record(channelSize)); + } } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java index cc32d79cff..c2b5da29fa 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java @@ -30,159 +30,137 @@ package com.google.api.gax.tracing; +import static io.opentelemetry.api.common.AttributeKey.stringKey; + import com.google.common.base.Stopwatch; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.metrics.DoubleHistogram; -import io.opentelemetry.api.metrics.LongCounter; +import io.opentelemetry.api.metrics.LongHistogram; import io.opentelemetry.api.metrics.Meter; -import org.threeten.bp.Duration; - import java.util.HashMap; import java.util.Map; import java.util.concurrent.TimeUnit; - -import static io.opentelemetry.api.common.AttributeKey.stringKey; +import org.threeten.bp.Duration; public class OpenTelemetryMetricsTracer implements ApiTracer { - protected Meter meter; - - private Stopwatch attemptTimer; - - private final Stopwatch operationTimer = Stopwatch.createStarted(); - - private SpanName spanName; - - private DoubleHistogram attemptLatencyRecorder; - - private DoubleHistogram operationLatencyRecorder; - - Map operationLatencyLabels = new HashMap<>(); - - public OpenTelemetryMetricsTracer(Meter meter, SpanName spanName) { - this.meter = meter; - this.spanName = spanName; - this.attemptLatencyRecorder = meter.histogramBuilder(attemptLatencyName()) - .setDescription("Duration of an individual operation attempt") - .setUnit("ms") - .build(); - this.operationLatencyRecorder = meter.histogramBuilder("operation_latency") - .setDescription("Total time until final operation success or failure, including retries and backoff.") - .setUnit("ms") - .build(); - } - - @Override - public Scope inScope() { - return () -> {}; - } - - @Override - public void operationSucceeded() { - - } - - //This is just one way to add labels, we could have another layer of abstraction(a separate class) just for get/set labels because the logic is generic. - public void addOperationLatencyLabels(String key, String value) { - operationLatencyLabels.put(key, value); - } - - public Map getOperationLatencyLabels() { - return operationLatencyLabels; - } - - @Override - public void operationSucceeded(Object response) { - AttributesBuilder attributesBuilder = Attributes.builder(); - getOperationLatencyLabels().forEach((key, value) -> { - attributesBuilder.put(stringKey(key), value); - }); - operationLatencyRecorder.record(operationTimer.elapsed(TimeUnit.MILLISECONDS), attributesBuilder.build()); - } - - @Override - public void operationCancelled() { - - } - - @Override - public void operationFailed(Throwable error) { - - } - - @Override - public void connectionSelected(String id) { - - } - - @Override - public void attemptStarted(int attemptNumber) { - - } - - @Override - public void attemptStarted(Object request, int attemptNumber) { - attemptTimer = Stopwatch.createStarted(); - LongCounter longCounter = meter.counterBuilder("attempt_count") - .setDescription("Attempt Count") - .setUnit("1") - .build(); - longCounter.add(1); - } - - @Override - public void attemptSucceeded() { - - } - - @Override - public void attemptSucceeded(Object response) { - Attributes attributes = Attributes.of(stringKey("method_name"), spanName.toString()); - attemptLatencyRecorder.record(attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); - } - - @Override - public void attemptCancelled() { - - } - - @Override - public void attemptFailed(Throwable error, Duration delay) { - - } - - @Override - public void attemptFailedRetriesExhausted(Throwable error) { - - } - - @Override - public void attemptPermanentFailure(Throwable error) { - - } - - @Override - public void lroStartFailed(Throwable error) { - - } - - @Override - public void lroStartSucceeded() { - - } - - @Override - public void responseReceived() { - - } - - @Override - public void requestSent() { - - } - - @Override - public void batchRequestSent(long elementCount, long requestSize) { - - } + protected Meter meter; + + private Stopwatch attemptTimer; + + private final Stopwatch operationTimer = Stopwatch.createStarted(); + + private SpanName spanName; + + private DoubleHistogram attemptLatencyRecorder; + + private DoubleHistogram operationLatencyRecorder; + private LongHistogram retryCountRecorder; + + Map operationLatencyLabels = new HashMap<>(); + + public OpenTelemetryMetricsTracer(Meter meter, SpanName spanName) { + this.meter = meter; + this.spanName = spanName; + this.attemptLatencyRecorder = + meter + .histogramBuilder(attemptLatencyName()) + .setDescription("Duration of an individual operation attempt") + .setUnit("ms") + .build(); + this.operationLatencyRecorder = + meter + .histogramBuilder("operation_latency") + .setDescription( + "Total time until final operation success or failure, including retries and backoff.") + .setUnit("ms") + .build(); + this.retryCountRecorder = + meter + .histogramBuilder("retry_count") + .setDescription("Number of additional attempts per operation after initial attempt") + .setUnit("1") + .ofLongs() + .build(); + } + + @Override + public Scope inScope() { + return () -> {}; + } + + @Override + public void operationSucceeded() {} + + // This is just one way to add labels, we could have another layer of abstraction(a separate + // class) just for get/set labels because the logic is generic. + public void addOperationLatencyLabels(String key, String value) { + operationLatencyLabels.put(key, value); + } + + @Override + public void operationSucceeded(Object response) { + AttributesBuilder attributesBuilder = Attributes.builder(); + operationLatencyLabels.forEach((key, value) -> attributesBuilder.put(stringKey(key), value)); + operationLatencyRecorder.record( + operationTimer.elapsed(TimeUnit.MILLISECONDS), attributesBuilder.build()); + } + + @Override + public void operationCancelled() {} + + @Override + public void operationFailed(Throwable error) {} + + @Override + public void connectionSelected(String id) {} + + @Override + public void attemptStarted(int attemptNumber) {} + + @Override + public void attemptStarted(Object request, int attemptNumber) { + attemptTimer = Stopwatch.createStarted(); + } + + @Override + public void attemptSucceeded() {} + + @Override + public void attemptSucceeded(Object response) { + Attributes attributes = Attributes.of(stringKey("method_name"), spanName.toString()); + attemptLatencyRecorder.record(attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); + } + + @Override + public void attemptCancelled() {} + + @Override + public void attemptFailed(Throwable error, Duration delay) {} + + @Override + public void attemptFailedRetriesExhausted(Throwable error) {} + + @Override + public void attemptPermanentFailure(Throwable error) {} + + @Override + public void retryCount(int count) { + retryCountRecorder.record(count); + } + + @Override + public void lroStartFailed(Throwable error) {} + + @Override + public void lroStartSucceeded() {} + + @Override + public void responseReceived() {} + + @Override + public void requestSent() {} + + @Override + public void batchRequestSent(long elementCount, long requestSize) {} } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracerFactory.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracerFactory.java index a7da78da69..7b68001a34 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracerFactory.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracerFactory.java @@ -45,22 +45,11 @@ @InternalApi("For google-cloud-java client use only") public class OpenTelemetryTracerFactory extends BaseApiTracerFactory { protected Meter meter; - public OpenTelemetryTracerFactory(OpenTelemetry openTelemetry, String libraryName, String libraryVersion) { - meter = openTelemetry.meterBuilder(libraryName) - .setInstrumentationVersion(libraryVersion) - .build(); - -// -// // Build an asynchronous instrument, e.g. Gauge -// ObservableDoubleGauge observableDoubleGauge = meter -// .gaugeBuilder("cpu_usage") -// .setDescription("CPU Usage") -// .setUnit("ms") -// -// .buildWithCallback(measurement -> { -// measurement.record(0.3, Attributes.of(stringKey("Key"), "SomeWork")); -// }); + public OpenTelemetryTracerFactory( + OpenTelemetry openTelemetry, String libraryName, String libraryVersion) { + meter = + openTelemetry.meterBuilder(libraryName).setInstrumentationVersion(libraryVersion).build(); } @Override From c2573b60346d4efe8253b3e0454ec3f9d3d639c4 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Mon, 17 Jul 2023 13:10:44 -0400 Subject: [PATCH 05/26] Add ApiTracerFactory to java-core. --- ...ory.java => OpenTelemetryMetricsFactory.java} | 9 +++++++-- .../gax/tracing/OpenTelemetryMetricsTracer.java | 9 ++++++--- .../java/com/google/cloud/ServiceOptions.java | 16 ++++++++++++++++ 3 files changed, 29 insertions(+), 5 deletions(-) rename gax-java/gax/src/main/java/com/google/api/gax/tracing/{OpenTelemetryTracerFactory.java => OpenTelemetryMetricsFactory.java} (91%) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracerFactory.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java similarity index 91% rename from gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracerFactory.java rename to gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java index 7b68001a34..7d59f7ea2c 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracerFactory.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java @@ -43,10 +43,10 @@ *

This class is thread safe. */ @InternalApi("For google-cloud-java client use only") -public class OpenTelemetryTracerFactory extends BaseApiTracerFactory { +public class OpenTelemetryMetricsFactory implements ApiTracerFactory { protected Meter meter; - public OpenTelemetryTracerFactory( + public OpenTelemetryMetricsFactory( OpenTelemetry openTelemetry, String libraryName, String libraryVersion) { meter = openTelemetry.meterBuilder(libraryName).setInstrumentationVersion(libraryVersion).build(); @@ -56,4 +56,9 @@ public OpenTelemetryTracerFactory( public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) { return new OpenTelemetryMetricsTracer(meter, spanName); } + + @Override + public ClientMetricsTracer newClientMetricsTracer() { + return new OpenTelemetryClientMetricsTracer(meter); + } } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java index c2b5da29fa..13f53012e0 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java @@ -57,6 +57,8 @@ public class OpenTelemetryMetricsTracer implements ApiTracer { private DoubleHistogram operationLatencyRecorder; private LongHistogram retryCountRecorder; + private Attributes defaultAttributes; + Map operationLatencyLabels = new HashMap<>(); public OpenTelemetryMetricsTracer(Meter meter, SpanName spanName) { @@ -82,6 +84,7 @@ public OpenTelemetryMetricsTracer(Meter meter, SpanName spanName) { .setUnit("1") .ofLongs() .build(); + this.defaultAttributes = Attributes.of(stringKey("method_name"), spanName.toString()); } @Override @@ -101,6 +104,7 @@ public void addOperationLatencyLabels(String key, String value) { @Override public void operationSucceeded(Object response) { AttributesBuilder attributesBuilder = Attributes.builder(); + attributesBuilder.putAll(defaultAttributes); operationLatencyLabels.forEach((key, value) -> attributesBuilder.put(stringKey(key), value)); operationLatencyRecorder.record( operationTimer.elapsed(TimeUnit.MILLISECONDS), attributesBuilder.build()); @@ -128,8 +132,7 @@ public void attemptSucceeded() {} @Override public void attemptSucceeded(Object response) { - Attributes attributes = Attributes.of(stringKey("method_name"), spanName.toString()); - attemptLatencyRecorder.record(attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); + attemptLatencyRecorder.record(attemptTimer.elapsed(TimeUnit.MILLISECONDS), defaultAttributes); } @Override @@ -146,7 +149,7 @@ public void attemptPermanentFailure(Throwable error) {} @Override public void retryCount(int count) { - retryCountRecorder.record(count); + retryCountRecorder.record(count, defaultAttributes); } @Override diff --git a/java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java b/java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java index 231b9040c9..a3b42a399a 100644 --- a/java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java +++ b/java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java @@ -40,6 +40,7 @@ import com.google.api.gax.rpc.FixedHeaderProvider; import com.google.api.gax.rpc.HeaderProvider; import com.google.api.gax.rpc.NoHeaderProvider; +import com.google.api.gax.tracing.ApiTracerFactory; import com.google.auth.Credentials; import com.google.auth.oauth2.GoogleCredentials; import com.google.auth.oauth2.QuotaProjectIdProvider; @@ -110,6 +111,8 @@ public abstract class ServiceOptions< private transient ServiceT service; private transient ServiceRpc rpc; + private final ApiTracerFactory apiTracerFactory; + /** * Builder for {@code ServiceOptions}. * @@ -136,6 +139,8 @@ public abstract static class Builder< private String clientLibToken = ServiceOptions.getGoogApiClientLibName(); private String quotaProjectId; + private ApiTracerFactory apiTracerFactory; + @InternalApi("This class should only be extended within google-cloud-java") protected Builder() {} @@ -151,6 +156,7 @@ protected Builder(ServiceOptions options) { transportOptions = options.transportOptions; clientLibToken = options.clientLibToken; quotaProjectId = options.quotaProjectId; + apiTracerFactory = options.apiTracerFactory; } protected abstract ServiceOptions build(); @@ -288,6 +294,11 @@ public B setQuotaProjectId(String quotaProjectId) { return self(); } + public B setApiTracerFactory(ApiTracerFactory apiTracerFactory) { + this.apiTracerFactory = apiTracerFactory; + return self(); + } + protected Set getAllowedClientLibTokens() { return allowedClientLibTokens; } @@ -328,6 +339,7 @@ protected ServiceOptions( builder.quotaProjectId != null ? builder.quotaProjectId : getValueFromCredentialsFile(getCredentialsPath(), "quota_project_id"); + apiTracerFactory = builder.apiTracerFactory; } private static String getCredentialsPath() { @@ -660,6 +672,10 @@ public String getLibraryVersion() { return GaxProperties.getLibraryVersion(this.getClass()); } + public ApiTracerFactory getApiTracerFactory() { + return apiTracerFactory; + } + @InternalApi public final HeaderProvider getMergedHeaderProvider(HeaderProvider internalHeaderProvider) { Map mergedHeaders = From 292beae2e59796d21051a767d71d162fae9d4b74 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Fri, 4 Aug 2023 16:50:14 -0400 Subject: [PATCH 06/26] chore: Add gRPC metrics for DirectPath. --- .../google/api/gax/grpc/GrpcCallContext.java | 5 +- .../google/api/gax/grpc/GrpcStreamTracer.java | 71 +++++++++++++++++++ .../com/google/api/gax/tracing/ApiTracer.java | 6 ++ .../tracing/OpenTelemetryMetricsTracer.java | 36 ++++++++++ 4 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcStreamTracer.java diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java index 5f33c36448..81bdc37337 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java @@ -504,7 +504,10 @@ public ApiTracer getTracer() { @Override public GrpcCallContext withTracer(@Nonnull ApiTracer tracer) { Preconditions.checkNotNull(tracer); - return withCallOptions(callOptions.withOption(TRACER_KEY, tracer)); + return withCallOptions( + callOptions + .withOption(TRACER_KEY, tracer) + .withStreamTracerFactory(new GrpcStreamTracer.Factory(tracer))); } /** {@inheritDoc} */ diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcStreamTracer.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcStreamTracer.java new file mode 100644 index 0000000000..e38e967f27 --- /dev/null +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcStreamTracer.java @@ -0,0 +1,71 @@ +/* + * Copyright 2023 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.api.gax.grpc; + +import com.google.api.gax.tracing.ApiTracer; +import com.google.common.base.Stopwatch; +import io.grpc.Attributes; +import io.grpc.ClientStreamTracer; +import io.grpc.Metadata; +import java.util.concurrent.TimeUnit; + +/** + * Records the time a request is enqueued in a grpc channel queue. Its primary purpose is to measure + * the transition time between asking gRPC to start an RPC and gRPC actually serializing that RPC. + */ +class GrpcStreamTracer extends ClientStreamTracer { + + private final Stopwatch stopwatch = Stopwatch.createUnstarted(); + private final ApiTracer tracer; + + public GrpcStreamTracer(ApiTracer tracer) { + this.tracer = tracer; + stopwatch.start(); + } + + @Override + public void createPendingStream() { + tracer.grpcTargetResolutionDelay(stopwatch.elapsed(TimeUnit.NANOSECONDS)); + stopwatch.reset(); + stopwatch.start(); + } + + @Override + public void streamCreated(Attributes transportAttrs, Metadata headers) { + tracer.grpcChannelReadinessDelay(stopwatch.elapsed(TimeUnit.NANOSECONDS)); + stopwatch.reset(); + stopwatch.start(); + } + + @Override + public void outboundMessageSent(int seqNo, long optionalWireSize, long optionalUncompressedSize) { + tracer.grpcCallSendDelay(stopwatch.elapsed(TimeUnit.NANOSECONDS)); + } + + static class Factory extends ClientStreamTracer.Factory { + + private final ApiTracer tracer; + + Factory(ApiTracer tracer) { + this.tracer = tracer; + } + + @Override + public ClientStreamTracer newClientStreamTracer(StreamInfo info, Metadata headers) { + return new GrpcStreamTracer(tracer); + } + } +} diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java index 73012d1bfd..8bd9fd6cbe 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java @@ -165,6 +165,12 @@ default String attemptLatencyName() { */ void batchRequestSent(long elementCount, long requestSize); + default void grpcTargetResolutionDelay(long elapsed) {}; + + default void grpcChannelReadinessDelay(long elapsed) {}; + + default void grpcCallSendDelay(long elapsed) {}; + /** * A context class to be used with {@link #inScope()} and a try-with-resources block. Closing a * {@link Scope} removes any context that the underlying implementation might've set in {@link diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java index 13f53012e0..a22f466cd2 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java @@ -57,6 +57,9 @@ public class OpenTelemetryMetricsTracer implements ApiTracer { private DoubleHistogram operationLatencyRecorder; private LongHistogram retryCountRecorder; + private DoubleHistogram targetResolutionDelayRecorder; + private DoubleHistogram channelReadinessDelayRecorder; + private DoubleHistogram callSendDelayRecorder; private Attributes defaultAttributes; Map operationLatencyLabels = new HashMap<>(); @@ -84,6 +87,24 @@ public OpenTelemetryMetricsTracer(Meter meter, SpanName spanName) { .setUnit("1") .ofLongs() .build(); + this.targetResolutionDelayRecorder = + meter + .histogramBuilder("target_resolution_delay") + .setDescription("Delay caused by name resolution") + .setUnit("ns") + .build(); + this.channelReadinessDelayRecorder = + meter + .histogramBuilder("channel_readiness_delay") + .setDescription("Delay caused by establishing connection") + .setUnit("ns") + .build(); + this.callSendDelayRecorder = + meter + .histogramBuilder("call_send_delay") + .setDescription("Call send delay. (after the connection is ready)") + .setUnit("ns") + .build(); this.defaultAttributes = Attributes.of(stringKey("method_name"), spanName.toString()); } @@ -166,4 +187,19 @@ public void requestSent() {} @Override public void batchRequestSent(long elementCount, long requestSize) {} + + @Override + public void grpcTargetResolutionDelay(long elapsed) { + this.targetResolutionDelayRecorder.record(elapsed, defaultAttributes); + } + + @Override + public void grpcChannelReadinessDelay(long elapsed) { + this.channelReadinessDelayRecorder.record(elapsed, defaultAttributes); + } + + @Override + public void grpcCallSendDelay(long elapsed) { + this.callSendDelayRecorder.record(elapsed, defaultAttributes); + } } From 9cfd35e3e914081cb9ef79908b24a24522bb36aa Mon Sep 17 00:00:00 2001 From: Blake Li Date: Tue, 22 Aug 2023 00:01:07 -0400 Subject: [PATCH 07/26] Add GFE metadata metrics. Add status labels. --- .../api/gax/grpc/GrpcDirectCallable.java | 70 ++++++++++++++-- .../com/google/api/gax/tracing/ApiTracer.java | 18 ++-- .../tracing/OpenTelemetryMetricsTracer.java | 83 ++++++++++++------- 3 files changed, 130 insertions(+), 41 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcDirectCallable.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcDirectCallable.java index 5b6a5f1bad..eaec58a1aa 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcDirectCallable.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcDirectCallable.java @@ -30,13 +30,20 @@ package com.google.api.gax.grpc; import com.google.api.core.ApiFuture; +import com.google.api.core.ApiFutureCallback; +import com.google.api.core.ApiFutures; import com.google.api.core.ListenableFutureToApiFuture; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.UnaryCallable; +import com.google.api.gax.tracing.ApiTracer; import com.google.common.base.Preconditions; +import com.google.common.util.concurrent.MoreExecutors; import io.grpc.ClientCall; +import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.stub.ClientCalls; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * {@code GrpcDirectCallable} creates gRPC calls. @@ -56,18 +63,71 @@ class GrpcDirectCallable extends UnaryCallable futureCall(RequestT request, ApiCallContext inputContext) { Preconditions.checkNotNull(request); Preconditions.checkNotNull(inputContext); - - ClientCall clientCall = GrpcClientCalls.newCall(descriptor, inputContext); - + final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); + GrpcCallContext grpcCallContext = responseMetadata.addHandlers(inputContext); + ClientCall clientCall = + GrpcClientCalls.newCall(descriptor, grpcCallContext); + GfeUnaryCallback callback = + new GfeUnaryCallback(inputContext.getTracer(), responseMetadata); + ApiFuture future; if (awaitTrailers) { - return new ListenableFutureToApiFuture<>(ClientCalls.futureUnaryCall(clientCall, request)); + future = new ListenableFutureToApiFuture<>(ClientCalls.futureUnaryCall(clientCall, request)); } else { - return GrpcClientCalls.eagerFutureUnaryCall(clientCall, request); + future = GrpcClientCalls.eagerFutureUnaryCall(clientCall, request); } + ApiFutures.addCallback(future, callback, MoreExecutors.directExecutor()); + return future; } @Override public String toString() { return String.format("direct(%s)", descriptor); } + + 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 class GfeUnaryCallback implements ApiFutureCallback { + + private final ApiTracer tracer; + private final GrpcResponseMetadata responseMetadata; + + GfeUnaryCallback(ApiTracer tracer, GrpcResponseMetadata responseMetadata) { + this.tracer = tracer; + this.responseMetadata = responseMetadata; + } + + @Override + public void onFailure(Throwable throwable) { + // Util.recordMetricsFromMetadata(responseMetadata, tracer, throwable); + } + + @Override + public void onSuccess(ResponseT response) { + Metadata metadata = responseMetadata.getMetadata(); + if (metadata == null) { + return; + } + String allKeys = metadata.keys().stream().reduce((a, b) -> a + ", " + b).get(); + // System.out.println( + // "************************ metadata size: " + // + metadata.keys().size() + // + ", all keys: " + // + allKeys); + if (metadata.get(SERVER_TIMING_HEADER_KEY) == null) { + return; + } + + String durMetadata = metadata.get(SERVER_TIMING_HEADER_KEY); + Matcher matcher = SERVER_TIMING_HEADER_PATTERN.matcher(durMetadata); + // this should always be true + if (matcher.find()) { + long latency = Long.valueOf(matcher.group("dur")); + tracer.recordGfeMetadata(latency); + } + // System.out.println("GFE metadata: " + metadata.get(SERVER_TIMING_HEADER_KEY)); + } + } } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java index 8bd9fd6cbe..e566167590 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java @@ -78,7 +78,7 @@ public interface ApiTracer { * * @param id the local connection identifier of the selected connection. */ - void connectionSelected(String id); + default void connectionSelected(String id) {}; /** * Adds an annotation that an attempt is about to start. In general this should occur at the very @@ -98,10 +98,10 @@ public interface ApiTracer { * @param attemptNumber the zero based sequential attempt number. * @param request request of this attempt. */ - void attemptStarted(Object request, int attemptNumber); + default void attemptStarted(Object request, int attemptNumber) {}; /** Adds an annotation that the attempt succeeded. */ - void attemptSucceeded(); + default void attemptSucceeded() {}; default void attemptSucceeded(Object response) {}; @@ -143,19 +143,19 @@ default String attemptLatencyName() { * * @param error the error that caused the long running operation fail. */ - void lroStartFailed(Throwable error); + default void lroStartFailed(Throwable error) {}; /** * Signals that the initial RPC successfully started the long running operation. The long running * operation will now be polled for completion. */ - void lroStartSucceeded(); + default void lroStartSucceeded() {}; /** Adds an annotation that a streaming response has been received. */ - void responseReceived(); + default void responseReceived() {}; /** Adds an annotation that a streaming request has been sent. */ - void requestSent(); + default void requestSent() {}; /** * Adds an annotation that a batch of writes has been flushed. @@ -163,7 +163,7 @@ default String attemptLatencyName() { * @param elementCount the number of elements in the batch. * @param requestSize the size of the batch in bytes. */ - void batchRequestSent(long elementCount, long requestSize); + default void batchRequestSent(long elementCount, long requestSize) {}; default void grpcTargetResolutionDelay(long elapsed) {}; @@ -171,6 +171,8 @@ default String attemptLatencyName() { default void grpcCallSendDelay(long elapsed) {}; + default void recordGfeMetadata(long latency) {}; + /** * A context class to be used with {@link #inScope()} and a try-with-resources block. Closing a * {@link Scope} removes any context that the underlying implementation might've set in {@link diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java index a22f466cd2..efb7d0345f 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java @@ -32,6 +32,8 @@ import static io.opentelemetry.api.common.AttributeKey.stringKey; +import com.google.api.gax.rpc.ApiException; +import com.google.api.gax.rpc.StatusCode; import com.google.common.base.Stopwatch; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; @@ -40,10 +42,13 @@ import io.opentelemetry.api.metrics.Meter; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.CancellationException; import java.util.concurrent.TimeUnit; +import javax.annotation.Nullable; import org.threeten.bp.Duration; public class OpenTelemetryMetricsTracer implements ApiTracer { + public static final String STATUS_ATTRIBUTE = "status"; protected Meter meter; private Stopwatch attemptTimer; @@ -56,11 +61,12 @@ public class OpenTelemetryMetricsTracer implements ApiTracer { private DoubleHistogram operationLatencyRecorder; private LongHistogram retryCountRecorder; + private LongHistogram gfeLatencyRecorder; private DoubleHistogram targetResolutionDelayRecorder; private DoubleHistogram channelReadinessDelayRecorder; private DoubleHistogram callSendDelayRecorder; - private Attributes defaultAttributes; + private Attributes attributes; Map operationLatencyLabels = new HashMap<>(); @@ -87,6 +93,13 @@ public OpenTelemetryMetricsTracer(Meter meter, SpanName spanName) { .setUnit("1") .ofLongs() .build(); + this.gfeLatencyRecorder = + meter + .histogramBuilder("gfe_latency") + .setDescription("GFE latency") + .setUnit("1") + .ofLongs() + .build(); this.targetResolutionDelayRecorder = meter .histogramBuilder("target_resolution_delay") @@ -105,7 +118,7 @@ public OpenTelemetryMetricsTracer(Meter meter, SpanName spanName) { .setDescription("Call send delay. (after the connection is ready)") .setUnit("ns") .build(); - this.defaultAttributes = Attributes.of(stringKey("method_name"), spanName.toString()); + this.attributes = Attributes.of(stringKey("method_name"), spanName.toString()); } @Override @@ -125,7 +138,8 @@ public void addOperationLatencyLabels(String key, String value) { @Override public void operationSucceeded(Object response) { AttributesBuilder attributesBuilder = Attributes.builder(); - attributesBuilder.putAll(defaultAttributes); + attributesBuilder.putAll(attributes); + attributesBuilder.put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()); operationLatencyLabels.forEach((key, value) -> attributesBuilder.put(stringKey(key), value)); operationLatencyRecorder.record( operationTimer.elapsed(TimeUnit.MILLISECONDS), attributesBuilder.build()); @@ -135,10 +149,11 @@ public void operationSucceeded(Object response) { public void operationCancelled() {} @Override - public void operationFailed(Throwable error) {} - - @Override - public void connectionSelected(String id) {} + public void operationFailed(Throwable error) { + Attributes newAttributes = + attributes.toBuilder().put(STATUS_ATTRIBUTE, extractStatus(error)).build(); + operationLatencyRecorder.record(operationTimer.elapsed(TimeUnit.MILLISECONDS), newAttributes); + } @Override public void attemptStarted(int attemptNumber) {} @@ -153,14 +168,20 @@ public void attemptSucceeded() {} @Override public void attemptSucceeded(Object response) { - attemptLatencyRecorder.record(attemptTimer.elapsed(TimeUnit.MILLISECONDS), defaultAttributes); + Attributes newAttributes = + attributes.toBuilder().put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()).build(); + attemptLatencyRecorder.record(attemptTimer.elapsed(TimeUnit.MILLISECONDS), newAttributes); } @Override public void attemptCancelled() {} @Override - public void attemptFailed(Throwable error, Duration delay) {} + public void attemptFailed(Throwable error, Duration delay) { + Attributes newAttributes = + attributes.toBuilder().put(STATUS_ATTRIBUTE, extractStatus(error)).build(); + attemptLatencyRecorder.record(attemptTimer.elapsed(TimeUnit.MILLISECONDS), newAttributes); + } @Override public void attemptFailedRetriesExhausted(Throwable error) {} @@ -170,36 +191,42 @@ public void attemptPermanentFailure(Throwable error) {} @Override public void retryCount(int count) { - retryCountRecorder.record(count, defaultAttributes); + retryCountRecorder.record(count, attributes); } - @Override - public void lroStartFailed(Throwable error) {} - - @Override - public void lroStartSucceeded() {} - - @Override - public void responseReceived() {} - - @Override - public void requestSent() {} - - @Override - public void batchRequestSent(long elementCount, long requestSize) {} - @Override public void grpcTargetResolutionDelay(long elapsed) { - this.targetResolutionDelayRecorder.record(elapsed, defaultAttributes); + this.targetResolutionDelayRecorder.record(elapsed, attributes); } @Override public void grpcChannelReadinessDelay(long elapsed) { - this.channelReadinessDelayRecorder.record(elapsed, defaultAttributes); + this.channelReadinessDelayRecorder.record(elapsed, attributes); } @Override public void grpcCallSendDelay(long elapsed) { - this.callSendDelayRecorder.record(elapsed, defaultAttributes); + this.callSendDelayRecorder.record(elapsed, attributes); + } + + @Override + public void recordGfeMetadata(long latency) { + this.gfeLatencyRecorder.record(latency); + } + + static String extractStatus(@Nullable Throwable error) { + final String statusString; + + if (error == null) { + return StatusCode.Code.OK.toString(); + } else if (error instanceof CancellationException) { + statusString = StatusCode.Code.CANCELLED.toString(); + } else if (error instanceof ApiException) { + statusString = ((ApiException) error).getStatusCode().getCode().toString(); + } else { + statusString = StatusCode.Code.UNKNOWN.toString(); + } + + return statusString; } } From 4d19614eb7392cdbe0d4732239222446a717fa12 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Wed, 23 Aug 2023 00:31:55 -0400 Subject: [PATCH 08/26] Add gax thread count metric. --- .../api/gax/tracing/ClientMetricsTracer.java | 2 ++ .../OpenTelemetryClientMetricsTracer.java | 19 ++++++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ClientMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ClientMetricsTracer.java index 12b7f534c5..a67b951808 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ClientMetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ClientMetricsTracer.java @@ -4,6 +4,8 @@ public interface ClientMetricsTracer { void recordCurrentChannelSize(int channelSize); + default void recordGaxThread(int threadCount) {}; + default String channelSizeName() { return "channel_size"; }; diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryClientMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryClientMetricsTracer.java index 61437760af..1537bde102 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryClientMetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryClientMetricsTracer.java @@ -5,18 +5,31 @@ public class OpenTelemetryClientMetricsTracer implements ClientMetricsTracer { - private final LongGaugeBuilder longGaugeBuilder; + private final LongGaugeBuilder channelSizeRecorder; + private final LongGaugeBuilder threadCountRecorder; private Meter meter; public OpenTelemetryClientMetricsTracer(Meter meter) { this.meter = meter; - longGaugeBuilder = + channelSizeRecorder = meter.gaugeBuilder(channelSizeName()).setDescription("Channel Size").setUnit("1").ofLongs(); + threadCountRecorder = + meter + .gaugeBuilder("client_thread_count") + .setDescription("Current Thread Count created by Client") + .setUnit("1") + .ofLongs(); } @Override public void recordCurrentChannelSize(int channelSize) { - longGaugeBuilder.buildWithCallback( + channelSizeRecorder.buildWithCallback( observableLongMeasurement -> observableLongMeasurement.record(channelSize)); } + + @Override + public void recordGaxThread(int threadCount) { + threadCountRecorder.buildWithCallback( + observableLongMeasurement -> observableLongMeasurement.record(threadCount)); + } } From 527985f81ec9e9579402ea83bd6b0d9eec6388b7 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Wed, 23 Aug 2023 22:12:30 -0400 Subject: [PATCH 09/26] Make metric recorders protected. --- .../tracing/OpenTelemetryMetricsTracer.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java index efb7d0345f..95a54cdf7a 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java @@ -55,18 +55,18 @@ public class OpenTelemetryMetricsTracer implements ApiTracer { private final Stopwatch operationTimer = Stopwatch.createStarted(); - private SpanName spanName; + private final SpanName spanName; - private DoubleHistogram attemptLatencyRecorder; + protected DoubleHistogram attemptLatencyRecorder; - private DoubleHistogram operationLatencyRecorder; - private LongHistogram retryCountRecorder; - private LongHistogram gfeLatencyRecorder; + protected DoubleHistogram operationLatencyRecorder; + protected LongHistogram retryCountRecorder; + protected LongHistogram gfeLatencyRecorder; - private DoubleHistogram targetResolutionDelayRecorder; - private DoubleHistogram channelReadinessDelayRecorder; - private DoubleHistogram callSendDelayRecorder; - private Attributes attributes; + protected DoubleHistogram targetResolutionDelayRecorder; + protected DoubleHistogram channelReadinessDelayRecorder; + protected DoubleHistogram callSendDelayRecorder; + protected Attributes attributes; Map operationLatencyLabels = new HashMap<>(); @@ -76,7 +76,7 @@ public OpenTelemetryMetricsTracer(Meter meter, SpanName spanName) { this.attemptLatencyRecorder = meter .histogramBuilder(attemptLatencyName()) - .setDescription("Duration of an individual operation attempt") + .setDescription("Duration of an individual attempt") .setUnit("ms") .build(); this.operationLatencyRecorder = From 56c999923176abc162335c6f11c683057b387635 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Fri, 1 Sep 2023 12:08:00 -0400 Subject: [PATCH 10/26] Add OperationCounter --- .../gax/tracing/OpenTelemetryMetricsTracer.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java index 95a54cdf7a..62126f0a87 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java @@ -38,6 +38,7 @@ import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.metrics.DoubleHistogram; +import io.opentelemetry.api.metrics.LongCounter; import io.opentelemetry.api.metrics.LongHistogram; import io.opentelemetry.api.metrics.Meter; import java.util.HashMap; @@ -66,6 +67,8 @@ public class OpenTelemetryMetricsTracer implements ApiTracer { protected DoubleHistogram targetResolutionDelayRecorder; protected DoubleHistogram channelReadinessDelayRecorder; protected DoubleHistogram callSendDelayRecorder; + protected LongCounter operationCountRecorder; + protected Attributes attributes; Map operationLatencyLabels = new HashMap<>(); @@ -118,6 +121,12 @@ public OpenTelemetryMetricsTracer(Meter meter, SpanName spanName) { .setDescription("Call send delay. (after the connection is ready)") .setUnit("ns") .build(); + this.operationCountRecorder = + meter + .counterBuilder("operation_count") + .setDescription("Count of Operations") + .setUnit("1") + .build(); this.attributes = Attributes.of(stringKey("method_name"), spanName.toString()); } @@ -141,8 +150,9 @@ public void operationSucceeded(Object response) { attributesBuilder.putAll(attributes); attributesBuilder.put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()); operationLatencyLabels.forEach((key, value) -> attributesBuilder.put(stringKey(key), value)); - operationLatencyRecorder.record( - operationTimer.elapsed(TimeUnit.MILLISECONDS), attributesBuilder.build()); + Attributes allAttributes = attributesBuilder.build(); + operationLatencyRecorder.record(operationTimer.elapsed(TimeUnit.MILLISECONDS), allAttributes); + operationCountRecorder.add(1, allAttributes); } @Override From d249b669d8ec7785bcb5b0e6d27abdc7933b1bac Mon Sep 17 00:00:00 2001 From: Blake Li Date: Wed, 4 Oct 2023 23:16:54 -0400 Subject: [PATCH 11/26] Add MetricsRecorder --- .../java/com/google/api/gax/tracing/MetricsRecorder.java | 5 +++++ .../google/api/gax/tracing/OpenTelemetryMetricsFactory.java | 2 ++ 2 files changed, 7 insertions(+) create mode 100644 gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java new file mode 100644 index 0000000000..6abdb75124 --- /dev/null +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java @@ -0,0 +1,5 @@ +package com.google.api.gax.tracing; + +public interface MetricsRecorder { + +} diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java index 7d59f7ea2c..ba6b8da8b6 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java @@ -46,6 +46,8 @@ public class OpenTelemetryMetricsFactory implements ApiTracerFactory { protected Meter meter; + protected MetricsRecorder metricsRecorder; + public OpenTelemetryMetricsFactory( OpenTelemetry openTelemetry, String libraryName, String libraryVersion) { meter = From 57571b2948425d021f5817ce12c09cda365a9b86 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Tue, 2 Jan 2024 17:58:41 -0500 Subject: [PATCH 12/26] Add MetricsRecorder --- .../google/api/gax/grpc/GrpcCallContext.java | 2 +- .../api/gax/tracing/MetricsRecorder.java | 86 ++++++++++++++++++- .../tracing/OpenTelemetryMetricsFactory.java | 9 +- .../tracing/OpenTelemetryMetricsTracer.java | 8 +- 4 files changed, 99 insertions(+), 6 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java index 81bdc37337..321177987a 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java @@ -66,7 +66,7 @@ */ @BetaApi("Reference ApiCallContext instead - this class is likely to experience breaking changes") public final class GrpcCallContext implements ApiCallContext { - static final CallOptions.Key TRACER_KEY = CallOptions.Key.create("gax.tracer"); + public static final CallOptions.Key TRACER_KEY = CallOptions.Key.create("gax.tracer"); private final Channel channel; private final CallOptions callOptions; diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java index 6abdb75124..7ee022a4c8 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java @@ -1,5 +1,89 @@ package com.google.api.gax.tracing; -public interface MetricsRecorder { +import com.google.common.base.Stopwatch; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.DoubleHistogram; +import io.opentelemetry.api.metrics.LongCounter; +import io.opentelemetry.api.metrics.LongHistogram; +import io.opentelemetry.api.metrics.Meter; +public class MetricsRecorder { + public static final String STATUS_ATTRIBUTE = "status"; + protected Meter meter; + + private Stopwatch attemptTimer; + + private final Stopwatch operationTimer = Stopwatch.createStarted(); + + protected DoubleHistogram attemptLatencyRecorder; + + protected DoubleHistogram operationLatencyRecorder; + protected LongHistogram retryCountRecorder; + protected LongHistogram gfeLatencyRecorder; + + protected DoubleHistogram targetResolutionDelayRecorder; + protected DoubleHistogram channelReadinessDelayRecorder; + protected DoubleHistogram callSendDelayRecorder; + protected LongCounter operationCountRecorder; + + protected Attributes attributes; + + public MetricsRecorder(Meter meter) { + this.meter = meter; + this.attemptLatencyRecorder = + meter + .histogramBuilder("attempt_latency") + .setDescription("Duration of an individual attempt") + .setUnit("ms") + .build(); + this.operationLatencyRecorder = + meter + .histogramBuilder("operation_latency") + .setDescription( + "Total time until final operation success or failure, including retries and backoff.") + .setUnit("ms") + .build(); + this.retryCountRecorder = + meter + .histogramBuilder("retry_count") + .setDescription("Number of additional attempts per operation after initial attempt") + .setUnit("1") + .ofLongs() + .build(); + this.gfeLatencyRecorder = + meter + .histogramBuilder("gfe_latency") + .setDescription("GFE latency") + .setUnit("1") + .ofLongs() + .build(); + this.targetResolutionDelayRecorder = + meter + .histogramBuilder("target_resolution_delay") + .setDescription("Delay caused by name resolution") + .setUnit("ns") + .build(); + this.channelReadinessDelayRecorder = + meter + .histogramBuilder("channel_readiness_delay") + .setDescription("Delay caused by establishing connection") + .setUnit("ns") + .build(); + this.callSendDelayRecorder = + meter + .histogramBuilder("call_send_delay") + .setDescription("Call send delay. (after the connection is ready)") + .setUnit("ns") + .build(); + this.operationCountRecorder = + meter + .counterBuilder("operation_count") + .setDescription("Count of Operations") + .setUnit("1") + .build(); + } + + public void recordAttemptLatency(double attemptLatency) { + attemptLatencyRecorder.record(attemptLatency); + } } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java index ba6b8da8b6..6157450c4d 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java @@ -30,6 +30,7 @@ package com.google.api.gax.tracing; import com.google.api.core.InternalApi; +import com.google.api.gax.core.GaxProperties; import io.opencensus.trace.Tracer; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.metrics.Meter; @@ -51,12 +52,16 @@ public class OpenTelemetryMetricsFactory implements ApiTracerFactory { public OpenTelemetryMetricsFactory( OpenTelemetry openTelemetry, String libraryName, String libraryVersion) { meter = - openTelemetry.meterBuilder(libraryName).setInstrumentationVersion(libraryVersion).build(); + openTelemetry + .meterBuilder("gax") + .setInstrumentationVersion(GaxProperties.getGaxVersion()) + .build(); + metricsRecorder = new MetricsRecorder(meter); } @Override public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) { - return new OpenTelemetryMetricsTracer(meter, spanName); + return new OpenTelemetryMetricsTracer(meter, spanName, metricsRecorder); } @Override diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java index 62126f0a87..20c2f32841 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java @@ -73,7 +73,10 @@ public class OpenTelemetryMetricsTracer implements ApiTracer { Map operationLatencyLabels = new HashMap<>(); - public OpenTelemetryMetricsTracer(Meter meter, SpanName spanName) { + protected MetricsRecorder metricsRecorder; + + public OpenTelemetryMetricsTracer( + Meter meter, SpanName spanName, MetricsRecorder metricsRecorder) { this.meter = meter; this.spanName = spanName; this.attemptLatencyRecorder = @@ -128,6 +131,7 @@ public OpenTelemetryMetricsTracer(Meter meter, SpanName spanName) { .setUnit("1") .build(); this.attributes = Attributes.of(stringKey("method_name"), spanName.toString()); + this.metricsRecorder = metricsRecorder; } @Override @@ -180,7 +184,7 @@ public void attemptSucceeded() {} public void attemptSucceeded(Object response) { Attributes newAttributes = attributes.toBuilder().put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()).build(); - attemptLatencyRecorder.record(attemptTimer.elapsed(TimeUnit.MILLISECONDS), newAttributes); + metricsRecorder.recordAttemptLatency(attemptTimer.elapsed(TimeUnit.MILLISECONDS)); } @Override From 3f240499366e9b4c8acf76c560426f00dc625bda Mon Sep 17 00:00:00 2001 From: Blake Li Date: Tue, 9 Jan 2024 10:45:15 -0500 Subject: [PATCH 13/26] Expose addAdditionalAttributes in ApiTracer. --- .../main/java/com/google/api/gax/tracing/ApiTracer.java | 1 + .../java/com/google/api/gax/tracing/MetricsRecorder.java | 8 ++++++-- .../api/gax/tracing/OpenTelemetryMetricsTracer.java | 8 +++++++- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java index e566167590..e18ed11e0e 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java @@ -173,6 +173,7 @@ default String attemptLatencyName() { default void recordGfeMetadata(long latency) {}; + default void addAdditionalAttributes(String key, String value) {}; /** * A context class to be used with {@link #inScope()} and a try-with-resources block. Closing a * {@link Scope} removes any context that the underlying implementation might've set in {@link diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java index 7ee022a4c8..d28458d204 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java @@ -2,10 +2,12 @@ import com.google.common.base.Stopwatch; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.LongCounter; import io.opentelemetry.api.metrics.LongHistogram; import io.opentelemetry.api.metrics.Meter; +import java.util.Map; public class MetricsRecorder { public static final String STATUS_ATTRIBUTE = "status"; @@ -83,7 +85,9 @@ public MetricsRecorder(Meter meter) { .build(); } - public void recordAttemptLatency(double attemptLatency) { - attemptLatencyRecorder.record(attemptLatency); + public void recordAttemptLatency(double attemptLatency, Map attributes) { + AttributesBuilder attributesBuilder = Attributes.builder(); + attributes.forEach(attributesBuilder::put); + attemptLatencyRecorder.record(attemptLatency, attributesBuilder.build()); } } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java index 20c2f32841..339b0aa592 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java @@ -72,6 +72,7 @@ public class OpenTelemetryMetricsTracer implements ApiTracer { protected Attributes attributes; Map operationLatencyLabels = new HashMap<>(); + private Map additionalAttributes = new HashMap<>(); protected MetricsRecorder metricsRecorder; @@ -184,7 +185,8 @@ public void attemptSucceeded() {} public void attemptSucceeded(Object response) { Attributes newAttributes = attributes.toBuilder().put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()).build(); - metricsRecorder.recordAttemptLatency(attemptTimer.elapsed(TimeUnit.MILLISECONDS)); + metricsRecorder.recordAttemptLatency( + attemptTimer.elapsed(TimeUnit.MILLISECONDS), additionalAttributes); } @Override @@ -243,4 +245,8 @@ static String extractStatus(@Nullable Throwable error) { return statusString; } + + public void addAdditionalAttributes(String key, String value) { + additionalAttributes.put(key, value); + } } From 09c7c7b33c936caa41c02d617cad7ff5ce78a6d5 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Tue, 9 Jan 2024 17:47:49 -0500 Subject: [PATCH 14/26] Move OpenTelemetry logics to MetricsRecorder --- .../api/gax/tracing/MetricsRecorder.java | 32 ++- .../google/api/gax/tracing/MetricsTracer.java | 141 ++++++++++ .../tracing/OpenTelemetryMetricsFactory.java | 2 +- .../tracing/OpenTelemetryMetricsTracer.java | 252 ------------------ 4 files changed, 167 insertions(+), 260 deletions(-) create mode 100644 gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java delete mode 100644 gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java index d28458d204..39b2b3ddff 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java @@ -1,6 +1,5 @@ package com.google.api.gax.tracing; -import com.google.common.base.Stopwatch; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.metrics.DoubleHistogram; @@ -10,13 +9,8 @@ import java.util.Map; public class MetricsRecorder { - public static final String STATUS_ATTRIBUTE = "status"; protected Meter meter; - private Stopwatch attemptTimer; - - private final Stopwatch operationTimer = Stopwatch.createStarted(); - protected DoubleHistogram attemptLatencyRecorder; protected DoubleHistogram operationLatencyRecorder; @@ -28,6 +22,8 @@ public class MetricsRecorder { protected DoubleHistogram callSendDelayRecorder; protected LongCounter operationCountRecorder; + protected LongCounter attemptCountRecorder; + protected Attributes attributes; public MetricsRecorder(Meter meter) { @@ -83,11 +79,33 @@ public MetricsRecorder(Meter meter) { .setDescription("Count of Operations") .setUnit("1") .build(); + this.attemptCountRecorder = + meter + .counterBuilder("attempt_count") + .setDescription("Count of Attempts") + .setUnit("1") + .build(); } public void recordAttemptLatency(double attemptLatency, Map attributes) { + attemptLatencyRecorder.record(attemptLatency, toOtelAttributes(attributes)); + } + + public void recordAttemptCount(long count, Map attributes) { + attemptCountRecorder.add(count, toOtelAttributes(attributes)); + } + + public void recordOperationLatency(double operationLatency, Map attributes) { + operationLatencyRecorder.record(operationLatency, toOtelAttributes(attributes)); + } + + public void recordOperationCount(long count, Map attributes) { + operationCountRecorder.add(count, toOtelAttributes(attributes)); + } + + private Attributes toOtelAttributes(Map attributes) { AttributesBuilder attributesBuilder = Attributes.builder(); attributes.forEach(attributesBuilder::put); - attemptLatencyRecorder.record(attemptLatency, attributesBuilder.build()); + return attributesBuilder.build(); } } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java new file mode 100644 index 0000000000..9c02a50a78 --- /dev/null +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java @@ -0,0 +1,141 @@ +/* + * Copyright 2023 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.google.api.gax.tracing; + +import com.google.api.gax.rpc.ApiException; +import com.google.api.gax.rpc.StatusCode; +import com.google.common.base.Stopwatch; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.CancellationException; +import java.util.concurrent.TimeUnit; +import javax.annotation.Nullable; +import org.threeten.bp.Duration; + +public class MetricsTracer implements ApiTracer { + public static final String STATUS_ATTRIBUTE = "status"; + + private Stopwatch attemptTimer; + + private final Stopwatch operationTimer = Stopwatch.createStarted(); + + private final Map attributes = new HashMap<>(); + + protected MetricsRecorder metricsRecorder; + + public MetricsTracer( + SpanName spanName, MetricsRecorder metricsRecorder) { + this.attributes.put("method_name", spanName.toString()); + this.metricsRecorder = metricsRecorder; + } + + @Override + public Scope inScope() { + return () -> {}; + } + + @Override + public void operationSucceeded() {} + + @Override + public void operationSucceeded(Object response) { + attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()); + metricsRecorder.recordOperationLatency(operationTimer.elapsed(TimeUnit.MILLISECONDS), + attributes); + metricsRecorder.recordOperationCount(1, attributes); + } + + @Override + public void operationCancelled() {} + + @Override + public void operationFailed(Throwable error) { + attributes.put(STATUS_ATTRIBUTE, extractStatus(error)); + metricsRecorder.recordOperationLatency(operationTimer.elapsed(TimeUnit.MILLISECONDS), + attributes); + metricsRecorder.recordOperationCount(1, attributes); + } + + @Override + public void attemptStarted(int attemptNumber) {} + + @Override + public void attemptStarted(Object request, int attemptNumber) { + attemptTimer = Stopwatch.createStarted(); + } + + @Override + public void attemptSucceeded() {} + + @Override + public void attemptSucceeded(Object response) { + attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()); + metricsRecorder.recordAttemptLatency( + attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); + metricsRecorder.recordAttemptCount(1, attributes); + } + + @Override + public void attemptCancelled() {} + + @Override + public void attemptFailed(Throwable error, Duration delay) { + attributes.put(STATUS_ATTRIBUTE, extractStatus(error)); + metricsRecorder.recordAttemptLatency(attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); + metricsRecorder.recordAttemptCount(1, attributes); + } + + @Override + public void attemptFailedRetriesExhausted(Throwable error) {} + + @Override + public void attemptPermanentFailure(Throwable error) {} + + static String extractStatus(@Nullable Throwable error) { + final String statusString; + + if (error == null) { + return StatusCode.Code.OK.toString(); + } else if (error instanceof CancellationException) { + statusString = StatusCode.Code.CANCELLED.toString(); + } else if (error instanceof ApiException) { + statusString = ((ApiException) error).getStatusCode().getCode().toString(); + } else { + statusString = StatusCode.Code.UNKNOWN.toString(); + } + + return statusString; + } + + public void addAdditionalAttributes(String key, String value) { + attributes.put(key, value); + } +} diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java index 6157450c4d..2eef2c347f 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java @@ -61,7 +61,7 @@ public OpenTelemetryMetricsFactory( @Override public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) { - return new OpenTelemetryMetricsTracer(meter, spanName, metricsRecorder); + return new MetricsTracer(spanName, metricsRecorder); } @Override diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java deleted file mode 100644 index 339b0aa592..0000000000 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsTracer.java +++ /dev/null @@ -1,252 +0,0 @@ -/* - * Copyright 2023 Google LLC - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following disclaimer - * in the documentation and/or other materials provided with the - * distribution. - * * Neither the name of Google LLC nor the names of its - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - -package com.google.api.gax.tracing; - -import static io.opentelemetry.api.common.AttributeKey.stringKey; - -import com.google.api.gax.rpc.ApiException; -import com.google.api.gax.rpc.StatusCode; -import com.google.common.base.Stopwatch; -import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.common.AttributesBuilder; -import io.opentelemetry.api.metrics.DoubleHistogram; -import io.opentelemetry.api.metrics.LongCounter; -import io.opentelemetry.api.metrics.LongHistogram; -import io.opentelemetry.api.metrics.Meter; -import java.util.HashMap; -import java.util.Map; -import java.util.concurrent.CancellationException; -import java.util.concurrent.TimeUnit; -import javax.annotation.Nullable; -import org.threeten.bp.Duration; - -public class OpenTelemetryMetricsTracer implements ApiTracer { - public static final String STATUS_ATTRIBUTE = "status"; - protected Meter meter; - - private Stopwatch attemptTimer; - - private final Stopwatch operationTimer = Stopwatch.createStarted(); - - private final SpanName spanName; - - protected DoubleHistogram attemptLatencyRecorder; - - protected DoubleHistogram operationLatencyRecorder; - protected LongHistogram retryCountRecorder; - protected LongHistogram gfeLatencyRecorder; - - protected DoubleHistogram targetResolutionDelayRecorder; - protected DoubleHistogram channelReadinessDelayRecorder; - protected DoubleHistogram callSendDelayRecorder; - protected LongCounter operationCountRecorder; - - protected Attributes attributes; - - Map operationLatencyLabels = new HashMap<>(); - private Map additionalAttributes = new HashMap<>(); - - protected MetricsRecorder metricsRecorder; - - public OpenTelemetryMetricsTracer( - Meter meter, SpanName spanName, MetricsRecorder metricsRecorder) { - this.meter = meter; - this.spanName = spanName; - this.attemptLatencyRecorder = - meter - .histogramBuilder(attemptLatencyName()) - .setDescription("Duration of an individual attempt") - .setUnit("ms") - .build(); - this.operationLatencyRecorder = - meter - .histogramBuilder("operation_latency") - .setDescription( - "Total time until final operation success or failure, including retries and backoff.") - .setUnit("ms") - .build(); - this.retryCountRecorder = - meter - .histogramBuilder("retry_count") - .setDescription("Number of additional attempts per operation after initial attempt") - .setUnit("1") - .ofLongs() - .build(); - this.gfeLatencyRecorder = - meter - .histogramBuilder("gfe_latency") - .setDescription("GFE latency") - .setUnit("1") - .ofLongs() - .build(); - this.targetResolutionDelayRecorder = - meter - .histogramBuilder("target_resolution_delay") - .setDescription("Delay caused by name resolution") - .setUnit("ns") - .build(); - this.channelReadinessDelayRecorder = - meter - .histogramBuilder("channel_readiness_delay") - .setDescription("Delay caused by establishing connection") - .setUnit("ns") - .build(); - this.callSendDelayRecorder = - meter - .histogramBuilder("call_send_delay") - .setDescription("Call send delay. (after the connection is ready)") - .setUnit("ns") - .build(); - this.operationCountRecorder = - meter - .counterBuilder("operation_count") - .setDescription("Count of Operations") - .setUnit("1") - .build(); - this.attributes = Attributes.of(stringKey("method_name"), spanName.toString()); - this.metricsRecorder = metricsRecorder; - } - - @Override - public Scope inScope() { - return () -> {}; - } - - @Override - public void operationSucceeded() {} - - // This is just one way to add labels, we could have another layer of abstraction(a separate - // class) just for get/set labels because the logic is generic. - public void addOperationLatencyLabels(String key, String value) { - operationLatencyLabels.put(key, value); - } - - @Override - public void operationSucceeded(Object response) { - AttributesBuilder attributesBuilder = Attributes.builder(); - attributesBuilder.putAll(attributes); - attributesBuilder.put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()); - operationLatencyLabels.forEach((key, value) -> attributesBuilder.put(stringKey(key), value)); - Attributes allAttributes = attributesBuilder.build(); - operationLatencyRecorder.record(operationTimer.elapsed(TimeUnit.MILLISECONDS), allAttributes); - operationCountRecorder.add(1, allAttributes); - } - - @Override - public void operationCancelled() {} - - @Override - public void operationFailed(Throwable error) { - Attributes newAttributes = - attributes.toBuilder().put(STATUS_ATTRIBUTE, extractStatus(error)).build(); - operationLatencyRecorder.record(operationTimer.elapsed(TimeUnit.MILLISECONDS), newAttributes); - } - - @Override - public void attemptStarted(int attemptNumber) {} - - @Override - public void attemptStarted(Object request, int attemptNumber) { - attemptTimer = Stopwatch.createStarted(); - } - - @Override - public void attemptSucceeded() {} - - @Override - public void attemptSucceeded(Object response) { - Attributes newAttributes = - attributes.toBuilder().put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()).build(); - metricsRecorder.recordAttemptLatency( - attemptTimer.elapsed(TimeUnit.MILLISECONDS), additionalAttributes); - } - - @Override - public void attemptCancelled() {} - - @Override - public void attemptFailed(Throwable error, Duration delay) { - Attributes newAttributes = - attributes.toBuilder().put(STATUS_ATTRIBUTE, extractStatus(error)).build(); - attemptLatencyRecorder.record(attemptTimer.elapsed(TimeUnit.MILLISECONDS), newAttributes); - } - - @Override - public void attemptFailedRetriesExhausted(Throwable error) {} - - @Override - public void attemptPermanentFailure(Throwable error) {} - - @Override - public void retryCount(int count) { - retryCountRecorder.record(count, attributes); - } - - @Override - public void grpcTargetResolutionDelay(long elapsed) { - this.targetResolutionDelayRecorder.record(elapsed, attributes); - } - - @Override - public void grpcChannelReadinessDelay(long elapsed) { - this.channelReadinessDelayRecorder.record(elapsed, attributes); - } - - @Override - public void grpcCallSendDelay(long elapsed) { - this.callSendDelayRecorder.record(elapsed, attributes); - } - - @Override - public void recordGfeMetadata(long latency) { - this.gfeLatencyRecorder.record(latency); - } - - static String extractStatus(@Nullable Throwable error) { - final String statusString; - - if (error == null) { - return StatusCode.Code.OK.toString(); - } else if (error instanceof CancellationException) { - statusString = StatusCode.Code.CANCELLED.toString(); - } else if (error instanceof ApiException) { - statusString = ((ApiException) error).getStatusCode().getCode().toString(); - } else { - statusString = StatusCode.Code.UNKNOWN.toString(); - } - - return statusString; - } - - public void addAdditionalAttributes(String key, String value) { - additionalAttributes.put(key, value); - } -} From d1bf7bd868718a58a5a6fe7dbcd5681b91bc8151 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Mon, 15 Jan 2024 23:40:40 -0500 Subject: [PATCH 15/26] Simplify the PoC --- .../com/google/api/gax/grpc/ChannelPool.java | 18 ----- .../google/api/gax/grpc/GrpcCallContext.java | 5 +- .../api/gax/grpc/GrpcDirectCallable.java | 70 ++---------------- .../google/api/gax/grpc/GrpcStreamTracer.java | 71 ------------------- .../InstantiatingGrpcChannelProvider.java | 11 +-- .../com/google/api/gax/rpc/ClientContext.java | 3 - .../api/gax/rpc/TransportChannelProvider.java | 3 - .../com/google/api/gax/tracing/ApiTracer.java | 6 -- .../api/gax/tracing/ApiTracerFactory.java | 5 -- .../api/gax/tracing/ClientMetricsTracer.java | 12 ---- .../api/gax/tracing/MetricsRecorder.java | 38 ---------- .../google/api/gax/tracing/MetricsTracer.java | 14 ++-- .../OpenTelemetryClientMetricsTracer.java | 35 --------- .../tracing/OpenTelemetryMetricsFactory.java | 5 -- 14 files changed, 13 insertions(+), 283 deletions(-) delete mode 100644 gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcStreamTracer.java delete mode 100644 gax-java/gax/src/main/java/com/google/api/gax/tracing/ClientMetricsTracer.java delete mode 100644 gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryClientMetricsTracer.java diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPool.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPool.java index ddc42f907c..c3e26dc4e2 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPool.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/ChannelPool.java @@ -30,7 +30,6 @@ package com.google.api.gax.grpc; import com.google.api.core.InternalApi; -import com.google.api.gax.tracing.ClientMetricsTracer; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -81,27 +80,11 @@ class ChannelPool extends ManagedChannel { private final AtomicInteger indexTicker = new AtomicInteger(); private final String authority; - private ClientMetricsTracer clientMetricsTracer; - static ChannelPool create(ChannelPoolSettings settings, ChannelFactory channelFactory) throws IOException { return new ChannelPool(settings, channelFactory, Executors.newSingleThreadScheduledExecutor()); } - static ChannelPool create( - ChannelPoolSettings settings, - ChannelFactory channelFactory, - ClientMetricsTracer clientMetricsTracer) - throws IOException { - ChannelPool channelPool = - new ChannelPool(settings, channelFactory, Executors.newSingleThreadScheduledExecutor()); - channelPool.clientMetricsTracer = clientMetricsTracer; - if (channelPool.clientMetricsTracer != null) { - channelPool.clientMetricsTracer.recordCurrentChannelSize(settings.getInitialChannelCount()); - } - return channelPool; - } - /** * Initializes the channel pool. Assumes that all channels have the same authority. * @@ -319,7 +302,6 @@ void resize() { shrink(dampenedTarget); } - clientMetricsTracer.recordCurrentChannelSize(entries.get().size()); } /** Not threadsafe, must be called under the entryWriteLock monitor */ diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java index 321177987a..9598267a51 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java @@ -504,10 +504,7 @@ public ApiTracer getTracer() { @Override public GrpcCallContext withTracer(@Nonnull ApiTracer tracer) { Preconditions.checkNotNull(tracer); - return withCallOptions( - callOptions - .withOption(TRACER_KEY, tracer) - .withStreamTracerFactory(new GrpcStreamTracer.Factory(tracer))); + return withCallOptions(callOptions.withOption(TRACER_KEY, tracer)); } /** {@inheritDoc} */ diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcDirectCallable.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcDirectCallable.java index eaec58a1aa..5b6a5f1bad 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcDirectCallable.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcDirectCallable.java @@ -30,20 +30,13 @@ package com.google.api.gax.grpc; import com.google.api.core.ApiFuture; -import com.google.api.core.ApiFutureCallback; -import com.google.api.core.ApiFutures; import com.google.api.core.ListenableFutureToApiFuture; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.UnaryCallable; -import com.google.api.gax.tracing.ApiTracer; import com.google.common.base.Preconditions; -import com.google.common.util.concurrent.MoreExecutors; import io.grpc.ClientCall; -import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.stub.ClientCalls; -import java.util.regex.Matcher; -import java.util.regex.Pattern; /** * {@code GrpcDirectCallable} creates gRPC calls. @@ -63,71 +56,18 @@ class GrpcDirectCallable extends UnaryCallable futureCall(RequestT request, ApiCallContext inputContext) { Preconditions.checkNotNull(request); Preconditions.checkNotNull(inputContext); - final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); - GrpcCallContext grpcCallContext = responseMetadata.addHandlers(inputContext); - ClientCall clientCall = - GrpcClientCalls.newCall(descriptor, grpcCallContext); - GfeUnaryCallback callback = - new GfeUnaryCallback(inputContext.getTracer(), responseMetadata); - ApiFuture future; + + ClientCall clientCall = GrpcClientCalls.newCall(descriptor, inputContext); + if (awaitTrailers) { - future = new ListenableFutureToApiFuture<>(ClientCalls.futureUnaryCall(clientCall, request)); + return new ListenableFutureToApiFuture<>(ClientCalls.futureUnaryCall(clientCall, request)); } else { - future = GrpcClientCalls.eagerFutureUnaryCall(clientCall, request); + return GrpcClientCalls.eagerFutureUnaryCall(clientCall, request); } - ApiFutures.addCallback(future, callback, MoreExecutors.directExecutor()); - return future; } @Override public String toString() { return String.format("direct(%s)", descriptor); } - - 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 class GfeUnaryCallback implements ApiFutureCallback { - - private final ApiTracer tracer; - private final GrpcResponseMetadata responseMetadata; - - GfeUnaryCallback(ApiTracer tracer, GrpcResponseMetadata responseMetadata) { - this.tracer = tracer; - this.responseMetadata = responseMetadata; - } - - @Override - public void onFailure(Throwable throwable) { - // Util.recordMetricsFromMetadata(responseMetadata, tracer, throwable); - } - - @Override - public void onSuccess(ResponseT response) { - Metadata metadata = responseMetadata.getMetadata(); - if (metadata == null) { - return; - } - String allKeys = metadata.keys().stream().reduce((a, b) -> a + ", " + b).get(); - // System.out.println( - // "************************ metadata size: " - // + metadata.keys().size() - // + ", all keys: " - // + allKeys); - if (metadata.get(SERVER_TIMING_HEADER_KEY) == null) { - return; - } - - String durMetadata = metadata.get(SERVER_TIMING_HEADER_KEY); - Matcher matcher = SERVER_TIMING_HEADER_PATTERN.matcher(durMetadata); - // this should always be true - if (matcher.find()) { - long latency = Long.valueOf(matcher.group("dur")); - tracer.recordGfeMetadata(latency); - } - // System.out.println("GFE metadata: " + metadata.get(SERVER_TIMING_HEADER_KEY)); - } - } } diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcStreamTracer.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcStreamTracer.java deleted file mode 100644 index e38e967f27..0000000000 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcStreamTracer.java +++ /dev/null @@ -1,71 +0,0 @@ -/* - * Copyright 2023 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.api.gax.grpc; - -import com.google.api.gax.tracing.ApiTracer; -import com.google.common.base.Stopwatch; -import io.grpc.Attributes; -import io.grpc.ClientStreamTracer; -import io.grpc.Metadata; -import java.util.concurrent.TimeUnit; - -/** - * Records the time a request is enqueued in a grpc channel queue. Its primary purpose is to measure - * the transition time between asking gRPC to start an RPC and gRPC actually serializing that RPC. - */ -class GrpcStreamTracer extends ClientStreamTracer { - - private final Stopwatch stopwatch = Stopwatch.createUnstarted(); - private final ApiTracer tracer; - - public GrpcStreamTracer(ApiTracer tracer) { - this.tracer = tracer; - stopwatch.start(); - } - - @Override - public void createPendingStream() { - tracer.grpcTargetResolutionDelay(stopwatch.elapsed(TimeUnit.NANOSECONDS)); - stopwatch.reset(); - stopwatch.start(); - } - - @Override - public void streamCreated(Attributes transportAttrs, Metadata headers) { - tracer.grpcChannelReadinessDelay(stopwatch.elapsed(TimeUnit.NANOSECONDS)); - stopwatch.reset(); - stopwatch.start(); - } - - @Override - public void outboundMessageSent(int seqNo, long optionalWireSize, long optionalUncompressedSize) { - tracer.grpcCallSendDelay(stopwatch.elapsed(TimeUnit.NANOSECONDS)); - } - - static class Factory extends ClientStreamTracer.Factory { - - private final ApiTracer tracer; - - Factory(ApiTracer tracer) { - this.tracer = tracer; - } - - @Override - public ClientStreamTracer newClientStreamTracer(StreamInfo info, Metadata headers) { - return new GrpcStreamTracer(tracer); - } - } -} diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 8779fe8b49..2703d13486 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -40,7 +40,6 @@ import com.google.api.gax.rpc.TransportChannelProvider; import com.google.api.gax.rpc.internal.EnvironmentProvider; import com.google.api.gax.rpc.mtls.MtlsProvider; -import com.google.api.gax.tracing.ClientMetricsTracer; import com.google.auth.Credentials; import com.google.auth.oauth2.ComputeEngineCredentials; import com.google.common.annotations.VisibleForTesting; @@ -118,7 +117,6 @@ public final class InstantiatingGrpcChannelProvider implements TransportChannelP @Nullable private final Boolean allowNonDefaultServiceAccount; @VisibleForTesting final ImmutableMap directPathServiceConfig; @Nullable private final MtlsProvider mtlsProvider; - @Nullable private ClientMetricsTracer clientMetricsTracer; @Nullable private final ApiFunction channelConfigurator; @@ -186,11 +184,6 @@ public String getTransportName() { return GrpcTransportChannel.getGrpcTransportName(); } - @Override - public void setClientMetricsTracer(ClientMetricsTracer clientMetricsTracer) { - this.clientMetricsTracer = clientMetricsTracer; - } - @Override public boolean needsEndpoint() { return endpoint == null; @@ -248,9 +241,7 @@ public TransportChannel getTransportChannel() throws IOException { private TransportChannel createChannel() throws IOException { return GrpcTransportChannel.create( ChannelPool.create( - channelPoolSettings, - InstantiatingGrpcChannelProvider.this::createSingleChannel, - clientMetricsTracer)); + channelPoolSettings, InstantiatingGrpcChannelProvider.this::createSingleChannel)); } private boolean isDirectPathEnabled() { diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index 46f7818281..f0132953a1 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -39,7 +39,6 @@ import com.google.api.gax.rpc.internal.QuotaProjectIdHidingCredentials; import com.google.api.gax.tracing.ApiTracerFactory; import com.google.api.gax.tracing.BaseApiTracerFactory; -import com.google.api.gax.tracing.ClientMetricsTracer; import com.google.auth.Credentials; import com.google.auth.oauth2.GdchCredentials; import com.google.auto.value.AutoValue; @@ -224,8 +223,6 @@ public static ClientContext create(StubSettings settings) throws IOException { if (transportChannelProvider.needsEndpoint()) { transportChannelProvider = transportChannelProvider.withEndpoint(endpoint); } - ClientMetricsTracer clientMetricsTracer = settings.getTracerFactory().newClientMetricsTracer(); - transportChannelProvider.setClientMetricsTracer(clientMetricsTracer); TransportChannel transportChannel = transportChannelProvider.getTransportChannel(); diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/TransportChannelProvider.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/TransportChannelProvider.java index 343ccd56e4..21f3c31f63 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/TransportChannelProvider.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/TransportChannelProvider.java @@ -31,7 +31,6 @@ import com.google.api.core.BetaApi; import com.google.api.core.InternalExtensionOnly; -import com.google.api.gax.tracing.ClientMetricsTracer; import com.google.auth.Credentials; import java.io.IOException; import java.util.Map; @@ -144,8 +143,6 @@ public interface TransportChannelProvider { */ String getTransportName(); - default void setClientMetricsTracer(ClientMetricsTracer clientMetricsTracer) {}; - /** * User set custom endpoint for the Transport Channel Provider * diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java index e18ed11e0e..f43af79a5d 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java @@ -165,12 +165,6 @@ default String attemptLatencyName() { */ default void batchRequestSent(long elementCount, long requestSize) {}; - default void grpcTargetResolutionDelay(long elapsed) {}; - - default void grpcChannelReadinessDelay(long elapsed) {}; - - default void grpcCallSendDelay(long elapsed) {}; - default void recordGfeMetadata(long latency) {}; default void addAdditionalAttributes(String key, String value) {}; diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java index d8cbef51bf..bb8345b88c 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java @@ -61,9 +61,4 @@ enum OperationType { * @param operationType the type of operation that the tracer will trace */ ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType); - - // This probably needs to be moved to a new factory - default ClientMetricsTracer newClientMetricsTracer() { - return null; - }; } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ClientMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ClientMetricsTracer.java deleted file mode 100644 index a67b951808..0000000000 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ClientMetricsTracer.java +++ /dev/null @@ -1,12 +0,0 @@ -package com.google.api.gax.tracing; - -public interface ClientMetricsTracer { - - void recordCurrentChannelSize(int channelSize); - - default void recordGaxThread(int threadCount) {}; - - default String channelSizeName() { - return "channel_size"; - }; -} diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java index 39b2b3ddff..45a2ebf803 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java @@ -4,7 +4,6 @@ import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.LongCounter; -import io.opentelemetry.api.metrics.LongHistogram; import io.opentelemetry.api.metrics.Meter; import java.util.Map; @@ -14,12 +13,7 @@ public class MetricsRecorder { protected DoubleHistogram attemptLatencyRecorder; protected DoubleHistogram operationLatencyRecorder; - protected LongHistogram retryCountRecorder; - protected LongHistogram gfeLatencyRecorder; - protected DoubleHistogram targetResolutionDelayRecorder; - protected DoubleHistogram channelReadinessDelayRecorder; - protected DoubleHistogram callSendDelayRecorder; protected LongCounter operationCountRecorder; protected LongCounter attemptCountRecorder; @@ -41,38 +35,6 @@ public MetricsRecorder(Meter meter) { "Total time until final operation success or failure, including retries and backoff.") .setUnit("ms") .build(); - this.retryCountRecorder = - meter - .histogramBuilder("retry_count") - .setDescription("Number of additional attempts per operation after initial attempt") - .setUnit("1") - .ofLongs() - .build(); - this.gfeLatencyRecorder = - meter - .histogramBuilder("gfe_latency") - .setDescription("GFE latency") - .setUnit("1") - .ofLongs() - .build(); - this.targetResolutionDelayRecorder = - meter - .histogramBuilder("target_resolution_delay") - .setDescription("Delay caused by name resolution") - .setUnit("ns") - .build(); - this.channelReadinessDelayRecorder = - meter - .histogramBuilder("channel_readiness_delay") - .setDescription("Delay caused by establishing connection") - .setUnit("ns") - .build(); - this.callSendDelayRecorder = - meter - .histogramBuilder("call_send_delay") - .setDescription("Call send delay. (after the connection is ready)") - .setUnit("ns") - .build(); this.operationCountRecorder = meter .counterBuilder("operation_count") diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java index 9c02a50a78..66a8df3d17 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java @@ -51,8 +51,7 @@ public class MetricsTracer implements ApiTracer { protected MetricsRecorder metricsRecorder; - public MetricsTracer( - SpanName spanName, MetricsRecorder metricsRecorder) { + public MetricsTracer(SpanName spanName, MetricsRecorder metricsRecorder) { this.attributes.put("method_name", spanName.toString()); this.metricsRecorder = metricsRecorder; } @@ -68,8 +67,8 @@ public void operationSucceeded() {} @Override public void operationSucceeded(Object response) { attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()); - metricsRecorder.recordOperationLatency(operationTimer.elapsed(TimeUnit.MILLISECONDS), - attributes); + metricsRecorder.recordOperationLatency( + operationTimer.elapsed(TimeUnit.MILLISECONDS), attributes); metricsRecorder.recordOperationCount(1, attributes); } @@ -79,8 +78,8 @@ public void operationCancelled() {} @Override public void operationFailed(Throwable error) { attributes.put(STATUS_ATTRIBUTE, extractStatus(error)); - metricsRecorder.recordOperationLatency(operationTimer.elapsed(TimeUnit.MILLISECONDS), - attributes); + metricsRecorder.recordOperationLatency( + operationTimer.elapsed(TimeUnit.MILLISECONDS), attributes); metricsRecorder.recordOperationCount(1, attributes); } @@ -98,8 +97,7 @@ public void attemptSucceeded() {} @Override public void attemptSucceeded(Object response) { attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()); - metricsRecorder.recordAttemptLatency( - attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); + metricsRecorder.recordAttemptLatency(attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); metricsRecorder.recordAttemptCount(1, attributes); } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryClientMetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryClientMetricsTracer.java deleted file mode 100644 index 1537bde102..0000000000 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryClientMetricsTracer.java +++ /dev/null @@ -1,35 +0,0 @@ -package com.google.api.gax.tracing; - -import io.opentelemetry.api.metrics.LongGaugeBuilder; -import io.opentelemetry.api.metrics.Meter; - -public class OpenTelemetryClientMetricsTracer implements ClientMetricsTracer { - - private final LongGaugeBuilder channelSizeRecorder; - private final LongGaugeBuilder threadCountRecorder; - private Meter meter; - - public OpenTelemetryClientMetricsTracer(Meter meter) { - this.meter = meter; - channelSizeRecorder = - meter.gaugeBuilder(channelSizeName()).setDescription("Channel Size").setUnit("1").ofLongs(); - threadCountRecorder = - meter - .gaugeBuilder("client_thread_count") - .setDescription("Current Thread Count created by Client") - .setUnit("1") - .ofLongs(); - } - - @Override - public void recordCurrentChannelSize(int channelSize) { - channelSizeRecorder.buildWithCallback( - observableLongMeasurement -> observableLongMeasurement.record(channelSize)); - } - - @Override - public void recordGaxThread(int threadCount) { - threadCountRecorder.buildWithCallback( - observableLongMeasurement -> observableLongMeasurement.record(threadCount)); - } -} diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java index 2eef2c347f..37d0e50144 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java @@ -63,9 +63,4 @@ public OpenTelemetryMetricsFactory( public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) { return new MetricsTracer(spanName, metricsRecorder); } - - @Override - public ClientMetricsTracer newClientMetricsTracer() { - return new OpenTelemetryClientMetricsTracer(meter); - } } From fed50c7c0dbdc47146171cc597db15ea5391348c Mon Sep 17 00:00:00 2001 From: Blake Li Date: Fri, 19 Jan 2024 16:15:29 -0500 Subject: [PATCH 16/26] feat: Make all methods in ApiTracer default. --- .../api/gax/retrying/BasicRetryingFuture.java | 3 +- .../com/google/api/gax/tracing/ApiTracer.java | 34 +++--- .../api/gax/tracing/MetricsRecorder.java | 63 +---------- .../google/api/gax/tracing/MetricsTracer.java | 103 +----------------- ...Factory.java => MetricsTracerFactory.java} | 17 +-- 5 files changed, 23 insertions(+), 197 deletions(-) rename gax-java/gax/src/main/java/com/google/api/gax/tracing/{OpenTelemetryMetricsFactory.java => MetricsTracerFactory.java} (80%) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java b/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java index 1a46409ab0..de7b5b5acb 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java @@ -199,8 +199,7 @@ void handleAttempt(Throwable throwable, ResponseT response) { } super.setException(throwable); } else { - tracer.attemptSucceeded(response); - tracer.retryCount(attemptSettings.getAttemptCount()); + tracer.attemptSucceeded(); super.set(response); } } catch (CancellationException e) { diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java index f43af79a5d..90c252654d 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java @@ -49,21 +49,23 @@ public interface ApiTracer { * between clients using gax and external resources to share the same implementation of the * tracing. For example OpenCensus will install a thread local that can read by the GRPC. */ - Scope inScope(); + default Scope inScope() { + return () -> { + // noop + }; + }; /** * Signals that the overall operation has finished successfully. The tracer is now considered * closed and should no longer be used. */ - void operationSucceeded(); - - default void operationSucceeded(Object response) {}; + default void operationSucceeded() {}; /** * Signals that the operation was cancelled by the user. The tracer is now considered closed and * should no longer be used. */ - void operationCancelled(); + default void operationCancelled() {}; /** * Signals that the overall operation has failed and no further attempts will be made. The tracer @@ -71,7 +73,7 @@ public interface ApiTracer { * * @param error the final error that caused the operation to fail. */ - void operationFailed(Throwable error); + default void operationFailed(Throwable error) {}; /** * Annotates the operation with selected connection id from the {@code ChannelPool}. @@ -88,7 +90,7 @@ public interface ApiTracer { * @deprecated Please use {@link #attemptStarted(Object, int)} instead. */ @Deprecated - void attemptStarted(int attemptNumber); + default void attemptStarted(int attemptNumber) {}; /** * Adds an annotation that an attempt is about to start with additional information from the @@ -103,15 +105,8 @@ public interface ApiTracer { /** Adds an annotation that the attempt succeeded. */ default void attemptSucceeded() {}; - default void attemptSucceeded(Object response) {}; - - // This is for libraries to override to intended name - default String attemptLatencyName() { - return "attempt_latency"; - }; - /** Add an annotation that the attempt was cancelled by the user. */ - void attemptCancelled(); + default void attemptCancelled() {}; /** * Adds an annotation that the attempt failed, but another attempt will be made after the delay. @@ -119,7 +114,7 @@ default String attemptLatencyName() { * @param error the transient error that caused the attempt to fail. * @param delay the amount of time to wait before the next attempt will start. */ - void attemptFailed(Throwable error, Duration delay); + default void attemptFailed(Throwable error, Duration delay) {}; /** * Adds an annotation that the attempt failed and that no further attempts will be made because @@ -127,7 +122,7 @@ default String attemptLatencyName() { * * @param error the last error received before retries were exhausted. */ - void attemptFailedRetriesExhausted(Throwable error); + default void attemptFailedRetriesExhausted(Throwable error) {}; /** * Adds an annotation that the attempt failed and that no further attempts will be made because @@ -135,9 +130,8 @@ default String attemptLatencyName() { * * @param error the error that caused the final attempt to fail. */ - void attemptPermanentFailure(Throwable error); + default void attemptPermanentFailure(Throwable error) {}; - default void retryCount(int count) {}; /** * Signals that the initial RPC for the long running operation failed. * @@ -165,8 +159,6 @@ default String attemptLatencyName() { */ default void batchRequestSent(long elementCount, long requestSize) {}; - default void recordGfeMetadata(long latency) {}; - default void addAdditionalAttributes(String key, String value) {}; /** * A context class to be used with {@link #inScope()} and a try-with-resources block. Closing a diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java index 45a2ebf803..45d0b15ba9 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java @@ -7,67 +7,14 @@ import io.opentelemetry.api.metrics.Meter; import java.util.Map; -public class MetricsRecorder { - protected Meter meter; +interface MetricsRecorder { - protected DoubleHistogram attemptLatencyRecorder; + default void recordAttemptLatency(double attemptLatency, Map attributes) {} - protected DoubleHistogram operationLatencyRecorder; + default void recordAttemptCount(long count, Map attributes) {} - protected LongCounter operationCountRecorder; + default void recordOperationLatency(double operationLatency, Map attributes) {} - protected LongCounter attemptCountRecorder; + default void recordOperationCount(long count, Map attributes) {} - protected Attributes attributes; - - public MetricsRecorder(Meter meter) { - this.meter = meter; - this.attemptLatencyRecorder = - meter - .histogramBuilder("attempt_latency") - .setDescription("Duration of an individual attempt") - .setUnit("ms") - .build(); - this.operationLatencyRecorder = - meter - .histogramBuilder("operation_latency") - .setDescription( - "Total time until final operation success or failure, including retries and backoff.") - .setUnit("ms") - .build(); - this.operationCountRecorder = - meter - .counterBuilder("operation_count") - .setDescription("Count of Operations") - .setUnit("1") - .build(); - this.attemptCountRecorder = - meter - .counterBuilder("attempt_count") - .setDescription("Count of Attempts") - .setUnit("1") - .build(); - } - - public void recordAttemptLatency(double attemptLatency, Map attributes) { - attemptLatencyRecorder.record(attemptLatency, toOtelAttributes(attributes)); - } - - public void recordAttemptCount(long count, Map attributes) { - attemptCountRecorder.add(count, toOtelAttributes(attributes)); - } - - public void recordOperationLatency(double operationLatency, Map attributes) { - operationLatencyRecorder.record(operationLatency, toOtelAttributes(attributes)); - } - - public void recordOperationCount(long count, Map attributes) { - operationCountRecorder.add(count, toOtelAttributes(attributes)); - } - - private Attributes toOtelAttributes(Map attributes) { - AttributesBuilder attributesBuilder = Attributes.builder(); - attributes.forEach(attributesBuilder::put); - return attributesBuilder.build(); - } } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java index 66a8df3d17..1b15d3212a 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 Google LLC + * Copyright 2024 Google LLC * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -30,110 +30,9 @@ package com.google.api.gax.tracing; -import com.google.api.gax.rpc.ApiException; -import com.google.api.gax.rpc.StatusCode; -import com.google.common.base.Stopwatch; -import java.util.HashMap; -import java.util.Map; -import java.util.concurrent.CancellationException; -import java.util.concurrent.TimeUnit; -import javax.annotation.Nullable; -import org.threeten.bp.Duration; - public class MetricsTracer implements ApiTracer { - public static final String STATUS_ATTRIBUTE = "status"; - - private Stopwatch attemptTimer; - - private final Stopwatch operationTimer = Stopwatch.createStarted(); - - private final Map attributes = new HashMap<>(); - - protected MetricsRecorder metricsRecorder; public MetricsTracer(SpanName spanName, MetricsRecorder metricsRecorder) { - this.attributes.put("method_name", spanName.toString()); - this.metricsRecorder = metricsRecorder; - } - - @Override - public Scope inScope() { - return () -> {}; - } - - @Override - public void operationSucceeded() {} - - @Override - public void operationSucceeded(Object response) { - attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()); - metricsRecorder.recordOperationLatency( - operationTimer.elapsed(TimeUnit.MILLISECONDS), attributes); - metricsRecorder.recordOperationCount(1, attributes); - } - - @Override - public void operationCancelled() {} - - @Override - public void operationFailed(Throwable error) { - attributes.put(STATUS_ATTRIBUTE, extractStatus(error)); - metricsRecorder.recordOperationLatency( - operationTimer.elapsed(TimeUnit.MILLISECONDS), attributes); - metricsRecorder.recordOperationCount(1, attributes); - } - - @Override - public void attemptStarted(int attemptNumber) {} - - @Override - public void attemptStarted(Object request, int attemptNumber) { - attemptTimer = Stopwatch.createStarted(); - } - - @Override - public void attemptSucceeded() {} - - @Override - public void attemptSucceeded(Object response) { - attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()); - metricsRecorder.recordAttemptLatency(attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); - metricsRecorder.recordAttemptCount(1, attributes); - } - - @Override - public void attemptCancelled() {} - - @Override - public void attemptFailed(Throwable error, Duration delay) { - attributes.put(STATUS_ATTRIBUTE, extractStatus(error)); - metricsRecorder.recordAttemptLatency(attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); - metricsRecorder.recordAttemptCount(1, attributes); - } - - @Override - public void attemptFailedRetriesExhausted(Throwable error) {} - - @Override - public void attemptPermanentFailure(Throwable error) {} - - static String extractStatus(@Nullable Throwable error) { - final String statusString; - - if (error == null) { - return StatusCode.Code.OK.toString(); - } else if (error instanceof CancellationException) { - statusString = StatusCode.Code.CANCELLED.toString(); - } else if (error instanceof ApiException) { - statusString = ((ApiException) error).getStatusCode().getCode().toString(); - } else { - statusString = StatusCode.Code.UNKNOWN.toString(); - } - - return statusString; - } - public void addAdditionalAttributes(String key, String value) { - attributes.put(key, value); } } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java similarity index 80% rename from gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java rename to gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java index 37d0e50144..a85e1545bf 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsFactory.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java @@ -30,10 +30,7 @@ package com.google.api.gax.tracing; import com.google.api.core.InternalApi; -import com.google.api.gax.core.GaxProperties; import io.opencensus.trace.Tracer; -import io.opentelemetry.api.OpenTelemetry; -import io.opentelemetry.api.metrics.Meter; /** * A {@link ApiTracerFactory} to build instances of {@link OpencensusTracer}. @@ -44,19 +41,11 @@ *

This class is thread safe. */ @InternalApi("For google-cloud-java client use only") -public class OpenTelemetryMetricsFactory implements ApiTracerFactory { - protected Meter meter; - +public class MetricsTracerFactory implements ApiTracerFactory { protected MetricsRecorder metricsRecorder; - public OpenTelemetryMetricsFactory( - OpenTelemetry openTelemetry, String libraryName, String libraryVersion) { - meter = - openTelemetry - .meterBuilder("gax") - .setInstrumentationVersion(GaxProperties.getGaxVersion()) - .build(); - metricsRecorder = new MetricsRecorder(meter); + public MetricsTracerFactory(MetricsRecorder metricsRecorder) { + this.metricsRecorder = metricsRecorder; } @Override From 69e8a90e48de32f26acf430ca1045ae76e715027 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Fri, 19 Jan 2024 16:35:54 -0500 Subject: [PATCH 17/26] Remove unused dependencies. --- gax-java/gax/pom.xml | 12 ------- .../api/gax/tracing/MetricsRecorder.java | 35 ++++++++++++++++--- .../api/gax/tracing/MetricsTracerFactory.java | 2 +- .../google/api/gax/tracing/TraceFinisher.java | 2 +- .../gax/tracing/TracedResponseObserver.java | 2 +- gax-java/pom.xml | 7 ---- 6 files changed, 33 insertions(+), 27 deletions(-) diff --git a/gax-java/gax/pom.xml b/gax-java/gax/pom.xml index 7f779cd7d0..01b565be60 100644 --- a/gax-java/gax/pom.xml +++ b/gax-java/gax/pom.xml @@ -57,18 +57,6 @@ graal-sdk provided - - io.opentelemetry - opentelemetry-api - - - io.opentelemetry - opentelemetry-sdk - - - io.opentelemetry - opentelemetry-exporter-otlp - diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java index 45d0b15ba9..161064ae3e 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java @@ -1,10 +1,35 @@ +/* + * Copyright 2024 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + package com.google.api.gax.tracing; -import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.common.AttributesBuilder; -import io.opentelemetry.api.metrics.DoubleHistogram; -import io.opentelemetry.api.metrics.LongCounter; -import io.opentelemetry.api.metrics.Meter; import java.util.Map; interface MetricsRecorder { diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java index a85e1545bf..a0d9e77cc5 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 Google LLC + * Copyright 2024 Google LLC * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/TraceFinisher.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/TraceFinisher.java index 1a517dfc7a..292a827759 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/TraceFinisher.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/TraceFinisher.java @@ -53,6 +53,6 @@ public void onFailure(Throwable throwable) { @Override public void onSuccess(T responseT) { - tracer.operationSucceeded(responseT); + tracer.operationSucceeded(); } } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedResponseObserver.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedResponseObserver.java index 768de72f12..ba72d2f5b7 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedResponseObserver.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedResponseObserver.java @@ -106,7 +106,7 @@ public void onError(Throwable t) { @Override public void onComplete() { - tracer.operationSucceeded(null); + tracer.operationSucceeded(); innerObserver.onComplete(); } } diff --git a/gax-java/pom.xml b/gax-java/pom.xml index 89df61d674..a8cea24b91 100644 --- a/gax-java/pom.xml +++ b/gax-java/pom.xml @@ -158,13 +158,6 @@ pom import - - io.opentelemetry - opentelemetry-bom - 1.27.0 - pom - import - From f771d7e8d353066addd6406e46b337d138402a25 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Fri, 19 Jan 2024 16:36:55 -0500 Subject: [PATCH 18/26] Revert ClientContext changes. --- .../gax/src/main/java/com/google/api/gax/rpc/ClientContext.java | 1 - 1 file changed, 1 deletion(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index f0132953a1..e7fac9d0c6 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -223,7 +223,6 @@ public static ClientContext create(StubSettings settings) throws IOException { if (transportChannelProvider.needsEndpoint()) { transportChannelProvider = transportChannelProvider.withEndpoint(endpoint); } - TransportChannel transportChannel = transportChannelProvider.getTransportChannel(); ApiCallContext defaultCallContext = From d714cf43bf7f823f12f899768249964f721b8688 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Fri, 19 Jan 2024 23:28:40 -0500 Subject: [PATCH 19/26] Introduce MethodName. Add Javadocs. --- .../google/api/gax/tracing/BaseApiTracer.java | 3 + .../google/api/gax/tracing/MethodName.java | 63 +++++++++++++++++++ .../api/gax/tracing/MetricsRecorder.java | 19 ++++++ .../google/api/gax/tracing/MetricsTracer.java | 12 +++- .../api/gax/tracing/MetricsTracerFactory.java | 13 ++-- 5 files changed, 103 insertions(+), 7 deletions(-) create mode 100644 gax-java/gax/src/main/java/com/google/api/gax/tracing/MethodName.java diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/BaseApiTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/BaseApiTracer.java index 538708b879..5da03d9309 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/BaseApiTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/BaseApiTracer.java @@ -34,6 +34,9 @@ /** * A base implementation of {@link ApiTracer} that does nothing. + * With the deprecation of Java 7 support, all the methods in {@link ApiTracer} are now made default, we no longer + * need a base class that does nothing. This class should be removed once all the references to it + * are removed in Google Cloud Client Libraries. * *

For internal use only. */ diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MethodName.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MethodName.java new file mode 100644 index 0000000000..300c7209f4 --- /dev/null +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MethodName.java @@ -0,0 +1,63 @@ +/* + * Copyright 2024 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.tracing; + +import com.google.api.core.BetaApi; +import com.google.api.core.InternalApi; +import com.google.api.gax.rpc.StubSettings; +import com.google.auto.value.AutoValue; + +/** A value class to represent the name of the RPC method in an {@link ApiTracer}. */ +@InternalApi +@AutoValue +public abstract class MethodName { + /** + * Creates a new instance of the RPC method name. + * + * @param serviceName The name of the service. In general this will be GAPIC generated service name {@link StubSettings#getServiceName()}. + * However, in some cases, when the GAPIC generated service is wrapped, this will be overridden + * to specify the manually written wrapper's name. + * @param methodName The name of the logical operation being traced. + */ + public static MethodName of(String serviceName, String methodName) { + return new AutoValue_MethodName(serviceName, methodName); + } + + /** The name of the service. ie BigtableData */ + public abstract String getServiceName(); + + /** The name of the logical operation being traced. ie. ReadRows. */ + public abstract String getMethodName(); + + @Override + public String toString() { + return getServiceName() + "." + getMethodName(); + } +} diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java index 161064ae3e..214e8420a8 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java @@ -30,16 +30,35 @@ package com.google.api.gax.tracing; +import com.google.api.core.BetaApi; +import com.google.api.core.InternalApi; import java.util.Map; +/** + * Provides an interface for metrics recording. The implementer is expected to use an observability framework, e.g. OpenTelemetry + */ +@BetaApi +@InternalApi interface MetricsRecorder { + /** + * TODO: Add Javadoc + */ default void recordAttemptLatency(double attemptLatency, Map attributes) {} + /** + * TODO: Add Javadoc + */ default void recordAttemptCount(long count, Map attributes) {} + /** + * TODO: Add Javadoc + */ default void recordOperationLatency(double operationLatency, Map attributes) {} + /** + * TODO: Add Javadoc + */ default void recordOperationCount(long count, Map attributes) {} } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java index 1b15d3212a..ae0a6f6b03 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java @@ -30,9 +30,19 @@ package com.google.api.gax.tracing; +import com.google.api.core.BetaApi; +import com.google.api.core.InternalApi; + +/** + * This class computes generic metrics that can be observed in the lifecycle of an RPC operation. + * The responsibility of recording metrics should delegate to {@link MetricsRecorder}, hence this + * class should not have any knowledge about the observability framework used for metrics recording. + */ +@BetaApi +@InternalApi public class MetricsTracer implements ApiTracer { - public MetricsTracer(SpanName spanName, MetricsRecorder metricsRecorder) { + public MetricsTracer(MethodName methodName, MetricsRecorder metricsRecorder) { } } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java index a0d9e77cc5..f8f989895f 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java @@ -29,18 +29,19 @@ */ package com.google.api.gax.tracing; +import com.google.api.core.BetaApi; import com.google.api.core.InternalApi; -import io.opencensus.trace.Tracer; /** - * A {@link ApiTracerFactory} to build instances of {@link OpencensusTracer}. + * A {@link ApiTracerFactory} to build instances of {@link MetricsTracer}. * - *

This class wraps the {@link Tracer} provided by Opencensus in {@code Tracing.getTracer()}. It - * will be used to create new spans and wrap them in {@link OpencensusTracer} defined in gax. + *

This class wraps the {@link MetricsRecorder} and pass it to {@link MetricsTracer}. It + * will be used to record metrics in {@link MetricsTracer}. * *

This class is thread safe. */ -@InternalApi("For google-cloud-java client use only") +@BetaApi +@InternalApi public class MetricsTracerFactory implements ApiTracerFactory { protected MetricsRecorder metricsRecorder; @@ -50,6 +51,6 @@ public MetricsTracerFactory(MetricsRecorder metricsRecorder) { @Override public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) { - return new MetricsTracer(spanName, metricsRecorder); + return new MetricsTracer(MethodName.of(spanName.getClientName(), spanName.getMethodName()), metricsRecorder); } } From 79775114dab263b6b7e1e054583549121da3b214 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Fri, 19 Jan 2024 23:38:20 -0500 Subject: [PATCH 20/26] Format --- .../google/api/gax/grpc/GrpcCallContext.java | 3 +++ .../google/api/gax/tracing/BaseApiTracer.java | 8 ++++---- .../google/api/gax/tracing/MethodName.java | 8 +++++--- .../api/gax/tracing/MetricsRecorder.java | 20 ++++++------------- .../google/api/gax/tracing/MetricsTracer.java | 4 +--- .../api/gax/tracing/MetricsTracerFactory.java | 7 ++++--- 6 files changed, 23 insertions(+), 27 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java index 9598267a51..64d3cad338 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java @@ -66,6 +66,9 @@ */ @BetaApi("Reference ApiCallContext instead - this class is likely to experience breaking changes") public final class GrpcCallContext implements ApiCallContext { + + // This field is made public for handwritten libraries to easily access the tracer from + // CallOptions public static final CallOptions.Key TRACER_KEY = CallOptions.Key.create("gax.tracer"); private final Channel channel; diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/BaseApiTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/BaseApiTracer.java index 5da03d9309..1e542f124d 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/BaseApiTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/BaseApiTracer.java @@ -33,10 +33,10 @@ import org.threeten.bp.Duration; /** - * A base implementation of {@link ApiTracer} that does nothing. - * With the deprecation of Java 7 support, all the methods in {@link ApiTracer} are now made default, we no longer - * need a base class that does nothing. This class should be removed once all the references to it - * are removed in Google Cloud Client Libraries. + * A base implementation of {@link ApiTracer} that does nothing. With the deprecation of Java 7 + * support, all the methods in {@link ApiTracer} are now made default, we no longer need a base + * class that does nothing. This class should be removed once all the references to it are removed + * in Google Cloud Client Libraries. * *

For internal use only. */ diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MethodName.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MethodName.java index 300c7209f4..1581c0ed38 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MethodName.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MethodName.java @@ -35,15 +35,17 @@ import com.google.auto.value.AutoValue; /** A value class to represent the name of the RPC method in an {@link ApiTracer}. */ +@BetaApi @InternalApi @AutoValue public abstract class MethodName { /** * Creates a new instance of the RPC method name. * - * @param serviceName The name of the service. In general this will be GAPIC generated service name {@link StubSettings#getServiceName()}. - * However, in some cases, when the GAPIC generated service is wrapped, this will be overridden - * to specify the manually written wrapper's name. + * @param serviceName The name of the service. In general this will be GAPIC generated service + * name {@link StubSettings#getServiceName()}. However, in some cases, when the GAPIC + * generated service is wrapped, this will be overridden to specify the manually written + * wrapper's name. * @param methodName The name of the logical operation being traced. */ public static MethodName of(String serviceName, String methodName) { diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java index 214e8420a8..51775148c4 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java @@ -35,30 +35,22 @@ import java.util.Map; /** - * Provides an interface for metrics recording. The implementer is expected to use an observability framework, e.g. OpenTelemetry + * Provides an interface for metrics recording. The implementer is expected to use an observability + * framework, e.g. OpenTelemetry */ @BetaApi @InternalApi interface MetricsRecorder { - /** - * TODO: Add Javadoc - */ + /** TODO: Add Javadoc */ default void recordAttemptLatency(double attemptLatency, Map attributes) {} - /** - * TODO: Add Javadoc - */ + /** TODO: Add Javadoc */ default void recordAttemptCount(long count, Map attributes) {} - /** - * TODO: Add Javadoc - */ + /** TODO: Add Javadoc */ default void recordOperationLatency(double operationLatency, Map attributes) {} - /** - * TODO: Add Javadoc - */ + /** TODO: Add Javadoc */ default void recordOperationCount(long count, Map attributes) {} - } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java index ae0a6f6b03..2db57ba6a1 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java @@ -42,7 +42,5 @@ @InternalApi public class MetricsTracer implements ApiTracer { - public MetricsTracer(MethodName methodName, MetricsRecorder metricsRecorder) { - - } + public MetricsTracer(MethodName methodName, MetricsRecorder metricsRecorder) {} } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java index f8f989895f..e4a844668e 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java @@ -35,8 +35,8 @@ /** * A {@link ApiTracerFactory} to build instances of {@link MetricsTracer}. * - *

This class wraps the {@link MetricsRecorder} and pass it to {@link MetricsTracer}. It - * will be used to record metrics in {@link MetricsTracer}. + *

This class wraps the {@link MetricsRecorder} and pass it to {@link MetricsTracer}. It will be + * used to record metrics in {@link MetricsTracer}. * *

This class is thread safe. */ @@ -51,6 +51,7 @@ public MetricsTracerFactory(MetricsRecorder metricsRecorder) { @Override public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) { - return new MetricsTracer(MethodName.of(spanName.getClientName(), spanName.getMethodName()), metricsRecorder); + return new MetricsTracer( + MethodName.of(spanName.getClientName(), spanName.getMethodName()), metricsRecorder); } } From 39d0c69793c530d38c7fbc4af1e9a7e8e6e05825 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Fri, 19 Jan 2024 23:47:38 -0500 Subject: [PATCH 21/26] Add Javadoc to ServiceOptions --- .../java/com/google/api/gax/tracing/MetricsRecorder.java | 2 +- .../src/main/java/com/google/cloud/ServiceOptions.java | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java index 51775148c4..55e9c2e068 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java @@ -40,7 +40,7 @@ */ @BetaApi @InternalApi -interface MetricsRecorder { +public interface MetricsRecorder { /** TODO: Add Javadoc */ default void recordAttemptLatency(double attemptLatency, Map attributes) {} diff --git a/java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java b/java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java index af63a18d52..9384e7823d 100644 --- a/java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java +++ b/java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java @@ -40,6 +40,7 @@ import com.google.api.gax.rpc.FixedHeaderProvider; import com.google.api.gax.rpc.HeaderProvider; import com.google.api.gax.rpc.NoHeaderProvider; +import com.google.api.gax.tracing.ApiTracer; import com.google.api.gax.tracing.ApiTracerFactory; import com.google.auth.Credentials; import com.google.auth.oauth2.GoogleCredentials; @@ -312,6 +313,12 @@ public B setQuotaProjectId(String quotaProjectId) { return self(); } + /** + * Sets the {@link ApiTracerFactory}. It will be used to create an {@link ApiTracer} that is + * annotated throughout the lifecycle of an RPC operation. + */ + @BetaApi + @InternalApi public B setApiTracerFactory(ApiTracerFactory apiTracerFactory) { this.apiTracerFactory = apiTracerFactory; return self(); From 0efbebadd33e7bf09271b8d705fa156bede31d8c Mon Sep 17 00:00:00 2001 From: Blake Li Date: Mon, 22 Jan 2024 12:49:39 -0500 Subject: [PATCH 22/26] Move addAttributes() from ApiTracer to MEtricsTracer. --- .../src/main/java/com/google/api/gax/tracing/ApiTracer.java | 1 - .../main/java/com/google/api/gax/tracing/MetricsTracer.java | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java index 90c252654d..6143772bac 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java @@ -159,7 +159,6 @@ default Scope inScope() { */ default void batchRequestSent(long elementCount, long requestSize) {}; - default void addAdditionalAttributes(String key, String value) {}; /** * A context class to be used with {@link #inScope()} and a try-with-resources block. Closing a * {@link Scope} removes any context that the underlying implementation might've set in {@link diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java index 2db57ba6a1..ae060fa04f 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java @@ -43,4 +43,9 @@ public class MetricsTracer implements ApiTracer { public MetricsTracer(MethodName methodName, MetricsRecorder metricsRecorder) {} + + /** + * TODO: Add Javadoc + */ + void addAttributes(String key, String value) {}; } From 6171779e601e7ec80a4405234e60289e0e817ca7 Mon Sep 17 00:00:00 2001 From: Blake Li Date: Mon, 22 Jan 2024 12:58:00 -0500 Subject: [PATCH 23/26] Format --- .../main/java/com/google/api/gax/tracing/MetricsTracer.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java index ae060fa04f..0fe5ca9dd7 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java @@ -44,8 +44,6 @@ public class MetricsTracer implements ApiTracer { public MetricsTracer(MethodName methodName, MetricsRecorder metricsRecorder) {} - /** - * TODO: Add Javadoc - */ + /** TODO: Add Javadoc */ void addAttributes(String key, String value) {}; } From 26786829226c9dde03d10ace917243c0865ab61c Mon Sep 17 00:00:00 2001 From: Blake Li Date: Mon, 22 Jan 2024 15:46:20 -0500 Subject: [PATCH 24/26] Add basic Java docs. --- .../google/api/gax/tracing/MetricsRecorder.java | 16 ++++++++++++---- .../google/api/gax/tracing/MetricsTracer.java | 7 +++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java index 55e9c2e068..1cf9306ee3 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java @@ -42,15 +42,23 @@ @InternalApi public interface MetricsRecorder { - /** TODO: Add Javadoc */ + /** + * Records the latency of an RPC attempt + */ default void recordAttemptLatency(double attemptLatency, Map attributes) {} - /** TODO: Add Javadoc */ + /** + * Records the count of RPC attempts + */ default void recordAttemptCount(long count, Map attributes) {} - /** TODO: Add Javadoc */ + /** + * Records the total end-to-end latency for an operation, including the initial RPC attempts and subsequent retries. + */ default void recordOperationLatency(double operationLatency, Map attributes) {} - /** TODO: Add Javadoc */ + /** + * Records the count of operations + */ default void recordOperationCount(long count, Map attributes) {} } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java index 0fe5ca9dd7..4b9f30dc0e 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java @@ -44,6 +44,9 @@ public class MetricsTracer implements ApiTracer { public MetricsTracer(MethodName methodName, MetricsRecorder metricsRecorder) {} - /** TODO: Add Javadoc */ - void addAttributes(String key, String value) {}; + /** + * Add attributes that will be attached to all metrics. This is expected to be called by handwritten client + * teams to add additional attributes that are not supposed be collected by Gax. + */ + public void addAttributes(String key, String value) {}; } From 9e32b79566dd560dd2fa1b5ab139670b19edcd2f Mon Sep 17 00:00:00 2001 From: Blake Li Date: Mon, 22 Jan 2024 15:46:46 -0500 Subject: [PATCH 25/26] Add basic Java docs. --- .../google/api/gax/tracing/MetricsRecorder.java | 15 +++++---------- .../com/google/api/gax/tracing/MetricsTracer.java | 5 +++-- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java index 1cf9306ee3..8060146ceb 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java @@ -42,23 +42,18 @@ @InternalApi public interface MetricsRecorder { - /** - * Records the latency of an RPC attempt - */ + /** Records the latency of an RPC attempt */ default void recordAttemptLatency(double attemptLatency, Map attributes) {} - /** - * Records the count of RPC attempts - */ + /** Records the count of RPC attempts */ default void recordAttemptCount(long count, Map attributes) {} /** - * Records the total end-to-end latency for an operation, including the initial RPC attempts and subsequent retries. + * Records the total end-to-end latency for an operation, including the initial RPC attempts and + * subsequent retries. */ default void recordOperationLatency(double operationLatency, Map attributes) {} - /** - * Records the count of operations - */ + /** Records the count of operations */ default void recordOperationCount(long count, Map attributes) {} } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java index 4b9f30dc0e..bf5dbdd046 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java @@ -45,8 +45,9 @@ public class MetricsTracer implements ApiTracer { public MetricsTracer(MethodName methodName, MetricsRecorder metricsRecorder) {} /** - * Add attributes that will be attached to all metrics. This is expected to be called by handwritten client - * teams to add additional attributes that are not supposed be collected by Gax. + * Add attributes that will be attached to all metrics. This is expected to be called by + * handwritten client teams to add additional attributes that are not supposed be collected by + * Gax. */ public void addAttributes(String key, String value) {}; } From 048350024c52ca9272b6e15d9198ed68bdd2b7be Mon Sep 17 00:00:00 2001 From: Blake Li Date: Mon, 22 Jan 2024 16:46:37 -0500 Subject: [PATCH 26/26] Update Java docs. --- .../main/java/com/google/api/gax/tracing/MetricsRecorder.java | 4 +++- .../java/com/google/api/gax/tracing/MetricsTracerFactory.java | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java index 8060146ceb..d2e221fb5b 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsRecorder.java @@ -36,7 +36,9 @@ /** * Provides an interface for metrics recording. The implementer is expected to use an observability - * framework, e.g. OpenTelemetry + * framework, e.g. OpenTelemetry. There should be only one instance of MetricsRecorder per client, + * all the methods in this class are expected to be called from multiple threads, hence the + * implementation must be thread safe. */ @BetaApi @InternalApi diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java index e4a844668e..d2b8d87fb4 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java @@ -38,7 +38,7 @@ *

This class wraps the {@link MetricsRecorder} and pass it to {@link MetricsTracer}. It will be * used to record metrics in {@link MetricsTracer}. * - *

This class is thread safe. + *

This class is expected to be initialized once during client initialization. */ @BetaApi @InternalApi