From 296f8c3be69c621492072294edb5f2d60cffc5d1 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Thu, 14 Feb 2019 14:08:19 -0500 Subject: [PATCH 01/10] WIP: integrate OpenCensus tracing into the bigtable data client --- .../data/v2/stub/EnhancedBigtableStub.java | 72 +++++++++++++++---- .../v2/stub/EnhancedBigtableStubSettings.java | 15 ++++ .../gaxx/tracing/WrappedTracerFactory.java | 48 +++++++++++++ 3 files changed, 120 insertions(+), 15 deletions(-) create mode 100644 google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/tracing/WrappedTracerFactory.java diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index b9159a26eabe..41041593ccda 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -26,6 +26,10 @@ import com.google.api.gax.rpc.ServerStreamingCallSettings; import com.google.api.gax.rpc.ServerStreamingCallable; import com.google.api.gax.rpc.UnaryCallable; +import com.google.api.gax.tracing.SpanName; +import com.google.api.gax.tracing.TracedBatchingCallable; +import com.google.api.gax.tracing.TracedServerStreamingCallable; +import com.google.api.gax.tracing.TracedUnaryCallable; import com.google.bigtable.v2.MutateRowsRequest; import com.google.bigtable.v2.ReadRowsRequest; import com.google.bigtable.v2.SampleRowKeysRequest; @@ -50,6 +54,7 @@ import com.google.cloud.bigtable.data.v2.stub.readrows.ReadRowsUserCallable; import com.google.cloud.bigtable.data.v2.stub.readrows.RowMergingCallable; import com.google.cloud.bigtable.gaxx.retrying.ApiResultRetryAlgorithm; +import com.google.cloud.bigtable.gaxx.tracing.WrappedTracerFactory; import java.io.IOException; import java.util.List; import org.threeten.bp.Duration; @@ -68,6 +73,9 @@ */ @InternalApi public class EnhancedBigtableStub implements AutoCloseable { + private static final String TRACING_OUTER_CLIENT_NAME = "Bigtable"; + private static final String TRACING_INNER_CLIENT_NAME = "BaseBigtable"; + private final EnhancedBigtableStubSettings settings; private final GrpcBigtableStub stub; private final ClientContext clientContext; @@ -91,7 +99,9 @@ public static EnhancedBigtableStub create(EnhancedBigtableStubSettings settings) .setEndpoint(settings.getEndpoint()) .setCredentialsProvider(settings.getCredentialsProvider()) .setStreamWatchdogProvider(settings.getStreamWatchdogProvider()) - .setStreamWatchdogCheckInterval(settings.getStreamWatchdogCheckInterval()); + .setStreamWatchdogCheckInterval(settings.getStreamWatchdogCheckInterval()) + // Force the base stub to use a different TracerFactory + .setTracerFactory(new WrappedTracerFactory(settings.getTracerFactory(), TRACING_INNER_CLIENT_NAME)); // ReadRow retries are handled in the overlay: disable retries in the base layer (but make // sure to preserve the exception callable settings). @@ -139,6 +149,11 @@ public static EnhancedBigtableStub create(EnhancedBigtableStubSettings settings) ClientContext clientContext = ClientContext.create(baseSettings); GrpcBigtableStub stub = new GrpcBigtableStub(baseSettings, clientContext); + // Make sure to keep the original tracer factory for the outer client. + clientContext = clientContext.toBuilder() + .setTracerFactory(settings.getTracerFactory()) + .build(); + return new EnhancedBigtableStub(settings, clientContext, stub); } @@ -246,15 +261,13 @@ private ServerStreamingCallable createReadRowsCallable( FilterMarkerRowsCallable filtering = new FilterMarkerRowsCallable<>(retrying2, rowAdapter); - ServerStreamingCallable withContext = - filtering.withDefaultCallContext(clientContext.getDefaultCallContext()); + ReadRowsUserCallable userFacing = new ReadRowsUserCallable<>(filtering, requestContext); - // NOTE: Ideally `withDefaultCallContext` should be the outer-most callable, however the - // ReadRowsUserCallable overrides the first() method. This override would be lost if - // ReadRowsUserCallable is wrapped by another callable. At some point in the future, - // gax-java should allow preserving these kind of overrides through callable chains, at which - // point this should be re-ordered. - return new ReadRowsUserCallable<>(withContext, requestContext); + TracedServerStreamingCallable traced = new TracedServerStreamingCallable<>( + userFacing, clientContext.getTracerFactory(), SpanName.of(TRACING_OUTER_CLIENT_NAME, "ReadRows") + ); + + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** @@ -278,7 +291,13 @@ private UnaryCallable> createSampleRowKeysCallable() { UnaryCallable> userFacing = new SampleRowKeysCallable(retryable, requestContext); - return userFacing.withDefaultCallContext(clientContext.getDefaultCallContext()); + UnaryCallable> traced = new TracedUnaryCallable<>( + userFacing, + clientContext.getTracerFactory(), + SpanName.of(TRACING_OUTER_CLIENT_NAME, "SampleRowKeys") + ); + + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** @@ -291,7 +310,13 @@ private UnaryCallable> createSampleRowKeysCallable() { private UnaryCallable createMutateRowCallable() { MutateRowCallable userFacing = new MutateRowCallable(stub.mutateRowCallable(), requestContext); - return userFacing.withDefaultCallContext(clientContext.getDefaultCallContext()); + UnaryCallable traced = new TracedUnaryCallable<>( + userFacing, + clientContext.getTracerFactory(), + SpanName.of(TRACING_OUTER_CLIENT_NAME, "MutateRow") + ); + + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** @@ -310,7 +335,13 @@ private UnaryCallable createMutateRowCallable() { */ private UnaryCallable createBulkMutateRowsCallable() { UnaryCallable baseCallable = createMutateRowsBaseCallable(); - return new BulkMutateRowsUserFacingCallable(baseCallable, requestContext); + BulkMutateRowsUserFacingCallable userFacing = new BulkMutateRowsUserFacingCallable( + baseCallable, requestContext); + + TracedUnaryCallable traced = new TracedUnaryCallable<>( + userFacing, clientContext.getTracerFactory(), SpanName.of(TRACING_OUTER_CLIENT_NAME, "BulkMutateRows")); + + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** @@ -337,8 +368,12 @@ private UnaryCallable createBulkMutateRowsBatchingCallable() BatchingCallSettings.newBuilder(new MutateRowsBatchingDescriptor()) .setBatchingSettings(settings.bulkMutateRowsSettings().getBatchingSettings()); + TracedBatchingCallable traced = new TracedBatchingCallable<>( + baseCallable, clientContext.getTracerFactory(), SpanName.of(TRACING_OUTER_CLIENT_NAME, "BulkMutateRows"), + batchingCallSettings.getBatchingDescriptor()); + UnaryCallable batching = - Callables.batching(baseCallable, batchingCallSettings.build(), clientContext); + Callables.batching(traced, batchingCallSettings.build(), clientContext); MutateRowsUserFacingCallable userFacing = new MutateRowsUserFacingCallable(batching, requestContext); @@ -380,7 +415,10 @@ private UnaryCallable createCheckAndMutateRowCa CheckAndMutateRowCallable userFacing = new CheckAndMutateRowCallable(stub.checkAndMutateRowCallable(), requestContext); - return userFacing.withDefaultCallContext(clientContext.getDefaultCallContext()); + TracedUnaryCallable traced = new TracedUnaryCallable<>( + userFacing, clientContext.getTracerFactory(), SpanName.of(TRACING_OUTER_CLIENT_NAME, "CheckAndMutateRow")); + + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** @@ -396,7 +434,11 @@ private UnaryCallable createReadModifyWriteRowCallable( ReadModifyWriteRowCallable userFacing = new ReadModifyWriteRowCallable(stub.readModifyWriteRowCallable(), requestContext); - return userFacing.withDefaultCallContext(clientContext.getDefaultCallContext()); + TracedUnaryCallable traced = new TracedUnaryCallable<>( + userFacing, clientContext.getTracerFactory(), + SpanName.of(TRACING_OUTER_CLIENT_NAME, "ReadModifyWriteRow")); + + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } // diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java index ca44142a91eb..17499828f070 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java @@ -19,7 +19,9 @@ import com.google.api.gax.batching.BatchingSettings; import com.google.api.gax.batching.FlowControlSettings; import com.google.api.gax.batching.FlowController.LimitExceededBehavior; +import com.google.api.gax.core.GaxProperties; import com.google.api.gax.core.GoogleCredentialsProvider; +import com.google.api.gax.grpc.GaxGrpcProperties; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.rpc.BatchingCallSettings; @@ -27,6 +29,7 @@ import com.google.api.gax.rpc.StatusCode.Code; import com.google.api.gax.rpc.StubSettings; import com.google.api.gax.rpc.UnaryCallSettings; +import com.google.api.gax.tracing.OpencensusTracerFactory; import com.google.cloud.bigtable.data.v2.internal.DummyBatchingDescriptor; import com.google.cloud.bigtable.data.v2.models.ConditionalRowMutation; import com.google.cloud.bigtable.data.v2.models.KeyOffset; @@ -36,8 +39,12 @@ import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; +import com.google.common.graph.ImmutableNetwork; import java.util.List; +import java.util.Map; import java.util.Set; import javax.annotation.Nonnull; import org.threeten.bp.Duration; @@ -269,6 +276,14 @@ private Builder() { setStreamWatchdogCheckInterval(baseDefaults.getStreamWatchdogCheckInterval()); setStreamWatchdogProvider(baseDefaults.getStreamWatchdogProvider()); + setTracerFactory(new OpencensusTracerFactory( + ImmutableMap.of( + "gax", GaxGrpcProperties.getGaxGrpcVersion(), + "grpc", GaxGrpcProperties.getGrpcVersion(), + "gapic", GaxProperties.getLibraryVersion(EnhancedBigtableStubSettings.class) + ) + )); + // Per-method settings using baseSettings for defaults. readRowsSettings = ServerStreamingCallSettings.newBuilder(); readRowsSettings diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/tracing/WrappedTracerFactory.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/tracing/WrappedTracerFactory.java new file mode 100644 index 000000000000..021a0afed6c7 --- /dev/null +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/tracing/WrappedTracerFactory.java @@ -0,0 +1,48 @@ +/* + * Copyright 2019 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.gaxx.tracing; + +import com.google.api.core.InternalApi; +import com.google.api.gax.tracing.ApiTracer; +import com.google.api.gax.tracing.ApiTracerFactory; +import com.google.api.gax.tracing.OpencensusTracerFactory; +import com.google.api.gax.tracing.SpanName; +import org.threeten.bp.Duration; + +/** + * Simple wrapper around {@link ApiTracerFactory} to augment the client name of the generated + * traces. + * + *

This is used to disambiguate traces in underlying GAPIC client from the manually written + * overlay. + */ +@InternalApi +public class WrappedTracerFactory implements ApiTracerFactory { + private final ApiTracerFactory innerFactory; + private final String clientName; + + public WrappedTracerFactory(ApiTracerFactory tracerFactory, String clientName) { + this.innerFactory = tracerFactory; + this.clientName = clientName; + } + + @Override + public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) { + spanName = SpanName.of(clientName, spanName.getMethodName()); + + return innerFactory.newTracer(parent, spanName, operationType); + } +} From 05e13b69da3701de180e00458b1bec46d069c9e7 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Thu, 14 Feb 2019 14:10:07 -0500 Subject: [PATCH 02/10] doc --- .../cloud/bigtable/gaxx/tracing/WrappedTracerFactory.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/tracing/WrappedTracerFactory.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/tracing/WrappedTracerFactory.java index 021a0afed6c7..bf113be9a9ac 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/tracing/WrappedTracerFactory.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/tracing/WrappedTracerFactory.java @@ -28,6 +28,8 @@ * *

