From 63801fb49e9eb8b09a881b78e041de28ae16d9aa Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Thu, 27 Dec 2018 17:08:00 -0500 Subject: [PATCH 1/4] Add plumbing for tracing This has been extracted from #613 This doesn't provide any extra functionality. Instead it just sets up the foundation for integrating tracing in to gax. The general idea is to add a tracing abstraction layer that all parts of gax can call into. This is accomplished by introducing - ApiTracer interface that has methods for all annotations that a span should contain - ApiTracerFactory interface: since an ApiTracer is stateful and has a 1:1 mapping with operations, a factory is introduced to cleanly switch implementations of the tracer By default a NoopApiTracerFactory is configured that will return NoopApiTracers. In the future a parallel OpenCensusApiTracerFactory will be added. A client implementation will be able to opt into tracing by setting the factory in StubSettings. The actual ApiTracer will be propagated by CallContext. --- .../google/api/gax/grpc/GrpcCallContext.java | 31 +++++ .../api/gax/httpjson/HttpJsonCallContext.java | 52 ++++++-- .../api/gax/retrying/NoopRetryingContext.java | 11 ++ .../api/gax/retrying/RetryingContext.java | 7 +- .../google/api/gax/rpc/ApiCallContext.java | 9 ++ .../com/google/api/gax/rpc/ClientContext.java | 13 +- .../com/google/api/gax/rpc/StubSettings.java | 30 +++++ .../com/google/api/gax/tracing/ApiTracer.java | 112 +++++++++++++++++ .../api/gax/tracing/ApiTracerFactory.java | 45 +++++++ .../google/api/gax/tracing/NoopApiTracer.java | 118 ++++++++++++++++++ .../api/gax/tracing/NoopApiTracerFactory.java | 56 +++++++++ .../com/google/api/gax/tracing/SpanName.java | 66 ++++++++++ .../api/gax/rpc/testing/FakeCallContext.java | 64 ++++++++-- 13 files changed, 590 insertions(+), 24 deletions(-) create mode 100644 gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java create mode 100644 gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java create mode 100644 gax/src/main/java/com/google/api/gax/tracing/NoopApiTracer.java create mode 100644 gax/src/main/java/com/google/api/gax/tracing/NoopApiTracerFactory.java create mode 100644 gax/src/main/java/com/google/api/gax/tracing/SpanName.java diff --git a/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java b/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java index aaeac38cf..3bbec167c 100644 --- a/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java +++ b/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java @@ -34,11 +34,14 @@ import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.TransportChannel; import com.google.api.gax.rpc.internal.Headers; +import com.google.api.gax.tracing.ApiTracer; +import com.google.api.gax.tracing.NoopApiTracer; import com.google.auth.Credentials; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import io.grpc.CallCredentials; import io.grpc.CallOptions; +import io.grpc.CallOptions.Key; import io.grpc.Channel; import io.grpc.Deadline; import io.grpc.Metadata; @@ -46,6 +49,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.threeten.bp.Duration; @@ -60,6 +64,8 @@ @BetaApi("Reference ApiCallContext instead - this class is likely to experience breaking changes") @InternalExtensionOnly public final class GrpcCallContext implements ApiCallContext { + private static final CallOptions.Key TRACER_KEY = Key.create("gax.tracer"); + private final Channel channel; private final CallOptions callOptions; @Nullable private final Duration timeout; @@ -254,6 +260,11 @@ public ApiCallContext merge(ApiCallContext inputCallContext) { newCallCredentials = this.callOptions.getCredentials(); } + ApiTracer newTracer = grpcCallContext.callOptions.getOption(TRACER_KEY); + if (newTracer == null) { + newTracer = this.callOptions.getOption(TRACER_KEY); + } + Duration newTimeout = grpcCallContext.timeout; if (newTimeout == null) { newTimeout = this.timeout; @@ -283,6 +294,10 @@ public ApiCallContext merge(ApiCallContext inputCallContext) { .withCallCredentials(newCallCredentials) .withDeadline(newDeadline); + if (newTracer != null) { + newCallOptions = newCallOptions.withOption(TRACER_KEY, newTracer); + } + return new GrpcCallContext( newChannel, newCallOptions, @@ -370,6 +385,22 @@ public GrpcCallContext withRequestParamsDynamicHeaderOption(String requestParams return withCallOptions(newCallOptions); } + @Override + @Nonnull + public ApiTracer getTracer() { + ApiTracer tracer = callOptions.getOption(TRACER_KEY); + if (tracer == null) { + tracer = NoopApiTracer.create(); + } + return tracer; + } + + @Override + public ApiCallContext withTracer(@Nonnull ApiTracer tracer) { + Preconditions.checkNotNull(tracer); + return withCallOptions(callOptions.withOption(TRACER_KEY, tracer)); + } + @Override public int hashCode() { return Objects.hash( diff --git a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java index e9a8c1615..6b17c7fa1 100644 --- a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java +++ b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java @@ -34,6 +34,8 @@ import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.TransportChannel; import com.google.api.gax.rpc.internal.Headers; +import com.google.api.gax.tracing.ApiTracer; +import com.google.api.gax.tracing.NoopApiTracer; import com.google.auth.Credentials; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; @@ -61,10 +63,12 @@ public final class HttpJsonCallContext implements ApiCallContext { private final Instant deadline; private final Credentials credentials; private final ImmutableMap> extraHeaders; + private final ApiTracer tracer; /** Returns an empty instance. */ public static HttpJsonCallContext createDefault() { - return new HttpJsonCallContext(null, null, null, null, ImmutableMap.>of()); + return new HttpJsonCallContext( + null, null, null, null, ImmutableMap.>of(), null); } private HttpJsonCallContext( @@ -72,12 +76,14 @@ private HttpJsonCallContext( Duration timeout, Instant deadline, Credentials credentials, - ImmutableMap> extraHeaders) { + ImmutableMap> extraHeaders, + ApiTracer tracer) { this.channel = channel; this.timeout = timeout; this.deadline = deadline; this.credentials = credentials; this.extraHeaders = extraHeaders; + this.tracer = tracer; } /** @@ -137,14 +143,19 @@ public HttpJsonCallContext merge(ApiCallContext inputCallContext) { ImmutableMap> newExtraHeaders = Headers.mergeHeaders(extraHeaders, httpJsonCallContext.extraHeaders); + ApiTracer newTracer = httpJsonCallContext.tracer; + if (newTracer == null) { + newTracer = this.tracer; + } + return new HttpJsonCallContext( - newChannel, newTimeout, newDeadline, newCredentials, newExtraHeaders); + newChannel, newTimeout, newDeadline, newCredentials, newExtraHeaders, newTracer); } @Override public HttpJsonCallContext withCredentials(Credentials newCredentials) { return new HttpJsonCallContext( - this.channel, this.timeout, this.deadline, newCredentials, this.extraHeaders); + this.channel, this.timeout, this.deadline, newCredentials, this.extraHeaders, this.tracer); } @Override @@ -171,7 +182,7 @@ public HttpJsonCallContext withTimeout(Duration timeout) { } return new HttpJsonCallContext( - this.channel, timeout, this.deadline, this.credentials, this.extraHeaders); + this.channel, timeout, this.deadline, this.credentials, this.extraHeaders, this.tracer); } @Nullable @@ -208,7 +219,8 @@ public ApiCallContext withExtraHeaders(Map> extraHeaders) { Preconditions.checkNotNull(extraHeaders); ImmutableMap> newExtraHeaders = Headers.mergeHeaders(this.extraHeaders, extraHeaders); - return new HttpJsonCallContext(channel, timeout, deadline, credentials, newExtraHeaders); + return new HttpJsonCallContext( + channel, timeout, deadline, credentials, newExtraHeaders, this.tracer); } @BetaApi("The surface for extra headers is not stable yet and may change in the future.") @@ -230,11 +242,30 @@ public Credentials getCredentials() { } public HttpJsonCallContext withChannel(HttpJsonChannel newChannel) { - return new HttpJsonCallContext(newChannel, timeout, deadline, credentials, extraHeaders); + return new HttpJsonCallContext( + newChannel, timeout, deadline, credentials, extraHeaders, this.tracer); } public HttpJsonCallContext withDeadline(Instant newDeadline) { - return new HttpJsonCallContext(channel, timeout, newDeadline, credentials, extraHeaders); + return new HttpJsonCallContext( + channel, timeout, newDeadline, credentials, extraHeaders, this.tracer); + } + + @Nonnull + @Override + public ApiTracer getTracer() { + if (tracer == null) { + return NoopApiTracer.create(); + } + return tracer; + } + + @Override + public ApiCallContext withTracer(@Nonnull ApiTracer newTracer) { + Preconditions.checkNotNull(newTracer); + + return new HttpJsonCallContext( + channel, timeout, deadline, credentials, extraHeaders, newTracer); } @Override @@ -250,11 +281,12 @@ public boolean equals(Object o) { && Objects.equals(timeout, that.timeout) && Objects.equals(deadline, that.deadline) && Objects.equals(credentials, that.credentials) - && Objects.equals(extraHeaders, that.extraHeaders); + && Objects.equals(extraHeaders, that.extraHeaders) + && Objects.equals(tracer, that.tracer); } @Override public int hashCode() { - return Objects.hash(channel, timeout, deadline, credentials, extraHeaders); + return Objects.hash(channel, timeout, deadline, credentials, extraHeaders, tracer); } } diff --git a/gax/src/main/java/com/google/api/gax/retrying/NoopRetryingContext.java b/gax/src/main/java/com/google/api/gax/retrying/NoopRetryingContext.java index def9cc796..03398f5e8 100644 --- a/gax/src/main/java/com/google/api/gax/retrying/NoopRetryingContext.java +++ b/gax/src/main/java/com/google/api/gax/retrying/NoopRetryingContext.java @@ -31,6 +31,11 @@ // TODO(igorbernstein2): Remove this class once RetryingExecutor#createFuture(Callable) is // deprecated and removed. + +import com.google.api.gax.tracing.ApiTracer; +import com.google.api.gax.tracing.NoopApiTracer; +import javax.annotation.Nonnull; + /** * Backwards compatibility class to aid in transition to adding operation state to {@link * RetryingFuture} implementations. @@ -39,4 +44,10 @@ class NoopRetryingContext implements RetryingContext { public static RetryingContext create() { return new NoopRetryingContext(); } + + @Nonnull + @Override + public ApiTracer getTracer() { + return NoopApiTracer.create(); + } } diff --git a/gax/src/main/java/com/google/api/gax/retrying/RetryingContext.java b/gax/src/main/java/com/google/api/gax/retrying/RetryingContext.java index 335d14611..b0d9c4308 100644 --- a/gax/src/main/java/com/google/api/gax/retrying/RetryingContext.java +++ b/gax/src/main/java/com/google/api/gax/retrying/RetryingContext.java @@ -30,6 +30,8 @@ package com.google.api.gax.retrying; import com.google.api.core.BetaApi; +import com.google.api.gax.tracing.ApiTracer; +import javax.annotation.Nonnull; /** * Context for a retryable operation. @@ -37,4 +39,7 @@ *

