From 62a20dfdd223b8763ce358e3c5bb2feb672d2551 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 3 Nov 2020 12:46:24 -0500 Subject: [PATCH 01/12] Extract server-timing trailer and create metrics for gfe latency --- .../data/v2/BigtableDataSettings.java | 2 + .../v2/stub/BigtableInterceptorProvider.java | 44 +++ .../v2/stub/CheckAndMutateRowCallable.java | 1 - .../data/v2/stub/ClientHeaderInterceptor.java | 96 ++++++ .../data/v2/stub/EnhancedBigtableStub.java | 46 ++- .../v2/stub/EnhancedBigtableStubSettings.java | 35 ++- .../data/v2/stub/metrics/HeaderTracer.java | 85 ++++++ .../v2/stub/metrics/RpcMeasureConstants.java | 17 +- .../v2/stub/metrics/RpcViewConstants.java | 23 ++ .../data/v2/stub/metrics/RpcViews.java | 4 +- .../v2/BigtableDataClientFactoryTest.java | 3 +- .../EnhancedBigtableStubSettingsTest.java | 1 + .../data/v2/stub/metrics/GFEMetricsTest.java | 288 ++++++++++++++++++ .../v2/stub/metrics/MetricsTracerTest.java | 121 ++------ .../data/v2/stub/metrics/StatsTestUtils.java | 79 ++++- 15 files changed, 732 insertions(+), 113 deletions(-) create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableInterceptorProvider.java create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ClientHeaderInterceptor.java create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracer.java create mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/GFEMetricsTest.java diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java index 105fbaae45..728ac0d6e3 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java @@ -23,6 +23,7 @@ import com.google.api.gax.rpc.UnaryCallSettings; import com.google.cloud.bigtable.data.v2.models.Query; import com.google.cloud.bigtable.data.v2.models.Row; +import com.google.cloud.bigtable.data.v2.stub.BigtableInterceptorProvider; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; import com.google.common.base.MoreObjects; import com.google.common.base.Strings; @@ -128,6 +129,7 @@ public ManagedChannelBuilder apply(ManagedChannelBuilder input) { .setKeepAliveTimeout( Duration.ofSeconds(10)) // wait this long before considering the connection dead .setKeepAliveWithoutCalls(true) // sends ping without active streams + .setInterceptorProvider(BigtableInterceptorProvider.createDefault()) .build()); LOGGER.info("Connecting to the Bigtable emulator at " + hostname + ":" + port); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableInterceptorProvider.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableInterceptorProvider.java new file mode 100644 index 0000000000..b889f2a28c --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableInterceptorProvider.java @@ -0,0 +1,44 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub; + +import com.google.api.gax.grpc.GrpcInterceptorProvider; +import com.google.common.collect.ImmutableList; +import io.grpc.ClientInterceptor; +import java.util.List; + +public class BigtableInterceptorProvider implements GrpcInterceptorProvider { + + private static final ClientHeaderInterceptor headerInterceptor = new ClientHeaderInterceptor(); + private List clientInterceptors; + + private BigtableInterceptorProvider(List interceptors) { + this.clientInterceptors = interceptors; + } + + public static BigtableInterceptorProvider createDefault() { + return new BigtableInterceptorProvider(ImmutableList.of(headerInterceptor)); + } + + public static BigtableInterceptorProvider create(List interceptors) { + return new BigtableInterceptorProvider(interceptors); + } + + @Override + public List getInterceptors() { + return clientInterceptors; + } +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CheckAndMutateRowCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CheckAndMutateRowCallable.java index 549e10f44b..b23abaebfe 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CheckAndMutateRowCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CheckAndMutateRowCallable.java @@ -42,7 +42,6 @@ class CheckAndMutateRowCallable extends UnaryCallable futureCall(ConditionalRowMutation request, ApiCallContext context) { ApiFuture rawResponse = inner.futureCall(request.toProto(requestContext), context); - return ApiFutures.transform( rawResponse, new ApiFunction() { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ClientHeaderInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ClientHeaderInterceptor.java new file mode 100644 index 0000000000..c27a83f8f6 --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ClientHeaderInterceptor.java @@ -0,0 +1,96 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub; + +import com.google.api.gax.tracing.SpanName; +import com.google.cloud.bigtable.data.v2.stub.metrics.HeaderTracer; +import com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants; +import io.grpc.CallOptions; +import io.grpc.Channel; +import io.grpc.ClientCall; +import io.grpc.ClientInterceptor; +import io.grpc.ForwardingClientCall.SimpleForwardingClientCall; +import io.grpc.ForwardingClientCallListener.SimpleForwardingClientCallListener; +import io.grpc.Metadata; +import io.grpc.MethodDescriptor; +import java.util.logging.Logger; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class ClientHeaderInterceptor implements ClientInterceptor { + public static final Metadata.Key SERVER_TIMING_HEADER_KEY = + Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER); + + private static final Logger LOGGER = Logger.getLogger(ClientHeaderInterceptor.class.getName()); + private static final String CLIENT_NAME = "Bigtable"; + private static final Pattern GFE_HEADER_PATTERN = Pattern.compile(".*dur=(?\\d+)"); + private static final Pattern METHOD_DESCRIPTOR_PATTERN = Pattern.compile(".*/(?[a-zA-Z]+)"); + + @Override + public ClientCall interceptCall( + final MethodDescriptor method, final CallOptions callOptions, Channel channel) { + final ClientCall clientCall = channel.newCall(method, callOptions); + final SpanName spanName = processMethodName(method.getFullMethodName()); + final HeaderTracer tracer = callOptions.getOption(HeaderTracer.HEADER_TRACER_CONTEXT_KEY); + return new SimpleForwardingClientCall(clientCall) { + @Override + public void start(Listener responseListener, Metadata headers) { + super.start( + new SimpleForwardingClientCallListener(responseListener) { + @Override + public void onHeaders(Metadata headers) { + processHeader(headers, tracer, spanName); + super.onHeaders(headers); + } + }, + headers); + } + }; + } + + private SpanName processMethodName(String method) { + Matcher matcher = METHOD_DESCRIPTOR_PATTERN.matcher(method); + if (matcher.find()) { + return SpanName.of(CLIENT_NAME, matcher.group("op")); + } + LOGGER.warning( + String.format("Failed to get bigtable op name. Received method descriptor: %s.", method)); + return null; + } + + private void processHeader(Metadata headers, HeaderTracer tracer, SpanName span) { + if (tracer == null) { + LOGGER.warning("Couldn't find HeaderTracer in call options. Skip extracting gfe metrics"); + return; + } + if (headers.get(SERVER_TIMING_HEADER_KEY) != null) { + String serverTiming = headers.get(SERVER_TIMING_HEADER_KEY); + Matcher matcher = GFE_HEADER_PATTERN.matcher(serverTiming); + tracer.recordHeader(RpcMeasureConstants.BIGTABLE_GFE_MISSING_COUNT, 0, span); + if (matcher.find()) { + long latency = Long.valueOf(matcher.group("dur")); + tracer.recordHeader(RpcMeasureConstants.BIGTABLE_GFE_LATENCY, latency, span); + } else { + LOGGER.warning( + String.format( + "Received invalid %s header: %s, failed to add to metrics.", + SERVER_TIMING_HEADER_KEY.name(), serverTiming)); + } + } else { + tracer.recordHeader(RpcMeasureConstants.BIGTABLE_GFE_MISSING_COUNT, 1l, span); + } + } +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index d729d6244d..650e86830b 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -23,6 +23,7 @@ import com.google.api.gax.core.FixedCredentialsProvider; import com.google.api.gax.core.GaxProperties; import com.google.api.gax.grpc.GaxGrpcProperties; +import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.GrpcCallSettings; import com.google.api.gax.grpc.GrpcRawCallableFactory; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; @@ -30,6 +31,7 @@ import com.google.api.gax.retrying.RetryAlgorithm; import com.google.api.gax.retrying.RetryingExecutorWithContext; import com.google.api.gax.retrying.ScheduledRetryingExecutor; +import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.Callables; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.RequestParamsExtractor; @@ -65,9 +67,7 @@ import com.google.cloud.bigtable.data.v2.models.RowAdapter; import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.cloud.bigtable.data.v2.models.RowMutationEntry; -import com.google.cloud.bigtable.data.v2.stub.metrics.CompositeTracerFactory; -import com.google.cloud.bigtable.data.v2.stub.metrics.MetricsTracerFactory; -import com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants; +import com.google.cloud.bigtable.data.v2.stub.metrics.*; import com.google.cloud.bigtable.data.v2.stub.mutaterows.BulkMutateRowsUserFacingCallable; import com.google.cloud.bigtable.data.v2.stub.mutaterows.MutateRowsBatchingDescriptor; import com.google.cloud.bigtable.data.v2.stub.mutaterows.MutateRowsRetryingCallable; @@ -82,6 +82,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.protobuf.ByteString; +import io.grpc.CallOptions; import io.opencensus.stats.Stats; import io.opencensus.stats.StatsRecorder; import io.opencensus.tags.TagKey; @@ -91,6 +92,7 @@ import java.io.IOException; import java.util.List; import java.util.Map; +import java.util.logging.Logger; import javax.annotation.Nonnull; /** @@ -109,6 +111,8 @@ public class EnhancedBigtableStub implements AutoCloseable { private static final String CLIENT_NAME = "Bigtable"; + private static final Logger LOGGER = Logger.getLogger(EnhancedBigtableStub.class.getName()); + private final EnhancedBigtableStubSettings settings; private final ClientContext clientContext; private final RequestContext requestContext; @@ -203,13 +207,26 @@ public static EnhancedBigtableStubSettings finalizeSettings( .build()), // Add user configured tracer settings.getTracerFactory()))); - + HeaderTracer headerTracer = builder.getHeaderTracer(); + headerTracer.setStats(stats); + headerTracer.setTagger(tagger); + headerTracer.setStatsAttributes( + ImmutableMap.builder() + .put(RpcMeasureConstants.BIGTABLE_PROJECT_ID, TagValue.create(settings.getProjectId())) + .put( + RpcMeasureConstants.BIGTABLE_INSTANCE_ID, TagValue.create(settings.getInstanceId())) + .put( + RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID, + TagValue.create(settings.getAppProfileId())) + .build()); + builder.setHeaderTracer(headerTracer); return builder.build(); } public EnhancedBigtableStub(EnhancedBigtableStubSettings settings, ClientContext clientContext) { this.settings = settings; this.clientContext = clientContext; + this.requestContext = RequestContext.create( settings.getProjectId(), settings.getInstanceId(), settings.getAppProfileId()); @@ -274,7 +291,7 @@ public ServerStreamingCallable createReadRowsCallable( clientContext.getTracerFactory(), SpanName.of(CLIENT_NAME, "ReadRows")); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext(getContextWithTracer()); } /** @@ -463,7 +480,7 @@ private UnaryCallable createBulkMutateRowsCallable() { new TracedUnaryCallable<>( userFacing, clientContext.getTracerFactory(), SpanName.of(CLIENT_NAME, "MutateRows")); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext(getContextWithTracer()); } /** @@ -638,7 +655,7 @@ private UnaryCallable createUserFacin new TracedUnaryCallable<>( inner, clientContext.getTracerFactory(), SpanName.of(CLIENT_NAME, methodName)); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext(getContextWithTracer()); } // @@ -686,6 +703,21 @@ public UnaryCallable readModifyWriteRowCallable() { } // + private GrpcCallContext getContextWithTracer() { + ApiCallContext apiCallContext = clientContext.getDefaultCallContext(); + if (!(apiCallContext instanceof GrpcCallContext)) { + LOGGER.warning( + "Failed to inject tracer in call context. Expected GrpcCallContext but had " + + apiCallContext.getClass()); + } + GrpcCallContext grpcCallContext = (GrpcCallContext) apiCallContext; + CallOptions options = grpcCallContext.getCallOptions(); + options = + options.withOption(HeaderTracer.HEADER_TRACER_CONTEXT_KEY, settings.getHeaderTracer()); + grpcCallContext = grpcCallContext.withCallOptions(options); + return grpcCallContext; + } + @Override public void close() { for (BackgroundResource backgroundResource : clientContext.getBackgroundResources()) { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java index 0ce05d3a52..301a604b10 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java @@ -24,12 +24,8 @@ import com.google.api.gax.core.GoogleCredentialsProvider; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.api.gax.retrying.RetrySettings; -import com.google.api.gax.rpc.FixedHeaderProvider; -import com.google.api.gax.rpc.ServerStreamingCallSettings; +import com.google.api.gax.rpc.*; import com.google.api.gax.rpc.StatusCode.Code; -import com.google.api.gax.rpc.StubSettings; -import com.google.api.gax.rpc.TransportChannelProvider; -import com.google.api.gax.rpc.UnaryCallSettings; import com.google.auth.Credentials; import com.google.cloud.bigtable.Version; import com.google.cloud.bigtable.data.v2.models.ConditionalRowMutation; @@ -38,6 +34,7 @@ import com.google.cloud.bigtable.data.v2.models.ReadModifyWriteRow; import com.google.cloud.bigtable.data.v2.models.Row; import com.google.cloud.bigtable.data.v2.models.RowMutation; +import com.google.cloud.bigtable.data.v2.stub.metrics.*; import com.google.cloud.bigtable.data.v2.stub.mutaterows.MutateRowsBatchingDescriptor; import com.google.cloud.bigtable.data.v2.stub.readrows.ReadRowsBatchingDescriptor; import com.google.common.base.MoreObjects; @@ -157,6 +154,7 @@ public class EnhancedBigtableStubSettings extends StubSettings primedTableIds; + private HeaderTracer headerTracer; private final ServerStreamingCallSettings readRowsSettings; private final UnaryCallSettings readRowSettings; @@ -196,6 +194,7 @@ private EnhancedBigtableStubSettings(Builder builder) { appProfileId = builder.appProfileId; isRefreshingChannel = builder.isRefreshingChannel; primedTableIds = builder.primedTableIds; + headerTracer = builder.headerTracer; // Per method settings. readRowsSettings = builder.readRowsSettings.build(); @@ -240,6 +239,12 @@ public List getPrimedTableIds() { return primedTableIds; } + /** Gets the tracer for capturing metrics in the header. */ + @BetaApi("This API is not currently stable and might change in the future") + public HeaderTracer getHeaderTracer() { + return headerTracer; + } + /** Returns a builder for the default ChannelProvider for this service. */ public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProviderBuilder() { return BigtableStubSettings.defaultGrpcTransportProviderBuilder() @@ -249,7 +254,10 @@ public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProvi .setKeepAliveTimeout( Duration.ofSeconds(10)) // wait this long before considering the connection dead .setKeepAliveWithoutCalls(true) // sends ping without active streams + .setInterceptorProvider(BigtableInterceptorProvider.createDefault()) // TODO(weiranf): Set this to true by default once DirectPath goes to public beta + // Attempts direct access to CBT service over gRPC to improve throughput, + // whether the attempt is allowed is totally controlled by service owner. .setAttemptDirectPath(isDirectPathEnabled()); } @@ -502,6 +510,7 @@ public static class Builder extends StubSettings.Builder primedTableIds; + private HeaderTracer headerTracer; private final ServerStreamingCallSettings.Builder readRowsSettings; private final UnaryCallSettings.Builder readRowSettings; @@ -525,6 +534,7 @@ private Builder() { this.appProfileId = SERVER_DEFAULT_APP_PROFILE_ID; this.isRefreshingChannel = false; primedTableIds = ImmutableList.of(); + headerTracer = new HeaderTracer(); setCredentialsProvider(defaultCredentialsProviderBuilder().build()); // Defaults provider @@ -638,6 +648,7 @@ private Builder(EnhancedBigtableStubSettings settings) { appProfileId = settings.appProfileId; isRefreshingChannel = settings.isRefreshingChannel; primedTableIds = settings.primedTableIds; + headerTracer = settings.headerTracer; // Per method settings. readRowsSettings = settings.readRowsSettings.toBuilder(); @@ -760,6 +771,19 @@ public List getPrimedTableIds() { return primedTableIds; } + // TODO: update comments + /** Configure the header tracer */ + @BetaApi("") + public Builder setHeaderTracer(HeaderTracer tracer) { + this.headerTracer = headerTracer; + return this; + } + + // TODO: update comments + public HeaderTracer getHeaderTracer() { + return headerTracer; + } + /** Returns the builder for the settings used for calls to readRows. */ public ServerStreamingCallSettings.Builder readRowsSettings() { return readRowsSettings; @@ -839,6 +863,7 @@ public String toString() { .add("appProfileId", appProfileId) .add("isRefreshingChannel", isRefreshingChannel) .add("primedTableIds", primedTableIds) + .add("headerTracer", headerTracer) .add("readRowsSettings", readRowsSettings) .add("readRowSettings", readRowSettings) .add("sampleRowKeysSettings", sampleRowKeysSettings) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracer.java new file mode 100644 index 0000000000..9101bfd764 --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracer.java @@ -0,0 +1,85 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub.metrics; + +import com.google.api.gax.tracing.SpanName; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableMap; +import io.grpc.CallOptions; +import io.opencensus.stats.Measure.MeasureDouble; +import io.opencensus.stats.Measure.MeasureLong; +import io.opencensus.stats.MeasureMap; +import io.opencensus.stats.Stats; +import io.opencensus.stats.StatsRecorder; +import io.opencensus.tags.TagContextBuilder; +import io.opencensus.tags.TagKey; +import io.opencensus.tags.TagValue; +import io.opencensus.tags.Tagger; +import io.opencensus.tags.Tags; +import java.util.Map; +import javax.annotation.Nullable; + +public class HeaderTracer { + public static final CallOptions.Key HEADER_TRACER_CONTEXT_KEY = + CallOptions.Key.create("BigtableHeaderTracer"); + + private Tagger tagger; + private StatsRecorder stats; + private Map statsAttributes; + + public HeaderTracer() { + this.tagger = Tags.getTagger(); + this.stats = Stats.getStatsRecorder(); + this.statsAttributes = ImmutableMap.of(); + } + + public void setTagger(Tagger tagger) { + this.tagger = tagger; + } + + public void setStats(StatsRecorder stats) { + this.stats = stats; + } + + public void setStatsAttributes(Map statsAttributes) { + this.statsAttributes = statsAttributes; + } + + public void recordHeader(MeasureLong measure, long value, @Nullable SpanName spanName) { + Preconditions.checkNotNull(measure, "Measure cannot be null."); + MeasureMap measures = stats.newMeasureMap().put(measure, value); + measures.record(newTagCtxBuilder(spanName).build()); + } + + public void recordHeader(MeasureDouble measure, double value, @Nullable SpanName spanName) { + Preconditions.checkNotNull(measure, "Measure cannot be null."); + MeasureMap measures = stats.newMeasureMap().put(measure, value); + measures.record(newTagCtxBuilder(spanName).build()); + } + + private TagContextBuilder newTagCtxBuilder(@Nullable SpanName spanName) { + TagContextBuilder tagContextBuilder = tagger.currentBuilder(); + if (spanName != null) { + tagContextBuilder.putLocal( + RpcMeasureConstants.BIGTABLE_OP, TagValue.create(spanName.toString())); + } + // Copy client level tags in + for (Map.Entry entry : statsAttributes.entrySet()) { + tagContextBuilder.putLocal(entry.getKey(), entry.getValue()); + } + return tagContextBuilder; + } +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java index 8c6e347a0f..75cca5bddf 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java @@ -16,6 +16,7 @@ package com.google.cloud.bigtable.data.v2.stub.metrics; import com.google.api.core.InternalApi; +import io.opencensus.stats.Measure; import io.opencensus.stats.Measure.MeasureLong; import io.opencensus.tags.TagKey; @@ -27,7 +28,7 @@ public class RpcMeasureConstants { public static final TagKey BIGTABLE_APP_PROFILE_ID = TagKey.create("bigtable_app_profile_id"); /** Tag key that represents a Bigtable operation name. */ - static final TagKey BIGTABLE_OP = TagKey.create("bigtable_op"); + public static final TagKey BIGTABLE_OP = TagKey.create("bigtable_op"); /** Tag key that represents the final status of the Bigtable operation. */ static final TagKey BIGTABLE_STATUS = TagKey.create("bigtable_status"); @@ -74,4 +75,18 @@ public class RpcMeasureConstants { "cloud.google.com/java/bigtable/read_rows_first_row_latency", "Time between request being sent to the first row received", MILLISECOND); + + /** GFE t4t7 latency extracted from server-timing trailer. */ + public static final MeasureLong BIGTABLE_GFE_LATENCY = + MeasureLong.create( + "cloud.google.com/java/bigtable/gfe_latency", + "GFE t4t7 latency. Time between GFE receives the first byte of the request and GFE reads the first byte of the response", + MILLISECOND); + + /** Number of responses without server-timing trailer. */ + public static final MeasureLong BIGTABLE_GFE_MISSING_COUNT = + Measure.MeasureLong.create( + "cloud.google.com/java/bigtable/gfe_missing_count", + "Number of responses without the server-timing trailer", + COUNT); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java index d21060c4ac..af72d2f492 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java @@ -17,6 +17,8 @@ import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID; import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_ATTEMPT_LATENCY; +import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_GFE_LATENCY; +import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_GFE_MISSING_COUNT; import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_INSTANCE_ID; import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_OP; import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_OP_ATTEMPT_COUNT; @@ -29,13 +31,16 @@ import io.opencensus.stats.Aggregation; import io.opencensus.stats.Aggregation.Count; import io.opencensus.stats.Aggregation.Distribution; +import io.opencensus.stats.Aggregation.Sum; import io.opencensus.stats.BucketBoundaries; import io.opencensus.stats.View; +import io.opencensus.tags.TagKey; import java.util.Arrays; class RpcViewConstants { // Aggregations private static final Aggregation COUNT = Count.create(); + private static final Aggregation SUM = Sum.create(); private static final Aggregation AGGREGATION_WITH_MILLIS_HISTOGRAM = Distribution.create( @@ -124,4 +129,22 @@ class RpcViewConstants { BIGTABLE_APP_PROFILE_ID, BIGTABLE_OP, BIGTABLE_STATUS)); + + static final View BIGTABLE_GFE_LATENCY_VIEW = + View.create( + View.Name.create("cloud.google.com/java/bigtable/gfe_latency"), + "GFE t4t7 latency in msecs", + BIGTABLE_GFE_LATENCY, + AGGREGATION_WITH_MILLIS_HISTOGRAM, + ImmutableList.of( + BIGTABLE_INSTANCE_ID, BIGTABLE_PROJECT_ID, BIGTABLE_APP_PROFILE_ID, BIGTABLE_OP)); + + static final View BIGTABLE_GFE_MISSING_COUNT_VIEW = + View.create( + View.Name.create("cloud.google.com/java/bigtable/gfe_missing_count"), + "Number of responses without the server-timing trailer", + BIGTABLE_GFE_MISSING_COUNT, + SUM, + ImmutableList.of( + BIGTABLE_INSTANCE_ID, BIGTABLE_PROJECT_ID, BIGTABLE_APP_PROFILE_ID, BIGTABLE_OP)); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViews.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViews.java index cc31539496..fc8f47b742 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViews.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViews.java @@ -31,7 +31,9 @@ public class RpcViews { RpcViewConstants.BIGTABLE_COMPLETED_OP_VIEW, RpcViewConstants.BIGTABLE_READ_ROWS_FIRST_ROW_LATENCY_VIEW, RpcViewConstants.BIGTABLE_ATTEMPT_LATENCY_VIEW, - RpcViewConstants.BIGTABLE_ATTEMPTS_PER_OP_VIEW); + RpcViewConstants.BIGTABLE_ATTEMPTS_PER_OP_VIEW, + RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW, + RpcViewConstants.BIGTABLE_GFE_MISSING_COUNT_VIEW); /** Registers all Bigtable specific views. */ public static void registerBigtableClientViews() { diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java index 25c341d650..2d6a4998ba 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java @@ -35,8 +35,7 @@ import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.common.base.Preconditions; import com.google.protobuf.ByteString; -import io.grpc.Attributes; -import io.grpc.ServerTransportFilter; +import io.grpc.*; import io.grpc.stub.StreamObserver; import java.io.IOException; import java.lang.reflect.Method; diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java index 2cd55a311c..4c8358e15c 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java @@ -628,6 +628,7 @@ public void isRefreshingChannelFalseValueTest() { "appProfileId", "isRefreshingChannel", "primedTableIds", + "headerTracer", "readRowsSettings", "readRowSettings", "sampleRowKeysSettings", diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/GFEMetricsTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/GFEMetricsTest.java new file mode 100644 index 0000000000..2daf6f4e75 --- /dev/null +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/GFEMetricsTest.java @@ -0,0 +1,288 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub.metrics; + +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doAnswer; + +import com.google.api.gax.rpc.ClientContext; +import com.google.bigtable.v2.BigtableGrpc; +import com.google.bigtable.v2.MutateRowRequest; +import com.google.bigtable.v2.MutateRowResponse; +import com.google.bigtable.v2.ReadRowsRequest; +import com.google.bigtable.v2.ReadRowsResponse; +import com.google.cloud.bigtable.data.v2.BigtableDataSettings; +import com.google.cloud.bigtable.data.v2.FakeServiceHelper; +import com.google.cloud.bigtable.data.v2.models.Query; +import com.google.cloud.bigtable.data.v2.models.RowMutation; +import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStub; +import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; +import com.google.common.collect.ImmutableMap; +import io.grpc.*; +import io.grpc.stub.StreamObserver; +import io.opencensus.impl.stats.StatsComponentImpl; +import io.opencensus.stats.*; +import io.opencensus.tags.TagKey; +import io.opencensus.tags.TagValue; +import io.opencensus.tags.Tags; +import java.util.Random; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.Answers; +import org.mockito.Mock; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; +import org.mockito.stubbing.Answer; + +@RunWith(JUnit4.class) +public class GFEMetricsTest { + @Rule public MockitoRule mockitoRule = MockitoJUnit.rule(); + + private FakeServiceHelper serviceHelper; + private FakeServiceHelper serviceHelperNoHeader; + + @Mock(answer = Answers.CALLS_REAL_METHODS) + private BigtableGrpc.BigtableImplBase fakeService; + + private StatsComponent localStats = new StatsComponentImpl(); + private EnhancedBigtableStub stub; + private EnhancedBigtableStub noHeaderStub; + + private static final String PROJECT_ID = "fake-project"; + private static final String INSTANCE_ID = "fake-instance"; + private static final String APP_PROFILE_ID = "default"; + private static final String TABLE_ID = "fake-table"; + + private static final long WAIT_FOR_METRICS_TIME_MS = 1_000; + + private int fakeServerTiming; + + @Before + public void setUp() throws Exception { + fakeServerTiming = new Random().nextInt(10000) + 1; + serviceHelper = + new FakeServiceHelper( + new ServerInterceptor() { + @Override + public ServerCall.Listener interceptCall( + ServerCall serverCall, + Metadata metadata, + ServerCallHandler serverCallHandler) { + return serverCallHandler.startCall( + new ForwardingServerCall.SimpleForwardingServerCall(serverCall) { + @Override + public void sendHeaders(Metadata headers) { + // inject fake server-timing header for testing + headers.put( + Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER), + String.format("gfet4t7; dur=%d", fakeServerTiming)); + super.sendHeaders(headers); + } + }, + metadata); + } + }, + fakeService); + serviceHelperNoHeader = new FakeServiceHelper(fakeService); + serviceHelper.start(); + serviceHelperNoHeader.start(); + + RpcViews.registerBigtableClientViews(localStats.getViewManager()); + + BigtableDataSettings settings = + BigtableDataSettings.newBuilderForEmulator(serviceHelper.getPort()) + .setProjectId(PROJECT_ID) + .setInstanceId(INSTANCE_ID) + .setAppProfileId(APP_PROFILE_ID) + .build(); + + EnhancedBigtableStubSettings stubSettings = + EnhancedBigtableStub.finalizeSettings( + settings.getStubSettings(), Tags.getTagger(), localStats.getStatsRecorder()); + stub = new EnhancedBigtableStub(stubSettings, ClientContext.create(stubSettings)); + + // Create another stub to connect to the service with no injected header. + BigtableDataSettings noHeaderSettings = + BigtableDataSettings.newBuilderForEmulator(serviceHelperNoHeader.getPort()) + .setProjectId(PROJECT_ID) + .setInstanceId(INSTANCE_ID) + .setAppProfileId(APP_PROFILE_ID) + .build(); + + EnhancedBigtableStubSettings noHeaderStubSettings = + EnhancedBigtableStub.finalizeSettings( + noHeaderSettings.getStubSettings(), Tags.getTagger(), localStats.getStatsRecorder()); + noHeaderStub = + new EnhancedBigtableStub(noHeaderStubSettings, ClientContext.create(noHeaderStubSettings)); + } + + @After + public void tearDown() { + stub.close(); + noHeaderStub.close(); + serviceHelper.shutdown(); + serviceHelperNoHeader.shutdown(); + } + + @Test + public void testGFELatencyMetricReadRows() throws InterruptedException { + doAnswer(new ReadRowsAnswer()) + .when(fakeService) + .readRows(any(ReadRowsRequest.class), anyObserver(ReadRowsResponse.class)); + + stub.readRowsCallable().call(Query.create(TABLE_ID)); + + Thread.sleep(WAIT_FOR_METRICS_TIME_MS); + + long latency = + StatsTestUtils.getAggregationValueAsLong( + localStats, + RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW, + ImmutableMap.of( + RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadRows")), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID); + + assertThat(latency).isEqualTo(fakeServerTiming); + } + + @Test + public void testGFELatencyMetricMutateRows() throws InterruptedException { + doAnswer(new MutateRowAnswer()) + .when(fakeService) + .mutateRow(any(MutateRowRequest.class), anyObserver(MutateRowResponse.class)); + + stub.mutateRowCallable().call(RowMutation.create(TABLE_ID, "fake-key")); + + Thread.sleep(WAIT_FOR_METRICS_TIME_MS); + + long latency = + StatsTestUtils.getAggregationValueAsLong( + localStats, + RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW, + ImmutableMap.of(RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.MutateRow")), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID); + + assertThat(latency).isEqualTo(fakeServerTiming); + } + + @Test + public void testGFEMissingHeaderMetric() throws InterruptedException { + doAnswer(new ReadRowsAnswer()) + .when(fakeService) + .readRows(any(ReadRowsRequest.class), anyObserver(ReadRowsResponse.class)); + doAnswer(new MutateRowAnswer()) + .when(fakeService) + .mutateRow(any(MutateRowRequest.class), anyObserver(MutateRowResponse.class)); + + // Make a few calls to the server that'll add server-timing header and the counter should be 0. + stub.readRowsCallable().call(Query.create(TABLE_ID)); + stub.mutateRowCallable().call(RowMutation.create(TABLE_ID, "key")); + + Thread.sleep(WAIT_FOR_METRICS_TIME_MS); + long mutateRowMissingCount = + StatsTestUtils.getAggregationValueAsLong( + localStats, + RpcViewConstants.BIGTABLE_GFE_MISSING_COUNT_VIEW, + ImmutableMap.of(RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.MutateRow")), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID); + long readRowsMissingCount = + StatsTestUtils.getAggregationValueAsLong( + localStats, + RpcViewConstants.BIGTABLE_GFE_MISSING_COUNT_VIEW, + ImmutableMap.of( + RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadRows")), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID); + + Thread.sleep(WAIT_FOR_METRICS_TIME_MS); + + assertThat(mutateRowMissingCount).isEqualTo(0); + assertThat(readRowsMissingCount).isEqualTo(0); + + // Make a few more calls to the service which won't add the header and the counter should match + // number of requests we sent. + int readRowsCalls = new Random().nextInt(10) + 1; + int mutateRowCalls = new Random().nextInt(10) + 1; + for (int i = 0; i < mutateRowCalls; i++) { + noHeaderStub.mutateRowCallable().call(RowMutation.create(TABLE_ID, "fake-key" + i)); + } + for (int i = 0; i < readRowsCalls; i++) { + noHeaderStub.readRowsCallable().call(Query.create(TABLE_ID)); + } + + Thread.sleep(WAIT_FOR_METRICS_TIME_MS); + + mutateRowMissingCount = + StatsTestUtils.getAggregationValueAsLong( + localStats, + RpcViewConstants.BIGTABLE_GFE_MISSING_COUNT_VIEW, + ImmutableMap.of(RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.MutateRow")), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID); + readRowsMissingCount = + StatsTestUtils.getAggregationValueAsLong( + localStats, + RpcViewConstants.BIGTABLE_GFE_MISSING_COUNT_VIEW, + ImmutableMap.of( + RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadRows")), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID); + + assertThat(mutateRowMissingCount).isEqualTo(mutateRowCalls); + assertThat(readRowsMissingCount).isEqualTo(readRowsCalls); + } + + private class ReadRowsAnswer implements Answer { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + StreamObserver observer = + (StreamObserver) invocation.getArguments()[1]; + observer.onNext(ReadRowsResponse.getDefaultInstance()); + observer.onCompleted(); + return null; + } + } + + private class MutateRowAnswer implements Answer { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + StreamObserver observer = + (StreamObserver) invocation.getArguments()[1]; + observer.onNext(MutateRowResponse.getDefaultInstance()); + observer.onCompleted(); + return null; + } + } + + private static StreamObserver anyObserver(Class returnType) { + return (StreamObserver) any(returnType); + } +} diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracerTest.java index 4b025303e4..56d65f8298 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracerTest.java @@ -39,24 +39,10 @@ import io.grpc.Status; import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; -import io.opencensus.common.Function; import io.opencensus.impl.stats.StatsComponentImpl; -import io.opencensus.stats.AggregationData; -import io.opencensus.stats.AggregationData.CountData; -import io.opencensus.stats.AggregationData.DistributionData; -import io.opencensus.stats.AggregationData.LastValueDataDouble; -import io.opencensus.stats.AggregationData.LastValueDataLong; -import io.opencensus.stats.AggregationData.SumDataDouble; -import io.opencensus.stats.AggregationData.SumDataLong; -import io.opencensus.stats.View; -import io.opencensus.stats.ViewData; import io.opencensus.tags.TagKey; import io.opencensus.tags.TagValue; import io.opencensus.tags.Tags; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.Objects; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import org.junit.After; @@ -117,7 +103,6 @@ public void setUp() throws Exception { EnhancedBigtableStubSettings stubSettings = EnhancedBigtableStub.finalizeSettings( settings.getStubSettings(), Tags.getTagger(), localStats.getStatsRecorder()); - stub = new EnhancedBigtableStub(stubSettings, ClientContext.create(stubSettings)); } @@ -155,11 +140,15 @@ public Object answer(InvocationOnMock invocation) throws Throwable { Thread.sleep(100); long opLatency = - getAggregationValueAsLong( + StatsTestUtils.getAggregationValueAsLong( + localStats, RpcViewConstants.BIGTABLE_OP_LATENCY_VIEW, ImmutableMap.of( RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadRows"), - RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create("OK"))); + RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create("OK")), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID); assertThat(opLatency).isIn(Range.closed(sleepTime, elapsed)); } @@ -187,11 +176,15 @@ public Object answer(InvocationOnMock invocation) { Thread.sleep(100); long opLatency = - getAggregationValueAsLong( + StatsTestUtils.getAggregationValueAsLong( + localStats, RpcViewConstants.BIGTABLE_COMPLETED_OP_VIEW, ImmutableMap.of( RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadRows"), - RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create("OK"))); + RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create("OK")), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID); assertThat(opLatency).isEqualTo(2); } @@ -225,9 +218,13 @@ public Object answer(InvocationOnMock invocation) throws Throwable { Thread.sleep(100); long firstRowLatency = - getAggregationValueAsLong( + StatsTestUtils.getAggregationValueAsLong( + localStats, RpcViewConstants.BIGTABLE_READ_ROWS_FIRST_ROW_LATENCY_VIEW, - ImmutableMap.of()); + ImmutableMap.of(), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID); // adding buffer time to the upper range to allow for a race between the emulator and the client // recording the duration @@ -267,11 +264,15 @@ public Object answer(InvocationOnMock invocation) { Thread.sleep(100); long opLatency = - getAggregationValueAsLong( + StatsTestUtils.getAggregationValueAsLong( + localStats, RpcViewConstants.BIGTABLE_ATTEMPTS_PER_OP_VIEW, ImmutableMap.of( RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadRows"), - RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create("OK"))); + RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create("OK")), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID); assertThat(opLatency).isEqualTo(2); } @@ -312,11 +313,15 @@ public Object answer(InvocationOnMock invocation) throws Throwable { Thread.sleep(100); long attemptLatency = - getAggregationValueAsLong( + StatsTestUtils.getAggregationValueAsLong( + localStats, RpcViewConstants.BIGTABLE_ATTEMPT_LATENCY_VIEW, ImmutableMap.of( RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadRows"), - RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create("OK"))); + RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create("OK")), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID); // Average attempt latency will be just a single wait (as opposed to op latency which will be 2x // sleeptime) assertThat(attemptLatency).isIn(Range.closed(sleepTime, elapsed - sleepTime)); @@ -326,70 +331,4 @@ public Object answer(InvocationOnMock invocation) throws Throwable { private static StreamObserver anyObserver(Class returnType) { return (StreamObserver) any(returnType); } - - private long getAggregationValueAsLong(View view, ImmutableMap tags) { - ViewData viewData = localStats.getViewManager().getView(view.getName()); - Map, AggregationData> aggregationMap = - Objects.requireNonNull(viewData).getAggregationMap(); - - List tagValues = new ArrayList<>(); - - for (TagKey column : view.getColumns()) { - if (RpcMeasureConstants.BIGTABLE_PROJECT_ID == column) { - tagValues.add(TagValue.create(PROJECT_ID)); - } else if (RpcMeasureConstants.BIGTABLE_INSTANCE_ID == column) { - tagValues.add(TagValue.create(INSTANCE_ID)); - } else if (RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID == column) { - tagValues.add(TagValue.create(APP_PROFILE_ID)); - } else { - tagValues.add(tags.get(column)); - } - } - - AggregationData aggregationData = aggregationMap.get(tagValues); - - return aggregationData.match( - new Function() { - @Override - public Long apply(SumDataDouble arg) { - return (long) arg.getSum(); - } - }, - new Function() { - @Override - public Long apply(SumDataLong arg) { - return arg.getSum(); - } - }, - new Function() { - @Override - public Long apply(CountData arg) { - return arg.getCount(); - } - }, - new Function() { - @Override - public Long apply(DistributionData arg) { - return (long) arg.getMean(); - } - }, - new Function() { - @Override - public Long apply(LastValueDataDouble arg) { - return (long) arg.getLastValue(); - } - }, - new Function() { - @Override - public Long apply(LastValueDataLong arg) { - return arg.getLastValue(); - } - }, - new Function() { - @Override - public Long apply(AggregationData arg) { - throw new UnsupportedOperationException(); - } - }); - } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/StatsTestUtils.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/StatsTestUtils.java index ff37e75a87..5f96bb989c 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/StatsTestUtils.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/StatsTestUtils.java @@ -23,9 +23,7 @@ import com.google.common.collect.Maps; import io.grpc.Context; import io.opencensus.common.Scope; -import io.opencensus.stats.Measure; -import io.opencensus.stats.MeasureMap; -import io.opencensus.stats.StatsRecorder; +import io.opencensus.stats.*; import io.opencensus.tags.Tag; import io.opencensus.tags.TagContext; import io.opencensus.tags.TagContextBuilder; @@ -35,8 +33,7 @@ import io.opencensus.tags.TagValue; import io.opencensus.tags.Tagger; import io.opencensus.tags.unsafe.ContextUtils; -import java.util.Iterator; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; @@ -264,4 +261,76 @@ private static ImmutableMap getTags(TagContext tags) { ? ((FakeTagContext) tags).getTags() : ImmutableMap.of(); } + + public static long getAggregationValueAsLong( + StatsComponent stats, + View view, + ImmutableMap tags, + String projectId, + String instanceId, + String appProfileId) { + ViewData viewData = stats.getViewManager().getView(view.getName()); + Map, AggregationData> aggregationMap = + Objects.requireNonNull(viewData).getAggregationMap(); + + List tagValues = new ArrayList<>(); + + for (TagKey column : view.getColumns()) { + if (RpcMeasureConstants.BIGTABLE_PROJECT_ID == column) { + tagValues.add(TagValue.create(projectId)); + } else if (RpcMeasureConstants.BIGTABLE_INSTANCE_ID == column) { + tagValues.add(TagValue.create(instanceId)); + } else if (RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID == column) { + tagValues.add(TagValue.create(appProfileId)); + } else { + tagValues.add(tags.get(column)); + } + } + + AggregationData aggregationData = aggregationMap.get(tagValues); + + return aggregationData.match( + new io.opencensus.common.Function() { + @Override + public Long apply(AggregationData.SumDataDouble arg) { + return (long) arg.getSum(); + } + }, + new io.opencensus.common.Function() { + @Override + public Long apply(AggregationData.SumDataLong arg) { + return arg.getSum(); + } + }, + new io.opencensus.common.Function() { + @Override + public Long apply(AggregationData.CountData arg) { + return arg.getCount(); + } + }, + new io.opencensus.common.Function() { + @Override + public Long apply(AggregationData.DistributionData arg) { + return (long) arg.getMean(); + } + }, + new io.opencensus.common.Function() { + @Override + public Long apply(AggregationData.LastValueDataDouble arg) { + return (long) arg.getLastValue(); + } + }, + new io.opencensus.common.Function() { + @Override + public Long apply(AggregationData.LastValueDataLong arg) { + return arg.getLastValue(); + } + }, + new io.opencensus.common.Function() { + @Override + public Long apply(AggregationData arg) { + throw new UnsupportedOperationException(); + } + }); + } } From 335d1dce7ee661631c05d2abb20cf0a45b46c2c8 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 12 Nov 2020 20:44:48 +0000 Subject: [PATCH 02/12] Add more tests and refactor --- .../v2/stub/BigtableInterceptorProvider.java | 3 + .../v2/stub/CheckAndMutateRowCallable.java | 1 + .../data/v2/stub/EnhancedBigtableStub.java | 61 +++++---- .../v2/stub/EnhancedBigtableStubSettings.java | 14 +-- .../ClientHeaderInterceptor.java | 46 +++---- .../data/v2/stub/metrics/HeaderTracer.java | 116 +++++++++++++++--- .../v2/stub/metrics/RpcMeasureConstants.java | 6 +- .../v2/BigtableDataClientFactoryTest.java | 3 +- .../EnhancedBigtableStubSettingsTest.java | 35 +++++- ....java => ClientHeaderInterceptorTest.java} | 2 +- .../v2/stub/metrics/HeaderTracerTest.java | 76 ++++++++++++ 11 files changed, 274 insertions(+), 89 deletions(-) rename google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/{ => metrics}/ClientHeaderInterceptor.java (69%) rename google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/{GFEMetricsTest.java => ClientHeaderInterceptorTest.java} (99%) create mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableInterceptorProvider.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableInterceptorProvider.java index b889f2a28c..a6442d6034 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableInterceptorProvider.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableInterceptorProvider.java @@ -15,11 +15,14 @@ */ package com.google.cloud.bigtable.data.v2.stub; +import com.google.api.core.BetaApi; import com.google.api.gax.grpc.GrpcInterceptorProvider; +import com.google.cloud.bigtable.data.v2.stub.metrics.ClientHeaderInterceptor; import com.google.common.collect.ImmutableList; import io.grpc.ClientInterceptor; import java.util.List; +@BetaApi public class BigtableInterceptorProvider implements GrpcInterceptorProvider { private static final ClientHeaderInterceptor headerInterceptor = new ClientHeaderInterceptor(); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CheckAndMutateRowCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CheckAndMutateRowCallable.java index b23abaebfe..549e10f44b 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CheckAndMutateRowCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CheckAndMutateRowCallable.java @@ -42,6 +42,7 @@ class CheckAndMutateRowCallable extends UnaryCallable futureCall(ConditionalRowMutation request, ApiCallContext context) { ApiFuture rawResponse = inner.futureCall(request.toProto(requestContext), context); + return ApiFutures.transform( rawResponse, new ApiFunction() { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 650e86830b..12b53f4511 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -67,7 +67,10 @@ import com.google.cloud.bigtable.data.v2.models.RowAdapter; import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.cloud.bigtable.data.v2.models.RowMutationEntry; -import com.google.cloud.bigtable.data.v2.stub.metrics.*; +import com.google.cloud.bigtable.data.v2.stub.metrics.CompositeTracerFactory; +import com.google.cloud.bigtable.data.v2.stub.metrics.HeaderTracer; +import com.google.cloud.bigtable.data.v2.stub.metrics.MetricsTracerFactory; +import com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants; import com.google.cloud.bigtable.data.v2.stub.mutaterows.BulkMutateRowsUserFacingCallable; import com.google.cloud.bigtable.data.v2.stub.mutaterows.MutateRowsBatchingDescriptor; import com.google.cloud.bigtable.data.v2.stub.mutaterows.MutateRowsRetryingCallable; @@ -207,26 +210,31 @@ public static EnhancedBigtableStubSettings finalizeSettings( .build()), // Add user configured tracer settings.getTracerFactory()))); - HeaderTracer headerTracer = builder.getHeaderTracer(); - headerTracer.setStats(stats); - headerTracer.setTagger(tagger); - headerTracer.setStatsAttributes( - ImmutableMap.builder() - .put(RpcMeasureConstants.BIGTABLE_PROJECT_ID, TagValue.create(settings.getProjectId())) - .put( - RpcMeasureConstants.BIGTABLE_INSTANCE_ID, TagValue.create(settings.getInstanceId())) - .put( - RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID, - TagValue.create(settings.getAppProfileId())) + builder.setHeaderTracer( + builder + .getHeaderTracer() + .toBuilder() + .setStats(stats) + .setTagger(tagger) + .setStatsAttributes( + ImmutableMap.builder() + .put( + RpcMeasureConstants.BIGTABLE_PROJECT_ID, + TagValue.create(settings.getProjectId())) + .put( + RpcMeasureConstants.BIGTABLE_INSTANCE_ID, + TagValue.create(settings.getInstanceId())) + .put( + RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID, + TagValue.create(settings.getAppProfileId())) + .build()) .build()); - builder.setHeaderTracer(headerTracer); return builder.build(); } public EnhancedBigtableStub(EnhancedBigtableStubSettings settings, ClientContext clientContext) { this.settings = settings; this.clientContext = clientContext; - this.requestContext = RequestContext.create( settings.getProjectId(), settings.getInstanceId(), settings.getAppProfileId()); @@ -285,13 +293,12 @@ public ServerStreamingCallable createReadRowsCallable( ServerStreamingCallable readRowsUserCallable = new ReadRowsUserCallable<>(readRowsCallable, requestContext); + SpanName spanName = SpanName.of(CLIENT_NAME, "ReadRows"); ServerStreamingCallable traced = new TracedServerStreamingCallable<>( - readRowsUserCallable, - clientContext.getTracerFactory(), - SpanName.of(CLIENT_NAME, "ReadRows")); + readRowsUserCallable, clientContext.getTracerFactory(), spanName); - return traced.withDefaultCallContext(getContextWithTracer()); + return traced.withDefaultCallContext(getContextWithTracer(spanName)); } /** @@ -476,11 +483,11 @@ private UnaryCallable createBulkMutateRowsCallable() { UnaryCallable userFacing = new BulkMutateRowsUserFacingCallable(baseCallable, requestContext); + SpanName spanName = SpanName.of(CLIENT_NAME, "MutateRows"); UnaryCallable traced = - new TracedUnaryCallable<>( - userFacing, clientContext.getTracerFactory(), SpanName.of(CLIENT_NAME, "MutateRows")); + new TracedUnaryCallable<>(userFacing, clientContext.getTracerFactory(), spanName); - return traced.withDefaultCallContext(getContextWithTracer()); + return traced.withDefaultCallContext(getContextWithTracer(spanName)); } /** @@ -651,11 +658,11 @@ public Map extract(ReadModifyWriteRowRequest request) { private UnaryCallable createUserFacingUnaryCallable( String methodName, UnaryCallable inner) { + SpanName spanName = SpanName.of(CLIENT_NAME, methodName); UnaryCallable traced = - new TracedUnaryCallable<>( - inner, clientContext.getTracerFactory(), SpanName.of(CLIENT_NAME, methodName)); + new TracedUnaryCallable<>(inner, clientContext.getTracerFactory(), spanName); - return traced.withDefaultCallContext(getContextWithTracer()); + return traced.withDefaultCallContext(getContextWithTracer(spanName)); } // @@ -703,7 +710,7 @@ public UnaryCallable readModifyWriteRowCallable() { } // - private GrpcCallContext getContextWithTracer() { + private GrpcCallContext getContextWithTracer(SpanName spanName) { ApiCallContext apiCallContext = clientContext.getDefaultCallContext(); if (!(apiCallContext instanceof GrpcCallContext)) { LOGGER.warning( @@ -713,7 +720,9 @@ private GrpcCallContext getContextWithTracer() { GrpcCallContext grpcCallContext = (GrpcCallContext) apiCallContext; CallOptions options = grpcCallContext.getCallOptions(); options = - options.withOption(HeaderTracer.HEADER_TRACER_CONTEXT_KEY, settings.getHeaderTracer()); + options + .withOption(HeaderTracer.HEADER_TRACER_CONTEXT_KEY, settings.getHeaderTracer()) + .withOption(HeaderTracer.SPAN_NAME_CONTEXT_KEY, spanName.toString()); grpcCallContext = grpcCallContext.withCallOptions(options); return grpcCallContext; } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java index 301a604b10..46b41d7771 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java @@ -240,7 +240,7 @@ public List getPrimedTableIds() { } /** Gets the tracer for capturing metrics in the header. */ - @BetaApi("This API is not currently stable and might change in the future") + @BetaApi public HeaderTracer getHeaderTracer() { return headerTracer; } @@ -534,7 +534,7 @@ private Builder() { this.appProfileId = SERVER_DEFAULT_APP_PROFILE_ID; this.isRefreshingChannel = false; primedTableIds = ImmutableList.of(); - headerTracer = new HeaderTracer(); + headerTracer = HeaderTracer.newBuilder().build(); setCredentialsProvider(defaultCredentialsProviderBuilder().build()); // Defaults provider @@ -771,15 +771,15 @@ public List getPrimedTableIds() { return primedTableIds; } - // TODO: update comments - /** Configure the header tracer */ - @BetaApi("") - public Builder setHeaderTracer(HeaderTracer tracer) { + /** Configure the header tracer for surfacing metrics in the header. */ + @BetaApi + public Builder setHeaderTracer(HeaderTracer headerTracer) { this.headerTracer = headerTracer; return this; } - // TODO: update comments + /** Gets the header tracer that'll be used to surface metrics in the header. */ + @BetaApi public HeaderTracer getHeaderTracer() { return headerTracer; } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ClientHeaderInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptor.java similarity index 69% rename from google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ClientHeaderInterceptor.java rename to google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptor.java index c27a83f8f6..0de60ae5b2 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ClientHeaderInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptor.java @@ -13,11 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.google.cloud.bigtable.data.v2.stub; +package com.google.cloud.bigtable.data.v2.stub.metrics; -import com.google.api.gax.tracing.SpanName; -import com.google.cloud.bigtable.data.v2.stub.metrics.HeaderTracer; -import com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants; +import com.google.api.core.BetaApi; import io.grpc.CallOptions; import io.grpc.Channel; import io.grpc.ClientCall; @@ -30,20 +28,26 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +/** + * Surface GFE server-timing metric. + * + *

This class exports the latency metric that tracks the time between GFE receives the first byte + * of a request and reads the first byte of the response. It also tracks the number of occurrences + * when this metric is missing from the header. + */ +@BetaApi public class ClientHeaderInterceptor implements ClientInterceptor { + private static final Logger LOGGER = Logger.getLogger(ClientHeaderInterceptor.class.getName()); + public static final Metadata.Key SERVER_TIMING_HEADER_KEY = Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER); - - private static final Logger LOGGER = Logger.getLogger(ClientHeaderInterceptor.class.getName()); - private static final String CLIENT_NAME = "Bigtable"; private static final Pattern GFE_HEADER_PATTERN = Pattern.compile(".*dur=(?\\d+)"); - private static final Pattern METHOD_DESCRIPTOR_PATTERN = Pattern.compile(".*/(?[a-zA-Z]+)"); @Override public ClientCall interceptCall( final MethodDescriptor method, final CallOptions callOptions, Channel channel) { final ClientCall clientCall = channel.newCall(method, callOptions); - final SpanName spanName = processMethodName(method.getFullMethodName()); + final String span = callOptions.getOption(HeaderTracer.SPAN_NAME_CONTEXT_KEY); final HeaderTracer tracer = callOptions.getOption(HeaderTracer.HEADER_TRACER_CONTEXT_KEY); return new SimpleForwardingClientCall(clientCall) { @Override @@ -52,7 +56,7 @@ public void start(Listener responseListener, Metadata headers) { new SimpleForwardingClientCallListener(responseListener) { @Override public void onHeaders(Metadata headers) { - processHeader(headers, tracer, spanName); + processHeader(headers, tracer, span); super.onHeaders(headers); } }, @@ -61,36 +65,26 @@ public void onHeaders(Metadata headers) { }; } - private SpanName processMethodName(String method) { - Matcher matcher = METHOD_DESCRIPTOR_PATTERN.matcher(method); - if (matcher.find()) { - return SpanName.of(CLIENT_NAME, matcher.group("op")); - } - LOGGER.warning( - String.format("Failed to get bigtable op name. Received method descriptor: %s.", method)); - return null; - } - - private void processHeader(Metadata headers, HeaderTracer tracer, SpanName span) { + private void processHeader(Metadata headers, HeaderTracer tracer, String span) { if (tracer == null) { - LOGGER.warning("Couldn't find HeaderTracer in call options. Skip extracting gfe metrics"); + LOGGER.warning("Couldn't find HeaderTracer in call options. Skip exporting gfe metrics"); return; } if (headers.get(SERVER_TIMING_HEADER_KEY) != null) { String serverTiming = headers.get(SERVER_TIMING_HEADER_KEY); Matcher matcher = GFE_HEADER_PATTERN.matcher(serverTiming); - tracer.recordHeader(RpcMeasureConstants.BIGTABLE_GFE_MISSING_COUNT, 0, span); + tracer.record(RpcMeasureConstants.BIGTABLE_GFE_MISSING_COUNT, 0L, span); if (matcher.find()) { long latency = Long.valueOf(matcher.group("dur")); - tracer.recordHeader(RpcMeasureConstants.BIGTABLE_GFE_LATENCY, latency, span); + tracer.record(RpcMeasureConstants.BIGTABLE_GFE_LATENCY, latency, span); } else { LOGGER.warning( String.format( - "Received invalid %s header: %s, failed to add to metrics.", + "Failed to get GFE latency from %s header: %s.", SERVER_TIMING_HEADER_KEY.name(), serverTiming)); } } else { - tracer.recordHeader(RpcMeasureConstants.BIGTABLE_GFE_MISSING_COUNT, 1l, span); + tracer.record(RpcMeasureConstants.BIGTABLE_GFE_MISSING_COUNT, 1L, span); } } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracer.java index 9101bfd764..89b165cf85 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracer.java @@ -15,7 +15,8 @@ */ package com.google.cloud.bigtable.data.v2.stub.metrics; -import com.google.api.gax.tracing.SpanName; +import com.google.api.core.BetaApi; +import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import io.grpc.CallOptions; @@ -30,51 +31,111 @@ import io.opencensus.tags.Tagger; import io.opencensus.tags.Tags; import java.util.Map; +import javax.annotation.Nonnull; import javax.annotation.Nullable; +@BetaApi public class HeaderTracer { public static final CallOptions.Key HEADER_TRACER_CONTEXT_KEY = CallOptions.Key.create("BigtableHeaderTracer"); + public static final CallOptions.Key SPAN_NAME_CONTEXT_KEY = + CallOptions.Key.create("BigtableSpanName"); private Tagger tagger; private StatsRecorder stats; private Map statsAttributes; - public HeaderTracer() { - this.tagger = Tags.getTagger(); - this.stats = Stats.getStatsRecorder(); - this.statsAttributes = ImmutableMap.of(); + private HeaderTracer(Builder builder) { + tagger = builder.getTagger(); + stats = builder.getStats(); + statsAttributes = builder.getStatsAttributes(); } - public void setTagger(Tagger tagger) { - this.tagger = tagger; + public static class Builder { + private Tagger tagger; + private StatsRecorder stats; + private Map statsAttributes; + + private Builder() { + tagger = Tags.getTagger(); + stats = Stats.getStatsRecorder(); + statsAttributes = ImmutableMap.of(); + } + + private Builder(HeaderTracer headerTracer) { + tagger = headerTracer.tagger; + stats = headerTracer.stats; + statsAttributes = headerTracer.statsAttributes; + } + + // + public Builder setTagger(@Nonnull Tagger tagger) { + Preconditions.checkNotNull(tagger); + this.tagger = tagger; + return this; + } + + public Builder setStats(@Nonnull StatsRecorder stats) { + Preconditions.checkNotNull(stats); + this.stats = stats; + return this; + } + + public Builder setStatsAttributes(@Nonnull Map statsAttributes) { + Preconditions.checkNotNull(statsAttributes); + this.statsAttributes = statsAttributes; + return this; + } + + public Tagger getTagger() { + return tagger; + } + + public StatsRecorder getStats() { + return stats; + } + + public Map getStatsAttributes() { + return statsAttributes; + } + + public HeaderTracer build() { + Preconditions.checkNotNull(stats, "StatsRecorder must be set"); + Preconditions.checkNotNull(tagger, "Tagger must be set"); + Preconditions.checkNotNull(statsAttributes, "Stats attributes must be set"); + return new HeaderTracer(this); + } + // + } + + public Tagger getTagger() { + return tagger; } - public void setStats(StatsRecorder stats) { - this.stats = stats; + public StatsRecorder getStats() { + return stats; } - public void setStatsAttributes(Map statsAttributes) { - this.statsAttributes = statsAttributes; + public Map getStatsAttributes() { + return statsAttributes; } - public void recordHeader(MeasureLong measure, long value, @Nullable SpanName spanName) { + public void record(MeasureLong measure, long value, @Nullable String span) { Preconditions.checkNotNull(measure, "Measure cannot be null."); MeasureMap measures = stats.newMeasureMap().put(measure, value); - measures.record(newTagCtxBuilder(spanName).build()); + measures.record(newTagCtxBuilder(span).build()); } - public void recordHeader(MeasureDouble measure, double value, @Nullable SpanName spanName) { + public void record(MeasureDouble measure, double value, @Nullable String span) { Preconditions.checkNotNull(measure, "Measure cannot be null."); MeasureMap measures = stats.newMeasureMap().put(measure, value); - measures.record(newTagCtxBuilder(spanName).build()); + measures.record(newTagCtxBuilder(span).build()); } - private TagContextBuilder newTagCtxBuilder(@Nullable SpanName spanName) { + private TagContextBuilder newTagCtxBuilder(@Nullable String span) { TagContextBuilder tagContextBuilder = tagger.currentBuilder(); - if (spanName != null) { - tagContextBuilder.putLocal( - RpcMeasureConstants.BIGTABLE_OP, TagValue.create(spanName.toString())); + if (span != null) { + tagContextBuilder.putLocal(RpcMeasureConstants.BIGTABLE_OP, TagValue.create(span)); } // Copy client level tags in for (Map.Entry entry : statsAttributes.entrySet()) { @@ -82,4 +143,21 @@ private TagContextBuilder newTagCtxBuilder(@Nullable SpanName spanName) { } return tagContextBuilder; } + + public static Builder newBuilder() { + return new Builder(); + } + + public Builder toBuilder() { + return new Builder(this); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("stats", stats) + .add("tagger", tagger) + .add("statsAttributes", stats) + .toString(); + } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java index 75cca5bddf..58dd8a1edd 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java @@ -28,7 +28,7 @@ public class RpcMeasureConstants { public static final TagKey BIGTABLE_APP_PROFILE_ID = TagKey.create("bigtable_app_profile_id"); /** Tag key that represents a Bigtable operation name. */ - public static final TagKey BIGTABLE_OP = TagKey.create("bigtable_op"); + static final TagKey BIGTABLE_OP = TagKey.create("bigtable_op"); /** Tag key that represents the final status of the Bigtable operation. */ static final TagKey BIGTABLE_STATUS = TagKey.create("bigtable_status"); @@ -80,10 +80,10 @@ public class RpcMeasureConstants { public static final MeasureLong BIGTABLE_GFE_LATENCY = MeasureLong.create( "cloud.google.com/java/bigtable/gfe_latency", - "GFE t4t7 latency. Time between GFE receives the first byte of the request and GFE reads the first byte of the response", + "GFE t4t7 latency. Time between GFE receives the first byte of a request and reads the first byte of the response", MILLISECOND); - /** Number of responses without server-timing trailer. */ + /** Number of responses without the server-timing trailer. */ public static final MeasureLong BIGTABLE_GFE_MISSING_COUNT = Measure.MeasureLong.create( "cloud.google.com/java/bigtable/gfe_missing_count", diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java index 2d6a4998ba..25c341d650 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java @@ -35,7 +35,8 @@ import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.common.base.Preconditions; import com.google.protobuf.ByteString; -import io.grpc.*; +import io.grpc.Attributes; +import io.grpc.ServerTransportFilter; import io.grpc.stub.StreamObserver; import java.io.IOException; import java.lang.reflect.Method; diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java index 4c8358e15c..10ba675826 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java @@ -32,6 +32,7 @@ import com.google.cloud.bigtable.data.v2.models.Query; import com.google.cloud.bigtable.data.v2.models.Row; import com.google.cloud.bigtable.data.v2.models.RowMutation; +import com.google.cloud.bigtable.data.v2.stub.metrics.HeaderTracer; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Range; @@ -75,6 +76,7 @@ public void settingsAreNotLostTest() { CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); WatchdogProvider watchdogProvider = Mockito.mock(WatchdogProvider.class); Duration watchdogInterval = Duration.ofSeconds(12); + HeaderTracer headerTracer = Mockito.mock(HeaderTracer.class); EnhancedBigtableStubSettings.Builder builder = EnhancedBigtableStubSettings.newBuilder() @@ -85,7 +87,8 @@ public void settingsAreNotLostTest() { .setEndpoint(endpoint) .setCredentialsProvider(credentialsProvider) .setStreamWatchdogProvider(watchdogProvider) - .setStreamWatchdogCheckInterval(watchdogInterval); + .setStreamWatchdogCheckInterval(watchdogInterval) + .setHeaderTracer(headerTracer); verifyBuilder( builder, @@ -96,7 +99,8 @@ public void settingsAreNotLostTest() { endpoint, credentialsProvider, watchdogProvider, - watchdogInterval); + watchdogInterval, + headerTracer); verifySettings( builder.build(), projectId, @@ -106,7 +110,8 @@ public void settingsAreNotLostTest() { endpoint, credentialsProvider, watchdogProvider, - watchdogInterval); + watchdogInterval, + headerTracer); verifyBuilder( builder.build().toBuilder(), projectId, @@ -116,7 +121,8 @@ public void settingsAreNotLostTest() { endpoint, credentialsProvider, watchdogProvider, - watchdogInterval); + watchdogInterval, + headerTracer); } private void verifyBuilder( @@ -128,7 +134,8 @@ private void verifyBuilder( String endpoint, CredentialsProvider credentialsProvider, WatchdogProvider watchdogProvider, - Duration watchdogInterval) { + Duration watchdogInterval, + HeaderTracer headerTracer) { assertThat(builder.getProjectId()).isEqualTo(projectId); assertThat(builder.getInstanceId()).isEqualTo(instanceId); assertThat(builder.getAppProfileId()).isEqualTo(appProfileId); @@ -137,6 +144,7 @@ private void verifyBuilder( assertThat(builder.getCredentialsProvider()).isEqualTo(credentialsProvider); assertThat(builder.getStreamWatchdogProvider()).isSameInstanceAs(watchdogProvider); assertThat(builder.getStreamWatchdogCheckInterval()).isEqualTo(watchdogInterval); + assertThat(builder.getHeaderTracer()).isEqualTo(headerTracer); } private void verifySettings( @@ -148,7 +156,8 @@ private void verifySettings( String endpoint, CredentialsProvider credentialsProvider, WatchdogProvider watchdogProvider, - Duration watchdogInterval) { + Duration watchdogInterval, + HeaderTracer headerTracer) { assertThat(settings.getProjectId()).isEqualTo(projectId); assertThat(settings.getInstanceId()).isEqualTo(instanceId); assertThat(settings.getAppProfileId()).isEqualTo(appProfileId); @@ -157,6 +166,7 @@ private void verifySettings( assertThat(settings.getCredentialsProvider()).isEqualTo(credentialsProvider); assertThat(settings.getStreamWatchdogProvider()).isSameInstanceAs(watchdogProvider); assertThat(settings.getStreamWatchdogCheckInterval()).isEqualTo(watchdogInterval); + assertThat(settings.getHeaderTracer()).isEqualTo(headerTracer); } @Test @@ -622,6 +632,19 @@ public void isRefreshingChannelFalseValueTest() { assertThat(builder.build().toBuilder().isRefreshingChannel()).isFalse(); } + @Test + public void verifyDefaultHeaderTracerNotNullTest() { + String dummyProjectId = "my-project"; + String dummyInstanceId = "my-instance"; + EnhancedBigtableStubSettings.Builder builder = + EnhancedBigtableStubSettings.newBuilder() + .setProjectId(dummyProjectId) + .setInstanceId(dummyInstanceId); + assertThat(builder.getHeaderTracer()).isNotNull(); + assertThat(builder.build().getHeaderTracer()).isNotNull(); + assertThat(builder.build().toBuilder().getHeaderTracer()).isNotNull(); + } + static final String[] SETTINGS_LIST = { "projectId", "instanceId", diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/GFEMetricsTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptorTest.java similarity index 99% rename from google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/GFEMetricsTest.java rename to google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptorTest.java index 2daf6f4e75..b35deff0ce 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/GFEMetricsTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptorTest.java @@ -54,7 +54,7 @@ import org.mockito.stubbing.Answer; @RunWith(JUnit4.class) -public class GFEMetricsTest { +public class ClientHeaderInterceptorTest { @Rule public MockitoRule mockitoRule = MockitoJUnit.rule(); private FakeServiceHelper serviceHelper; diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java new file mode 100644 index 0000000000..5da7ba884b --- /dev/null +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java @@ -0,0 +1,76 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub.metrics; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableMap; +import io.opencensus.impl.stats.StatsComponentImpl; +import io.opencensus.stats.StatsComponent; +import io.opencensus.stats.StatsRecorder; +import io.opencensus.tags.TagKey; +import io.opencensus.tags.TagValue; +import io.opencensus.tags.Tagger; +import io.opencensus.tags.Tags; +import java.util.Map; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class HeaderTracerTest { + + private StatsComponent localStats = new StatsComponentImpl(); + + @Test + public void testEmptyBuilder() { + HeaderTracer.Builder builder = HeaderTracer.newBuilder(); + assertThat(builder.getStats()).isNotNull(); + assertThat(builder.getTagger()).isNotNull(); + assertThat(builder.getStatsAttributes()).isNotNull(); + assertThat(builder.getStatsAttributes()).isEmpty(); + } + + @Test + public void testBuilder() { + HeaderTracer.Builder builder = HeaderTracer.newBuilder(); + Map attrs = + ImmutableMap.of(TagKey.create("fake-key"), TagValue.create("fake-value")); + Tagger tagger = Tags.getTagger(); + StatsRecorder stats = localStats.getStatsRecorder(); + builder.setStats(stats).setStatsAttributes(attrs).setTagger(tagger); + HeaderTracer headerTracer = builder.build(); + assertThat(headerTracer.getStats()).isEqualTo(stats); + assertThat(headerTracer.getTagger()).isEqualTo(tagger); + assertThat(headerTracer.getStatsAttributes()).isEqualTo(attrs); + } + + @Test + public void testToBuilder() { + HeaderTracer.Builder builder = HeaderTracer.newBuilder(); + Map attrs = + ImmutableMap.of(TagKey.create("fake-key"), TagValue.create("fake-value")); + Tagger tagger = Tags.getTagger(); + StatsRecorder stats = localStats.getStatsRecorder(); + builder.setStats(stats).setStatsAttributes(attrs).setTagger(tagger); + HeaderTracer headerTracer = builder.build(); + + HeaderTracer.Builder newBuilder = headerTracer.toBuilder(); + assertThat(newBuilder.getStats()).isEqualTo(stats); + assertThat(newBuilder.getTagger()).isEqualTo(tagger); + assertThat(newBuilder.getStatsAttributes()).isEqualTo(attrs); + } +} From 5fac4bdbdc672a76a8f82f89db200a41265d77c3 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 12 Nov 2020 22:33:58 +0000 Subject: [PATCH 03/12] Refactor comments and imports --- .../data/v2/stub/EnhancedBigtableStub.java | 9 +++- .../v2/stub/EnhancedBigtableStubSettings.java | 8 +++- .../stub/metrics/ClientHeaderInterceptor.java | 4 +- .../v2/stub/metrics/RpcMeasureConstants.java | 13 +++--- .../v2/stub/metrics/RpcViewConstants.java | 15 +++---- .../data/v2/stub/metrics/RpcViews.java | 2 +- .../metrics/ClientHeaderInterceptorTest.java | 44 +++++++++++-------- .../v2/stub/metrics/HeaderTracerTest.java | 5 ++- .../data/v2/stub/metrics/StatsTestUtils.java | 14 +++++- 9 files changed, 69 insertions(+), 45 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 12b53f4511..9277ce7037 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -710,12 +710,17 @@ public UnaryCallable readModifyWriteRowCallable() { } // - private GrpcCallContext getContextWithTracer(SpanName spanName) { + /** + * Adds HeaderTracer and SpanName to CallOptions so we could surface metrics in the header + * with {@link com.google.cloud.bigtable.data.v2.stub.metrics.ClientHeaderInterceptor}. + * */ + private ApiCallContext getContextWithTracer(SpanName spanName) { ApiCallContext apiCallContext = clientContext.getDefaultCallContext(); if (!(apiCallContext instanceof GrpcCallContext)) { LOGGER.warning( - "Failed to inject tracer in call context. Expected GrpcCallContext but had " + "Failed to add tracer in call context. Expected GrpcCallContext but had " + apiCallContext.getClass()); + return apiCallContext; } GrpcCallContext grpcCallContext = (GrpcCallContext) apiCallContext; CallOptions options = grpcCallContext.getCallOptions(); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java index 46b41d7771..4ab9a4a665 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java @@ -24,8 +24,12 @@ import com.google.api.gax.core.GoogleCredentialsProvider; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.api.gax.retrying.RetrySettings; -import com.google.api.gax.rpc.*; +import com.google.api.gax.rpc.FixedHeaderProvider; +import com.google.api.gax.rpc.ServerStreamingCallSettings; import com.google.api.gax.rpc.StatusCode.Code; +import com.google.api.gax.rpc.StubSettings; +import com.google.api.gax.rpc.TransportChannelProvider; +import com.google.api.gax.rpc.UnaryCallSettings; import com.google.auth.Credentials; import com.google.cloud.bigtable.Version; import com.google.cloud.bigtable.data.v2.models.ConditionalRowMutation; @@ -34,7 +38,7 @@ import com.google.cloud.bigtable.data.v2.models.ReadModifyWriteRow; import com.google.cloud.bigtable.data.v2.models.Row; import com.google.cloud.bigtable.data.v2.models.RowMutation; -import com.google.cloud.bigtable.data.v2.stub.metrics.*; +import com.google.cloud.bigtable.data.v2.stub.metrics.HeaderTracer; import com.google.cloud.bigtable.data.v2.stub.mutaterows.MutateRowsBatchingDescriptor; import com.google.cloud.bigtable.data.v2.stub.readrows.ReadRowsBatchingDescriptor; import com.google.common.base.MoreObjects; diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptor.java index 0de60ae5b2..b3aad83705 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptor.java @@ -73,7 +73,7 @@ private void processHeader(Metadata headers, HeaderTracer tracer, String span) { if (headers.get(SERVER_TIMING_HEADER_KEY) != null) { String serverTiming = headers.get(SERVER_TIMING_HEADER_KEY); Matcher matcher = GFE_HEADER_PATTERN.matcher(serverTiming); - tracer.record(RpcMeasureConstants.BIGTABLE_GFE_MISSING_COUNT, 0L, span); + tracer.record(RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT, 0L, span); if (matcher.find()) { long latency = Long.valueOf(matcher.group("dur")); tracer.record(RpcMeasureConstants.BIGTABLE_GFE_LATENCY, latency, span); @@ -84,7 +84,7 @@ private void processHeader(Metadata headers, HeaderTracer tracer, String span) { SERVER_TIMING_HEADER_KEY.name(), serverTiming)); } } else { - tracer.record(RpcMeasureConstants.BIGTABLE_GFE_MISSING_COUNT, 1L, span); + tracer.record(RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT, 1L, span); } } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java index 58dd8a1edd..54cf10851e 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java @@ -16,7 +16,6 @@ package com.google.cloud.bigtable.data.v2.stub.metrics; import com.google.api.core.InternalApi; -import io.opencensus.stats.Measure; import io.opencensus.stats.Measure.MeasureLong; import io.opencensus.tags.TagKey; @@ -76,17 +75,17 @@ public class RpcMeasureConstants { "Time between request being sent to the first row received", MILLISECOND); - /** GFE t4t7 latency extracted from server-timing trailer. */ + /** GFE t4t7 latency extracted from server-timing header. */ public static final MeasureLong BIGTABLE_GFE_LATENCY = MeasureLong.create( "cloud.google.com/java/bigtable/gfe_latency", "GFE t4t7 latency. Time between GFE receives the first byte of a request and reads the first byte of the response", MILLISECOND); - /** Number of responses without the server-timing trailer. */ - public static final MeasureLong BIGTABLE_GFE_MISSING_COUNT = - Measure.MeasureLong.create( - "cloud.google.com/java/bigtable/gfe_missing_count", - "Number of responses without the server-timing trailer", + /** Number of responses without the server-timing header. */ + public static final MeasureLong BIGTABLE_GFE_HEADER_MISSING_COUNT = + MeasureLong.create( + "cloud.google.com/java/bigtable/gfe_header_missing_count", + "Number of responses without the server-timing header", COUNT); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java index af72d2f492..7df6d88c2e 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java @@ -18,7 +18,7 @@ import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID; import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_ATTEMPT_LATENCY; import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_GFE_LATENCY; -import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_GFE_MISSING_COUNT; +import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT; import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_INSTANCE_ID; import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_OP; import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_OP_ATTEMPT_COUNT; @@ -34,7 +34,6 @@ import io.opencensus.stats.Aggregation.Sum; import io.opencensus.stats.BucketBoundaries; import io.opencensus.stats.View; -import io.opencensus.tags.TagKey; import java.util.Arrays; class RpcViewConstants { @@ -136,15 +135,15 @@ class RpcViewConstants { "GFE t4t7 latency in msecs", BIGTABLE_GFE_LATENCY, AGGREGATION_WITH_MILLIS_HISTOGRAM, - ImmutableList.of( + ImmutableList.of( BIGTABLE_INSTANCE_ID, BIGTABLE_PROJECT_ID, BIGTABLE_APP_PROFILE_ID, BIGTABLE_OP)); - static final View BIGTABLE_GFE_MISSING_COUNT_VIEW = + static final View BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW = View.create( - View.Name.create("cloud.google.com/java/bigtable/gfe_missing_count"), - "Number of responses without the server-timing trailer", - BIGTABLE_GFE_MISSING_COUNT, + View.Name.create("cloud.google.com/java/bigtable/gfe_header_missing_count"), + "Number of responses without the server-timing header", + BIGTABLE_GFE_HEADER_MISSING_COUNT, SUM, - ImmutableList.of( + ImmutableList.of( BIGTABLE_INSTANCE_ID, BIGTABLE_PROJECT_ID, BIGTABLE_APP_PROFILE_ID, BIGTABLE_OP)); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViews.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViews.java index fc8f47b742..84d1a50fd9 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViews.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViews.java @@ -33,7 +33,7 @@ public class RpcViews { RpcViewConstants.BIGTABLE_ATTEMPT_LATENCY_VIEW, RpcViewConstants.BIGTABLE_ATTEMPTS_PER_OP_VIEW, RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW, - RpcViewConstants.BIGTABLE_GFE_MISSING_COUNT_VIEW); + RpcViewConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW); /** Registers all Bigtable specific views. */ public static void registerBigtableClientViews() { diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptorTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptorTest.java index b35deff0ce..7b83207a6c 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptorTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptorTest.java @@ -32,10 +32,14 @@ import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStub; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; import com.google.common.collect.ImmutableMap; -import io.grpc.*; +import io.grpc.ForwardingServerCall.SimpleForwardingServerCall; +import io.grpc.Metadata; +import io.grpc.ServerCall; +import io.grpc.ServerCallHandler; +import io.grpc.ServerInterceptor; import io.grpc.stub.StreamObserver; import io.opencensus.impl.stats.StatsComponentImpl; -import io.opencensus.stats.*; +import io.opencensus.stats.StatsComponent; import io.opencensus.tags.TagKey; import io.opencensus.tags.TagValue; import io.opencensus.tags.Tags; @@ -78,7 +82,11 @@ public class ClientHeaderInterceptorTest { @Before public void setUp() throws Exception { - fakeServerTiming = new Random().nextInt(10000) + 1; + RpcViews.registerBigtableClientViews(localStats.getViewManager()); + + // Create a server that'll inject a server-timing header with a random number and a stub that + // connects to this server. + fakeServerTiming = new Random().nextInt(1000) + 1; serviceHelper = new FakeServiceHelper( new ServerInterceptor() { @@ -88,10 +96,9 @@ public ServerCall.Listener interceptCall( Metadata metadata, ServerCallHandler serverCallHandler) { return serverCallHandler.startCall( - new ForwardingServerCall.SimpleForwardingServerCall(serverCall) { + new SimpleForwardingServerCall(serverCall) { @Override public void sendHeaders(Metadata headers) { - // inject fake server-timing header for testing headers.put( Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER), String.format("gfet4t7; dur=%d", fakeServerTiming)); @@ -102,11 +109,7 @@ public void sendHeaders(Metadata headers) { } }, fakeService); - serviceHelperNoHeader = new FakeServiceHelper(fakeService); serviceHelper.start(); - serviceHelperNoHeader.start(); - - RpcViews.registerBigtableClientViews(localStats.getViewManager()); BigtableDataSettings settings = BigtableDataSettings.newBuilderForEmulator(serviceHelper.getPort()) @@ -114,20 +117,22 @@ public void sendHeaders(Metadata headers) { .setInstanceId(INSTANCE_ID) .setAppProfileId(APP_PROFILE_ID) .build(); - EnhancedBigtableStubSettings stubSettings = EnhancedBigtableStub.finalizeSettings( settings.getStubSettings(), Tags.getTagger(), localStats.getStatsRecorder()); stub = new EnhancedBigtableStub(stubSettings, ClientContext.create(stubSettings)); - // Create another stub to connect to the service with no injected header. + // Create another server without injecting the server-timing header and another stub that + // connects to it. + serviceHelperNoHeader = new FakeServiceHelper(fakeService); + serviceHelperNoHeader.start(); + BigtableDataSettings noHeaderSettings = BigtableDataSettings.newBuilderForEmulator(serviceHelperNoHeader.getPort()) .setProjectId(PROJECT_ID) .setInstanceId(INSTANCE_ID) .setAppProfileId(APP_PROFILE_ID) .build(); - EnhancedBigtableStubSettings noHeaderStubSettings = EnhancedBigtableStub.finalizeSettings( noHeaderSettings.getStubSettings(), Tags.getTagger(), localStats.getStatsRecorder()); @@ -197,7 +202,8 @@ public void testGFEMissingHeaderMetric() throws InterruptedException { .when(fakeService) .mutateRow(any(MutateRowRequest.class), anyObserver(MutateRowResponse.class)); - // Make a few calls to the server that'll add server-timing header and the counter should be 0. + // Make a few calls to the server which will inject the server-timing header and the counter + // should be 0. stub.readRowsCallable().call(Query.create(TABLE_ID)); stub.mutateRowCallable().call(RowMutation.create(TABLE_ID, "key")); @@ -205,7 +211,7 @@ public void testGFEMissingHeaderMetric() throws InterruptedException { long mutateRowMissingCount = StatsTestUtils.getAggregationValueAsLong( localStats, - RpcViewConstants.BIGTABLE_GFE_MISSING_COUNT_VIEW, + RpcViewConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW, ImmutableMap.of(RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.MutateRow")), PROJECT_ID, INSTANCE_ID, @@ -213,7 +219,7 @@ public void testGFEMissingHeaderMetric() throws InterruptedException { long readRowsMissingCount = StatsTestUtils.getAggregationValueAsLong( localStats, - RpcViewConstants.BIGTABLE_GFE_MISSING_COUNT_VIEW, + RpcViewConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW, ImmutableMap.of( RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadRows")), PROJECT_ID, @@ -225,8 +231,8 @@ public void testGFEMissingHeaderMetric() throws InterruptedException { assertThat(mutateRowMissingCount).isEqualTo(0); assertThat(readRowsMissingCount).isEqualTo(0); - // Make a few more calls to the service which won't add the header and the counter should match - // number of requests we sent. + // Make a few more calls to the server which won't add the header and the counter should match + // the number of requests sent. int readRowsCalls = new Random().nextInt(10) + 1; int mutateRowCalls = new Random().nextInt(10) + 1; for (int i = 0; i < mutateRowCalls; i++) { @@ -241,7 +247,7 @@ public void testGFEMissingHeaderMetric() throws InterruptedException { mutateRowMissingCount = StatsTestUtils.getAggregationValueAsLong( localStats, - RpcViewConstants.BIGTABLE_GFE_MISSING_COUNT_VIEW, + RpcViewConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW, ImmutableMap.of(RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.MutateRow")), PROJECT_ID, INSTANCE_ID, @@ -249,7 +255,7 @@ public void testGFEMissingHeaderMetric() throws InterruptedException { readRowsMissingCount = StatsTestUtils.getAggregationValueAsLong( localStats, - RpcViewConstants.BIGTABLE_GFE_MISSING_COUNT_VIEW, + RpcViewConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW, ImmutableMap.of( RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadRows")), PROJECT_ID, diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java index 5da7ba884b..5b1e0a0680 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java @@ -29,6 +29,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.mockito.Mockito; @RunWith(JUnit4.class) public class HeaderTracerTest { @@ -49,7 +50,7 @@ public void testBuilder() { HeaderTracer.Builder builder = HeaderTracer.newBuilder(); Map attrs = ImmutableMap.of(TagKey.create("fake-key"), TagValue.create("fake-value")); - Tagger tagger = Tags.getTagger(); + Tagger tagger = Mockito.mock(Tagger.class); StatsRecorder stats = localStats.getStatsRecorder(); builder.setStats(stats).setStatsAttributes(attrs).setTagger(tagger); HeaderTracer headerTracer = builder.build(); @@ -63,7 +64,7 @@ public void testToBuilder() { HeaderTracer.Builder builder = HeaderTracer.newBuilder(); Map attrs = ImmutableMap.of(TagKey.create("fake-key"), TagValue.create("fake-value")); - Tagger tagger = Tags.getTagger(); + Tagger tagger = Mockito.mock(Tagger.class); StatsRecorder stats = localStats.getStatsRecorder(); builder.setStats(stats).setStatsAttributes(attrs).setTagger(tagger); HeaderTracer headerTracer = builder.build(); diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/StatsTestUtils.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/StatsTestUtils.java index 5f96bb989c..6aede96161 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/StatsTestUtils.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/StatsTestUtils.java @@ -23,7 +23,13 @@ import com.google.common.collect.Maps; import io.grpc.Context; import io.opencensus.common.Scope; -import io.opencensus.stats.*; +import io.opencensus.stats.AggregationData; +import io.opencensus.stats.Measure; +import io.opencensus.stats.MeasureMap; +import io.opencensus.stats.StatsComponent; +import io.opencensus.stats.StatsRecorder; +import io.opencensus.stats.View; +import io.opencensus.stats.ViewData; import io.opencensus.tags.Tag; import io.opencensus.tags.TagContext; import io.opencensus.tags.TagContextBuilder; @@ -33,8 +39,12 @@ import io.opencensus.tags.TagValue; import io.opencensus.tags.Tagger; import io.opencensus.tags.unsafe.ContextUtils; -import java.util.*; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; From 8ae57a761d36f94a0d6001c941cce3b14900a358 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 13 Nov 2020 00:14:54 +0000 Subject: [PATCH 04/12] reformatting --- .../cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java | 6 +++--- .../bigtable/data/v2/stub/metrics/RpcViewConstants.java | 2 +- .../bigtable/data/v2/stub/metrics/HeaderTracerTest.java | 1 - 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 9277ce7037..50c74f5baa 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -711,9 +711,9 @@ public UnaryCallable readModifyWriteRowCallable() { // /** - * Adds HeaderTracer and SpanName to CallOptions so we could surface metrics in the header - * with {@link com.google.cloud.bigtable.data.v2.stub.metrics.ClientHeaderInterceptor}. - * */ + * Adds HeaderTracer and SpanName to CallOptions so we could surface metrics in the header with + * {@link com.google.cloud.bigtable.data.v2.stub.metrics.ClientHeaderInterceptor}. + */ private ApiCallContext getContextWithTracer(SpanName spanName) { ApiCallContext apiCallContext = clientContext.getDefaultCallContext(); if (!(apiCallContext instanceof GrpcCallContext)) { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java index 7df6d88c2e..e84c9e7a2b 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java @@ -17,8 +17,8 @@ import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID; import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_ATTEMPT_LATENCY; -import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_GFE_LATENCY; import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT; +import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_GFE_LATENCY; import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_INSTANCE_ID; import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_OP; import static com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants.BIGTABLE_OP_ATTEMPT_COUNT; diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java index 5b1e0a0680..fb8457af72 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java @@ -24,7 +24,6 @@ import io.opencensus.tags.TagKey; import io.opencensus.tags.TagValue; import io.opencensus.tags.Tagger; -import io.opencensus.tags.Tags; import java.util.Map; import org.junit.Test; import org.junit.runner.RunWith; From dff4d283f5f210fb034ec3238ba84b5efea87002 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 13 Nov 2020 00:38:38 +0000 Subject: [PATCH 05/12] Clean up comments --- .../bigtable/data/v2/stub/EnhancedBigtableStub.java | 5 +++-- .../data/v2/stub/EnhancedBigtableStubSettings.java | 2 -- .../data/v2/stub/metrics/ClientHeaderInterceptor.java | 10 +++++----- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 50c74f5baa..bacab8c7c1 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -711,8 +711,9 @@ public UnaryCallable readModifyWriteRowCallable() { // /** - * Adds HeaderTracer and SpanName to CallOptions so we could surface metrics in the header with - * {@link com.google.cloud.bigtable.data.v2.stub.metrics.ClientHeaderInterceptor}. + * Adds a HeaderTracer instance and current span name to CallOptions so we could surface metrics + * in the header with {@link + * com.google.cloud.bigtable.data.v2.stub.metrics.ClientHeaderInterceptor}. */ private ApiCallContext getContextWithTracer(SpanName spanName) { ApiCallContext apiCallContext = clientContext.getDefaultCallContext(); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java index 4ab9a4a665..a58c4ed463 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java @@ -260,8 +260,6 @@ public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProvi .setKeepAliveWithoutCalls(true) // sends ping without active streams .setInterceptorProvider(BigtableInterceptorProvider.createDefault()) // TODO(weiranf): Set this to true by default once DirectPath goes to public beta - // Attempts direct access to CBT service over gRPC to improve throughput, - // whether the attempt is allowed is totally controlled by service owner. .setAttemptDirectPath(isDirectPathEnabled()); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptor.java index b3aad83705..19ecf68ee4 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptor.java @@ -31,9 +31,9 @@ /** * Surface GFE server-timing metric. * - *

This class exports the latency metric that tracks the time between GFE receives the first byte - * of a request and reads the first byte of the response. It also tracks the number of occurrences - * when this metric is missing from the header. + *

This class exports the metric from server-timing header that tracks the latency between GFE + * receives the first byte of a request and reads the first byte of the response. It also tracks the + * number of occurrences of missing server-timing header. */ @BetaApi public class ClientHeaderInterceptor implements ClientInterceptor { @@ -41,7 +41,7 @@ public class ClientHeaderInterceptor implements ClientInterceptor { public static final Metadata.Key SERVER_TIMING_HEADER_KEY = Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER); - private static final Pattern GFE_HEADER_PATTERN = Pattern.compile(".*dur=(?\\d+)"); + private static final Pattern SERVER_TIMING_HEADER_PATTERN = Pattern.compile(".*dur=(?\\d+)"); @Override public ClientCall interceptCall( @@ -72,7 +72,7 @@ private void processHeader(Metadata headers, HeaderTracer tracer, String span) { } if (headers.get(SERVER_TIMING_HEADER_KEY) != null) { String serverTiming = headers.get(SERVER_TIMING_HEADER_KEY); - Matcher matcher = GFE_HEADER_PATTERN.matcher(serverTiming); + Matcher matcher = SERVER_TIMING_HEADER_PATTERN.matcher(serverTiming); tracer.record(RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT, 0L, span); if (matcher.find()) { long latency = Long.valueOf(matcher.group("dur")); From c55d9d6891f3f4a4b3479779b6b867e6bd666666 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Mon, 30 Nov 2020 17:56:12 +0000 Subject: [PATCH 06/12] Refactor, use GrpcMetadataResponse to get the trailer --- .../data/v2/BigtableDataSettings.java | 8 +- .../v2/stub/BigtableInterceptorProvider.java | 47 ------ .../data/v2/stub/EnhancedBigtableStub.java | 90 ++++++----- .../v2/stub/EnhancedBigtableStubSettings.java | 4 +- .../stub/metrics/ClientHeaderInterceptor.java | 90 ----------- .../data/v2/stub/metrics/HeaderTracer.java | 147 +++++++----------- .../HeaderTracerStreamingCallable.java | 122 +++++++++++++++ .../metrics/HeaderTracerUnaryCallable.java | 65 ++++++++ .../data/v2/stub/metrics/RpcViews.java | 17 +- ...est.java => HeaderTracerCallableTest.java} | 4 +- 10 files changed, 313 insertions(+), 281 deletions(-) delete mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableInterceptorProvider.java delete mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptor.java create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerUnaryCallable.java rename google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/{ClientHeaderInterceptorTest.java => HeaderTracerCallableTest.java} (98%) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java index d3c35029c5..ff20075f6c 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java @@ -23,7 +23,6 @@ import com.google.api.gax.rpc.UnaryCallSettings; import com.google.cloud.bigtable.data.v2.models.Query; import com.google.cloud.bigtable.data.v2.models.Row; -import com.google.cloud.bigtable.data.v2.stub.BigtableInterceptorProvider; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; import com.google.common.base.MoreObjects; import com.google.common.base.Strings; @@ -128,7 +127,6 @@ public ManagedChannelBuilder apply(ManagedChannelBuilder input) { .setKeepAliveTime(Duration.ofSeconds(30)) // sends ping in this interval .setKeepAliveTimeout( Duration.ofSeconds(10)) // wait this long before considering the connection dead - .setInterceptorProvider(BigtableInterceptorProvider.createDefault()) .build()); LOGGER.info("Connecting to the Bigtable emulator at " + hostname + ":" + port); @@ -177,6 +175,12 @@ public static void enableOpenCensusStats() { // io.opencensus.contrib.grpc.metrics.RpcViews.registerClientGrpcBasicViews(); } + /** Enables OpenCensus GFE metric aggregations. */ + @BetaApi("OpenCensus stats integration is currently unstable and may change in the future") + public static void enableGfeOpenCensusStats() { + com.google.cloud.bigtable.data.v2.stub.metrics.RpcViews.registerBigtableClientGfeViews(); + } + /** Returns the target project id. */ public String getProjectId() { return stubSettings.getProjectId(); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableInterceptorProvider.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableInterceptorProvider.java deleted file mode 100644 index a6442d6034..0000000000 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableInterceptorProvider.java +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright 2020 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.google.cloud.bigtable.data.v2.stub; - -import com.google.api.core.BetaApi; -import com.google.api.gax.grpc.GrpcInterceptorProvider; -import com.google.cloud.bigtable.data.v2.stub.metrics.ClientHeaderInterceptor; -import com.google.common.collect.ImmutableList; -import io.grpc.ClientInterceptor; -import java.util.List; - -@BetaApi -public class BigtableInterceptorProvider implements GrpcInterceptorProvider { - - private static final ClientHeaderInterceptor headerInterceptor = new ClientHeaderInterceptor(); - private List clientInterceptors; - - private BigtableInterceptorProvider(List interceptors) { - this.clientInterceptors = interceptors; - } - - public static BigtableInterceptorProvider createDefault() { - return new BigtableInterceptorProvider(ImmutableList.of(headerInterceptor)); - } - - public static BigtableInterceptorProvider create(List interceptors) { - return new BigtableInterceptorProvider(interceptors); - } - - @Override - public List getInterceptors() { - return clientInterceptors; - } -} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index bacab8c7c1..025794bff1 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -23,7 +23,6 @@ import com.google.api.gax.core.FixedCredentialsProvider; import com.google.api.gax.core.GaxProperties; import com.google.api.gax.grpc.GaxGrpcProperties; -import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.GrpcCallSettings; import com.google.api.gax.grpc.GrpcRawCallableFactory; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; @@ -31,7 +30,6 @@ import com.google.api.gax.retrying.RetryAlgorithm; import com.google.api.gax.retrying.RetryingExecutorWithContext; import com.google.api.gax.retrying.ScheduledRetryingExecutor; -import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.Callables; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.RequestParamsExtractor; @@ -68,7 +66,8 @@ import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.cloud.bigtable.data.v2.models.RowMutationEntry; import com.google.cloud.bigtable.data.v2.stub.metrics.CompositeTracerFactory; -import com.google.cloud.bigtable.data.v2.stub.metrics.HeaderTracer; +import com.google.cloud.bigtable.data.v2.stub.metrics.HeaderTracerStreamingCallable; +import com.google.cloud.bigtable.data.v2.stub.metrics.HeaderTracerUnaryCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.MetricsTracerFactory; import com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants; import com.google.cloud.bigtable.data.v2.stub.mutaterows.BulkMutateRowsUserFacingCallable; @@ -85,7 +84,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.protobuf.ByteString; -import io.grpc.CallOptions; import io.opencensus.stats.Stats; import io.opencensus.stats.StatsRecorder; import io.opencensus.tags.TagKey; @@ -293,12 +291,12 @@ public ServerStreamingCallable createReadRowsCallable( ServerStreamingCallable readRowsUserCallable = new ReadRowsUserCallable<>(readRowsCallable, requestContext); - SpanName spanName = SpanName.of(CLIENT_NAME, "ReadRows"); + SpanName span = getSpanName("ReadRows"); ServerStreamingCallable traced = new TracedServerStreamingCallable<>( - readRowsUserCallable, clientContext.getTracerFactory(), spanName); + readRowsUserCallable, clientContext.getTracerFactory(), span); - return traced.withDefaultCallContext(getContextWithTracer(spanName)); + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** @@ -339,6 +337,7 @@ public UnaryCallable createReadRowCallable(RowAdapter *

  • Upon receiving the response stream, it will merge the {@link * com.google.bigtable.v2.ReadRowsResponse.CellChunk}s in logical rows. The actual row * implementation can be configured by the {@code rowAdapter} parameter. + *
  • Add header tracer for tracking GFE metrics. *
  • Retry/resume on failure. *
  • Filter out marker rows. * @@ -380,10 +379,14 @@ public Map extract(ReadRowsRequest readRowsRequest) { ServerStreamingCallable watched = Callables.watched(merging, innerSettings, clientContext); + ServerStreamingCallable withHeaderTracer = + new HeaderTracerStreamingCallable<>( + watched, settings.getHeaderTracer(), getSpanName("ReadRows").toString()); + // Retry logic is split into 2 parts to workaround a rare edge case described in // ReadRowsRetryCompletedCallable ServerStreamingCallable retrying1 = - new ReadRowsRetryCompletedCallable<>(watched); + new ReadRowsRetryCompletedCallable<>(withHeaderTracer); ServerStreamingCallable retrying2 = Callables.retrying(retrying1, innerSettings, clientContext); @@ -404,6 +407,8 @@ public Map extract(ReadRowsRequest readRowsRequest) { * */ private UnaryCallable> createSampleRowKeysCallable() { + String methodName = "SampleRowKeys"; + ServerStreamingCallable base = GrpcRawCallableFactory.createServerStreamingCallable( GrpcCallSettings.newBuilder() @@ -423,11 +428,15 @@ public Map extract( UnaryCallable> spoolable = base.all(); + HeaderTracerUnaryCallable> headerMetrics = + new HeaderTracerUnaryCallable<>( + spoolable, settings.getHeaderTracer(), getSpanName(methodName).toString()); + UnaryCallable> retryable = - Callables.retrying(spoolable, settings.sampleRowKeysSettings(), clientContext); + Callables.retrying(headerMetrics, settings.sampleRowKeysSettings(), clientContext); return createUserFacingUnaryCallable( - "SampleRowKeys", new SampleRowKeysCallable(retryable, requestContext)); + methodName, new SampleRowKeysCallable(retryable, requestContext)); } /** @@ -439,6 +448,7 @@ public Map extract( * */ private UnaryCallable createMutateRowCallable() { + String methodName = "MutateRow"; UnaryCallable base = GrpcRawCallableFactory.createUnaryCallable( GrpcCallSettings.newBuilder() @@ -455,11 +465,15 @@ public Map extract(MutateRowRequest mutateRowRequest) { .build(), settings.mutateRowSettings().getRetryableCodes()); + HeaderTracerUnaryCallable headerMetrics = + new HeaderTracerUnaryCallable<>( + base, settings.getHeaderTracer(), getSpanName(methodName).toString()); + UnaryCallable retrying = - Callables.retrying(base, settings.mutateRowSettings(), clientContext); + Callables.retrying(headerMetrics, settings.mutateRowSettings(), clientContext); return createUserFacingUnaryCallable( - "MutateRow", new MutateRowCallable(retrying, requestContext)); + methodName, new MutateRowCallable(retrying, requestContext)); } /** @@ -483,11 +497,13 @@ private UnaryCallable createBulkMutateRowsCallable() { UnaryCallable userFacing = new BulkMutateRowsUserFacingCallable(baseCallable, requestContext); - SpanName spanName = SpanName.of(CLIENT_NAME, "MutateRows"); + SpanName spanName = getSpanName("MutateRows"); UnaryCallable traced = new TracedUnaryCallable<>(userFacing, clientContext.getTracerFactory(), spanName); + HeaderTracerUnaryCallable headerMetric = + new HeaderTracerUnaryCallable<>(traced, settings.getHeaderTracer(), spanName.toString()); - return traced.withDefaultCallContext(getContextWithTracer(spanName)); + return headerMetric.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** @@ -593,6 +609,7 @@ public Map extract(MutateRowsRequest mutateRowsRequest) { * */ private UnaryCallable createCheckAndMutateRowCallable() { + String methodName = "CheckAndMutateRow"; UnaryCallable base = GrpcRawCallableFactory.createUnaryCallable( GrpcCallSettings.newBuilder() @@ -610,11 +627,15 @@ public Map extract( .build(), settings.checkAndMutateRowSettings().getRetryableCodes()); + HeaderTracerUnaryCallable headerMetric = + new HeaderTracerUnaryCallable<>( + base, settings.getHeaderTracer(), getSpanName(methodName).toString()); + UnaryCallable retrying = - Callables.retrying(base, settings.checkAndMutateRowSettings(), clientContext); + Callables.retrying(headerMetric, settings.checkAndMutateRowSettings(), clientContext); return createUserFacingUnaryCallable( - "CheckAndMutateRow", new CheckAndMutateRowCallable(retrying, requestContext)); + methodName, new CheckAndMutateRowCallable(retrying, requestContext)); } /** @@ -643,12 +664,15 @@ public Map extract(ReadModifyWriteRowRequest request) { }) .build(), settings.readModifyWriteRowSettings().getRetryableCodes()); - + String methodName = "ReadModifyWriteRow"; + HeaderTracerUnaryCallable headerMetrics = + new HeaderTracerUnaryCallable<>( + base, settings.getHeaderTracer(), getSpanName(methodName).toString()); UnaryCallable retrying = - Callables.retrying(base, settings.readModifyWriteRowSettings(), clientContext); + Callables.retrying(headerMetrics, settings.readModifyWriteRowSettings(), clientContext); return createUserFacingUnaryCallable( - "ReadModifyWriteRow", new ReadModifyWriteRowCallable(retrying, requestContext)); + methodName, new ReadModifyWriteRowCallable(retrying, requestContext)); } /** @@ -658,11 +682,10 @@ public Map extract(ReadModifyWriteRowRequest request) { private UnaryCallable createUserFacingUnaryCallable( String methodName, UnaryCallable inner) { - SpanName spanName = SpanName.of(CLIENT_NAME, methodName); UnaryCallable traced = - new TracedUnaryCallable<>(inner, clientContext.getTracerFactory(), spanName); + new TracedUnaryCallable<>(inner, clientContext.getTracerFactory(), getSpanName(methodName)); - return traced.withDefaultCallContext(getContextWithTracer(spanName)); + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } // @@ -710,27 +733,8 @@ public UnaryCallable readModifyWriteRowCallable() { } // - /** - * Adds a HeaderTracer instance and current span name to CallOptions so we could surface metrics - * in the header with {@link - * com.google.cloud.bigtable.data.v2.stub.metrics.ClientHeaderInterceptor}. - */ - private ApiCallContext getContextWithTracer(SpanName spanName) { - ApiCallContext apiCallContext = clientContext.getDefaultCallContext(); - if (!(apiCallContext instanceof GrpcCallContext)) { - LOGGER.warning( - "Failed to add tracer in call context. Expected GrpcCallContext but had " - + apiCallContext.getClass()); - return apiCallContext; - } - GrpcCallContext grpcCallContext = (GrpcCallContext) apiCallContext; - CallOptions options = grpcCallContext.getCallOptions(); - options = - options - .withOption(HeaderTracer.HEADER_TRACER_CONTEXT_KEY, settings.getHeaderTracer()) - .withOption(HeaderTracer.SPAN_NAME_CONTEXT_KEY, spanName.toString()); - grpcCallContext = grpcCallContext.withCallOptions(options); - return grpcCallContext; + private SpanName getSpanName(String methodName) { + return SpanName.of(CLIENT_NAME, methodName); } @Override diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java index 4448e700b5..8aaf2eae64 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java @@ -244,8 +244,7 @@ public List getPrimedTableIds() { } /** Gets the tracer for capturing metrics in the header. */ - @BetaApi - public HeaderTracer getHeaderTracer() { + HeaderTracer getHeaderTracer() { return headerTracer; } @@ -257,7 +256,6 @@ public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProvi .setKeepAliveTime(Duration.ofSeconds(30)) // sends ping in this interval .setKeepAliveTimeout( Duration.ofSeconds(10)) // wait this long before considering the connection dead - .setInterceptorProvider(BigtableInterceptorProvider.createDefault()) // TODO(weiranf): Set this to true by default once DirectPath goes to public beta .setAttemptDirectPath(isDirectPathEnabled()); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptor.java deleted file mode 100644 index 19ecf68ee4..0000000000 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptor.java +++ /dev/null @@ -1,90 +0,0 @@ -/* - * Copyright 2020 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.google.cloud.bigtable.data.v2.stub.metrics; - -import com.google.api.core.BetaApi; -import io.grpc.CallOptions; -import io.grpc.Channel; -import io.grpc.ClientCall; -import io.grpc.ClientInterceptor; -import io.grpc.ForwardingClientCall.SimpleForwardingClientCall; -import io.grpc.ForwardingClientCallListener.SimpleForwardingClientCallListener; -import io.grpc.Metadata; -import io.grpc.MethodDescriptor; -import java.util.logging.Logger; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -/** - * Surface GFE server-timing metric. - * - *

    This class exports the metric from server-timing header that tracks the latency between GFE - * receives the first byte of a request and reads the first byte of the response. It also tracks the - * number of occurrences of missing server-timing header. - */ -@BetaApi -public class ClientHeaderInterceptor implements ClientInterceptor { - private static final Logger LOGGER = Logger.getLogger(ClientHeaderInterceptor.class.getName()); - - public static final Metadata.Key SERVER_TIMING_HEADER_KEY = - Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER); - private static final Pattern SERVER_TIMING_HEADER_PATTERN = Pattern.compile(".*dur=(?\\d+)"); - - @Override - public ClientCall interceptCall( - final MethodDescriptor method, final CallOptions callOptions, Channel channel) { - final ClientCall clientCall = channel.newCall(method, callOptions); - final String span = callOptions.getOption(HeaderTracer.SPAN_NAME_CONTEXT_KEY); - final HeaderTracer tracer = callOptions.getOption(HeaderTracer.HEADER_TRACER_CONTEXT_KEY); - return new SimpleForwardingClientCall(clientCall) { - @Override - public void start(Listener responseListener, Metadata headers) { - super.start( - new SimpleForwardingClientCallListener(responseListener) { - @Override - public void onHeaders(Metadata headers) { - processHeader(headers, tracer, span); - super.onHeaders(headers); - } - }, - headers); - } - }; - } - - private void processHeader(Metadata headers, HeaderTracer tracer, String span) { - if (tracer == null) { - LOGGER.warning("Couldn't find HeaderTracer in call options. Skip exporting gfe metrics"); - return; - } - if (headers.get(SERVER_TIMING_HEADER_KEY) != null) { - String serverTiming = headers.get(SERVER_TIMING_HEADER_KEY); - Matcher matcher = SERVER_TIMING_HEADER_PATTERN.matcher(serverTiming); - tracer.record(RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT, 0L, span); - if (matcher.find()) { - long latency = Long.valueOf(matcher.group("dur")); - tracer.record(RpcMeasureConstants.BIGTABLE_GFE_LATENCY, latency, span); - } else { - LOGGER.warning( - String.format( - "Failed to get GFE latency from %s header: %s.", - SERVER_TIMING_HEADER_KEY.name(), serverTiming)); - } - } else { - tracer.record(RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT, 1L, span); - } - } -} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracer.java index 89b165cf85..877b77b748 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracer.java @@ -15,13 +15,11 @@ */ package com.google.cloud.bigtable.data.v2.stub.metrics; -import com.google.api.core.BetaApi; +import com.google.api.core.InternalApi; +import com.google.auto.value.AutoValue; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableMap; -import io.grpc.CallOptions; -import io.opencensus.stats.Measure.MeasureDouble; -import io.opencensus.stats.Measure.MeasureLong; +import io.grpc.Metadata; import io.opencensus.stats.MeasureMap; import io.opencensus.stats.Stats; import io.opencensus.stats.StatsRecorder; @@ -30,134 +28,97 @@ import io.opencensus.tags.TagValue; import io.opencensus.tags.Tagger; import io.opencensus.tags.Tags; +import java.util.Collections; import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.annotation.Nonnull; import javax.annotation.Nullable; -@BetaApi -public class HeaderTracer { - public static final CallOptions.Key HEADER_TRACER_CONTEXT_KEY = - CallOptions.Key.create("BigtableHeaderTracer"); - public static final CallOptions.Key SPAN_NAME_CONTEXT_KEY = - CallOptions.Key.create("BigtableSpanName"); - - private Tagger tagger; - private StatsRecorder stats; - private Map statsAttributes; - - private HeaderTracer(Builder builder) { - tagger = builder.getTagger(); - stats = builder.getStats(); - statsAttributes = builder.getStatsAttributes(); - } - - public static class Builder { - private Tagger tagger; - private StatsRecorder stats; - private Map statsAttributes; +@InternalApi +@AutoValue +public abstract class HeaderTracer { - private Builder() { - tagger = Tags.getTagger(); - stats = Stats.getStatsRecorder(); - statsAttributes = ImmutableMap.of(); - } - - private Builder(HeaderTracer headerTracer) { - tagger = headerTracer.tagger; - stats = headerTracer.stats; - statsAttributes = headerTracer.statsAttributes; - } + public static final Metadata.Key SERVER_TIMING_HEADER_KEY = + Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER); + public static final Pattern SERVER_TIMING_HEADER_PATTERN = Pattern.compile(".*dur=(?\\d+)"); + @AutoValue.Builder + public abstract static class Builder { // - public Builder setTagger(@Nonnull Tagger tagger) { - Preconditions.checkNotNull(tagger); - this.tagger = tagger; - return this; - } + public abstract Builder setTagger(@Nonnull Tagger tagger); - public Builder setStats(@Nonnull StatsRecorder stats) { - Preconditions.checkNotNull(stats); - this.stats = stats; - return this; - } + public abstract Builder setStats(@Nonnull StatsRecorder stats); - public Builder setStatsAttributes(@Nonnull Map statsAttributes) { - Preconditions.checkNotNull(statsAttributes); - this.statsAttributes = statsAttributes; - return this; - } + public abstract Builder setStatsAttributes(@Nonnull Map statsAttributes); - public Tagger getTagger() { - return tagger; - } + public abstract Tagger getTagger(); - public StatsRecorder getStats() { - return stats; - } + public abstract StatsRecorder getStats(); - public Map getStatsAttributes() { - return statsAttributes; - } + public abstract Map getStatsAttributes(); + + abstract HeaderTracer autoBuild(); public HeaderTracer build() { - Preconditions.checkNotNull(stats, "StatsRecorder must be set"); - Preconditions.checkNotNull(tagger, "Tagger must be set"); - Preconditions.checkNotNull(statsAttributes, "Stats attributes must be set"); - return new HeaderTracer(this); + HeaderTracer headerTracer = autoBuild(); + Preconditions.checkNotNull(headerTracer.getStats(), "StatsRecorder must be set"); + Preconditions.checkNotNull(headerTracer.getTagger(), "Tagger must be set"); + Preconditions.checkNotNull(headerTracer.getStatsAttributes(), "Stats attributes must be set"); + return headerTracer; } // } - public Tagger getTagger() { - return tagger; - } + public abstract Tagger getTagger(); - public StatsRecorder getStats() { - return stats; - } + public abstract StatsRecorder getStats(); - public Map getStatsAttributes() { - return statsAttributes; - } + public abstract Map getStatsAttributes(); - public void record(MeasureLong measure, long value, @Nullable String span) { - Preconditions.checkNotNull(measure, "Measure cannot be null."); - MeasureMap measures = stats.newMeasureMap().put(measure, value); - measures.record(newTagCtxBuilder(span).build()); - } - - public void record(MeasureDouble measure, double value, @Nullable String span) { - Preconditions.checkNotNull(measure, "Measure cannot be null."); - MeasureMap measures = stats.newMeasureMap().put(measure, value); - measures.record(newTagCtxBuilder(span).build()); + public void recordGfeMetrics(@Nonnull Metadata metadata, String spanName) { + MeasureMap measures = getStats().newMeasureMap(); + if (metadata.get(SERVER_TIMING_HEADER_KEY) != null) { + String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY); + Matcher matcher = SERVER_TIMING_HEADER_PATTERN.matcher(serverTiming); + measures.put(RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT, 0L); + if (matcher.find()) { + long latency = Long.valueOf(matcher.group("dur")); + measures.put(RpcMeasureConstants.BIGTABLE_GFE_LATENCY, latency); + } + } else { + measures.put(RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT, 1L); + } + measures.record(newTagCtxBuilder(spanName).build()); } private TagContextBuilder newTagCtxBuilder(@Nullable String span) { - TagContextBuilder tagContextBuilder = tagger.currentBuilder(); + TagContextBuilder tagContextBuilder = getTagger().currentBuilder(); if (span != null) { tagContextBuilder.putLocal(RpcMeasureConstants.BIGTABLE_OP, TagValue.create(span)); } // Copy client level tags in - for (Map.Entry entry : statsAttributes.entrySet()) { + for (Map.Entry entry : getStatsAttributes().entrySet()) { tagContextBuilder.putLocal(entry.getKey(), entry.getValue()); } return tagContextBuilder; } public static Builder newBuilder() { - return new Builder(); + return new AutoValue_HeaderTracer.Builder() + .setTagger(Tags.getTagger()) + .setStats(Stats.getStatsRecorder()) + .setStatsAttributes(Collections.emptyMap()); } - public Builder toBuilder() { - return new Builder(this); - } + public abstract Builder toBuilder(); @Override public String toString() { return MoreObjects.toStringHelper(this) - .add("stats", stats) - .add("tagger", tagger) - .add("statsAttributes", stats) + .add("stats", getStats()) + .add("tagger", getTagger()) + .add("statsAttributes", getStatsAttributes()) .toString(); } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java new file mode 100644 index 0000000000..f25b89f885 --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java @@ -0,0 +1,122 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub.metrics; + +import com.google.api.core.InternalApi; +import com.google.api.gax.grpc.GrpcResponseMetadata; +import com.google.api.gax.rpc.ApiCallContext; +import com.google.api.gax.rpc.ResponseObserver; +import com.google.api.gax.rpc.ServerStreamingCallable; +import com.google.api.gax.rpc.StreamController; +import io.grpc.Metadata; +import javax.annotation.Nonnull; + +/** + * Record GFE metrics. + * + *

    This class is considered an internal implementation detail and not meant to be used by + * applications. + */ +@InternalApi +public class HeaderTracerStreamingCallable + extends ServerStreamingCallable { + + private ServerStreamingCallable innerCallable; + private HeaderTracer headerTracer; + private String spanName; + + public HeaderTracerStreamingCallable( + @Nonnull ServerStreamingCallable callable, + @Nonnull HeaderTracer headerTracer, + @Nonnull String spanName) { + this.innerCallable = callable; + this.headerTracer = headerTracer; + this.spanName = spanName; + } + + @Override + public void call( + RequestT request, ResponseObserver responseObserver, ApiCallContext context) { + final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); + HeaderTracerResponseObserver innerObserver = + new HeaderTracerResponseObserver<>( + responseObserver, headerTracer, responseMetadata, spanName); + innerCallable.call(request, innerObserver, responseMetadata.addHandlers(context)); + } + + private class HeaderTracerResponseObserver implements ResponseObserver { + + private ResponseObserver outerObserver; + private HeaderTracer headerTracer; + private GrpcResponseMetadata responseMetadata; + private String spanName; + + HeaderTracerResponseObserver( + ResponseObserver observer, + HeaderTracer headerTracer, + GrpcResponseMetadata metadata, + String spanName) { + this.outerObserver = observer; + this.headerTracer = headerTracer; + this.responseMetadata = metadata; + this.spanName = spanName; + } + + @Override + public void onStart(final StreamController controller) { + outerObserver.onStart( + new StreamController() { + @Override + public void cancel() { + controller.cancel(); + } + + @Override + public void disableAutoInboundFlowControl() { + controller.disableAutoInboundFlowControl(); + } + + @Override + public void request(int count) { + controller.request(count); + } + }); + } + + @Override + public void onResponse(ResponseT response) { + outerObserver.onResponse(response); + } + + @Override + public void onError(Throwable t) { + Metadata metadata = responseMetadata.getMetadata(); + if (metadata != null) { + headerTracer.recordGfeMetrics(metadata, spanName); + } + outerObserver.onError(t); + } + + @Override + public void onComplete() { + Metadata metadata = responseMetadata.getMetadata(); + if (metadata != null) { + headerTracer.recordGfeMetrics(metadata, spanName); + } + outerObserver.onComplete(); + } + } +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerUnaryCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerUnaryCallable.java new file mode 100644 index 0000000000..0190878d13 --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerUnaryCallable.java @@ -0,0 +1,65 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub.metrics; + +import com.google.api.core.ApiFuture; +import com.google.api.core.InternalApi; +import com.google.api.gax.grpc.GrpcResponseMetadata; +import com.google.api.gax.rpc.ApiCallContext; +import com.google.api.gax.rpc.UnaryCallable; +import com.google.common.util.concurrent.MoreExecutors; +import io.grpc.Metadata; + +/** + * Record GFE metrics. + * + *

    This class is considered an internal implementation detail and not meant to be used by + * applications. + */ +@InternalApi +public class HeaderTracerUnaryCallable + extends UnaryCallable { + + private UnaryCallable innerCallable; + private HeaderTracer tracer; + private String spanName; + + public HeaderTracerUnaryCallable( + UnaryCallable callable, HeaderTracer tracer, String spanName) { + this.innerCallable = callable; + this.tracer = tracer; + this.spanName = spanName; + } + + @Override + public ApiFuture futureCall(RequestT request, ApiCallContext context) { + final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); + ApiFuture future = + innerCallable.futureCall(request, responseMetadata.addHandlers(context)); + future.addListener( + new Runnable() { + @Override + public void run() { + Metadata metadata = responseMetadata.getMetadata(); + if (metadata != null) { + tracer.recordGfeMetrics(metadata, spanName); + } + } + }, + MoreExecutors.directExecutor()); + return future; + } +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViews.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViews.java index 84d1a50fd9..1623f5972e 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViews.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViews.java @@ -31,7 +31,10 @@ public class RpcViews { RpcViewConstants.BIGTABLE_COMPLETED_OP_VIEW, RpcViewConstants.BIGTABLE_READ_ROWS_FIRST_ROW_LATENCY_VIEW, RpcViewConstants.BIGTABLE_ATTEMPT_LATENCY_VIEW, - RpcViewConstants.BIGTABLE_ATTEMPTS_PER_OP_VIEW, + RpcViewConstants.BIGTABLE_ATTEMPTS_PER_OP_VIEW); + + private static final ImmutableSet GFE_VIEW_SET = + ImmutableSet.of( RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW, RpcViewConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW); @@ -40,10 +43,22 @@ public static void registerBigtableClientViews() { registerBigtableClientViews(Stats.getViewManager()); } + /** Register views for GFE metrics. */ + public static void registerBigtableClientGfeViews() { + registerBigtableClientGfeViews(Stats.getViewManager()); + } + @VisibleForTesting static void registerBigtableClientViews(ViewManager viewManager) { for (View view : BIGTABLE_CLIENT_VIEWS_SET) { viewManager.registerView(view); } } + + @VisibleForTesting + static void registerBigtableClientGfeViews(ViewManager viewManager) { + for (View view : GFE_VIEW_SET) { + viewManager.registerView(view); + } + } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptorTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerCallableTest.java similarity index 98% rename from google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptorTest.java rename to google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerCallableTest.java index 7b83207a6c..51ee6b9c0a 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ClientHeaderInterceptorTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerCallableTest.java @@ -58,7 +58,7 @@ import org.mockito.stubbing.Answer; @RunWith(JUnit4.class) -public class ClientHeaderInterceptorTest { +public class HeaderTracerCallableTest { @Rule public MockitoRule mockitoRule = MockitoJUnit.rule(); private FakeServiceHelper serviceHelper; @@ -82,7 +82,7 @@ public class ClientHeaderInterceptorTest { @Before public void setUp() throws Exception { - RpcViews.registerBigtableClientViews(localStats.getViewManager()); + RpcViews.registerBigtableClientGfeViews(localStats.getViewManager()); // Create a server that'll inject a server-timing header with a random number and a stub that // connects to this server. From ea9bb3017c9ba009fa7b7b1d9e113fea50984383 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Mon, 30 Nov 2020 22:09:44 +0000 Subject: [PATCH 07/12] Fix based on the code review --- .../data/v2/stub/EnhancedBigtableStub.java | 33 +++++++++---------- .../v2/stub/EnhancedBigtableStubSettings.java | 6 ++-- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 025794bff1..9bb1c4b247 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -93,7 +93,6 @@ import java.io.IOException; import java.util.List; import java.util.Map; -import java.util.logging.Logger; import javax.annotation.Nonnull; /** @@ -112,8 +111,6 @@ public class EnhancedBigtableStub implements AutoCloseable { private static final String CLIENT_NAME = "Bigtable"; - private static final Logger LOGGER = Logger.getLogger(EnhancedBigtableStub.class.getName()); - private final EnhancedBigtableStubSettings settings; private final ClientContext clientContext; private final RequestContext requestContext; @@ -428,12 +425,12 @@ public Map extract( UnaryCallable> spoolable = base.all(); - HeaderTracerUnaryCallable> headerMetrics = + HeaderTracerUnaryCallable> withHeaderTracer = new HeaderTracerUnaryCallable<>( spoolable, settings.getHeaderTracer(), getSpanName(methodName).toString()); UnaryCallable> retryable = - Callables.retrying(headerMetrics, settings.sampleRowKeysSettings(), clientContext); + Callables.retrying(withHeaderTracer, settings.sampleRowKeysSettings(), clientContext); return createUserFacingUnaryCallable( methodName, new SampleRowKeysCallable(retryable, requestContext)); @@ -465,12 +462,12 @@ public Map extract(MutateRowRequest mutateRowRequest) { .build(), settings.mutateRowSettings().getRetryableCodes()); - HeaderTracerUnaryCallable headerMetrics = + HeaderTracerUnaryCallable withHeaderTracer = new HeaderTracerUnaryCallable<>( base, settings.getHeaderTracer(), getSpanName(methodName).toString()); UnaryCallable retrying = - Callables.retrying(headerMetrics, settings.mutateRowSettings(), clientContext); + Callables.retrying(withHeaderTracer, settings.mutateRowSettings(), clientContext); return createUserFacingUnaryCallable( methodName, new MutateRowCallable(retrying, requestContext)); @@ -500,10 +497,10 @@ private UnaryCallable createBulkMutateRowsCallable() { SpanName spanName = getSpanName("MutateRows"); UnaryCallable traced = new TracedUnaryCallable<>(userFacing, clientContext.getTracerFactory(), spanName); - HeaderTracerUnaryCallable headerMetric = + HeaderTracerUnaryCallable withHeaderTracer = new HeaderTracerUnaryCallable<>(traced, settings.getHeaderTracer(), spanName.toString()); - return headerMetric.withDefaultCallContext(clientContext.getDefaultCallContext()); + return withHeaderTracer.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** @@ -627,12 +624,13 @@ public Map extract( .build(), settings.checkAndMutateRowSettings().getRetryableCodes()); - HeaderTracerUnaryCallable headerMetric = - new HeaderTracerUnaryCallable<>( - base, settings.getHeaderTracer(), getSpanName(methodName).toString()); + HeaderTracerUnaryCallable + withHeaderTracer = + new HeaderTracerUnaryCallable<>( + base, settings.getHeaderTracer(), getSpanName(methodName).toString()); UnaryCallable retrying = - Callables.retrying(headerMetric, settings.checkAndMutateRowSettings(), clientContext); + Callables.retrying(withHeaderTracer, settings.checkAndMutateRowSettings(), clientContext); return createUserFacingUnaryCallable( methodName, new CheckAndMutateRowCallable(retrying, requestContext)); @@ -665,11 +663,12 @@ public Map extract(ReadModifyWriteRowRequest request) { .build(), settings.readModifyWriteRowSettings().getRetryableCodes()); String methodName = "ReadModifyWriteRow"; - HeaderTracerUnaryCallable headerMetrics = - new HeaderTracerUnaryCallable<>( - base, settings.getHeaderTracer(), getSpanName(methodName).toString()); + HeaderTracerUnaryCallable + withHeaderTracer = + new HeaderTracerUnaryCallable<>( + base, settings.getHeaderTracer(), getSpanName(methodName).toString()); UnaryCallable retrying = - Callables.retrying(headerMetrics, settings.readModifyWriteRowSettings(), clientContext); + Callables.retrying(withHeaderTracer, settings.readModifyWriteRowSettings(), clientContext); return createUserFacingUnaryCallable( methodName, new ReadModifyWriteRowCallable(retrying, requestContext)); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java index 8aaf2eae64..48647fcd1c 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java @@ -771,15 +771,13 @@ public List getPrimedTableIds() { } /** Configure the header tracer for surfacing metrics in the header. */ - @BetaApi - public Builder setHeaderTracer(HeaderTracer headerTracer) { + Builder setHeaderTracer(HeaderTracer headerTracer) { this.headerTracer = headerTracer; return this; } /** Gets the header tracer that'll be used to surface metrics in the header. */ - @BetaApi - public HeaderTracer getHeaderTracer() { + HeaderTracer getHeaderTracer() { return headerTracer; } From d2e8f8c6e1a23168dc022a39e5b5e529c02824c9 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 1 Dec 2020 02:39:22 +0000 Subject: [PATCH 08/12] clean up HeaderTracerResponseObserver --- .../metrics/HeaderTracerStreamingCallable.java | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java index f25b89f885..7f6194f7d2 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java @@ -77,23 +77,7 @@ private class HeaderTracerResponseObserver implements ResponseObserve @Override public void onStart(final StreamController controller) { - outerObserver.onStart( - new StreamController() { - @Override - public void cancel() { - controller.cancel(); - } - - @Override - public void disableAutoInboundFlowControl() { - controller.disableAutoInboundFlowControl(); - } - - @Override - public void request(int count) { - controller.request(count); - } - }); + outerObserver.onStart(controller); } @Override From 8ee3b64a1bf454d6afdfeaf55324c00bec9a5c38 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 1 Dec 2020 20:13:26 +0000 Subject: [PATCH 09/12] Add more tests for all the ops --- .../metrics/HeaderTracerCallableTest.java | 169 +++++++++++++++++- 1 file changed, 168 insertions(+), 1 deletion(-) diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerCallableTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerCallableTest.java index 51ee6b9c0a..4bfa664975 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerCallableTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerCallableTest.java @@ -21,17 +21,33 @@ import com.google.api.gax.rpc.ClientContext; import com.google.bigtable.v2.BigtableGrpc; +import com.google.bigtable.v2.CheckAndMutateRowRequest; +import com.google.bigtable.v2.CheckAndMutateRowResponse; import com.google.bigtable.v2.MutateRowRequest; import com.google.bigtable.v2.MutateRowResponse; +import com.google.bigtable.v2.MutateRowsRequest; +import com.google.bigtable.v2.MutateRowsResponse; +import com.google.bigtable.v2.ReadModifyWriteRowRequest; +import com.google.bigtable.v2.ReadModifyWriteRowResponse; import com.google.bigtable.v2.ReadRowsRequest; import com.google.bigtable.v2.ReadRowsResponse; +import com.google.bigtable.v2.SampleRowKeysRequest; +import com.google.bigtable.v2.SampleRowKeysResponse; import com.google.cloud.bigtable.data.v2.BigtableDataSettings; import com.google.cloud.bigtable.data.v2.FakeServiceHelper; +import com.google.cloud.bigtable.data.v2.models.BulkMutation; +import com.google.cloud.bigtable.data.v2.models.ConditionalRowMutation; +import com.google.cloud.bigtable.data.v2.models.Mutation; import com.google.cloud.bigtable.data.v2.models.Query; +import com.google.cloud.bigtable.data.v2.models.ReadModifyWriteRow; +import com.google.cloud.bigtable.data.v2.models.Row; +import com.google.cloud.bigtable.data.v2.models.RowCell; import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStub; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.protobuf.ByteString; import io.grpc.ForwardingServerCall.SimpleForwardingServerCall; import io.grpc.Metadata; import io.grpc.ServerCall; @@ -172,7 +188,7 @@ public void testGFELatencyMetricReadRows() throws InterruptedException { } @Test - public void testGFELatencyMetricMutateRows() throws InterruptedException { + public void testGFELatencyMetricMutateRow() throws InterruptedException { doAnswer(new MutateRowAnswer()) .when(fakeService) .mutateRow(any(MutateRowRequest.class), anyObserver(MutateRowResponse.class)); @@ -193,6 +209,102 @@ public void testGFELatencyMetricMutateRows() throws InterruptedException { assertThat(latency).isEqualTo(fakeServerTiming); } + @Test + public void testGFELatencyMetricMutateRows() throws InterruptedException { + doAnswer(new MutateRowsAnswer()) + .when(fakeService) + .mutateRows(any(MutateRowsRequest.class), anyObserver(MutateRowsResponse.class)); + + BulkMutation mutations = + BulkMutation.create(TABLE_ID) + .add("key", Mutation.create().setCell("fake-family", "fake-qualifier", "fake-value")); + stub.bulkMutateRowsCallable().call(mutations); + + Thread.sleep(WAIT_FOR_METRICS_TIME_MS); + + long latency = + StatsTestUtils.getAggregationValueAsLong( + localStats, + RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW, + ImmutableMap.of( + RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.MutateRows")), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID); + + assertThat(latency).isEqualTo(fakeServerTiming); + } + + @Test + public void testGFELatencySampleRowKeys() throws InterruptedException { + doAnswer(new SampleRowKeysAnswer()) + .when(fakeService) + .sampleRowKeys(any(SampleRowKeysRequest.class), anyObserver(SampleRowKeysResponse.class)); + stub.sampleRowKeysCallable().call(TABLE_ID); + + Thread.sleep(WAIT_FOR_METRICS_TIME_MS); + long latency = + StatsTestUtils.getAggregationValueAsLong( + localStats, + RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW, + ImmutableMap.of( + RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.SampleRowKeys")), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID); + assertThat(latency).isEqualTo(fakeServerTiming); + } + + @Test + public void testGFELatencyCheckAndMutateRow() throws InterruptedException { + doAnswer(new CheckAndMutateRowAnswer()) + .when(fakeService) + .checkAndMutateRow( + any(CheckAndMutateRowRequest.class), anyObserver(CheckAndMutateRowResponse.class)); + + ConditionalRowMutation mutation = + ConditionalRowMutation.create(TABLE_ID, "fake-key") + .then(Mutation.create().setCell("fake-family", "fake-qualifier", "fake-value")); + stub.checkAndMutateRowCallable().call(mutation); + + Thread.sleep(WAIT_FOR_METRICS_TIME_MS); + long latency = + StatsTestUtils.getAggregationValueAsLong( + localStats, + RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW, + ImmutableMap.of( + RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.CheckAndMutateRow")), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID); + assertThat(latency).isEqualTo(fakeServerTiming); + } + + @Test + public void testGFELatencyReadModifyWriteRow() throws InterruptedException { + doAnswer(new ReadModifyWriteRowAnswer()) + .when(fakeService) + .readModifyWriteRow( + any(ReadModifyWriteRowRequest.class), anyObserver(ReadModifyWriteRowResponse.class)); + + ReadModifyWriteRow request = + ReadModifyWriteRow.create(TABLE_ID, "fake-key") + .append("fake-family", "fake-qualifier", "suffix"); + stub.readModifyWriteRowCallable().call(request); + + Thread.sleep(WAIT_FOR_METRICS_TIME_MS); + long latency = + StatsTestUtils.getAggregationValueAsLong( + localStats, + RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW, + ImmutableMap.of( + RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadModifyWriteRow")), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID); + assertThat(latency).isEqualTo(fakeServerTiming); + } + @Test public void testGFEMissingHeaderMetric() throws InterruptedException { doAnswer(new ReadRowsAnswer()) @@ -266,6 +378,17 @@ public void testGFEMissingHeaderMetric() throws InterruptedException { assertThat(readRowsMissingCount).isEqualTo(readRowsCalls); } + private class ReadRowAnswer implements Answer { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + StreamObserver observer = (StreamObserver) invocation.getArguments()[1]; + observer.onNext( + Row.create(ByteString.copyFromUtf8("fake-row-key"), ImmutableList.of())); + observer.onCompleted(); + return null; + } + } + private class ReadRowsAnswer implements Answer { @Override public Object answer(InvocationOnMock invocation) throws Throwable { @@ -288,6 +411,50 @@ public Object answer(InvocationOnMock invocation) throws Throwable { } } + private class MutateRowsAnswer implements Answer { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + StreamObserver observer = + (StreamObserver) invocation.getArguments()[1]; + observer.onNext(MutateRowsResponse.getDefaultInstance()); + observer.onCompleted(); + return null; + } + } + + private class SampleRowKeysAnswer implements Answer { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + StreamObserver observer = + (StreamObserver) invocation.getArguments()[1]; + observer.onNext(SampleRowKeysResponse.getDefaultInstance()); + observer.onCompleted(); + return null; + } + } + + private class CheckAndMutateRowAnswer implements Answer { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + StreamObserver observer = + (StreamObserver) invocation.getArguments()[1]; + observer.onNext(CheckAndMutateRowResponse.getDefaultInstance()); + observer.onCompleted(); + return null; + } + } + + private class ReadModifyWriteRowAnswer implements Answer { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + StreamObserver observer = + (StreamObserver) invocation.getArguments()[1]; + observer.onNext(ReadModifyWriteRowResponse.getDefaultInstance()); + observer.onCompleted(); + return null; + } + } + private static StreamObserver anyObserver(Class returnType) { return (StreamObserver) any(returnType); } From 797e9b2ff0333fd5e4d3cae27937737ae709e3f8 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 3 Dec 2020 18:48:48 +0000 Subject: [PATCH 10/12] Improve documents, changes for directPath and more tests --- README.md | 14 +- .../data/v2/BigtableDataSettings.java | 10 +- .../data/v2/stub/EnhancedBigtableStub.java | 58 ++---- .../data/v2/stub/metrics/HeaderTracer.java | 29 ++- .../HeaderTracerStreamingCallable.java | 38 +++- .../metrics/HeaderTracerUnaryCallable.java | 61 ++++-- .../v2/stub/metrics/RpcMeasureConstants.java | 4 +- .../v2/stub/metrics/RpcViewConstants.java | 4 +- .../data/v2/stub/metrics/RpcViews.java | 19 +- .../metrics/HeaderTracerCallableTest.java | 181 +++++++----------- .../v2/stub/metrics/HeaderTracerTest.java | 17 +- 11 files changed, 233 insertions(+), 202 deletions(-) diff --git a/README.md b/README.md index 2f3b015d6e..7779c39000 100644 --- a/README.md +++ b/README.md @@ -262,12 +262,22 @@ metrics will be tagged with: each client RPC, tagged by operation name and the attempt status. Under normal circumstances, this will be identical to op_latency. However, when the client receives transient errors, op_latency will be the sum of all attempt_latencies - and the exponential delays + and the exponential delays. * `cloud.google.com/java/bigtable/attempts_per_op`: A distribution of attempts that each operation required, tagged by operation name and final operation status. Under normal circumstances, this will be 1. +#### GFE metric views: + +* `cloud.google.com/java/bigtable/gfe_latency`: A distribution of the latency + between Google's network receives an RPC and reads back the first byte of + the response. + +* `cloud.google.com/java/bigtable/gfe_header_missing_count`: A counter of the + number of RPC responses received without the server-timing header, which + indicates that the request probably never reached Google's network. + By default, the functionality is disabled. For example to enable metrics using [Google Stackdriver](https://cloud.google.com/monitoring/docs/): @@ -323,6 +333,8 @@ StackdriverStatsExporter.createAndRegister( ); BigtableDataSettings.enableOpenCensusStats(); +// Enable GFE metric views +BigtableDataSettings.enableGfeOpenCensusStats(); ``` ## Version Conflicts diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java index ff20075f6c..3002aa6113 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java @@ -175,7 +175,15 @@ public static void enableOpenCensusStats() { // io.opencensus.contrib.grpc.metrics.RpcViews.registerClientGrpcBasicViews(); } - /** Enables OpenCensus GFE metric aggregations. */ + /** + * Enables OpenCensus GFE metric aggregations. + * + *

    This will register views for gfe_latency and gfe_header_missing_count metrics. + * + *

    gfe_latency measures the latency between Google's network receives an RPC and reads back the + * first byte of the response. gfe_header_missing_count is a counter of the number of RPC + * responses received without the server-timing header. + */ @BetaApi("OpenCensus stats integration is currently unstable and may change in the future") public static void enableGfeOpenCensusStats() { com.google.cloud.bigtable.data.v2.stub.metrics.RpcViews.registerBigtableClientGfeViews(); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 9bb1c4b247..448390396f 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -164,6 +164,15 @@ public static EnhancedBigtableStubSettings finalizeSettings( .build()); } + ImmutableMap attributes = + ImmutableMap.builder() + .put(RpcMeasureConstants.BIGTABLE_PROJECT_ID, TagValue.create(settings.getProjectId())) + .put( + RpcMeasureConstants.BIGTABLE_INSTANCE_ID, TagValue.create(settings.getInstanceId())) + .put( + RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID, + TagValue.create(settings.getAppProfileId())) + .build(); // Inject Opencensus instrumentation builder.setTracerFactory( new CompositeTracerFactory( @@ -189,20 +198,7 @@ public static EnhancedBigtableStubSettings finalizeSettings( GaxProperties.getLibraryVersion(EnhancedBigtableStubSettings.class)) .build()), // Add OpenCensus Metrics - MetricsTracerFactory.create( - tagger, - stats, - ImmutableMap.builder() - .put( - RpcMeasureConstants.BIGTABLE_PROJECT_ID, - TagValue.create(settings.getProjectId())) - .put( - RpcMeasureConstants.BIGTABLE_INSTANCE_ID, - TagValue.create(settings.getInstanceId())) - .put( - RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID, - TagValue.create(settings.getAppProfileId())) - .build()), + MetricsTracerFactory.create(tagger, stats, attributes), // Add user configured tracer settings.getTracerFactory()))); builder.setHeaderTracer( @@ -211,18 +207,7 @@ public static EnhancedBigtableStubSettings finalizeSettings( .toBuilder() .setStats(stats) .setTagger(tagger) - .setStatsAttributes( - ImmutableMap.builder() - .put( - RpcMeasureConstants.BIGTABLE_PROJECT_ID, - TagValue.create(settings.getProjectId())) - .put( - RpcMeasureConstants.BIGTABLE_INSTANCE_ID, - TagValue.create(settings.getInstanceId())) - .put( - RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID, - TagValue.create(settings.getAppProfileId())) - .build()) + .setStatsAttributes(attributes) .build()); return builder.build(); } @@ -425,7 +410,7 @@ public Map extract( UnaryCallable> spoolable = base.all(); - HeaderTracerUnaryCallable> withHeaderTracer = + UnaryCallable> withHeaderTracer = new HeaderTracerUnaryCallable<>( spoolable, settings.getHeaderTracer(), getSpanName(methodName).toString()); @@ -462,7 +447,7 @@ public Map extract(MutateRowRequest mutateRowRequest) { .build(), settings.mutateRowSettings().getRetryableCodes()); - HeaderTracerUnaryCallable withHeaderTracer = + UnaryCallable withHeaderTracer = new HeaderTracerUnaryCallable<>( base, settings.getHeaderTracer(), getSpanName(methodName).toString()); @@ -497,7 +482,7 @@ private UnaryCallable createBulkMutateRowsCallable() { SpanName spanName = getSpanName("MutateRows"); UnaryCallable traced = new TracedUnaryCallable<>(userFacing, clientContext.getTracerFactory(), spanName); - HeaderTracerUnaryCallable withHeaderTracer = + UnaryCallable withHeaderTracer = new HeaderTracerUnaryCallable<>(traced, settings.getHeaderTracer(), spanName.toString()); return withHeaderTracer.withDefaultCallContext(clientContext.getDefaultCallContext()); @@ -624,10 +609,9 @@ public Map extract( .build(), settings.checkAndMutateRowSettings().getRetryableCodes()); - HeaderTracerUnaryCallable - withHeaderTracer = - new HeaderTracerUnaryCallable<>( - base, settings.getHeaderTracer(), getSpanName(methodName).toString()); + UnaryCallable withHeaderTracer = + new HeaderTracerUnaryCallable<>( + base, settings.getHeaderTracer(), getSpanName(methodName).toString()); UnaryCallable retrying = Callables.retrying(withHeaderTracer, settings.checkAndMutateRowSettings(), clientContext); @@ -663,10 +647,10 @@ public Map extract(ReadModifyWriteRowRequest request) { .build(), settings.readModifyWriteRowSettings().getRetryableCodes()); String methodName = "ReadModifyWriteRow"; - HeaderTracerUnaryCallable - withHeaderTracer = - new HeaderTracerUnaryCallable<>( - base, settings.getHeaderTracer(), getSpanName(methodName).toString()); + UnaryCallable withHeaderTracer = + new HeaderTracerUnaryCallable<>( + base, settings.getHeaderTracer(), getSpanName(methodName).toString()); + UnaryCallable retrying = Callables.retrying(withHeaderTracer, settings.readModifyWriteRowSettings(), clientContext); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracer.java index 877b77b748..f3eb0ef1e2 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracer.java @@ -18,7 +18,6 @@ import com.google.api.core.InternalApi; import com.google.auto.value.AutoValue; import com.google.common.base.MoreObjects; -import com.google.common.base.Preconditions; import io.grpc.Metadata; import io.opencensus.stats.MeasureMap; import io.opencensus.stats.Stats; @@ -33,15 +32,14 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.annotation.Nonnull; -import javax.annotation.Nullable; @InternalApi @AutoValue public abstract class HeaderTracer { - public static final Metadata.Key SERVER_TIMING_HEADER_KEY = + private static final Metadata.Key SERVER_TIMING_HEADER_KEY = Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER); - public static final Pattern SERVER_TIMING_HEADER_PATTERN = Pattern.compile(".*dur=(?\\d+)"); + private static final Pattern SERVER_TIMING_HEADER_PATTERN = Pattern.compile(".*dur=(?\\d+)"); @AutoValue.Builder public abstract static class Builder { @@ -52,19 +50,10 @@ public abstract static class Builder { public abstract Builder setStatsAttributes(@Nonnull Map statsAttributes); - public abstract Tagger getTagger(); - - public abstract StatsRecorder getStats(); - - public abstract Map getStatsAttributes(); - abstract HeaderTracer autoBuild(); public HeaderTracer build() { HeaderTracer headerTracer = autoBuild(); - Preconditions.checkNotNull(headerTracer.getStats(), "StatsRecorder must be set"); - Preconditions.checkNotNull(headerTracer.getTagger(), "Tagger must be set"); - Preconditions.checkNotNull(headerTracer.getStatsAttributes(), "Stats attributes must be set"); return headerTracer; } // @@ -76,7 +65,11 @@ public HeaderTracer build() { public abstract Map getStatsAttributes(); - public void recordGfeMetrics(@Nonnull Metadata metadata, String spanName) { + /** + * If the header has a server-timing field, extract the metric and publish it to OpenCensus. + * Otherwise increment the gfe header missing counter by 1. + */ + public void recordGfeMetadata(@Nonnull Metadata metadata, @Nonnull String spanName) { MeasureMap measures = getStats().newMeasureMap(); if (metadata.get(SERVER_TIMING_HEADER_KEY) != null) { String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY); @@ -92,7 +85,13 @@ public void recordGfeMetrics(@Nonnull Metadata metadata, String spanName) { measures.record(newTagCtxBuilder(spanName).build()); } - private TagContextBuilder newTagCtxBuilder(@Nullable String span) { + public void recordGfeMissingHeader(@Nonnull String spanName) { + MeasureMap measures = + getStats().newMeasureMap().put(RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT, 1L); + measures.record(newTagCtxBuilder(spanName).build()); + } + + private TagContextBuilder newTagCtxBuilder(String span) { TagContextBuilder tagContextBuilder = getTagger().currentBuilder(); if (span != null) { tagContextBuilder.putLocal(RpcMeasureConstants.BIGTABLE_OP, TagValue.create(span)); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java index 7f6194f7d2..8c5932f9c9 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java @@ -21,11 +21,20 @@ import com.google.api.gax.rpc.ResponseObserver; import com.google.api.gax.rpc.ServerStreamingCallable; import com.google.api.gax.rpc.StreamController; +import com.google.common.base.Preconditions; import io.grpc.Metadata; import javax.annotation.Nonnull; /** - * Record GFE metrics. + * This callable will inject a {@link GrpcResponseMetadata} to access the headers and trailers + * returned by gRPC methods upon completion. The {@link HeaderTracer} will process metrics that were + * injected in the header/trailer and publish them to OpenCensus. If {@link + * GrpcResponseMetadata#getMetadata()} returned null, it probably means that the request has never + * reached GFE, and it'll increment the gfe_header_missing_counter in this case. + * + *

    If GFE metrics are not registered in {@link RpcViews}, skip injecting GrpcResponseMetadata. + * This is for the case where direct path is enabled, all the requests won't go through GFE and + * therefore won't have the server-timing header. * *

    This class is considered an internal implementation detail and not meant to be used by * applications. @@ -34,7 +43,7 @@ public class HeaderTracerStreamingCallable extends ServerStreamingCallable { - private ServerStreamingCallable innerCallable; + private final ServerStreamingCallable innerCallable; private HeaderTracer headerTracer; private String spanName; @@ -42,6 +51,9 @@ public HeaderTracerStreamingCallable( @Nonnull ServerStreamingCallable callable, @Nonnull HeaderTracer headerTracer, @Nonnull String spanName) { + Preconditions.checkNotNull(callable, "Inner callable must be set"); + Preconditions.checkNotNull(headerTracer, "HeaderTracer must be set"); + Preconditions.checkNotNull(spanName, "Span name must be set"); this.innerCallable = callable; this.headerTracer = headerTracer; this.spanName = spanName; @@ -51,10 +63,14 @@ public HeaderTracerStreamingCallable( public void call( RequestT request, ResponseObserver responseObserver, ApiCallContext context) { final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); - HeaderTracerResponseObserver innerObserver = - new HeaderTracerResponseObserver<>( - responseObserver, headerTracer, responseMetadata, spanName); - innerCallable.call(request, innerObserver, responseMetadata.addHandlers(context)); + if (RpcViews.isGfeMetricsRegistered()) { + HeaderTracerResponseObserver innerObserver = + new HeaderTracerResponseObserver<>( + responseObserver, headerTracer, responseMetadata, spanName); + innerCallable.call(request, innerObserver, responseMetadata.addHandlers(context)); + } else { + innerCallable.call(request, responseObserver, context); + } } private class HeaderTracerResponseObserver implements ResponseObserver { @@ -87,9 +103,13 @@ public void onResponse(ResponseT response) { @Override public void onError(Throwable t) { + // server-timing metric will be added through GrpcResponseMetadata#onHeaders(Metadata), + // so it's not checking trailing metadata here. Metadata metadata = responseMetadata.getMetadata(); if (metadata != null) { - headerTracer.recordGfeMetrics(metadata, spanName); + headerTracer.recordGfeMetadata(metadata, spanName); + } else { + headerTracer.recordGfeMissingHeader(spanName); } outerObserver.onError(t); } @@ -98,7 +118,9 @@ public void onError(Throwable t) { public void onComplete() { Metadata metadata = responseMetadata.getMetadata(); if (metadata != null) { - headerTracer.recordGfeMetrics(metadata, spanName); + headerTracer.recordGfeMetadata(metadata, spanName); + } else { + headerTracer.recordGfeMissingHeader(spanName); } outerObserver.onComplete(); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerUnaryCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerUnaryCallable.java index 0190878d13..9ea28e8f8a 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerUnaryCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerUnaryCallable.java @@ -20,11 +20,21 @@ import com.google.api.gax.grpc.GrpcResponseMetadata; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.UnaryCallable; +import com.google.common.base.Preconditions; import com.google.common.util.concurrent.MoreExecutors; import io.grpc.Metadata; +import javax.annotation.Nonnull; /** - * Record GFE metrics. + * This callable will inject a {@link GrpcResponseMetadata} to access the headers and trailers + * returned by gRPC methods upon completion. The {@link HeaderTracer} will process metrics that were + * injected in the header/trailer and publish them to OpenCensus. If {@link + * GrpcResponseMetadata#getMetadata()} returned null, it probably means that the request has never + * reached GFE, and it'll increment the gfe_header_missing_counter in this case. + * + *

    If GFE metrics are not registered in {@link RpcViews}, skip injecting GrpcResponseMetadata. + * This is for the case where direct path is enabled, all the requests won't go through GFE and + * therefore won't have the server-timing header. * *

    This class is considered an internal implementation detail and not meant to be used by * applications. @@ -33,33 +43,44 @@ public class HeaderTracerUnaryCallable extends UnaryCallable { - private UnaryCallable innerCallable; - private HeaderTracer tracer; + private final UnaryCallable innerCallable; + private HeaderTracer headerTracer; private String spanName; public HeaderTracerUnaryCallable( - UnaryCallable callable, HeaderTracer tracer, String spanName) { - this.innerCallable = callable; - this.tracer = tracer; + @Nonnull UnaryCallable innerCallable, + @Nonnull HeaderTracer headerTracer, + @Nonnull String spanName) { + Preconditions.checkNotNull(innerCallable, "Inner callable must be set"); + Preconditions.checkNotNull(headerTracer, "HeaderTracer must be set"); + Preconditions.checkNotNull(spanName, "Span name must be set"); + this.innerCallable = innerCallable; + this.headerTracer = headerTracer; this.spanName = spanName; } @Override public ApiFuture futureCall(RequestT request, ApiCallContext context) { - final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); - ApiFuture future = - innerCallable.futureCall(request, responseMetadata.addHandlers(context)); - future.addListener( - new Runnable() { - @Override - public void run() { - Metadata metadata = responseMetadata.getMetadata(); - if (metadata != null) { - tracer.recordGfeMetrics(metadata, spanName); + if (RpcViews.isGfeMetricsRegistered()) { + final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); + ApiFuture future = + innerCallable.futureCall(request, responseMetadata.addHandlers(context)); + future.addListener( + new Runnable() { + @Override + public void run() { + Metadata metadata = responseMetadata.getMetadata(); + if (metadata != null) { + headerTracer.recordGfeMetadata(metadata, spanName); + } else { + headerTracer.recordGfeMissingHeader(spanName); + } } - } - }, - MoreExecutors.directExecutor()); - return future; + }, + MoreExecutors.directExecutor()); + return future; + } else { + return innerCallable.futureCall(request, context); + } } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java index 54cf10851e..4774cdb05b 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java @@ -79,13 +79,13 @@ public class RpcMeasureConstants { public static final MeasureLong BIGTABLE_GFE_LATENCY = MeasureLong.create( "cloud.google.com/java/bigtable/gfe_latency", - "GFE t4t7 latency. Time between GFE receives the first byte of a request and reads the first byte of the response", + "Latency between Google's network receives an RPC and reads back the first byte of the response", MILLISECOND); /** Number of responses without the server-timing header. */ public static final MeasureLong BIGTABLE_GFE_HEADER_MISSING_COUNT = MeasureLong.create( "cloud.google.com/java/bigtable/gfe_header_missing_count", - "Number of responses without the server-timing header", + "Number of RPC responses without the server-timing header, most likely means that the RPC never reached Google's network", COUNT); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java index e84c9e7a2b..84aa635503 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java @@ -132,7 +132,7 @@ class RpcViewConstants { static final View BIGTABLE_GFE_LATENCY_VIEW = View.create( View.Name.create("cloud.google.com/java/bigtable/gfe_latency"), - "GFE t4t7 latency in msecs", + "Latency between Google's network receives an RPC and reads back the first byte of the response", BIGTABLE_GFE_LATENCY, AGGREGATION_WITH_MILLIS_HISTOGRAM, ImmutableList.of( @@ -141,7 +141,7 @@ class RpcViewConstants { static final View BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW = View.create( View.Name.create("cloud.google.com/java/bigtable/gfe_header_missing_count"), - "Number of responses without the server-timing header", + "Number of RPC responses without the server-timing header, most likely means that the RPC never reached Google's network", BIGTABLE_GFE_HEADER_MISSING_COUNT, SUM, ImmutableList.of( diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViews.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViews.java index 1623f5972e..9e8f6084a2 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViews.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViews.java @@ -38,12 +38,19 @@ public class RpcViews { RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW, RpcViewConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW); + private static boolean gfeMetricsRegistered = false; + /** Registers all Bigtable specific views. */ public static void registerBigtableClientViews() { registerBigtableClientViews(Stats.getViewManager()); } - /** Register views for GFE metrics. */ + /** + * Register views for GFE metrics, including gfe_latency and gfe_header_missing_count. gfe_latency + * measures the latency between Google's network receives an RPC and reads back the first byte of + * the response. gfe_header_missing_count is a counter of the number of RPC responses without a + * server-timing header. + */ public static void registerBigtableClientGfeViews() { registerBigtableClientGfeViews(Stats.getViewManager()); } @@ -60,5 +67,15 @@ static void registerBigtableClientGfeViews(ViewManager viewManager) { for (View view : GFE_VIEW_SET) { viewManager.registerView(view); } + gfeMetricsRegistered = true; + } + + static boolean isGfeMetricsRegistered() { + return gfeMetricsRegistered; + } + + @VisibleForTesting + static void setGfeMetricsRegistered(boolean gfeMetricsRegistered) { + RpcViews.gfeMetricsRegistered = gfeMetricsRegistered; } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerCallableTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerCallableTest.java index 4bfa664975..9538b6a135 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerCallableTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerCallableTest.java @@ -16,11 +16,11 @@ package com.google.cloud.bigtable.data.v2.stub.metrics; import static com.google.common.truth.Truth.assertThat; -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.doAnswer; +import static org.junit.Assert.fail; import com.google.api.gax.rpc.ClientContext; -import com.google.bigtable.v2.BigtableGrpc; +import com.google.api.gax.rpc.UnavailableException; +import com.google.bigtable.v2.BigtableGrpc.BigtableImplBase; import com.google.bigtable.v2.CheckAndMutateRowRequest; import com.google.bigtable.v2.CheckAndMutateRowResponse; import com.google.bigtable.v2.MutateRowRequest; @@ -35,57 +35,49 @@ import com.google.bigtable.v2.SampleRowKeysResponse; import com.google.cloud.bigtable.data.v2.BigtableDataSettings; import com.google.cloud.bigtable.data.v2.FakeServiceHelper; +import com.google.cloud.bigtable.data.v2.internal.NameUtil; import com.google.cloud.bigtable.data.v2.models.BulkMutation; import com.google.cloud.bigtable.data.v2.models.ConditionalRowMutation; import com.google.cloud.bigtable.data.v2.models.Mutation; import com.google.cloud.bigtable.data.v2.models.Query; import com.google.cloud.bigtable.data.v2.models.ReadModifyWriteRow; -import com.google.cloud.bigtable.data.v2.models.Row; -import com.google.cloud.bigtable.data.v2.models.RowCell; import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStub; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.protobuf.ByteString; import io.grpc.ForwardingServerCall.SimpleForwardingServerCall; import io.grpc.Metadata; import io.grpc.ServerCall; import io.grpc.ServerCallHandler; import io.grpc.ServerInterceptor; +import io.grpc.Status; +import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; import io.opencensus.impl.stats.StatsComponentImpl; import io.opencensus.stats.StatsComponent; +import io.opencensus.stats.ViewData; import io.opencensus.tags.TagKey; import io.opencensus.tags.TagValue; import io.opencensus.tags.Tags; import java.util.Random; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.After; import org.junit.Before; -import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import org.mockito.Answers; -import org.mockito.Mock; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.junit.MockitoJUnit; -import org.mockito.junit.MockitoRule; -import org.mockito.stubbing.Answer; @RunWith(JUnit4.class) public class HeaderTracerCallableTest { - @Rule public MockitoRule mockitoRule = MockitoJUnit.rule(); - private FakeServiceHelper serviceHelper; private FakeServiceHelper serviceHelperNoHeader; - @Mock(answer = Answers.CALLS_REAL_METHODS) - private BigtableGrpc.BigtableImplBase fakeService; + private FakeService fakeService = new FakeService(); - private StatsComponent localStats = new StatsComponentImpl(); + private final StatsComponent localStats = new StatsComponentImpl(); private EnhancedBigtableStub stub; private EnhancedBigtableStub noHeaderStub; + private int attempts; private static final String PROJECT_ID = "fake-project"; private static final String INSTANCE_ID = "fake-instance"; @@ -94,7 +86,7 @@ public class HeaderTracerCallableTest { private static final long WAIT_FOR_METRICS_TIME_MS = 1_000; - private int fakeServerTiming; + private AtomicInteger fakeServerTiming; @Before public void setUp() throws Exception { @@ -102,7 +94,7 @@ public void setUp() throws Exception { // Create a server that'll inject a server-timing header with a random number and a stub that // connects to this server. - fakeServerTiming = new Random().nextInt(1000) + 1; + fakeServerTiming = new AtomicInteger(new Random().nextInt(1000) + 1); serviceHelper = new FakeServiceHelper( new ServerInterceptor() { @@ -117,7 +109,7 @@ public ServerCall.Listener interceptCall( public void sendHeaders(Metadata headers) { headers.put( Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER), - String.format("gfet4t7; dur=%d", fakeServerTiming)); + String.format("gfet4t7; dur=%d", fakeServerTiming.get())); super.sendHeaders(headers); } }, @@ -136,6 +128,7 @@ public void sendHeaders(Metadata headers) { EnhancedBigtableStubSettings stubSettings = EnhancedBigtableStub.finalizeSettings( settings.getStubSettings(), Tags.getTagger(), localStats.getStatsRecorder()); + attempts = stubSettings.readRowsSettings().getRetrySettings().getMaxAttempts(); stub = new EnhancedBigtableStub(stubSettings, ClientContext.create(stubSettings)); // Create another server without injecting the server-timing header and another stub that @@ -166,10 +159,6 @@ public void tearDown() { @Test public void testGFELatencyMetricReadRows() throws InterruptedException { - doAnswer(new ReadRowsAnswer()) - .when(fakeService) - .readRows(any(ReadRowsRequest.class), anyObserver(ReadRowsResponse.class)); - stub.readRowsCallable().call(Query.create(TABLE_ID)); Thread.sleep(WAIT_FOR_METRICS_TIME_MS); @@ -184,15 +173,11 @@ public void testGFELatencyMetricReadRows() throws InterruptedException { INSTANCE_ID, APP_PROFILE_ID); - assertThat(latency).isEqualTo(fakeServerTiming); + assertThat(latency).isEqualTo(fakeServerTiming.get()); } @Test public void testGFELatencyMetricMutateRow() throws InterruptedException { - doAnswer(new MutateRowAnswer()) - .when(fakeService) - .mutateRow(any(MutateRowRequest.class), anyObserver(MutateRowResponse.class)); - stub.mutateRowCallable().call(RowMutation.create(TABLE_ID, "fake-key")); Thread.sleep(WAIT_FOR_METRICS_TIME_MS); @@ -206,15 +191,11 @@ public void testGFELatencyMetricMutateRow() throws InterruptedException { INSTANCE_ID, APP_PROFILE_ID); - assertThat(latency).isEqualTo(fakeServerTiming); + assertThat(latency).isEqualTo(fakeServerTiming.get()); } @Test public void testGFELatencyMetricMutateRows() throws InterruptedException { - doAnswer(new MutateRowsAnswer()) - .when(fakeService) - .mutateRows(any(MutateRowsRequest.class), anyObserver(MutateRowsResponse.class)); - BulkMutation mutations = BulkMutation.create(TABLE_ID) .add("key", Mutation.create().setCell("fake-family", "fake-qualifier", "fake-value")); @@ -232,14 +213,11 @@ public void testGFELatencyMetricMutateRows() throws InterruptedException { INSTANCE_ID, APP_PROFILE_ID); - assertThat(latency).isEqualTo(fakeServerTiming); + assertThat(latency).isEqualTo(fakeServerTiming.get()); } @Test public void testGFELatencySampleRowKeys() throws InterruptedException { - doAnswer(new SampleRowKeysAnswer()) - .when(fakeService) - .sampleRowKeys(any(SampleRowKeysRequest.class), anyObserver(SampleRowKeysResponse.class)); stub.sampleRowKeysCallable().call(TABLE_ID); Thread.sleep(WAIT_FOR_METRICS_TIME_MS); @@ -252,16 +230,11 @@ public void testGFELatencySampleRowKeys() throws InterruptedException { PROJECT_ID, INSTANCE_ID, APP_PROFILE_ID); - assertThat(latency).isEqualTo(fakeServerTiming); + assertThat(latency).isEqualTo(fakeServerTiming.get()); } @Test public void testGFELatencyCheckAndMutateRow() throws InterruptedException { - doAnswer(new CheckAndMutateRowAnswer()) - .when(fakeService) - .checkAndMutateRow( - any(CheckAndMutateRowRequest.class), anyObserver(CheckAndMutateRowResponse.class)); - ConditionalRowMutation mutation = ConditionalRowMutation.create(TABLE_ID, "fake-key") .then(Mutation.create().setCell("fake-family", "fake-qualifier", "fake-value")); @@ -277,16 +250,11 @@ public void testGFELatencyCheckAndMutateRow() throws InterruptedException { PROJECT_ID, INSTANCE_ID, APP_PROFILE_ID); - assertThat(latency).isEqualTo(fakeServerTiming); + assertThat(latency).isEqualTo(fakeServerTiming.get()); } @Test public void testGFELatencyReadModifyWriteRow() throws InterruptedException { - doAnswer(new ReadModifyWriteRowAnswer()) - .when(fakeService) - .readModifyWriteRow( - any(ReadModifyWriteRowRequest.class), anyObserver(ReadModifyWriteRowResponse.class)); - ReadModifyWriteRow request = ReadModifyWriteRow.create(TABLE_ID, "fake-key") .append("fake-family", "fake-qualifier", "suffix"); @@ -302,18 +270,11 @@ public void testGFELatencyReadModifyWriteRow() throws InterruptedException { PROJECT_ID, INSTANCE_ID, APP_PROFILE_ID); - assertThat(latency).isEqualTo(fakeServerTiming); + assertThat(latency).isEqualTo(fakeServerTiming.get()); } @Test public void testGFEMissingHeaderMetric() throws InterruptedException { - doAnswer(new ReadRowsAnswer()) - .when(fakeService) - .readRows(any(ReadRowsRequest.class), anyObserver(ReadRowsResponse.class)); - doAnswer(new MutateRowAnswer()) - .when(fakeService) - .mutateRow(any(MutateRowRequest.class), anyObserver(MutateRowResponse.class)); - // Make a few calls to the server which will inject the server-timing header and the counter // should be 0. stub.readRowsCallable().call(Query.create(TABLE_ID)); @@ -378,84 +339,90 @@ public void testGFEMissingHeaderMetric() throws InterruptedException { assertThat(readRowsMissingCount).isEqualTo(readRowsCalls); } - private class ReadRowAnswer implements Answer { - @Override - public Object answer(InvocationOnMock invocation) throws Throwable { - StreamObserver observer = (StreamObserver) invocation.getArguments()[1]; - observer.onNext( - Row.create(ByteString.copyFromUtf8("fake-row-key"), ImmutableList.of())); - observer.onCompleted(); - return null; + @Test + public void testMetricsWithErrorResponse() throws InterruptedException { + try { + stub.readRowsCallable().call(Query.create("random-table-id")).iterator().next(); + fail("readrows should throw exception"); + } catch (Exception e) { + assertThat(e).isInstanceOf(UnavailableException.class); } + + Thread.sleep(WAIT_FOR_METRICS_TIME_MS); + long missingCount = + StatsTestUtils.getAggregationValueAsLong( + localStats, + RpcViewConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW, + ImmutableMap.of(RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadRows")), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID); + assertThat(missingCount).isEqualTo(attempts); + } + + @Test + public void testCallableBypassed() throws InterruptedException { + RpcViews.setGfeMetricsRegistered(false); + stub.readRowsCallable().call(Query.create(TABLE_ID)); + Thread.sleep(WAIT_FOR_METRICS_TIME_MS); + ViewData headerMissingView = + localStats + .getViewManager() + .getView(RpcViewConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW.getName()); + ViewData latencyView = + localStats.getViewManager().getView(RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW.getName()); + // Verify that the view is registered by it's not collecting metrics + assertThat(headerMissingView).isNotNull(); + assertThat(latencyView).isNotNull(); + assertThat(headerMissingView.getAggregationMap()).isEmpty(); + assertThat(latencyView.getAggregationMap()).isEmpty(); } - private class ReadRowsAnswer implements Answer { + private class FakeService extends BigtableImplBase { + private final String defaultTableName = + NameUtil.formatTableName(PROJECT_ID, INSTANCE_ID, TABLE_ID); + @Override - public Object answer(InvocationOnMock invocation) throws Throwable { - StreamObserver observer = - (StreamObserver) invocation.getArguments()[1]; + public void readRows(ReadRowsRequest request, StreamObserver observer) { + if (!request.getTableName().equals(defaultTableName)) { + observer.onError(new StatusRuntimeException(Status.UNAVAILABLE)); + return; + } observer.onNext(ReadRowsResponse.getDefaultInstance()); observer.onCompleted(); - return null; } - } - private class MutateRowAnswer implements Answer { @Override - public Object answer(InvocationOnMock invocation) throws Throwable { - StreamObserver observer = - (StreamObserver) invocation.getArguments()[1]; + public void mutateRow(MutateRowRequest request, StreamObserver observer) { observer.onNext(MutateRowResponse.getDefaultInstance()); observer.onCompleted(); - return null; } - } - private class MutateRowsAnswer implements Answer { @Override - public Object answer(InvocationOnMock invocation) throws Throwable { - StreamObserver observer = - (StreamObserver) invocation.getArguments()[1]; + public void mutateRows(MutateRowsRequest request, StreamObserver observer) { observer.onNext(MutateRowsResponse.getDefaultInstance()); observer.onCompleted(); - return null; } - } - private class SampleRowKeysAnswer implements Answer { @Override - public Object answer(InvocationOnMock invocation) throws Throwable { - StreamObserver observer = - (StreamObserver) invocation.getArguments()[1]; + public void sampleRowKeys( + SampleRowKeysRequest request, StreamObserver observer) { observer.onNext(SampleRowKeysResponse.getDefaultInstance()); observer.onCompleted(); - return null; } - } - private class CheckAndMutateRowAnswer implements Answer { @Override - public Object answer(InvocationOnMock invocation) throws Throwable { - StreamObserver observer = - (StreamObserver) invocation.getArguments()[1]; + public void checkAndMutateRow( + CheckAndMutateRowRequest request, StreamObserver observer) { observer.onNext(CheckAndMutateRowResponse.getDefaultInstance()); observer.onCompleted(); - return null; } - } - private class ReadModifyWriteRowAnswer implements Answer { @Override - public Object answer(InvocationOnMock invocation) throws Throwable { - StreamObserver observer = - (StreamObserver) invocation.getArguments()[1]; + public void readModifyWriteRow( + ReadModifyWriteRowRequest request, StreamObserver observer) { observer.onNext(ReadModifyWriteRowResponse.getDefaultInstance()); observer.onCompleted(); - return null; } } - - private static StreamObserver anyObserver(Class returnType) { - return (StreamObserver) any(returnType); - } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java index fb8457af72..ec616d0e98 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java @@ -36,12 +36,13 @@ public class HeaderTracerTest { private StatsComponent localStats = new StatsComponentImpl(); @Test - public void testEmptyBuilder() { + public void testDefaultBuilder() { HeaderTracer.Builder builder = HeaderTracer.newBuilder(); - assertThat(builder.getStats()).isNotNull(); - assertThat(builder.getTagger()).isNotNull(); - assertThat(builder.getStatsAttributes()).isNotNull(); - assertThat(builder.getStatsAttributes()).isEmpty(); + HeaderTracer tracer = builder.build(); + assertThat(tracer.getStats()).isNotNull(); + assertThat(tracer.getTagger()).isNotNull(); + assertThat(tracer.getStatsAttributes()).isNotNull(); + assertThat(tracer.getStatsAttributes()).isEmpty(); } @Test @@ -69,8 +70,8 @@ public void testToBuilder() { HeaderTracer headerTracer = builder.build(); HeaderTracer.Builder newBuilder = headerTracer.toBuilder(); - assertThat(newBuilder.getStats()).isEqualTo(stats); - assertThat(newBuilder.getTagger()).isEqualTo(tagger); - assertThat(newBuilder.getStatsAttributes()).isEqualTo(attrs); + assertThat(newBuilder.build().getStats()).isEqualTo(stats); + assertThat(newBuilder.build().getTagger()).isEqualTo(tagger); + assertThat(newBuilder.build().getStatsAttributes()).isEqualTo(attrs); } } From 20ec8d6bd0fdb8df8dc27265606986a07abca661 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 3 Dec 2020 19:07:38 +0000 Subject: [PATCH 11/12] Small fixes in the doc --- README.md | 2 +- .../bigtable/data/v2/stub/metrics/RpcMeasureConstants.java | 2 +- .../cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java | 2 +- .../cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index af5a6a5b55..8f58eac543 100644 --- a/README.md +++ b/README.md @@ -268,7 +268,7 @@ metrics will be tagged with: each operation required, tagged by operation name and final operation status. Under normal circumstances, this will be 1. -#### GFE metric views: +### GFE metric views: * `cloud.google.com/java/bigtable/gfe_latency`: A distribution of the latency between Google's network receives an RPC and reads back the first byte of diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java index 4774cdb05b..e6e5c70db1 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcMeasureConstants.java @@ -86,6 +86,6 @@ public class RpcMeasureConstants { public static final MeasureLong BIGTABLE_GFE_HEADER_MISSING_COUNT = MeasureLong.create( "cloud.google.com/java/bigtable/gfe_header_missing_count", - "Number of RPC responses without the server-timing header, most likely means that the RPC never reached Google's network", + "Number of RPC responses received without the server-timing header, most likely means that the RPC never reached Google's network", COUNT); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java index 84aa635503..8a14c01b13 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/RpcViewConstants.java @@ -141,7 +141,7 @@ class RpcViewConstants { static final View BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW = View.create( View.Name.create("cloud.google.com/java/bigtable/gfe_header_missing_count"), - "Number of RPC responses without the server-timing header, most likely means that the RPC never reached Google's network", + "Number of RPC responses received without the server-timing header, most likely means that the RPC never reached Google's network", BIGTABLE_GFE_HEADER_MISSING_COUNT, SUM, ImmutableList.of( diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java index ec616d0e98..30e24dbfe3 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerTest.java @@ -33,7 +33,7 @@ @RunWith(JUnit4.class) public class HeaderTracerTest { - private StatsComponent localStats = new StatsComponentImpl(); + private final StatsComponent localStats = new StatsComponentImpl(); @Test public void testDefaultBuilder() { From 757d12b3d3735d02c8bddabf7fc7ac63ab82e02a Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 10 Dec 2020 01:14:37 +0000 Subject: [PATCH 12/12] small clean up --- .../stub/metrics/HeaderTracerStreamingCallable.java | 13 +++++-------- .../v2/stub/metrics/HeaderTracerUnaryCallable.java | 13 +++++-------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java index 8c5932f9c9..fdca9297aa 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java @@ -44,19 +44,16 @@ public class HeaderTracerStreamingCallable extends ServerStreamingCallable { private final ServerStreamingCallable innerCallable; - private HeaderTracer headerTracer; - private String spanName; + private final HeaderTracer headerTracer; + private final String spanName; public HeaderTracerStreamingCallable( @Nonnull ServerStreamingCallable callable, @Nonnull HeaderTracer headerTracer, @Nonnull String spanName) { - Preconditions.checkNotNull(callable, "Inner callable must be set"); - Preconditions.checkNotNull(headerTracer, "HeaderTracer must be set"); - Preconditions.checkNotNull(spanName, "Span name must be set"); - this.innerCallable = callable; - this.headerTracer = headerTracer; - this.spanName = spanName; + this.innerCallable = Preconditions.checkNotNull(callable, "Inner callable must be set"); + this.headerTracer = Preconditions.checkNotNull(headerTracer, "HeaderTracer must be set"); + this.spanName = Preconditions.checkNotNull(spanName, "Span name must be set"); } @Override diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerUnaryCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerUnaryCallable.java index 9ea28e8f8a..17d84b2a2e 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerUnaryCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerUnaryCallable.java @@ -44,19 +44,16 @@ public class HeaderTracerUnaryCallable extends UnaryCallable { private final UnaryCallable innerCallable; - private HeaderTracer headerTracer; - private String spanName; + private final HeaderTracer headerTracer; + private final String spanName; public HeaderTracerUnaryCallable( @Nonnull UnaryCallable innerCallable, @Nonnull HeaderTracer headerTracer, @Nonnull String spanName) { - Preconditions.checkNotNull(innerCallable, "Inner callable must be set"); - Preconditions.checkNotNull(headerTracer, "HeaderTracer must be set"); - Preconditions.checkNotNull(spanName, "Span name must be set"); - this.innerCallable = innerCallable; - this.headerTracer = headerTracer; - this.spanName = spanName; + this.innerCallable = Preconditions.checkNotNull(innerCallable, "Inner callable must be set"); + this.headerTracer = Preconditions.checkNotNull(headerTracer, "HeaderTracer must be set"); + this.spanName = Preconditions.checkNotNull(spanName, "Span name must be set"); } @Override