This is used to disambiguate traces in underlying GAPIC client from the manually written * overlay. + * + *

For internal use, public for technical reasons. */ @InternalApi public class WrappedTracerFactory implements ApiTracerFactory { From 8579ac1bd646991f1f164d6ab6c747c3b43032b5 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Thu, 21 Feb 2019 17:04:13 -0500 Subject: [PATCH 03/10] format --- .../data/v2/stub/EnhancedBigtableStub.java | 70 +++++++++++-------- .../v2/stub/EnhancedBigtableStubSettings.java | 16 ++--- .../gaxx/tracing/WrappedTracerFactory.java | 2 - 3 files changed, 47 insertions(+), 41 deletions(-) diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 41041593ccda..a587fe2ed788 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -101,7 +101,8 @@ public static EnhancedBigtableStub create(EnhancedBigtableStubSettings settings) .setStreamWatchdogProvider(settings.getStreamWatchdogProvider()) .setStreamWatchdogCheckInterval(settings.getStreamWatchdogCheckInterval()) // Force the base stub to use a different TracerFactory - .setTracerFactory(new WrappedTracerFactory(settings.getTracerFactory(), TRACING_INNER_CLIENT_NAME)); + .setTracerFactory( + new WrappedTracerFactory(settings.getTracerFactory(), TRACING_INNER_CLIENT_NAME)); // ReadRow retries are handled in the overlay: disable retries in the base layer (but make // sure to preserve the exception callable settings). @@ -150,9 +151,7 @@ public static EnhancedBigtableStub create(EnhancedBigtableStubSettings settings) GrpcBigtableStub stub = new GrpcBigtableStub(baseSettings, clientContext); // Make sure to keep the original tracer factory for the outer client. - clientContext = clientContext.toBuilder() - .setTracerFactory(settings.getTracerFactory()) - .build(); + clientContext = clientContext.toBuilder().setTracerFactory(settings.getTracerFactory()).build(); return new EnhancedBigtableStub(settings, clientContext, stub); } @@ -263,9 +262,11 @@ private ServerStreamingCallable createReadRowsCallable( ReadRowsUserCallable userFacing = new ReadRowsUserCallable<>(filtering, requestContext); - TracedServerStreamingCallable traced = new TracedServerStreamingCallable<>( - userFacing, clientContext.getTracerFactory(), SpanName.of(TRACING_OUTER_CLIENT_NAME, "ReadRows") - ); + TracedServerStreamingCallable traced = + new TracedServerStreamingCallable<>( + userFacing, + clientContext.getTracerFactory(), + SpanName.of(TRACING_OUTER_CLIENT_NAME, "ReadRows")); return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -291,11 +292,11 @@ private UnaryCallable> createSampleRowKeysCallable() { UnaryCallable> userFacing = new SampleRowKeysCallable(retryable, requestContext); - UnaryCallable> traced = new TracedUnaryCallable<>( - userFacing, - clientContext.getTracerFactory(), - SpanName.of(TRACING_OUTER_CLIENT_NAME, "SampleRowKeys") - ); + UnaryCallable> traced = + new TracedUnaryCallable<>( + userFacing, + clientContext.getTracerFactory(), + SpanName.of(TRACING_OUTER_CLIENT_NAME, "SampleRowKeys")); return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -310,11 +311,11 @@ private UnaryCallable> createSampleRowKeysCallable() { private UnaryCallable createMutateRowCallable() { MutateRowCallable userFacing = new MutateRowCallable(stub.mutateRowCallable(), requestContext); - UnaryCallable traced = new TracedUnaryCallable<>( - userFacing, - clientContext.getTracerFactory(), - SpanName.of(TRACING_OUTER_CLIENT_NAME, "MutateRow") - ); + UnaryCallable traced = + new TracedUnaryCallable<>( + userFacing, + clientContext.getTracerFactory(), + SpanName.of(TRACING_OUTER_CLIENT_NAME, "MutateRow")); return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -335,11 +336,14 @@ private UnaryCallable createMutateRowCallable() { */ private UnaryCallable createBulkMutateRowsCallable() { UnaryCallable baseCallable = createMutateRowsBaseCallable(); - BulkMutateRowsUserFacingCallable userFacing = new BulkMutateRowsUserFacingCallable( - baseCallable, requestContext); + BulkMutateRowsUserFacingCallable userFacing = + new BulkMutateRowsUserFacingCallable(baseCallable, requestContext); - TracedUnaryCallable traced = new TracedUnaryCallable<>( - userFacing, clientContext.getTracerFactory(), SpanName.of(TRACING_OUTER_CLIENT_NAME, "BulkMutateRows")); + TracedUnaryCallable traced = + new TracedUnaryCallable<>( + userFacing, + clientContext.getTracerFactory(), + SpanName.of(TRACING_OUTER_CLIENT_NAME, "BulkMutateRows")); return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -368,9 +372,12 @@ private UnaryCallable createBulkMutateRowsBatchingCallable() BatchingCallSettings.newBuilder(new MutateRowsBatchingDescriptor()) .setBatchingSettings(settings.bulkMutateRowsSettings().getBatchingSettings()); - TracedBatchingCallable traced = new TracedBatchingCallable<>( - baseCallable, clientContext.getTracerFactory(), SpanName.of(TRACING_OUTER_CLIENT_NAME, "BulkMutateRows"), - batchingCallSettings.getBatchingDescriptor()); + TracedBatchingCallable traced = + new TracedBatchingCallable<>( + baseCallable, + clientContext.getTracerFactory(), + SpanName.of(TRACING_OUTER_CLIENT_NAME, "BulkMutateRows"), + batchingCallSettings.getBatchingDescriptor()); UnaryCallable batching = Callables.batching(traced, batchingCallSettings.build(), clientContext); @@ -415,8 +422,11 @@ private UnaryCallable createCheckAndMutateRowCa CheckAndMutateRowCallable userFacing = new CheckAndMutateRowCallable(stub.checkAndMutateRowCallable(), requestContext); - TracedUnaryCallable traced = new TracedUnaryCallable<>( - userFacing, clientContext.getTracerFactory(), SpanName.of(TRACING_OUTER_CLIENT_NAME, "CheckAndMutateRow")); + TracedUnaryCallable traced = + new TracedUnaryCallable<>( + userFacing, + clientContext.getTracerFactory(), + SpanName.of(TRACING_OUTER_CLIENT_NAME, "CheckAndMutateRow")); return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -434,9 +444,11 @@ private UnaryCallable createReadModifyWriteRowCallable( ReadModifyWriteRowCallable userFacing = new ReadModifyWriteRowCallable(stub.readModifyWriteRowCallable(), requestContext); - TracedUnaryCallable traced = new TracedUnaryCallable<>( - userFacing, clientContext.getTracerFactory(), - SpanName.of(TRACING_OUTER_CLIENT_NAME, "ReadModifyWriteRow")); + TracedUnaryCallable traced = + new TracedUnaryCallable<>( + userFacing, + clientContext.getTracerFactory(), + SpanName.of(TRACING_OUTER_CLIENT_NAME, "ReadModifyWriteRow")); return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java index 17499828f070..30eb2a938c58 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java @@ -41,10 +41,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Maps; -import com.google.common.graph.ImmutableNetwork; import java.util.List; -import java.util.Map; import java.util.Set; import javax.annotation.Nonnull; import org.threeten.bp.Duration; @@ -276,13 +273,12 @@ private Builder() { setStreamWatchdogCheckInterval(baseDefaults.getStreamWatchdogCheckInterval()); setStreamWatchdogProvider(baseDefaults.getStreamWatchdogProvider()); - setTracerFactory(new OpencensusTracerFactory( - ImmutableMap.of( - "gax", GaxGrpcProperties.getGaxGrpcVersion(), - "grpc", GaxGrpcProperties.getGrpcVersion(), - "gapic", GaxProperties.getLibraryVersion(EnhancedBigtableStubSettings.class) - ) - )); + setTracerFactory( + new OpencensusTracerFactory( + ImmutableMap.of( + "gax", GaxGrpcProperties.getGaxGrpcVersion(), + "grpc", GaxGrpcProperties.getGrpcVersion(), + "gapic", GaxProperties.getLibraryVersion(EnhancedBigtableStubSettings.class)))); // Per-method settings using baseSettings for defaults. readRowsSettings = ServerStreamingCallSettings.newBuilder(); diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/tracing/WrappedTracerFactory.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/tracing/WrappedTracerFactory.java index bf113be9a9ac..253d7a207ad9 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/tracing/WrappedTracerFactory.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/tracing/WrappedTracerFactory.java @@ -18,9 +18,7 @@ import com.google.api.core.InternalApi; import com.google.api.gax.tracing.ApiTracer; import com.google.api.gax.tracing.ApiTracerFactory; -import com.google.api.gax.tracing.OpencensusTracerFactory; import com.google.api.gax.tracing.SpanName; -import org.threeten.bp.Duration; /** * Simple wrapper around {@link ApiTracerFactory} to augment the client name of the generated From 740709652822e95bc8d83027d9035c60967e299a Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Thu, 21 Feb 2019 21:43:31 -0500 Subject: [PATCH 04/10] fix tracing for bulk mutations --- .../cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java | 4 ++-- .../v2/stub/mutaterows/MutateRowsAttemptCallable.java | 4 ++++ .../v2/stub/mutaterows/MutateRowsRetryingCallable.java | 8 ++++---- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 093978275090..2cc036e9837c 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -18,7 +18,7 @@ import com.google.api.core.InternalApi; import com.google.api.gax.retrying.ExponentialRetryAlgorithm; import com.google.api.gax.retrying.RetryAlgorithm; -import com.google.api.gax.retrying.RetryingExecutor; +import com.google.api.gax.retrying.RetryingExecutorWithContext; import com.google.api.gax.retrying.ScheduledRetryingExecutor; import com.google.api.gax.rpc.BatchingCallSettings; import com.google.api.gax.rpc.Callables; @@ -401,7 +401,7 @@ private UnaryCallable createMutateRowsBaseCallable() { new ApiResultRetryAlgorithm(), new ExponentialRetryAlgorithm( settings.bulkMutateRowsSettings().getRetrySettings(), clientContext.getClock())); - RetryingExecutor retryingExecutor = + RetryingExecutorWithContext retryingExecutor = new ScheduledRetryingExecutor<>(retryAlgorithm, clientContext.getExecutor()); return new MutateRowsRetryingCallable( diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java index 77f85f4b28fb..22266b6b81e6 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java @@ -179,6 +179,10 @@ public Void call() { return null; } + callContext + .getTracer() + .attemptStarted(externalFuture.getAttemptSettings().getOverallAttemptCount()); + // Make the actual call ApiFuture> innerFuture = innerCallable.futureCall(currentRequest, currentCallContext); diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsRetryingCallable.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsRetryingCallable.java index ab5b872af962..ff0daf78bb2d 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsRetryingCallable.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsRetryingCallable.java @@ -16,7 +16,7 @@ package com.google.cloud.bigtable.data.v2.stub.mutaterows; import com.google.api.core.InternalApi; -import com.google.api.gax.retrying.RetryingExecutor; +import com.google.api.gax.retrying.RetryingExecutorWithContext; import com.google.api.gax.retrying.RetryingFuture; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ServerStreamingCallable; @@ -42,13 +42,13 @@ public class MutateRowsRetryingCallable extends UnaryCallable { private final ApiCallContext callContextPrototype; private final ServerStreamingCallable callable; - private final RetryingExecutor executor; + private final RetryingExecutorWithContext executor; private final ImmutableSet retryCodes; public MutateRowsRetryingCallable( @Nonnull ApiCallContext callContextPrototype, @Nonnull ServerStreamingCallable callable, - @Nonnull RetryingExecutor executor, + @Nonnull RetryingExecutorWithContext executor, @Nonnull Set retryCodes) { this.callContextPrototype = Preconditions.checkNotNull(callContextPrototype); this.callable = Preconditions.checkNotNull(callable); @@ -62,7 +62,7 @@ public RetryingFuture futureCall(MutateRowsRequest request, ApiCallContext MutateRowsAttemptCallable retryCallable = new MutateRowsAttemptCallable(callable.all(), request, context, retryCodes); - RetryingFuture retryingFuture = executor.createFuture(retryCallable); + RetryingFuture retryingFuture = executor.createFuture(retryCallable, context); retryCallable.setExternalFuture(retryingFuture); retryCallable.call(); From 60d1e130fa79be07266759d05aee8b7cef1514b0 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Thu, 21 Feb 2019 22:21:09 -0500 Subject: [PATCH 05/10] Add instructions for tracing to readme --- .../google-cloud-bigtable/README.md | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/google-cloud-clients/google-cloud-bigtable/README.md b/google-cloud-clients/google-cloud-bigtable/README.md index deff3a9debe4..15b29d08b49b 100644 --- a/google-cloud-clients/google-cloud-bigtable/README.md +++ b/google-cloud-clients/google-cloud-bigtable/README.md @@ -130,6 +130,65 @@ try { } ``` +## Opencensus Tracing + +Cloud Bigtable client supports [Opencensus Tracing](https://opencensus.io/tracing/), which gives +insight into the client internals and aids in debugging production issues. By default, the +functionality is disabled. To enable, you need to add a couple of dependencies and configure an +exporter. For example to enable tracing using [Google Stackdriver](https://cloud.google.com/trace/docs/): + +[//]: # TODO: figure out how to keep opencensus version in sync with pom.xml + +If you are using Maven, add this to your pom.xml file +```xml + + io.opencensus + io.opencensus + 0.18.0 + + + io.opencensus + opencensus-exporter-trace-stackdriver + 0.18.0 + +``` +If you are using Gradle, add this to your dependencies +```Groovy +compile 'io.opencensus:opencensus-impl:0.18.0' +compile 'io.opencensus:opencensus-exporter-trace-stackdriver:0.18.0' +``` +If you are using SBT, add this to your dependencies +```Scala +libraryDependencies += "io.opencensus" % "opencensus-impl" % "0.18.0" +libraryDependencies += "io.opencensus" % "opencensus-exporter-trace-stackdriver" % "0.18.0" +``` + +Then at the start of your application configure the exporter: + +```java +import io.opencensus.exporter.trace.stackdriver.StackdriverTraceConfiguration; +import io.opencensus.exporter.trace.stackdriver.StackdriverTraceExporter; + +StackdriverTraceExporter.createAndRegister( + StackdriverTraceConfiguration.builder() + .setProjectId("YOUR-PROJECT_ID") + .build()); +``` + +By default traces are [sampled](https://opencensus.io/tracing/sampling) at a rate of about 1/10,000. +You can configure a higher rate by updating the active tracing params: + +```java +import io.opencensus.trace.Tracing; +import io.opencensus.trace.samplers.Samplers; + +Tracing.getTraceConfig().updateActiveTraceParams( + Tracing.getTraceConfig().getActiveTraceParams().toBuilder() + .setSampler(Samplers.probabilitySampler(0.01)) + .build() +); +``` + ## Troubleshooting To get help, follow the instructions in the [shared Troubleshooting From ed5e5a8bbbdc90655eeae9ae68940b76bf2933bc Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Thu, 21 Feb 2019 22:22:45 -0500 Subject: [PATCH 06/10] format --- google-cloud-clients/google-cloud-bigtable/README.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/google-cloud-clients/google-cloud-bigtable/README.md b/google-cloud-clients/google-cloud-bigtable/README.md index 15b29d08b49b..12661804098f 100644 --- a/google-cloud-clients/google-cloud-bigtable/README.md +++ b/google-cloud-clients/google-cloud-bigtable/README.md @@ -132,10 +132,11 @@ try { ## Opencensus Tracing -Cloud Bigtable client supports [Opencensus Tracing](https://opencensus.io/tracing/), which gives -insight into the client internals and aids in debugging production issues. By default, the -functionality is disabled. To enable, you need to add a couple of dependencies and configure an -exporter. For example to enable tracing using [Google Stackdriver](https://cloud.google.com/trace/docs/): +Cloud Bigtable client supports [Opencensus Tracing](https://opencensus.io/tracing/), +which gives insight into the client internals and aids in debugging production issues. +By default, the functionality is disabled. To enable, you need to add a couple of +dependencies and configure an exporter. For example to enable tracing using +[Google Stackdriver](https://cloud.google.com/trace/docs/): [//]: # TODO: figure out how to keep opencensus version in sync with pom.xml From b485322ab3da94a557b16e59059bcf80c82ee2bb Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Thu, 21 Feb 2019 22:30:28 -0500 Subject: [PATCH 07/10] fix todo comment --- google-cloud-clients/google-cloud-bigtable/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-clients/google-cloud-bigtable/README.md b/google-cloud-clients/google-cloud-bigtable/README.md index 06099a2b7993..f55ea3677ac7 100644 --- a/google-cloud-clients/google-cloud-bigtable/README.md +++ b/google-cloud-clients/google-cloud-bigtable/README.md @@ -138,7 +138,7 @@ By default, the functionality is disabled. To enable, you need to add a couple o dependencies and configure an exporter. For example to enable tracing using [Google Stackdriver](https://cloud.google.com/trace/docs/): -[//]: # TODO: figure out how to keep opencensus version in sync with pom.xml +[//]: # (TODO: figure out how to keep opencensus version in sync with pom.xml) If you are using Maven, add this to your pom.xml file ```xml From 7983fd42ed1f754f1dd18bdfcc2abe2e015c4551 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Mon, 25 Feb 2019 14:10:42 -0500 Subject: [PATCH 08/10] typo in readme --- google-cloud-clients/google-cloud-bigtable/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-clients/google-cloud-bigtable/README.md b/google-cloud-clients/google-cloud-bigtable/README.md index f55ea3677ac7..2aecd81f1d6e 100644 --- a/google-cloud-clients/google-cloud-bigtable/README.md +++ b/google-cloud-clients/google-cloud-bigtable/README.md @@ -144,7 +144,7 @@ If you are using Maven, add this to your pom.xml file ```xml io.opencensus - io.opencensus + opencensus-impl 0.18.0 From c48e4e19b161f2a74c0ca0eeb0a404f9649a85b5 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Mon, 25 Feb 2019 17:31:09 -0500 Subject: [PATCH 09/10] address feedback --- .../data/v2/stub/EnhancedBigtableStub.java | 98 +++++++++---------- 1 file changed, 46 insertions(+), 52 deletions(-) diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 2cc036e9837c..58153d06a068 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -261,15 +261,8 @@ private ServerStreamingCallable createReadRowsCallable( FilterMarkerRowsCallable filtering = new FilterMarkerRowsCallable<>(retrying2, rowAdapter); - ReadRowsUserCallable userFacing = new ReadRowsUserCallable<>(filtering, requestContext); - - TracedServerStreamingCallable traced = - new TracedServerStreamingCallable<>( - userFacing, - clientContext.getTracerFactory(), - SpanName.of(TRACING_OUTER_CLIENT_NAME, "ReadRows")); - - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return createUserFacingServerStreamingCallable( + "ReadRows", new ReadRowsUserCallable<>(filtering, requestContext)); } /** @@ -290,16 +283,8 @@ private UnaryCallable> createSampleRowKeysCallable() { UnaryCallable> retryable = Callables.retrying(spoolable, settings.sampleRowKeysSettings(), clientContext); - UnaryCallable> userFacing = - new SampleRowKeysCallable(retryable, requestContext); - - UnaryCallable> traced = - new TracedUnaryCallable<>( - userFacing, - clientContext.getTracerFactory(), - SpanName.of(TRACING_OUTER_CLIENT_NAME, "SampleRowKeys")); - - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return createUserFacingUnaryCallable( + "SampleRowKeys", new SampleRowKeysCallable(retryable, requestContext)); } /** @@ -310,15 +295,8 @@ private UnaryCallable> createSampleRowKeysCallable() { * */ private UnaryCallable createMutateRowCallable() { - MutateRowCallable userFacing = new MutateRowCallable(stub.mutateRowCallable(), requestContext); - - UnaryCallable traced = - new TracedUnaryCallable<>( - userFacing, - clientContext.getTracerFactory(), - SpanName.of(TRACING_OUTER_CLIENT_NAME, "MutateRow")); - - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return createUserFacingUnaryCallable( + "MutateRow", new MutateRowCallable(stub.mutateRowCallable(), requestContext)); } /** @@ -337,16 +315,11 @@ private UnaryCallable createMutateRowCallable() { */ private UnaryCallable createBulkMutateRowsCallable() { UnaryCallable baseCallable = createMutateRowsBaseCallable(); - BulkMutateRowsUserFacingCallable userFacing = - new BulkMutateRowsUserFacingCallable(baseCallable, requestContext); - TracedUnaryCallable traced = - new TracedUnaryCallable<>( - userFacing, - clientContext.getTracerFactory(), - SpanName.of(TRACING_OUTER_CLIENT_NAME, "BulkMutateRows")); - - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return createUserFacingUnaryCallable( + "BulkMutateRows", + new BulkMutateRowsUserFacingCallable(baseCallable, requestContext) + ); } /** @@ -373,6 +346,8 @@ private UnaryCallable createBulkMutateRowsBatchingCallable() BatchingCallSettings.newBuilder(new MutateRowsBatchingDescriptor()) .setBatchingSettings(settings.bulkMutateRowsSettings().getBatchingSettings()); + // This is a special case, the tracing starts after the batching, so we can't use + // createUserFacingUnaryCallable TracedBatchingCallable traced = new TracedBatchingCallable<>( baseCallable, @@ -420,16 +395,9 @@ private UnaryCallable createMutateRowsBaseCallable() { * */ private UnaryCallable createCheckAndMutateRowCallable() { - CheckAndMutateRowCallable userFacing = - new CheckAndMutateRowCallable(stub.checkAndMutateRowCallable(), requestContext); - - TracedUnaryCallable traced = - new TracedUnaryCallable<>( - userFacing, - clientContext.getTracerFactory(), - SpanName.of(TRACING_OUTER_CLIENT_NAME, "CheckAndMutateRow")); - - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return createUserFacingUnaryCallable( + "CheckAndMutateRow", + new CheckAndMutateRowCallable(stub.checkAndMutateRowCallable(), requestContext)); } /** @@ -442,14 +410,40 @@ private UnaryCallable createCheckAndMutateRowCa * */ private UnaryCallable createReadModifyWriteRowCallable() { - ReadModifyWriteRowCallable userFacing = - new ReadModifyWriteRowCallable(stub.readModifyWriteRowCallable(), requestContext); + return createUserFacingUnaryCallable( + "ReadModifyWriteRow", + new ReadModifyWriteRowCallable(stub.readModifyWriteRowCallable(), requestContext)); + } + + /** + * Wraps a callable chain in a user presentable callable that will inject the default call context + * and trace the call. + */ + private UnaryCallable createUserFacingUnaryCallable( + String methodName, UnaryCallable inner) { - TracedUnaryCallable traced = + UnaryCallable traced = new TracedUnaryCallable<>( - userFacing, + inner, + clientContext.getTracerFactory(), + SpanName.of(TRACING_OUTER_CLIENT_NAME, methodName)); + + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + } + + /** + * Wraps a callable chain in a user presentable callable that will inject the default call context + * and trace the call. + */ + private + ServerStreamingCallable createUserFacingServerStreamingCallable( + String methodName, ServerStreamingCallable inner) { + + ServerStreamingCallable traced = + new TracedServerStreamingCallable<>( + inner, clientContext.getTracerFactory(), - SpanName.of(TRACING_OUTER_CLIENT_NAME, "ReadModifyWriteRow")); + SpanName.of(TRACING_OUTER_CLIENT_NAME, methodName)); return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } From 8c1a89c0f82f99473fea4bf4735b98501f77b1b3 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Mon, 25 Feb 2019 18:11:40 -0500 Subject: [PATCH 10/10] format --- .../cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 58153d06a068..668632fb38c6 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -317,9 +317,7 @@ private UnaryCallable createBulkMutateRowsCallable() { UnaryCallable baseCallable = createMutateRowsBaseCallable(); return createUserFacingUnaryCallable( - "BulkMutateRows", - new BulkMutateRowsUserFacingCallable(baseCallable, requestContext) - ); + "BulkMutateRows", new BulkMutateRowsUserFacingCallable(baseCallable, requestContext)); } /**