It provides state to individual {@link RetryingFuture}s via the {@link RetryingExecutor}. */ @BetaApi("The surface for passing per operation state is not yet stable") -public interface RetryingContext {} +public interface RetryingContext { + @Nonnull + ApiTracer getTracer(); +} diff --git a/gax/src/main/java/com/google/api/gax/rpc/ApiCallContext.java b/gax/src/main/java/com/google/api/gax/rpc/ApiCallContext.java index 658453ef7..31a53b1a2 100644 --- a/gax/src/main/java/com/google/api/gax/rpc/ApiCallContext.java +++ b/gax/src/main/java/com/google/api/gax/rpc/ApiCallContext.java @@ -32,9 +32,11 @@ import com.google.api.core.BetaApi; import com.google.api.core.InternalExtensionOnly; import com.google.api.gax.retrying.RetryingContext; +import com.google.api.gax.tracing.ApiTracer; import com.google.auth.Credentials; import java.util.List; import java.util.Map; +import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.threeten.bp.Duration; @@ -130,6 +132,13 @@ public interface ApiCallContext extends RetryingContext { @Nullable Duration getStreamIdleTimeout(); + @BetaApi("The surface for tracing is not stable yet and may change in the future") + @Nonnull + ApiTracer getTracer(); + + @BetaApi("The surface for tracing is not stable yet and may change in the future") + ApiCallContext withTracer(@Nonnull ApiTracer tracer); + /** If inputContext is not null, returns it; if it is null, returns the present instance. */ ApiCallContext nullToSelf(ApiCallContext inputContext); diff --git a/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index 761f3baee..5e4dc6c29 100644 --- a/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -35,6 +35,8 @@ import com.google.api.gax.core.BackgroundResource; import com.google.api.gax.core.ExecutorAsBackgroundResource; import com.google.api.gax.core.ExecutorProvider; +import com.google.api.gax.tracing.ApiTracerFactory; +import com.google.api.gax.tracing.NoopApiTracerFactory; import com.google.auth.Credentials; import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; @@ -93,6 +95,10 @@ public abstract class ClientContext { @Nullable public abstract String getEndpoint(); + @BetaApi("The surface for tracing is not stable yet and may change in the future.") + @Nonnull + public abstract ApiTracerFactory getTracerFactory(); + public static Builder newBuilder() { return new AutoValue_ClientContext.Builder() .setBackgroundResources(Collections.emptyList()) @@ -101,7 +107,8 @@ public static Builder newBuilder() { .setInternalHeaders(Collections.emptyMap()) .setClock(NanoClock.getDefaultClock()) .setStreamWatchdog(null) - .setStreamWatchdogCheckInterval(Duration.ZERO); + .setStreamWatchdogCheckInterval(Duration.ZERO) + .setTracerFactory(new NoopApiTracerFactory()); } public abstract Builder toBuilder(); @@ -186,6 +193,7 @@ public static ClientContext create(StubSettings settings) throws IOException { .setEndpoint(settings.getEndpoint()) .setStreamWatchdog(watchdog) .setStreamWatchdogCheckInterval(settings.getStreamWatchdogCheckInterval()) + .setTracerFactory(settings.getTracerFactory()) .build(); } @@ -218,6 +226,9 @@ public abstract static class Builder { @BetaApi("The surface for streaming is not stable yet and may change in the future.") public abstract Builder setStreamWatchdogCheckInterval(Duration duration); + @BetaApi("The surface for tracing is not stable yet and may change in the future.") + public abstract Builder setTracerFactory(ApiTracerFactory tracerFactory); + public abstract ClientContext build(); } } diff --git a/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java b/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java index b0afdba88..fe170a108 100644 --- a/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java +++ b/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java @@ -39,6 +39,8 @@ import com.google.api.gax.core.FixedExecutorProvider; import com.google.api.gax.core.InstantiatingExecutorProvider; import com.google.api.gax.core.NoCredentialsProvider; +import com.google.api.gax.tracing.ApiTracerFactory; +import com.google.api.gax.tracing.NoopApiTracerFactory; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import java.io.IOException; @@ -67,6 +69,7 @@ public abstract class StubSettings> { private final String endpoint; @Nullable private final WatchdogProvider streamWatchdogProvider; @Nonnull private final Duration streamWatchdogCheckInterval; + @Nonnull private final ApiTracerFactory tracerFactory; /** Constructs an instance of StubSettings. */ protected StubSettings(Builder builder) { @@ -79,6 +82,7 @@ protected StubSettings(Builder builder) { this.endpoint = builder.endpoint; this.streamWatchdogProvider = builder.streamWatchdogProvider; this.streamWatchdogCheckInterval = builder.streamWatchdogCheckInterval; + this.tracerFactory = builder.tracerFactory; } public final ExecutorProvider getExecutorProvider() { @@ -123,6 +127,12 @@ public final Duration getStreamWatchdogCheckInterval() { return streamWatchdogCheckInterval; } + @BetaApi("The surface for tracing is not stable yet and may change in the future.") + @Nonnull + public ApiTracerFactory getTracerFactory() { + return tracerFactory; + } + public String toString() { return MoreObjects.toStringHelper(this) .add("executorProvider", executorProvider) @@ -134,6 +144,7 @@ public String toString() { .add("endpoint", endpoint) .add("streamWatchdogProvider", streamWatchdogProvider) .add("streamWatchdogCheckInterval", streamWatchdogCheckInterval) + .add("tracerFactory", tracerFactory) .toString(); } @@ -151,6 +162,7 @@ public abstract static class Builder< private String endpoint; @Nullable private WatchdogProvider streamWatchdogProvider; @Nonnull private Duration streamWatchdogCheckInterval; + @Nonnull private ApiTracerFactory tracerFactory; /** Create a builder from a StubSettings object. */ protected Builder(StubSettings settings) { @@ -163,6 +175,7 @@ protected Builder(StubSettings settings) { this.endpoint = settings.endpoint; this.streamWatchdogProvider = settings.streamWatchdogProvider; this.streamWatchdogCheckInterval = settings.streamWatchdogCheckInterval; + this.tracerFactory = settings.tracerFactory; } protected Builder(ClientContext clientContext) { @@ -176,6 +189,7 @@ protected Builder(ClientContext clientContext) { this.endpoint = null; this.streamWatchdogProvider = InstantiatingWatchdogProvider.create(); this.streamWatchdogCheckInterval = Duration.ofSeconds(10); + this.tracerFactory = new NoopApiTracerFactory(); } else { this.executorProvider = FixedExecutorProvider.create(clientContext.getExecutor()); this.transportChannelProvider = @@ -189,6 +203,7 @@ protected Builder(ClientContext clientContext) { this.streamWatchdogProvider = FixedWatchdogProvider.create(clientContext.getStreamWatchdog()); this.streamWatchdogCheckInterval = clientContext.getStreamWatchdogCheckInterval(); + this.tracerFactory = clientContext.getTracerFactory(); } } @@ -290,6 +305,14 @@ public B setStreamWatchdogCheckInterval(@Nonnull Duration checkInterval) { return self(); } + /** Configures the tracing implementation. */ + @BetaApi("The surface for tracing is not stable yet and may change in the future.") + public B setTracerFactory(@Nonnull ApiTracerFactory tracerFactory) { + Preconditions.checkNotNull(tracerFactory); + this.tracerFactory = tracerFactory; + return self(); + } + /** Gets the ExecutorProvider that was previously set on this Builder. */ public ExecutorProvider getExecutorProvider() { return executorProvider; @@ -339,6 +362,12 @@ public Duration getStreamWatchdogCheckInterval() { return streamWatchdogCheckInterval; } + @BetaApi("The surface for tracing is not stable yet and may change in the future.") + @Nonnull + public ApiTracerFactory getTracerFactory() { + return tracerFactory; + } + /** Applies the given settings updater function to the given method settings builders. */ protected static void applyToAllUnaryMethods( Iterable> methodSettingsBuilders, @@ -361,6 +390,7 @@ public String toString() { .add("endpoint", endpoint) .add("streamWatchdogProvider", streamWatchdogProvider) .add("streamWatchdogCheckInterval", streamWatchdogCheckInterval) + .add("tracerFactory", tracerFactory) .toString(); } } diff --git a/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java b/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java new file mode 100644 index 000000000..6d847cc7d --- /dev/null +++ b/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java @@ -0,0 +1,112 @@ +/* + * Copyright 2018 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.InternalExtensionOnly; +import org.threeten.bp.Duration; + +/** + * Implementations of this class trace the logical flow of a google cloud client. + * + *

A single instance of a tracer represents a logical operation that can be annotated throughout + * its lifecycle. Constructing an instance of a subclass will implicitly signal the start of a new + * operation. + */ +@BetaApi("Surface for tracing is not yet stable") +@InternalExtensionOnly +public interface ApiTracer { + /** + * Asks the underlying implementation to install itself as a thread local. This allows for interop + * 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(); + + /** + * Signals that the overall operation has finished successfully. The tracer is now considered + * closed and should no longer be used. + */ + void operationSucceeded(); + + /** + * Signals that the overall operation has failed and no further attempts will be made. The tracer + * is now considered closed and should no longer be used. + */ + void operationFailed(Throwable error); + + /** Annotates the operation with selected connection id from the {@code ChannelPool}. */ + void connectionSelected(int id); + + /** + * Adds an annotation that an attempt is about to start. In general this should occur at the very + * start of the operation. The attemptNumber is zero based. So the initial attempt will be 0. + */ + void startAttempt(int attemptNumber); + + /** Adds an annotation that the attempt succeeded. */ + void attemptSucceeded(); + + /** + * Adds an annotation that the attempt failed, but another attempt will be made after the delay. + */ + void retryableFailure(Throwable error, Duration delay); + + /** + * Adds an annotation that the attempt failed and that no further attempts will be made because + * retry limits have been reached. + */ + void retriesExhausted(); + + /** + * Adds an annotation that the attempt failed and that no further attempts will be made because + * the last error was not retryable. + */ + void permanentFailure(Throwable error); + + /** Adds an annotation that a streaming response has been received. */ + void receivedResponse(); + + /** Adds an annotation that a streaming request has been sent. */ + void sentRequest(); + + /** Adds an annotation that a batch of writes has been flushed. */ + void sentBatchRequest(long elementCount, long requestSize); + + /** + * 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 + * #inScope()}. + */ + interface Scope extends AutoCloseable { + @Override + void close(); + } +} diff --git a/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java b/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java new file mode 100644 index 000000000..1fcc144ac --- /dev/null +++ b/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java @@ -0,0 +1,45 @@ +/* + * Copyright 2017 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.InternalExtensionOnly; + +/** + * A factory to create new instances of {@link ApiTracer}s. + * + *

In general a single instance of an {@link ApiTracer} will correspond to a single logical + * operation. + */ +@BetaApi("Surface for tracing is not yet stable") +@InternalExtensionOnly +public interface ApiTracerFactory { + ApiTracer newTracer(SpanName spanName); +} diff --git a/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracer.java b/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracer.java new file mode 100644 index 000000000..c7d16f406 --- /dev/null +++ b/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracer.java @@ -0,0 +1,118 @@ +/* + * Copyright 2018 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 org.threeten.bp.Duration; + +/** + * A fake implementation of {@link ApiTracer} that does nothing. + * + *

For internal use only. + */ +@InternalApi +public final class NoopApiTracer implements ApiTracer { + private static final ApiTracer INSTANCE = new NoopApiTracer(); + + private static final Scope NOOP_SCOPE = + new Scope() { + @Override + public void close() { + // noop + } + }; + + private NoopApiTracer() {} + + public static ApiTracer create() { + return INSTANCE; + } + + @Override + public Scope inScope() { + return NOOP_SCOPE; + } + + @Override + public void operationSucceeded() { + // noop + } + + @Override + public void operationFailed(Throwable error) { + // noop + } + + @Override + public void connectionSelected(int id) { + // noop + } + + @Override + public void startAttempt(int attemptNumber) { + // noop + } + + @Override + public void attemptSucceeded() { + // noop + } + + @Override + public void retryableFailure(Throwable error, Duration delay) { + // noop + } + + @Override + public void retriesExhausted() { + // noop + } + + @Override + public void permanentFailure(Throwable error) { + // noop + + } + + @Override + public void receivedResponse() { + // noop + } + + @Override + public void sentRequest() { + // noop + } + + @Override + public void sentBatchRequest(long elementCount, long requestSize) { + // noop + } +} diff --git a/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracerFactory.java b/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracerFactory.java new file mode 100644 index 000000000..3ca7d02ea --- /dev/null +++ b/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracerFactory.java @@ -0,0 +1,56 @@ +/* + * Copyright 2018 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; + +/** + * Factory that will build fake {@link ApiTracer}s. + * + *

For internal use only. + */ +@InternalApi +public final class NoopApiTracerFactory implements ApiTracerFactory { + /** {@inheritDoc} */ + @Override + public ApiTracer newTracer(SpanName spanName) { + return NoopApiTracer.create(); + } + + @Override + public int hashCode() { + return 1; + } + + @Override + public boolean equals(Object obj) { + return obj instanceof NoopApiTracerFactory; + } +} diff --git a/gax/src/main/java/com/google/api/gax/tracing/SpanName.java b/gax/src/main/java/com/google/api/gax/tracing/SpanName.java new file mode 100644 index 000000000..61e583a05 --- /dev/null +++ b/gax/src/main/java/com/google/api/gax/tracing/SpanName.java @@ -0,0 +1,66 @@ +/* + * Copyright 2017 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.InternalExtensionOnly; +import com.google.auto.value.AutoValue; + +/** A value class to represent the name of the operation in an {@link ApiTracer}. */ +@BetaApi("Surface for tracing is not yet stable") +@InternalExtensionOnly +@AutoValue +public abstract class SpanName { + /** + * Creates a new instance of the name. + * + * @param clientName The name of the client. ie BigtableData + * @param methodName The name of the logical operation being traced. ie. ReadRows. + */ + public static SpanName of(String clientName, String methodName) { + return new AutoValue_SpanName(clientName, methodName); + } + + /** The name of the client. ie BigtableData */ + public abstract String getClientName(); + + /** The name of the logical operation being traced. ie. ReadRows. */ + public abstract String getMethodName(); + + /** Creates a new instance with the clientName overriden. */ + public SpanName withClientName(String clientName) { + return of(clientName, getMethodName()); + } + + /** Creates a new instance with the methodName overriden. */ + public SpanName withMethodName(String methodName) { + return of(getClientName(), methodName); + } +} diff --git a/gax/src/test/java/com/google/api/gax/rpc/testing/FakeCallContext.java b/gax/src/test/java/com/google/api/gax/rpc/testing/FakeCallContext.java index e4106d6b6..88bbe96ec 100644 --- a/gax/src/test/java/com/google/api/gax/rpc/testing/FakeCallContext.java +++ b/gax/src/test/java/com/google/api/gax/rpc/testing/FakeCallContext.java @@ -34,6 +34,7 @@ import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.TransportChannel; import com.google.api.gax.rpc.internal.Headers; +import com.google.api.gax.tracing.ApiTracer; import com.google.auth.Credentials; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; @@ -51,6 +52,7 @@ public class FakeCallContext implements ApiCallContext { private final Duration streamWaitTimeout; private final Duration streamIdleTimeout; private final ImmutableMap> extraHeaders; + private final ApiTracer tracer; private FakeCallContext( Credentials credentials, @@ -58,18 +60,20 @@ private FakeCallContext( Duration timeout, Duration streamWaitTimeout, Duration streamIdleTimeout, - ImmutableMap> extraHeaders) { + ImmutableMap> extraHeaders, + ApiTracer tracer) { this.credentials = credentials; this.channel = channel; this.timeout = timeout; this.streamWaitTimeout = streamWaitTimeout; this.streamIdleTimeout = streamIdleTimeout; this.extraHeaders = extraHeaders; + this.tracer = tracer; } public static FakeCallContext createDefault() { return new FakeCallContext( - null, null, null, null, null, ImmutableMap.>of()); + null, null, null, null, null, ImmutableMap.>of(), null); } @Override @@ -125,6 +129,11 @@ public ApiCallContext merge(ApiCallContext inputCallContext) { newStreamIdleTimeout = streamIdleTimeout; } + ApiTracer newTracer = fakeCallContext.tracer; + if (newTracer == null) { + newTracer = this.tracer; + } + ImmutableMap> newExtraHeaders = Headers.mergeHeaders(extraHeaders, fakeCallContext.extraHeaders); return new FakeCallContext( @@ -133,7 +142,8 @@ public ApiCallContext merge(ApiCallContext inputCallContext) { newTimeout, newStreamWaitTimeout, newStreamIdleTimeout, - newExtraHeaders); + newExtraHeaders, + newTracer); } public Credentials getCredentials() { @@ -169,7 +179,8 @@ public FakeCallContext withCredentials(Credentials credentials) { this.timeout, this.streamWaitTimeout, this.streamIdleTimeout, - this.extraHeaders); + this.extraHeaders, + this.tracer); } @Override @@ -190,7 +201,8 @@ public FakeCallContext withChannel(FakeChannel channel) { this.timeout, this.streamWaitTimeout, this.streamIdleTimeout, - this.extraHeaders); + this.extraHeaders, + this.tracer); } @Override @@ -211,23 +223,24 @@ public FakeCallContext withTimeout(Duration timeout) { timeout, this.streamWaitTimeout, this.streamIdleTimeout, - this.extraHeaders); + this.extraHeaders, + this.tracer); } @Override - public ApiCallContext withStreamWaitTimeout(@Nonnull Duration streamWaitTimeout) { - Preconditions.checkNotNull(streamWaitTimeout); + public ApiCallContext withStreamWaitTimeout(@Nullable Duration streamWaitTimeout) { return new FakeCallContext( this.credentials, this.channel, this.timeout, streamWaitTimeout, this.streamIdleTimeout, - this.extraHeaders); + this.extraHeaders, + this.tracer); } @Override - public ApiCallContext withStreamIdleTimeout(@Nonnull Duration streamIdleTimeout) { + public ApiCallContext withStreamIdleTimeout(@Nullable Duration streamIdleTimeout) { Preconditions.checkNotNull(streamIdleTimeout); return new FakeCallContext( this.credentials, @@ -235,7 +248,8 @@ public ApiCallContext withStreamIdleTimeout(@Nonnull Duration streamIdleTimeout) this.timeout, this.streamWaitTimeout, streamIdleTimeout, - this.extraHeaders); + this.extraHeaders, + this.tracer); } @Override @@ -244,7 +258,13 @@ public ApiCallContext withExtraHeaders(Map> extraHeaders) { ImmutableMap> newExtraHeaders = Headers.mergeHeaders(this.extraHeaders, extraHeaders); return new FakeCallContext( - credentials, channel, timeout, streamWaitTimeout, streamIdleTimeout, newExtraHeaders); + credentials, + channel, + timeout, + streamWaitTimeout, + streamIdleTimeout, + newExtraHeaders, + this.tracer); } @Override @@ -252,6 +272,26 @@ public Map> getExtraHeaders() { return this.extraHeaders; } + @Override + @Nonnull + public ApiTracer getTracer() { + return tracer; + } + + @Override + public ApiCallContext withTracer(@Nonnull ApiTracer tracer) { + Preconditions.checkNotNull(tracer); + + return new FakeCallContext( + this.credentials, + this.channel, + this.timeout, + this.streamWaitTimeout, + this.streamIdleTimeout, + this.extraHeaders, + tracer); + } + public static FakeCallContext create(ClientContext clientContext) { return FakeCallContext.createDefault() .withTransportChannel(clientContext.getTransportChannel()) From 3336fe6b5edcc09537882142ad1109cbed2cba60 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Fri, 28 Dec 2018 09:13:39 -0500 Subject: [PATCH 2/4] rename tracer method to subject action --- .../java/com/google/api/gax/tracing/ApiTracer.java | 14 +++++++------- .../com/google/api/gax/tracing/NoopApiTracer.java | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java b/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java index 6d847cc7d..b7c2e431a 100644 --- a/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java +++ b/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java @@ -69,7 +69,7 @@ public interface ApiTracer { * Adds an annotation that an attempt is about to start. In general this should occur at the very * start of the operation. The attemptNumber is zero based. So the initial attempt will be 0. */ - void startAttempt(int attemptNumber); + void attemptStarted(int attemptNumber); /** Adds an annotation that the attempt succeeded. */ void attemptSucceeded(); @@ -77,28 +77,28 @@ public interface ApiTracer { /** * Adds an annotation that the attempt failed, but another attempt will be made after the delay. */ - void retryableFailure(Throwable error, Duration delay); + void attemptFailed(Throwable error, Duration delay); /** * Adds an annotation that the attempt failed and that no further attempts will be made because * retry limits have been reached. */ - void retriesExhausted(); + void attemptFailedRetriesExhausted(); /** * Adds an annotation that the attempt failed and that no further attempts will be made because * the last error was not retryable. */ - void permanentFailure(Throwable error); + void attemptPermanentFailure(Throwable error); /** Adds an annotation that a streaming response has been received. */ - void receivedResponse(); + void responseReceived(); /** Adds an annotation that a streaming request has been sent. */ - void sentRequest(); + void requestSent(); /** Adds an annotation that a batch of writes has been flushed. */ - void sentBatchRequest(long elementCount, long requestSize); + void batchRequestSent(long elementCount, long requestSize); /** * A context class to be used with {@link #inScope()} and a try-with-resources block. Closing a diff --git a/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracer.java b/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracer.java index c7d16f406..7aba0f2fe 100644 --- a/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracer.java +++ b/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracer.java @@ -76,7 +76,7 @@ public void connectionSelected(int id) { } @Override - public void startAttempt(int attemptNumber) { + public void attemptStarted(int attemptNumber) { // noop } @@ -86,33 +86,33 @@ public void attemptSucceeded() { } @Override - public void retryableFailure(Throwable error, Duration delay) { + public void attemptFailed(Throwable error, Duration delay) { // noop } @Override - public void retriesExhausted() { + public void attemptFailedRetriesExhausted() { // noop } @Override - public void permanentFailure(Throwable error) { + public void attemptPermanentFailure(Throwable error) { // noop } @Override - public void receivedResponse() { + public void responseReceived() { // noop } @Override - public void sentRequest() { + public void requestSent() { // noop } @Override - public void sentBatchRequest(long elementCount, long requestSize) { + public void batchRequestSent(long elementCount, long requestSize) { // noop } } From 69388798af1eb41b4f6d050c0efebe40e4511a14 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Fri, 4 Jan 2019 11:00:27 -0500 Subject: [PATCH 3/4] address feedback --- .../google/api/gax/grpc/GrpcCallContext.java | 6 +++-- .../api/gax/grpc/GrpcCallContextTest.java | 23 +++++++++++++++++++ .../api/gax/httpjson/HttpJsonCallContext.java | 5 ++-- .../gax/httpjson/HttpJsonCallContextTest.java | 23 +++++++++++++++++++ .../api/gax/retrying/NoopRetryingContext.java | 3 ++- .../api/gax/retrying/RetryingContext.java | 1 + .../google/api/gax/rpc/ApiCallContext.java | 14 +++++++++++ .../com/google/api/gax/rpc/ClientContext.java | 8 ++++++- .../com/google/api/gax/rpc/StubSettings.java | 12 ++++++++-- .../com/google/api/gax/tracing/ApiTracer.java | 22 ++++++++++++++++-- .../api/gax/tracing/ApiTracerFactory.java | 2 +- .../google/api/gax/tracing/NoopApiTracer.java | 4 ++-- .../api/gax/tracing/NoopApiTracerFactory.java | 20 ++++++++-------- .../com/google/api/gax/tracing/SpanName.java | 6 ++--- .../api/gax/rpc/testing/FakeCallContext.java | 2 ++ 15 files changed, 124 insertions(+), 27 deletions(-) diff --git a/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java b/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java index 3bbec167c..6f1af6a2c 100644 --- a/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java +++ b/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java @@ -385,18 +385,20 @@ public GrpcCallContext withRequestParamsDynamicHeaderOption(String requestParams return withCallOptions(newCallOptions); } + /** {@inheritDoc} */ @Override @Nonnull public ApiTracer getTracer() { ApiTracer tracer = callOptions.getOption(TRACER_KEY); if (tracer == null) { - tracer = NoopApiTracer.create(); + tracer = NoopApiTracer.getInstance(); } return tracer; } + /** {@inheritDoc} */ @Override - public ApiCallContext withTracer(@Nonnull ApiTracer tracer) { + public GrpcCallContext withTracer(@Nonnull ApiTracer tracer) { Preconditions.checkNotNull(tracer); return withCallOptions(callOptions.withOption(TRACER_KEY, tracer)); } diff --git a/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java b/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java index 230ee25b3..b13042adb 100644 --- a/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java +++ b/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java @@ -35,6 +35,7 @@ import com.google.api.gax.rpc.testing.FakeCallContext; import com.google.api.gax.rpc.testing.FakeChannel; import com.google.api.gax.rpc.testing.FakeTransportChannel; +import com.google.api.gax.tracing.ApiTracer; import com.google.auth.Credentials; import com.google.common.collect.ImmutableMap; import com.google.common.truth.Truth; @@ -297,6 +298,28 @@ public void testMergeWithExtraHeaders() { Truth.assertThat(gotExtraHeaders).containsExactlyEntriesIn(expectedExtraHeaders); } + @Test + public void testMergeWithTracer() { + ApiTracer explicitTracer = Mockito.mock(ApiTracer.class); + GrpcCallContext ctxWithExplicitTracer = + GrpcCallContext.createDefault().withTracer(explicitTracer); + + GrpcCallContext ctxWithDefaultTracer = GrpcCallContext.createDefault(); + ApiTracer defaultTracer = ctxWithDefaultTracer.getTracer(); + + // Explicit tracers override default tracers + Truth.assertThat(ctxWithDefaultTracer.merge(ctxWithExplicitTracer).getTracer()) + .isSameAs(explicitTracer); + + // Default tracer do not override explicit tracers + Truth.assertThat(ctxWithExplicitTracer.merge(ctxWithDefaultTracer).getTracer()) + .isSameAs(explicitTracer); + + // Default tracer does not override another default tracer + Truth.assertThat(ctxWithDefaultTracer.merge(GrpcCallContext.createDefault()).getTracer()) + .isSameAs(defaultTracer); + } + private static Map> createTestExtraHeaders(String... keyValues) { Map> extraHeaders = new HashMap<>(); for (int i = 0; i < keyValues.length; i += 2) { diff --git a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java index 6b17c7fa1..ebe11b599 100644 --- a/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java +++ b/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java @@ -255,13 +255,14 @@ public HttpJsonCallContext withDeadline(Instant newDeadline) { @Override public ApiTracer getTracer() { if (tracer == null) { - return NoopApiTracer.create(); + return NoopApiTracer.getInstance(); } return tracer; } + /** {@inheritDoc} */ @Override - public ApiCallContext withTracer(@Nonnull ApiTracer newTracer) { + public HttpJsonCallContext withTracer(@Nonnull ApiTracer newTracer) { Preconditions.checkNotNull(newTracer); return new HttpJsonCallContext( diff --git a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonCallContextTest.java b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonCallContextTest.java index 63111de5c..7d1bb5f78 100644 --- a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonCallContextTest.java +++ b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonCallContextTest.java @@ -32,6 +32,7 @@ import com.google.api.gax.rpc.testing.FakeCallContext; import com.google.api.gax.rpc.testing.FakeChannel; import com.google.api.gax.rpc.testing.FakeTransportChannel; +import com.google.api.gax.tracing.ApiTracer; import com.google.auth.Credentials; import com.google.common.truth.Truth; import org.junit.Rule; @@ -152,4 +153,26 @@ public void testMergeWithTimeout() { Truth.assertThat(ctx1.merge(ctx2).getTimeout()).isEqualTo(timeout); } + + @Test + public void testMergeWithTracer() { + ApiTracer explicitTracer = Mockito.mock(ApiTracer.class); + HttpJsonCallContext ctxWithExplicitTracer = + HttpJsonCallContext.createDefault().withTracer(explicitTracer); + + HttpJsonCallContext ctxWithDefaultTracer = HttpJsonCallContext.createDefault(); + ApiTracer defaultTracer = ctxWithDefaultTracer.getTracer(); + + // Explicit tracers override default tracers + Truth.assertThat(ctxWithDefaultTracer.merge(ctxWithExplicitTracer).getTracer()) + .isSameAs(explicitTracer); + + // Default tracer do not override explicit tracers + Truth.assertThat(ctxWithExplicitTracer.merge(ctxWithDefaultTracer).getTracer()) + .isSameAs(explicitTracer); + + // Default tracer does not override another default tracer + Truth.assertThat(ctxWithDefaultTracer.merge(HttpJsonCallContext.createDefault()).getTracer()) + .isSameAs(defaultTracer); + } } diff --git a/gax/src/main/java/com/google/api/gax/retrying/NoopRetryingContext.java b/gax/src/main/java/com/google/api/gax/retrying/NoopRetryingContext.java index 03398f5e8..c2649d995 100644 --- a/gax/src/main/java/com/google/api/gax/retrying/NoopRetryingContext.java +++ b/gax/src/main/java/com/google/api/gax/retrying/NoopRetryingContext.java @@ -45,9 +45,10 @@ public static RetryingContext create() { return new NoopRetryingContext(); } + /** {@inheritDoc} */ @Nonnull @Override public ApiTracer getTracer() { - return NoopApiTracer.create(); + return NoopApiTracer.getInstance(); } } diff --git a/gax/src/main/java/com/google/api/gax/retrying/RetryingContext.java b/gax/src/main/java/com/google/api/gax/retrying/RetryingContext.java index b0d9c4308..04f4bb929 100644 --- a/gax/src/main/java/com/google/api/gax/retrying/RetryingContext.java +++ b/gax/src/main/java/com/google/api/gax/retrying/RetryingContext.java @@ -40,6 +40,7 @@ */ @BetaApi("The surface for passing per operation state is not yet stable") public interface RetryingContext { + /** Returns the {@link ApiTracer} associated with the current operation. */ @Nonnull ApiTracer getTracer(); } diff --git a/gax/src/main/java/com/google/api/gax/rpc/ApiCallContext.java b/gax/src/main/java/com/google/api/gax/rpc/ApiCallContext.java index 31a53b1a2..1115027d6 100644 --- a/gax/src/main/java/com/google/api/gax/rpc/ApiCallContext.java +++ b/gax/src/main/java/com/google/api/gax/rpc/ApiCallContext.java @@ -132,10 +132,24 @@ public interface ApiCallContext extends RetryingContext { @Nullable Duration getStreamIdleTimeout(); + /** + * The {@link ApiTracer} that was previously set for this context. + * + *

The {@link ApiTracer} will be used to trace the current operation and to annotate various + * events like retries. + */ @BetaApi("The surface for tracing is not stable yet and may change in the future") @Nonnull ApiTracer getTracer(); + /** + * Returns a new {@link ApiCallContext} with the given {@link ApiTracer}. + * + *

The {@link ApiTracer} will be used to trace the current operation and to annotate various + * events like retries. + * + * @param tracer the {@link ApiTracer} to set. + */ @BetaApi("The surface for tracing is not stable yet and may change in the future") ApiCallContext withTracer(@Nonnull ApiTracer tracer); diff --git a/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index 5e4dc6c29..086f89a08 100644 --- a/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -95,6 +95,7 @@ public abstract class ClientContext { @Nullable public abstract String getEndpoint(); + /** Gets the {@link ApiTracerFactory} that will be used to generate traces for operations. */ @BetaApi("The surface for tracing is not stable yet and may change in the future.") @Nonnull public abstract ApiTracerFactory getTracerFactory(); @@ -108,7 +109,7 @@ public static Builder newBuilder() { .setClock(NanoClock.getDefaultClock()) .setStreamWatchdog(null) .setStreamWatchdogCheckInterval(Duration.ZERO) - .setTracerFactory(new NoopApiTracerFactory()); + .setTracerFactory(NoopApiTracerFactory.getInstance()); } public abstract Builder toBuilder(); @@ -226,6 +227,11 @@ public abstract static class Builder { @BetaApi("The surface for streaming is not stable yet and may change in the future.") public abstract Builder setStreamWatchdogCheckInterval(Duration duration); + /** + * Set the {@link ApiTracerFactory} that will be used to generate traces for operations. + * + * @param tracerFactory an instance {@link ApiTracerFactory}. + */ @BetaApi("The surface for tracing is not stable yet and may change in the future.") public abstract Builder setTracerFactory(ApiTracerFactory tracerFactory); diff --git a/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java b/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java index fe170a108..2cac53602 100644 --- a/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java +++ b/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java @@ -127,6 +127,10 @@ public final Duration getStreamWatchdogCheckInterval() { return streamWatchdogCheckInterval; } + /** + * Gets the configured {@link ApiTracerFactory} that will be used to generate traces for + * operations. + */ @BetaApi("The surface for tracing is not stable yet and may change in the future.") @Nonnull public ApiTracerFactory getTracerFactory() { @@ -189,7 +193,7 @@ protected Builder(ClientContext clientContext) { this.endpoint = null; this.streamWatchdogProvider = InstantiatingWatchdogProvider.create(); this.streamWatchdogCheckInterval = Duration.ofSeconds(10); - this.tracerFactory = new NoopApiTracerFactory(); + this.tracerFactory = NoopApiTracerFactory.getInstance(); } else { this.executorProvider = FixedExecutorProvider.create(clientContext.getExecutor()); this.transportChannelProvider = @@ -305,7 +309,11 @@ public B setStreamWatchdogCheckInterval(@Nonnull Duration checkInterval) { return self(); } - /** Configures the tracing implementation. */ + /** + * Configures the {@link ApiTracerFactory} that will be used to generate traces. + * + * @param tracerFactory an instance of {@link ApiTracerFactory} to set. + */ @BetaApi("The surface for tracing is not stable yet and may change in the future.") public B setTracerFactory(@Nonnull ApiTracerFactory tracerFactory) { Preconditions.checkNotNull(tracerFactory); diff --git a/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java b/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java index b7c2e431a..8a137b10b 100644 --- a/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java +++ b/gax/src/main/java/com/google/api/gax/tracing/ApiTracer.java @@ -59,15 +59,23 @@ public interface ApiTracer { /** * Signals that the overall operation has failed and no further attempts will be made. The tracer * is now considered closed and should no longer be used. + * + * @param error the final error that caused the operation to fail. */ void operationFailed(Throwable error); - /** Annotates the operation with selected connection id from the {@code ChannelPool}. */ + /** + * Annotates the operation with selected connection id from the {@code ChannelPool}. + * + * @param id the local connection identifier of the selected connection. + */ void connectionSelected(int id); /** * Adds an annotation that an attempt is about to start. In general this should occur at the very * start of the operation. The attemptNumber is zero based. So the initial attempt will be 0. + * + * @param attemptNumber the zero based sequential attempt number. */ void attemptStarted(int attemptNumber); @@ -76,6 +84,9 @@ public interface ApiTracer { /** * Adds an annotation that the attempt failed, but another attempt will be made after the delay. + * + * @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); @@ -88,6 +99,8 @@ public interface ApiTracer { /** * Adds an annotation that the attempt failed and that no further attempts will be made because * the last error was not retryable. + * + * @param error the error that caused the final attempt to fail. */ void attemptPermanentFailure(Throwable error); @@ -97,7 +110,12 @@ public interface ApiTracer { /** Adds an annotation that a streaming request has been sent. */ void requestSent(); - /** Adds an annotation that a batch of writes has been flushed. */ + /** + * Adds an annotation that a batch of writes has been flushed. + * + * @param elementCount the number of elements in the batch. + * @param requestSize the size of the batch in bytes. + */ void batchRequestSent(long elementCount, long requestSize); /** diff --git a/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java b/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java index 1fcc144ac..d306cbd24 100644 --- a/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java +++ b/gax/src/main/java/com/google/api/gax/tracing/ApiTracerFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 Google LLC + * Copyright 2018 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/src/main/java/com/google/api/gax/tracing/NoopApiTracer.java b/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracer.java index 7aba0f2fe..4b569b7f9 100644 --- a/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracer.java +++ b/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracer.java @@ -33,7 +33,7 @@ import org.threeten.bp.Duration; /** - * A fake implementation of {@link ApiTracer} that does nothing. + * An implementation of {@link ApiTracer} that does nothing. * *

For internal use only. */ @@ -51,7 +51,7 @@ public void close() { private NoopApiTracer() {} - public static ApiTracer create() { + public static ApiTracer getInstance() { return INSTANCE; } diff --git a/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracerFactory.java b/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracerFactory.java index 3ca7d02ea..421b4df7b 100644 --- a/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracerFactory.java +++ b/gax/src/main/java/com/google/api/gax/tracing/NoopApiTracerFactory.java @@ -32,25 +32,23 @@ import com.google.api.core.InternalApi; /** - * Factory that will build fake {@link ApiTracer}s. + * Factory that will build {@link ApiTracer}s that do nothing. * *

For internal use only. */ @InternalApi public final class NoopApiTracerFactory implements ApiTracerFactory { - /** {@inheritDoc} */ - @Override - public ApiTracer newTracer(SpanName spanName) { - return NoopApiTracer.create(); - } + private static final NoopApiTracerFactory INSTANCE = new NoopApiTracerFactory(); - @Override - public int hashCode() { - return 1; + public static NoopApiTracerFactory getInstance() { + return INSTANCE; } + private NoopApiTracerFactory() {} + + /** {@inheritDoc} */ @Override - public boolean equals(Object obj) { - return obj instanceof NoopApiTracerFactory; + public ApiTracer newTracer(SpanName spanName) { + return NoopApiTracer.getInstance(); } } diff --git a/gax/src/main/java/com/google/api/gax/tracing/SpanName.java b/gax/src/main/java/com/google/api/gax/tracing/SpanName.java index 61e583a05..20e9436d4 100644 --- a/gax/src/main/java/com/google/api/gax/tracing/SpanName.java +++ b/gax/src/main/java/com/google/api/gax/tracing/SpanName.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 Google LLC + * Copyright 2018 Google LLC * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -41,8 +41,8 @@ public abstract class SpanName { /** * Creates a new instance of the name. * - * @param clientName The name of the client. ie BigtableData - * @param methodName The name of the logical operation being traced. ie. ReadRows. + * @param clientName The name of the client. + * @param methodName The name of the logical operation being traced. */ public static SpanName of(String clientName, String methodName) { return new AutoValue_SpanName(clientName, methodName); diff --git a/gax/src/test/java/com/google/api/gax/rpc/testing/FakeCallContext.java b/gax/src/test/java/com/google/api/gax/rpc/testing/FakeCallContext.java index 88bbe96ec..aa2a7dcb9 100644 --- a/gax/src/test/java/com/google/api/gax/rpc/testing/FakeCallContext.java +++ b/gax/src/test/java/com/google/api/gax/rpc/testing/FakeCallContext.java @@ -272,12 +272,14 @@ public Map> getExtraHeaders() { return this.extraHeaders; } + /** {@inheritDoc} */ @Override @Nonnull public ApiTracer getTracer() { return tracer; } + /** {@inheritDoc} */ @Override public ApiCallContext withTracer(@Nonnull ApiTracer tracer) { Preconditions.checkNotNull(tracer); From 986f42c5315191df33ba9112498ff2353929feae Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Sat, 5 Jan 2019 01:30:45 -0500 Subject: [PATCH 4/4] address feedback --- .../java/com/google/api/gax/grpc/GrpcCallContextTest.java | 6 +++--- .../google/api/gax/httpjson/HttpJsonCallContextTest.java | 6 +++--- .../main/java/com/google/api/gax/tracing/SpanName.java | 8 +++++--- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java b/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java index b13042adb..483133b69 100644 --- a/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java +++ b/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java @@ -307,15 +307,15 @@ public void testMergeWithTracer() { GrpcCallContext ctxWithDefaultTracer = GrpcCallContext.createDefault(); ApiTracer defaultTracer = ctxWithDefaultTracer.getTracer(); - // Explicit tracers override default tracers + // Explicit tracer overrides the default tracer. Truth.assertThat(ctxWithDefaultTracer.merge(ctxWithExplicitTracer).getTracer()) .isSameAs(explicitTracer); - // Default tracer do not override explicit tracers + // Default tracer does not override an explicit tracer. Truth.assertThat(ctxWithExplicitTracer.merge(ctxWithDefaultTracer).getTracer()) .isSameAs(explicitTracer); - // Default tracer does not override another default tracer + // Default tracer does not override another default tracer. Truth.assertThat(ctxWithDefaultTracer.merge(GrpcCallContext.createDefault()).getTracer()) .isSameAs(defaultTracer); } diff --git a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonCallContextTest.java b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonCallContextTest.java index 7d1bb5f78..d368bd768 100644 --- a/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonCallContextTest.java +++ b/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonCallContextTest.java @@ -163,15 +163,15 @@ public void testMergeWithTracer() { HttpJsonCallContext ctxWithDefaultTracer = HttpJsonCallContext.createDefault(); ApiTracer defaultTracer = ctxWithDefaultTracer.getTracer(); - // Explicit tracers override default tracers + // Explicit tracer overrides the default tracer. Truth.assertThat(ctxWithDefaultTracer.merge(ctxWithExplicitTracer).getTracer()) .isSameAs(explicitTracer); - // Default tracer do not override explicit tracers + // Default tracer does not override an explicit tracer. Truth.assertThat(ctxWithExplicitTracer.merge(ctxWithDefaultTracer).getTracer()) .isSameAs(explicitTracer); - // Default tracer does not override another default tracer + // Default tracer does not override another default tracer. Truth.assertThat(ctxWithDefaultTracer.merge(HttpJsonCallContext.createDefault()).getTracer()) .isSameAs(defaultTracer); } diff --git a/gax/src/main/java/com/google/api/gax/tracing/SpanName.java b/gax/src/main/java/com/google/api/gax/tracing/SpanName.java index 20e9436d4..80c3ed5a6 100644 --- a/gax/src/main/java/com/google/api/gax/tracing/SpanName.java +++ b/gax/src/main/java/com/google/api/gax/tracing/SpanName.java @@ -30,18 +30,20 @@ package com.google.api.gax.tracing; import com.google.api.core.BetaApi; -import com.google.api.core.InternalExtensionOnly; +import com.google.api.core.InternalApi; import com.google.auto.value.AutoValue; /** A value class to represent the name of the operation in an {@link ApiTracer}. */ @BetaApi("Surface for tracing is not yet stable") -@InternalExtensionOnly +@InternalApi("For google-cloud-java client use only") @AutoValue public abstract class SpanName { /** * Creates a new instance of the name. * - * @param clientName The name of the client. + * @param clientName The name of the client. In general this will be GAPIC generated client name. + * However, in some cases, when the GAPIC generated client 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 SpanName of(String clientName, String methodName) {