From d1c29ed6cda7b0082c3882d42bacb9d138a801a9 Mon Sep 17 00:00:00 2001 From: Meeral Date: Sun, 5 May 2024 16:43:35 +0000 Subject: [PATCH 01/18] trying out new interceptor --- .../data/v2/stub/EnhancedBigtableStub.java | 2 +- .../v2/stub/EnhancedBigtableStubSettings.java | 18 +++-- .../v2/stub/TargetEndpointInterceptor.java | 79 +++++++++++++++++++ 3 files changed, 93 insertions(+), 6 deletions(-) create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/TargetEndpointInterceptor.java 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 57d9748cca..e13db621c2 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 @@ -275,7 +275,7 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set } managedChannelBuilder.intercept(errorCountPerConnectionMetricTracker.getInterceptor()); - + managedChannelBuilder.intercept(new TargetEndpointInterceptor()); if (oldChannelConfigurator != null) { managedChannelBuilder = oldChannelConfigurator.apply(managedChannelBuilder); } 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 f07a8fb7fc..3c5d3db73b 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 @@ -100,6 +100,7 @@ public class EnhancedBigtableStubSettings extends StubSettings IDEMPOTENT_RETRY_CODES = ImmutableSet.of(Code.DEADLINE_EXCEEDED, Code.UNAVAILABLE); @@ -345,7 +346,17 @@ public boolean getEnableRetryInfo() { /** Returns a builder for the default ChannelProvider for this service. */ public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProviderBuilder() { - return BigtableStubSettings.defaultGrpcTransportProviderBuilder() + String enableDirectpathEnv = System.getenv(CBT_ENABLE_DIRECTPATH); + Boolean isDirectpathEnabled = Boolean.parseBoolean(enableDirectpathEnv); + + InstantiatingGrpcChannelProvider.Builder grpcTransportProviderBuilder = + BigtableStubSettings.defaultGrpcTransportProviderBuilder(); + if (isDirectpathEnabled) { + // Attempts direct access to CBT service over gRPC to improve throughput, + // whether the attempt is allowed is totally controlled by service owner. + grpcTransportProviderBuilder.setAttemptDirectPathXds().setAttemptDirectPath(true); + } + return grpcTransportProviderBuilder .setChannelPoolSettings( ChannelPoolSettings.builder() .setInitialChannelCount(10) @@ -356,10 +367,7 @@ public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProvi .setMaxInboundMessageSize(MAX_MESSAGE_SIZE) .setKeepAliveTime(Duration.ofSeconds(30)) // sends ping in this interval .setKeepAliveTimeout( - Duration.ofSeconds(10)) // wait this long before considering the connection dead - // Attempts direct access to CBT service over gRPC to improve throughput, - // whether the attempt is allowed is totally controlled by service owner. - .setAttemptDirectPath(true); + Duration.ofSeconds(10)); // wait this long before considering the connection dead } @SuppressWarnings("WeakerAccess") diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/TargetEndpointInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/TargetEndpointInterceptor.java new file mode 100644 index 0000000000..eb9ee03838 --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/TargetEndpointInterceptor.java @@ -0,0 +1,79 @@ +/* + * Copyright 2018 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 static java.lang.String.*; + +import io.grpc.CallOptions; +import io.grpc.Channel; +import io.grpc.ClientCall; +import io.grpc.ClientInterceptor; +import io.grpc.ForwardingClientCall; +import io.grpc.ForwardingClientCall.SimpleForwardingClientCall; +import io.grpc.ForwardingClientCallListener; +import io.grpc.Grpc; +import io.grpc.Metadata; +import io.grpc.MethodDescriptor; +import java.net.InetSocketAddress; +import java.net.SocketAddress; +import java.util.logging.Level; +import java.util.logging.Logger; + +/** + * stuff. + */ +public class TargetEndpointInterceptor implements ClientInterceptor { + + private static final Logger LOG = Logger.getLogger(TargetEndpointInterceptor.class.getName()); + + @Override + public ClientCall interceptCall( + MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { + System.out.println("INTECERCEPT!!!"); + final ForwardingClientCall.SimpleForwardingClientCall simpleForwardingClientCall = + (SimpleForwardingClientCall) channel.newCall(methodDescriptor, callOptions); + return new ForwardingClientCall.SimpleForwardingClientCall( + simpleForwardingClientCall) { + @Override + public void start(Listener responseListener, Metadata headers) { + super.start( + new ForwardingClientCallListener.SimpleForwardingClientCallListener( + responseListener) { + @Override + public void onHeaders(Metadata headers) { + SocketAddress remoteAddr = + simpleForwardingClientCall.getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR); + if(remoteAddr == null) { + System.out.println("Remote address is null!!!"); + } else { + System.out.println( + format( + "Target Address is: %s", + ((InetSocketAddress) remoteAddr).getAddress().toString())); + LOG.log( + Level.INFO, + format( + "Target Address is: %s", + ((InetSocketAddress) remoteAddr).getAddress().toString())); + } + super.onHeaders(headers); + } + }, + headers); + } + }; + } +} From 5155fb2b3beb3a36849d739ee0ed0ce98e9d5b9a Mon Sep 17 00:00:00 2001 From: Meeral Date: Thu, 9 May 2024 15:10:15 +0000 Subject: [PATCH 02/18] testing adding new target label --- .../data/v2/stub/EnhancedBigtableStub.java | 1 + .../stub/metrics/BigtableExporterUtils.java | 3 +- .../stub/metrics/BuiltinMetricsConstants.java | 3 + .../v2/stub/metrics/BuiltinMetricsTracer.java | 19 ++++- .../TargetEndpointInterceptor.java | 33 +++++---- .../stub/metrics/TargetEndpointTracker.java | 74 +++++++++++++++++++ .../bigtable/data/v2/stub/metrics/Util.java | 7 +- 7 files changed, 123 insertions(+), 17 deletions(-) rename google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/{ => metrics}/TargetEndpointInterceptor.java (79%) create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointTracker.java 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 e13db621c2..40d34fac39 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 @@ -112,6 +112,7 @@ import com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants; import com.google.cloud.bigtable.data.v2.stub.metrics.StatsHeadersServerStreamingCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.StatsHeadersUnaryCallable; +import com.google.cloud.bigtable.data.v2.stub.metrics.TargetEndpointInterceptor; import com.google.cloud.bigtable.data.v2.stub.metrics.TracedBatcherUnaryCallable; import com.google.cloud.bigtable.data.v2.stub.mutaterows.BulkMutateRowsUserFacingCallable; import com.google.cloud.bigtable.data.v2.stub.mutaterows.MutateRowsAttemptResult; diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java index 5bf6688e17..b08747c408 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java @@ -47,6 +47,7 @@ import com.google.monitoring.v3.TimeSeries; import com.google.monitoring.v3.TypedValue; import com.google.protobuf.util.Timestamps; +import io.grpc.Grpc; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.sdk.metrics.data.AggregationTemporality; @@ -60,6 +61,7 @@ import io.opentelemetry.sdk.metrics.data.SumData; import java.lang.management.ManagementFactory; import java.net.InetAddress; +import java.net.SocketAddress; import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Collection; @@ -109,7 +111,6 @@ static String getDefaultTaskValue() { static String getProjectId(PointData pointData) { return pointData.getAttributes().get(BIGTABLE_PROJECT_ID_KEY); } - static List convertToBigtableTimeSeries(List collection, String taskId) { List allTimeSeries = new ArrayList<>(); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java index d85300828b..10c7bb682b 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java @@ -24,6 +24,8 @@ import io.opentelemetry.sdk.metrics.InstrumentSelector; import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.View; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -48,6 +50,7 @@ public class BuiltinMetricsConstants { public static final AttributeKey CLIENT_NAME_KEY = AttributeKey.stringKey("client_name"); static final AttributeKey METHOD_KEY = AttributeKey.stringKey("method"); static final AttributeKey STATUS_KEY = AttributeKey.stringKey("status"); + static AttributeKey> TARGET_KEY = AttributeKey.stringArrayKey("target"); static final AttributeKey CLIENT_UID_KEY = AttributeKey.stringKey("client_uid"); // Metric names diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index abd214d760..92c48e6b23 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -22,6 +22,7 @@ import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.STATUS_KEY; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.STREAMING_KEY; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.TABLE_ID_KEY; +import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.TARGET_KEY; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.ZONE_ID_KEY; import com.google.api.gax.retrying.ServerStreamingAttemptException; @@ -29,9 +30,13 @@ import com.google.cloud.bigtable.Version; import com.google.common.base.Stopwatch; import com.google.common.math.IntMath; +import io.grpc.CallOptions; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.LongCounter; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; import java.util.concurrent.CancellationException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -46,6 +51,8 @@ */ class BuiltinMetricsTracer extends BigtableTracer { + static final CallOptions.Key BUILTIN_METRICSTRACER_KEY = + CallOptions.Key.create("builtin-metrics-tracer"); private static final String NAME = "java-bigtable/" + Version.VERSION; private final OperationType operationType; private final SpanName spanName; @@ -85,6 +92,8 @@ class BuiltinMetricsTracer extends BigtableTracer { private Long serverLatencies = null; + private List targets; + // OpenCensus (and server) histogram buckets use [start, end), however OpenTelemetry uses (start, // end]. To work around this, we measure all the latencies in nanoseconds and convert them // to milliseconds and use DoubleHistogram. This should minimize the chance of a data @@ -175,6 +184,10 @@ public void attemptSucceeded() { recordAttemptCompletion(null); } + public void addTarget(String target) { + System.out.println(String.format("[IMPORTANT] Remote Address added by inteceptor: %s",target)); + this.targets.add(target); + } @Override public void attemptCancelled() { recordAttemptCompletion(new CancellationException()); @@ -293,8 +306,11 @@ private void recordOperationCompletion(@Nullable Throwable status) { .put(CLIENT_NAME_KEY, NAME) .put(STREAMING_KEY, isStreaming) .put(STATUS_KEY, statusStr) + .put(TARGET_KEY, this.targets) .build(); + baseAttributes.asMap().forEach((key,value)->System.out.println(String.format("Attribute key: %s, value %s", key,value))); + long operationLatencyNano = operationTimer.elapsed(TimeUnit.NANOSECONDS); // Only record when retry count is greater than 0 so the retry @@ -338,7 +354,7 @@ private void recordAttemptCompletion(@Nullable Throwable status) { } String statusStr = Util.extractStatus(status); - + System.out.println(String.format("Target key is: %s", targets.get(0))); Attributes attributes = baseAttributes .toBuilder() @@ -349,6 +365,7 @@ private void recordAttemptCompletion(@Nullable Throwable status) { .put(CLIENT_NAME_KEY, NAME) .put(STREAMING_KEY, isStreaming) .put(STATUS_KEY, statusStr) + .put(TARGET_KEY, this.targets) .build(); clientBlockingLatenciesHistogram.record(convertToMs(totalClientBlockingTime.get()), attributes); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/TargetEndpointInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java similarity index 79% rename from google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/TargetEndpointInterceptor.java rename to google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java index eb9ee03838..69f9230106 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/TargetEndpointInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java @@ -13,10 +13,12 @@ * 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 static java.lang.String.*; +import com.google.api.gax.grpc.GrpcCallContext; +import com.google.api.gax.tracing.ApiTracer; import io.grpc.CallOptions; import io.grpc.Channel; import io.grpc.ClientCall; @@ -27,6 +29,8 @@ import io.grpc.Grpc; import io.grpc.Metadata; import io.grpc.MethodDescriptor; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; import java.net.InetSocketAddress; import java.net.SocketAddress; import java.util.logging.Level; @@ -37,38 +41,41 @@ */ public class TargetEndpointInterceptor implements ClientInterceptor { + private String target; private static final Logger LOG = Logger.getLogger(TargetEndpointInterceptor.class.getName()); + public String getTarget() { + return target; + } + @Override public ClientCall interceptCall( MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { System.out.println("INTECERCEPT!!!"); + final ForwardingClientCall.SimpleForwardingClientCall simpleForwardingClientCall = (SimpleForwardingClientCall) channel.newCall(methodDescriptor, callOptions); return new ForwardingClientCall.SimpleForwardingClientCall( simpleForwardingClientCall) { + @Override public void start(Listener responseListener, Metadata headers) { + super.start( new ForwardingClientCallListener.SimpleForwardingClientCallListener( responseListener) { + @Override public void onHeaders(Metadata headers) { SocketAddress remoteAddr = simpleForwardingClientCall.getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR); - if(remoteAddr == null) { - System.out.println("Remote address is null!!!"); - } else { - System.out.println( - format( - "Target Address is: %s", - ((InetSocketAddress) remoteAddr).getAddress().toString())); - LOG.log( - Level.INFO, - format( - "Target Address is: %s", - ((InetSocketAddress) remoteAddr).getAddress().toString())); + target = ((InetSocketAddress) remoteAddr).getAddress().toString(); + + BuiltinMetricsTracer apiTracer = callOptions.getOption(BuiltinMetricsTracer.BUILTIN_METRICSTRACER_KEY); + if (apiTracer != null && target != null) { + apiTracer.addTarget(target); } + super.onHeaders(headers); } }, diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointTracker.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointTracker.java new file mode 100644 index 0000000000..9c71f29f32 --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointTracker.java @@ -0,0 +1,74 @@ +/* + * Copyright 2024 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.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.METER_NAME; +import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME; + +import com.google.api.core.InternalApi; +import io.grpc.ClientInterceptor; +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.LongHistogram; +import io.opentelemetry.api.metrics.Meter; +import java.lang.annotation.Target; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; +import java.util.WeakHashMap; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +/* Background task that goes through all connections and updates the errors_per_connection metric. */ +@InternalApi("For internal use only") +public class TargetEndpointTracker implements Runnable { + + private static final Integer PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS = 60; + + private final LongHistogram perConnectionErrorCountHistogram; + private final Attributes attributes; + + private final Set targetEndpointInterceptors; + private final Object interceptorsLock = new Object(); + + public TargetEndpointTracker(OpenTelemetry openTelemetry, Attributes attributes) { + targetEndpointInterceptors = + Collections.synchronizedSet(Collections.newSetFromMap(new WeakHashMap<>())); + + HashSet targets = new HashSet<>(); + for(TargetEndpointInterceptor interceptor: targetEndpointInterceptors) { + targets.add(interceptor.getTarget()); + } + + attributes.toBuilder().put("targets", (String[])targets.toArray()); + this.attributes = attributes; + } + + public void setTargetEndpointTracker(ScheduledExecutorService scheduler) { + scheduler.scheduleAtFixedRate( + this, 0, PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS, TimeUnit.SECONDS); + } + + public ClientInterceptor getInterceptor() { + TargetEndpointInterceptor targetEndpointInterceptor = + new TargetEndpointInterceptor(); + synchronized (interceptorsLock) { + targetEndpointInterceptors.add(targetEndpointInterceptor); + } + return targetEndpointInterceptor; + } +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index 4c3fd7a42d..0e89e240a1 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -34,6 +34,7 @@ import com.google.common.collect.ImmutableMap; import com.google.protobuf.InvalidProtocolBufferException; import io.grpc.CallOptions; +import io.grpc.Grpc; import io.grpc.Metadata; import io.grpc.Status; import io.grpc.StatusException; @@ -65,6 +66,8 @@ public class Util { static final Metadata.Key LOCATION_METADATA_KEY = Metadata.Key.of("x-goog-ext-425905942-bin", Metadata.BINARY_BYTE_MARSHALLER); + // Grpc.TRANSPORT_ATTR_REMOTE_ADDR; + /** Convert an exception into a value that can be used to create an OpenCensus tag value. */ static String extractStatus(@Nullable Throwable error) { final String statusString; @@ -151,8 +154,7 @@ static Map> createStatsHeaders(ApiCallContext apiCallContex int attemptCount = ((BigtableTracer) apiCallContext.getTracer()).getAttempt(); headers.put(ATTEMPT_HEADER_KEY.name(), Arrays.asList(String.valueOf(attemptCount))); } - return headers.build(); - } + return headers.build();} private static Long getGfeLatency(@Nullable Metadata metadata) { if (metadata == null) { @@ -209,6 +211,7 @@ static void recordMetricsFromMetadata( if (responseParams != null && latency == null) { latency = 0L; } + // Record gfe metrics tracer.recordGfeMetadata(latency, throwable); } From 48b75896358958b99b3b865d791581dea9404d30 Mon Sep 17 00:00:00 2001 From: Meeral Date: Thu, 9 May 2024 15:12:56 +0000 Subject: [PATCH 03/18] remove tracker --- .../stub/metrics/TargetEndpointTracker.java | 74 ------------------- 1 file changed, 74 deletions(-) delete mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointTracker.java diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointTracker.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointTracker.java deleted file mode 100644 index 9c71f29f32..0000000000 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointTracker.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright 2024 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.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.METER_NAME; -import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME; - -import com.google.api.core.InternalApi; -import io.grpc.ClientInterceptor; -import io.opentelemetry.api.OpenTelemetry; -import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.metrics.LongHistogram; -import io.opentelemetry.api.metrics.Meter; -import java.lang.annotation.Target; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.Set; -import java.util.WeakHashMap; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; - -/* Background task that goes through all connections and updates the errors_per_connection metric. */ -@InternalApi("For internal use only") -public class TargetEndpointTracker implements Runnable { - - private static final Integer PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS = 60; - - private final LongHistogram perConnectionErrorCountHistogram; - private final Attributes attributes; - - private final Set targetEndpointInterceptors; - private final Object interceptorsLock = new Object(); - - public TargetEndpointTracker(OpenTelemetry openTelemetry, Attributes attributes) { - targetEndpointInterceptors = - Collections.synchronizedSet(Collections.newSetFromMap(new WeakHashMap<>())); - - HashSet targets = new HashSet<>(); - for(TargetEndpointInterceptor interceptor: targetEndpointInterceptors) { - targets.add(interceptor.getTarget()); - } - - attributes.toBuilder().put("targets", (String[])targets.toArray()); - this.attributes = attributes; - } - - public void setTargetEndpointTracker(ScheduledExecutorService scheduler) { - scheduler.scheduleAtFixedRate( - this, 0, PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS, TimeUnit.SECONDS); - } - - public ClientInterceptor getInterceptor() { - TargetEndpointInterceptor targetEndpointInterceptor = - new TargetEndpointInterceptor(); - synchronized (interceptorsLock) { - targetEndpointInterceptors.add(targetEndpointInterceptor); - } - return targetEndpointInterceptor; - } -} From 55f65ac6c3e5625d398ef26a4dbb1a4c2f9ec244 Mon Sep 17 00:00:00 2001 From: Meeral Date: Thu, 9 May 2024 15:27:47 +0000 Subject: [PATCH 04/18] fix tracer --- .../bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 92c48e6b23..b937aeaaf0 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -92,7 +92,7 @@ class BuiltinMetricsTracer extends BigtableTracer { private Long serverLatencies = null; - private List targets; + private List targets = new ArrayList<>(); // OpenCensus (and server) histogram buckets use [start, end), however OpenTelemetry uses (start, // end]. To work around this, we measure all the latencies in nanoseconds and convert them @@ -354,7 +354,6 @@ private void recordAttemptCompletion(@Nullable Throwable status) { } String statusStr = Util.extractStatus(status); - System.out.println(String.format("Target key is: %s", targets.get(0))); Attributes attributes = baseAttributes .toBuilder() @@ -368,6 +367,8 @@ private void recordAttemptCompletion(@Nullable Throwable status) { .put(TARGET_KEY, this.targets) .build(); + attributes.asMap().forEach((key,value)->System.out.println(String.format("Attribute key: %s, value %s", key,value))); + clientBlockingLatenciesHistogram.record(convertToMs(totalClientBlockingTime.get()), attributes); attemptLatenciesHistogram.record( From 3ff32234caf8a3d6aef02c06a43dc56bfdb59d63 Mon Sep 17 00:00:00 2001 From: Meeral Date: Thu, 9 May 2024 15:33:19 +0000 Subject: [PATCH 05/18] fix interceptor --- .../data/v2/stub/metrics/TargetEndpointInterceptor.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java index 69f9230106..7cf354ee77 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java @@ -61,10 +61,19 @@ public ClientCall interceptCall( @Override public void start(Listener responseListener, Metadata headers) { + SocketAddress remoteAddr = + simpleForwardingClientCall.getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR); + target = ((InetSocketAddress) remoteAddr).getAddress().toString(); + + BuiltinMetricsTracer apiTracer = callOptions.getOption(BuiltinMetricsTracer.BUILTIN_METRICSTRACER_KEY); + if (apiTracer != null && target != null) { + apiTracer.addTarget(target); + } super.start( new ForwardingClientCallListener.SimpleForwardingClientCallListener( responseListener) { + @Override public void onHeaders(Metadata headers) { SocketAddress remoteAddr = From c1875fec35f58c205dc7589c40509aca6465e604 Mon Sep 17 00:00:00 2001 From: Meeral Date: Thu, 9 May 2024 15:40:54 +0000 Subject: [PATCH 06/18] fix interceptor --- .../v2/stub/metrics/TargetEndpointInterceptor.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java index 7cf354ee77..3d13764148 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java @@ -60,15 +60,7 @@ public ClientCall interceptCall( @Override public void start(Listener responseListener, Metadata headers) { - - SocketAddress remoteAddr = - simpleForwardingClientCall.getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR); - target = ((InetSocketAddress) remoteAddr).getAddress().toString(); - - BuiltinMetricsTracer apiTracer = callOptions.getOption(BuiltinMetricsTracer.BUILTIN_METRICSTRACER_KEY); - if (apiTracer != null && target != null) { - apiTracer.addTarget(target); - } + super.start( new ForwardingClientCallListener.SimpleForwardingClientCallListener( responseListener) { From 0a34918c29974e1aab1d263091ebf64f26b21199 Mon Sep 17 00:00:00 2001 From: Meeral Date: Thu, 9 May 2024 17:11:07 +0000 Subject: [PATCH 07/18] add null check --- .../data/v2/stub/metrics/TargetEndpointInterceptor.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java index 3d13764148..5e566d7b42 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java @@ -60,7 +60,7 @@ public ClientCall interceptCall( @Override public void start(Listener responseListener, Metadata headers) { - + super.start( new ForwardingClientCallListener.SimpleForwardingClientCallListener( responseListener) { @@ -73,7 +73,11 @@ public void onHeaders(Metadata headers) { target = ((InetSocketAddress) remoteAddr).getAddress().toString(); BuiltinMetricsTracer apiTracer = callOptions.getOption(BuiltinMetricsTracer.BUILTIN_METRICSTRACER_KEY); + if (apiTracer == null) { + System.out.println("api tracer is null"); + } if (apiTracer != null && target != null) { + apiTracer.addTarget(target); } From 6fa4b3fb8a9f96b614241d04c4ec4ff17459f8e4 Mon Sep 17 00:00:00 2001 From: Meeral Date: Thu, 9 May 2024 17:17:14 +0000 Subject: [PATCH 08/18] testing tracer --- .../data/v2/stub/metrics/TargetEndpointInterceptor.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java index 5e566d7b42..3d27abe026 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java @@ -52,6 +52,11 @@ public String getTarget() { public ClientCall interceptCall( MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { System.out.println("INTECERCEPT!!!"); + ApiTracer tracer = callOptions.getOption(BuiltinMetricsTracer.BUILTIN_METRICSTRACER_KEY); + if (tracer == null) { + System.out.println("tracer is null outside of header"); + } + ((BuiltinMetricsTracer) tracer).addTarget(target); final ForwardingClientCall.SimpleForwardingClientCall simpleForwardingClientCall = (SimpleForwardingClientCall) channel.newCall(methodDescriptor, callOptions); @@ -77,8 +82,8 @@ public void onHeaders(Metadata headers) { System.out.println("api tracer is null"); } if (apiTracer != null && target != null) { - apiTracer.addTarget(target); + ((BuiltinMetricsTracer) tracer).addTarget(target); } super.onHeaders(headers); From 87e0a54c6fe48efa92485135c84c9d0b3443014c Mon Sep 17 00:00:00 2001 From: Meeral Date: Thu, 9 May 2024 17:34:01 +0000 Subject: [PATCH 09/18] try using grpc tracer --- .../v2/stub/metrics/TargetEndpointInterceptor.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java index 3d27abe026..4f0a2842d1 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java @@ -52,9 +52,9 @@ public String getTarget() { public ClientCall interceptCall( MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { System.out.println("INTECERCEPT!!!"); - ApiTracer tracer = callOptions.getOption(BuiltinMetricsTracer.BUILTIN_METRICSTRACER_KEY); + ApiTracer tracer = callOptions.getOption(GrpcCallContext.TRACER_KEY); if (tracer == null) { - System.out.println("tracer is null outside of header"); + System.out.println("grpc tracer is null outside of header"); } ((BuiltinMetricsTracer) tracer).addTarget(target); @@ -77,12 +77,10 @@ public void onHeaders(Metadata headers) { simpleForwardingClientCall.getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR); target = ((InetSocketAddress) remoteAddr).getAddress().toString(); - BuiltinMetricsTracer apiTracer = callOptions.getOption(BuiltinMetricsTracer.BUILTIN_METRICSTRACER_KEY); - if (apiTracer == null) { - System.out.println("api tracer is null"); + if (tracer == null) { + System.out.println("gax tracer is null"); } - if (apiTracer != null && target != null) { - apiTracer.addTarget(target); + if (tracer != null && target != null) { ((BuiltinMetricsTracer) tracer).addTarget(target); } From f8a3abec151e71b1be9e032c06bf1ad9c147a6b4 Mon Sep 17 00:00:00 2001 From: Meeral Date: Thu, 9 May 2024 18:36:16 +0000 Subject: [PATCH 10/18] adding response header --- .../data/v2/stub/metrics/BigtableTracer.java | 4 ++++ .../metrics/TargetEndpointInterceptor.java | 18 +++++------------- .../bigtable/data/v2/stub/metrics/Util.java | 6 +++++- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java index 3445514f7b..3a77bfbc81 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java @@ -86,4 +86,8 @@ public void setLocations(String zone, String cluster) { public void grpcChannelQueuedLatencies(long queuedTimeMs) { // noop } + + public void addTarget(String target) { + // noop + } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java index 4f0a2842d1..b128defa4e 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java @@ -23,6 +23,7 @@ import io.grpc.Channel; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; +import io.grpc.ClientStreamTracer; import io.grpc.ForwardingClientCall; import io.grpc.ForwardingClientCall.SimpleForwardingClientCall; import io.grpc.ForwardingClientCallListener; @@ -33,6 +34,7 @@ import io.opentelemetry.api.common.Attributes; import java.net.InetSocketAddress; import java.net.SocketAddress; +import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; @@ -52,11 +54,6 @@ public String getTarget() { public ClientCall interceptCall( MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { System.out.println("INTECERCEPT!!!"); - ApiTracer tracer = callOptions.getOption(GrpcCallContext.TRACER_KEY); - if (tracer == null) { - System.out.println("grpc tracer is null outside of header"); - } - ((BuiltinMetricsTracer) tracer).addTarget(target); final ForwardingClientCall.SimpleForwardingClientCall simpleForwardingClientCall = (SimpleForwardingClientCall) channel.newCall(methodDescriptor, callOptions); @@ -75,15 +72,10 @@ public void start(Listener responseListener, Metadata headers) { public void onHeaders(Metadata headers) { SocketAddress remoteAddr = simpleForwardingClientCall.getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR); - target = ((InetSocketAddress) remoteAddr).getAddress().toString(); - - if (tracer == null) { - System.out.println("gax tracer is null"); - } - if (tracer != null && target != null) { - ((BuiltinMetricsTracer) tracer).addTarget(target); - } + String target = ((InetSocketAddress) remoteAddr).getAddress().toString(); + System.out.println("Adding custom hedererrwerss"); + headers.put(Util.TARGET_METADATA_KEY, target); super.onHeaders(headers); } }, diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index 0e89e240a1..2ccfaed31c 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -66,7 +66,7 @@ public class Util { static final Metadata.Key LOCATION_METADATA_KEY = Metadata.Key.of("x-goog-ext-425905942-bin", Metadata.BINARY_BYTE_MARSHALLER); - // Grpc.TRANSPORT_ATTR_REMOTE_ADDR; + static final Metadata.Key TARGET_METADATA_KEY = Metadata.Key.of("target",Metadata.ASCII_STRING_MARSHALLER); /** Convert an exception into a value that can be used to create an OpenCensus tag value. */ static String extractStatus(@Nullable Throwable error) { @@ -214,6 +214,10 @@ static void recordMetricsFromMetadata( // Record gfe metrics tracer.recordGfeMetadata(latency, throwable); + + System.out.println("Adding target to tracer"); + //Record target + tracer.addTarget(responseMetadata.getMetadata().get(TARGET_METADATA_KEY)); } /** From 29f29a15af335626c82a70e85a92447033cd343e Mon Sep 17 00:00:00 2001 From: Meeral Date: Thu, 9 May 2024 18:40:15 +0000 Subject: [PATCH 11/18] logging target --- .../com/google/cloud/bigtable/data/v2/stub/metrics/Util.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index 2ccfaed31c..5d57e190b9 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -215,7 +215,7 @@ static void recordMetricsFromMetadata( // Record gfe metrics tracer.recordGfeMetadata(latency, throwable); - System.out.println("Adding target to tracer"); + System.out.println(String.format("Adding target to tracer: %s", responseMetadata.getMetadata().get(TARGET_METADATA_KEY))); //Record target tracer.addTarget(responseMetadata.getMetadata().get(TARGET_METADATA_KEY)); } From 85cd43bcf7b46dd56c9654894a38bac1fecc7c73 Mon Sep 17 00:00:00 2001 From: Meeral Date: Thu, 9 May 2024 18:47:10 +0000 Subject: [PATCH 12/18] add method to composite tracer --- .../bigtable/data/v2/stub/metrics/CompositeTracer.java | 6 ++++++ .../google/cloud/bigtable/data/v2/stub/metrics/Util.java | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java index 774c6d9f22..3ccde82d69 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java @@ -225,4 +225,10 @@ public void grpcChannelQueuedLatencies(long queuedTimeMs) { tracer.grpcChannelQueuedLatencies(queuedTimeMs); } } + + public void addTarget(String target) { + for (BigtableTracer tracer : bigtableTracers) { + tracer.addTarget(target); + } + } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index 5d57e190b9..267cd4450b 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -215,9 +215,10 @@ static void recordMetricsFromMetadata( // Record gfe metrics tracer.recordGfeMetadata(latency, throwable); - System.out.println(String.format("Adding target to tracer: %s", responseMetadata.getMetadata().get(TARGET_METADATA_KEY))); + String target = responseMetadata.getMetadata().get(TARGET_METADATA_KEY); + System.out.println(String.format("Adding target to tracer: %s",target)); //Record target - tracer.addTarget(responseMetadata.getMetadata().get(TARGET_METADATA_KEY)); + tracer.addTarget(target); } /** From 0f41fdad25736a83978708b1c3a2dff5e419f885 Mon Sep 17 00:00:00 2001 From: Meeral Date: Sun, 5 May 2024 16:43:35 +0000 Subject: [PATCH 13/18] Add target label to attributes, populated via new interceptor. Inteceptor pulls target from GRPC response header. --- .../v2/stub/metrics/TargetEndpointInterceptor.java | 11 +++++++++++ .../cloud/bigtable/data/v2/stub/metrics/Util.java | 9 +++++++++ .../v2/stub/metrics/BuiltinMetricsTracerTest.java | 3 +++ 3 files changed, 23 insertions(+) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java index b128defa4e..934b6e024c 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java @@ -53,7 +53,10 @@ public String getTarget() { @Override public ClientCall interceptCall( MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { +<<<<<<< HEAD System.out.println("INTECERCEPT!!!"); +======= +>>>>>>> 20566b2b (Add target label to attributes, populated via new interceptor. Inteceptor pulls target from GRPC response header.) final ForwardingClientCall.SimpleForwardingClientCall simpleForwardingClientCall = (SimpleForwardingClientCall) channel.newCall(methodDescriptor, callOptions); @@ -72,11 +75,19 @@ public void start(Listener responseListener, Metadata headers) { public void onHeaders(Metadata headers) { SocketAddress remoteAddr = simpleForwardingClientCall.getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR); +<<<<<<< HEAD String target = ((InetSocketAddress) remoteAddr).getAddress().toString(); System.out.println("Adding custom hedererrwerss"); headers.put(Util.TARGET_METADATA_KEY, target); super.onHeaders(headers); +======= + if(remoteAddr != null && ((InetSocketAddress) remoteAddr).getAddress() != null) { + String target = ((InetSocketAddress) remoteAddr).getAddress().toString(); + headers.put(Util.TARGET_METADATA_KEY, target); + super.onHeaders(headers); + } +>>>>>>> 20566b2b (Add target label to attributes, populated via new interceptor. Inteceptor pulls target from GRPC response header.) } }, headers); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index 267cd4450b..fa2196d067 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -214,11 +214,20 @@ static void recordMetricsFromMetadata( // Record gfe metrics tracer.recordGfeMetadata(latency, throwable); +<<<<<<< HEAD String target = responseMetadata.getMetadata().get(TARGET_METADATA_KEY); System.out.println(String.format("Adding target to tracer: %s",target)); //Record target tracer.addTarget(target); +======= + if(responseMetadata.getMetadata() != null) { + String target = responseMetadata.getMetadata().get(TARGET_METADATA_KEY); + System.out.println(String.format("Adding target to tracer: %s",target)); + //Record target + tracer.addTarget(target); + } +>>>>>>> 20566b2b (Add target label to attributes, populated via new interceptor. Inteceptor pulls target from GRPC response header.) } /** diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java index 2dd4bcabb3..22a3954563 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java @@ -28,6 +28,7 @@ import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.STATUS_KEY; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.STREAMING_KEY; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.TABLE_ID_KEY; +import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.TARGET_KEY; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.ZONE_ID_KEY; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsTestUtils.getAggregatedValue; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsTestUtils.getMetricData; @@ -185,6 +186,7 @@ public void sendHeaders(Metadata headers) { headers.put( Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER), String.format("gfet4t7; dur=%d", FAKE_SERVER_TIMING)); + headers.put(Metadata.Key.of("io.grpc.Grpc.TRANSPORT_ATTR_REMOTE_ADDR", Metadata.ASCII_STRING_MARSHALLER), "localhost"); ResponseParams params = ResponseParams.newBuilder().setZoneId(ZONE).setClusterId(CLUSTER).build(); @@ -296,6 +298,7 @@ public void testReadRowsOperationLatencies() { .put(METHOD_KEY, "Bigtable.ReadRows") .put(STREAMING_KEY, true) .put(CLIENT_NAME_KEY, CLIENT_NAME) + .put(TARGET_KEY, "") .build(); Collection allMetricData = metricReader.collectAllMetrics(); From 369391246d396400f077100f4ce61d408e1c2c49 Mon Sep 17 00:00:00 2001 From: Meeral Date: Thu, 13 Jun 2024 19:24:32 +0000 Subject: [PATCH 14/18] mend --- .../data/v2/stub/EnhancedBigtableStub.java | 2 - .../stub/metrics/BigtableExporterUtils.java | 3 +- .../stub/metrics/BuiltinMetricsConstants.java | 4 +- .../v2/stub/metrics/BuiltinMetricsTracer.java | 20 ++-- .../data/v2/stub/metrics/CompositeTracer.java | 7 +- .../metrics/TargetEndpointInterceptor.java | 97 ------------------- .../bigtable/data/v2/stub/metrics/Util.java | 29 +++--- .../stub/metrics/BuiltinMetricsTestUtils.java | 2 + .../metrics/BuiltinMetricsTracerTest.java | 39 +++++++- 9 files changed, 68 insertions(+), 135 deletions(-) delete mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java 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 40d34fac39..7e0f54a928 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 @@ -112,7 +112,6 @@ import com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants; import com.google.cloud.bigtable.data.v2.stub.metrics.StatsHeadersServerStreamingCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.StatsHeadersUnaryCallable; -import com.google.cloud.bigtable.data.v2.stub.metrics.TargetEndpointInterceptor; import com.google.cloud.bigtable.data.v2.stub.metrics.TracedBatcherUnaryCallable; import com.google.cloud.bigtable.data.v2.stub.mutaterows.BulkMutateRowsUserFacingCallable; import com.google.cloud.bigtable.data.v2.stub.mutaterows.MutateRowsAttemptResult; @@ -276,7 +275,6 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set } managedChannelBuilder.intercept(errorCountPerConnectionMetricTracker.getInterceptor()); - managedChannelBuilder.intercept(new TargetEndpointInterceptor()); if (oldChannelConfigurator != null) { managedChannelBuilder = oldChannelConfigurator.apply(managedChannelBuilder); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java index b08747c408..5bf6688e17 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java @@ -47,7 +47,6 @@ import com.google.monitoring.v3.TimeSeries; import com.google.monitoring.v3.TypedValue; import com.google.protobuf.util.Timestamps; -import io.grpc.Grpc; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.sdk.metrics.data.AggregationTemporality; @@ -61,7 +60,6 @@ import io.opentelemetry.sdk.metrics.data.SumData; import java.lang.management.ManagementFactory; import java.net.InetAddress; -import java.net.SocketAddress; import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Collection; @@ -111,6 +109,7 @@ static String getDefaultTaskValue() { static String getProjectId(PointData pointData) { return pointData.getAttributes().get(BIGTABLE_PROJECT_ID_KEY); } + static List convertToBigtableTimeSeries(List collection, String taskId) { List allTimeSeries = new ArrayList<>(); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java index 10c7bb682b..dcb1b32f02 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java @@ -24,7 +24,6 @@ import io.opentelemetry.sdk.metrics.InstrumentSelector; import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.View; -import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; @@ -110,7 +109,8 @@ public class BuiltinMetricsConstants { CLUSTER_ID_KEY, ZONE_ID_KEY, METHOD_KEY, - CLIENT_NAME_KEY); + CLIENT_NAME_KEY, + TARGET_KEY); static void defineView( ImmutableMap.Builder viewMap, diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index b937aeaaf0..f6835e8bea 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -32,11 +32,11 @@ import com.google.common.math.IntMath; import io.grpc.CallOptions; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.internal.StringUtils; import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.LongCounter; import java.util.ArrayList; -import java.util.List; -import java.util.Set; +import java.util.HashSet; import java.util.concurrent.CancellationException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -92,7 +92,7 @@ class BuiltinMetricsTracer extends BigtableTracer { private Long serverLatencies = null; - private List targets = new ArrayList<>(); + private HashSet targets = new HashSet<>(); // OpenCensus (and server) histogram buckets use [start, end), however OpenTelemetry uses (start, // end]. To work around this, we measure all the latencies in nanoseconds and convert them @@ -185,9 +185,11 @@ public void attemptSucceeded() { } public void addTarget(String target) { - System.out.println(String.format("[IMPORTANT] Remote Address added by inteceptor: %s",target)); - this.targets.add(target); + if (!StringUtils.isNullOrEmpty(target)) { + this.targets.add(target); + } } + @Override public void attemptCancelled() { recordAttemptCompletion(new CancellationException()); @@ -306,11 +308,9 @@ private void recordOperationCompletion(@Nullable Throwable status) { .put(CLIENT_NAME_KEY, NAME) .put(STREAMING_KEY, isStreaming) .put(STATUS_KEY, statusStr) - .put(TARGET_KEY, this.targets) + .put(TARGET_KEY, new ArrayList<>(this.targets)) .build(); - baseAttributes.asMap().forEach((key,value)->System.out.println(String.format("Attribute key: %s, value %s", key,value))); - long operationLatencyNano = operationTimer.elapsed(TimeUnit.NANOSECONDS); // Only record when retry count is greater than 0 so the retry @@ -364,11 +364,9 @@ private void recordAttemptCompletion(@Nullable Throwable status) { .put(CLIENT_NAME_KEY, NAME) .put(STREAMING_KEY, isStreaming) .put(STATUS_KEY, statusStr) - .put(TARGET_KEY, this.targets) + .put(TARGET_KEY, new ArrayList<>(this.targets)) .build(); - attributes.asMap().forEach((key,value)->System.out.println(String.format("Attribute key: %s, value %s", key,value))); - clientBlockingLatenciesHistogram.record(convertToMs(totalClientBlockingTime.get()), attributes); attemptLatenciesHistogram.record( diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java index 3ccde82d69..1e36fb723f 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java @@ -17,6 +17,7 @@ import com.google.api.gax.tracing.ApiTracer; import com.google.common.collect.ImmutableList; +import io.opentelemetry.api.internal.StringUtils; import java.util.ArrayList; import java.util.List; import javax.annotation.Nullable; @@ -227,8 +228,10 @@ public void grpcChannelQueuedLatencies(long queuedTimeMs) { } public void addTarget(String target) { - for (BigtableTracer tracer : bigtableTracers) { - tracer.addTarget(target); + if (StringUtils.isNullOrEmpty(target)) { + for (BigtableTracer tracer : bigtableTracers) { + tracer.addTarget(target); + } } } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java deleted file mode 100644 index 934b6e024c..0000000000 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetEndpointInterceptor.java +++ /dev/null @@ -1,97 +0,0 @@ -/* - * Copyright 2018 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 java.lang.String.*; - -import com.google.api.gax.grpc.GrpcCallContext; -import com.google.api.gax.tracing.ApiTracer; -import io.grpc.CallOptions; -import io.grpc.Channel; -import io.grpc.ClientCall; -import io.grpc.ClientInterceptor; -import io.grpc.ClientStreamTracer; -import io.grpc.ForwardingClientCall; -import io.grpc.ForwardingClientCall.SimpleForwardingClientCall; -import io.grpc.ForwardingClientCallListener; -import io.grpc.Grpc; -import io.grpc.Metadata; -import io.grpc.MethodDescriptor; -import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.api.common.Attributes; -import java.net.InetSocketAddress; -import java.net.SocketAddress; -import java.util.List; -import java.util.logging.Level; -import java.util.logging.Logger; - -/** - * stuff. - */ -public class TargetEndpointInterceptor implements ClientInterceptor { - - private String target; - private static final Logger LOG = Logger.getLogger(TargetEndpointInterceptor.class.getName()); - - public String getTarget() { - return target; - } - - @Override - public ClientCall interceptCall( - MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { -<<<<<<< HEAD - System.out.println("INTECERCEPT!!!"); -======= ->>>>>>> 20566b2b (Add target label to attributes, populated via new interceptor. Inteceptor pulls target from GRPC response header.) - - final ForwardingClientCall.SimpleForwardingClientCall simpleForwardingClientCall = - (SimpleForwardingClientCall) channel.newCall(methodDescriptor, callOptions); - return new ForwardingClientCall.SimpleForwardingClientCall( - simpleForwardingClientCall) { - - @Override - public void start(Listener responseListener, Metadata headers) { - - super.start( - new ForwardingClientCallListener.SimpleForwardingClientCallListener( - responseListener) { - - - @Override - public void onHeaders(Metadata headers) { - SocketAddress remoteAddr = - simpleForwardingClientCall.getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR); -<<<<<<< HEAD - String target = ((InetSocketAddress) remoteAddr).getAddress().toString(); - - System.out.println("Adding custom hedererrwerss"); - headers.put(Util.TARGET_METADATA_KEY, target); - super.onHeaders(headers); -======= - if(remoteAddr != null && ((InetSocketAddress) remoteAddr).getAddress() != null) { - String target = ((InetSocketAddress) remoteAddr).getAddress().toString(); - headers.put(Util.TARGET_METADATA_KEY, target); - super.onHeaders(headers); - } ->>>>>>> 20566b2b (Add target label to attributes, populated via new interceptor. Inteceptor pulls target from GRPC response header.) - } - }, - headers); - } - }; - } -} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index fa2196d067..6e6a12bc2c 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -34,12 +34,12 @@ import com.google.common.collect.ImmutableMap; import com.google.protobuf.InvalidProtocolBufferException; import io.grpc.CallOptions; -import io.grpc.Grpc; import io.grpc.Metadata; import io.grpc.Status; import io.grpc.StatusException; import io.grpc.StatusRuntimeException; import io.opencensus.tags.TagValue; +import io.opentelemetry.api.internal.StringUtils; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Arrays; @@ -66,7 +66,8 @@ public class Util { static final Metadata.Key LOCATION_METADATA_KEY = Metadata.Key.of("x-goog-ext-425905942-bin", Metadata.BINARY_BYTE_MARSHALLER); - static final Metadata.Key TARGET_METADATA_KEY = Metadata.Key.of("target",Metadata.ASCII_STRING_MARSHALLER); + static final Metadata.Key TARGET_METADATA_KEY = + Metadata.Key.of("target", Metadata.ASCII_STRING_MARSHALLER); /** Convert an exception into a value that can be used to create an OpenCensus tag value. */ static String extractStatus(@Nullable Throwable error) { @@ -154,7 +155,8 @@ static Map> createStatsHeaders(ApiCallContext apiCallContex int attemptCount = ((BigtableTracer) apiCallContext.getTracer()).getAttempt(); headers.put(ATTEMPT_HEADER_KEY.name(), Arrays.asList(String.valueOf(attemptCount))); } - return headers.build();} + return headers.build(); + } private static Long getGfeLatency(@Nullable Metadata metadata) { if (metadata == null) { @@ -214,20 +216,15 @@ static void recordMetricsFromMetadata( // Record gfe metrics tracer.recordGfeMetadata(latency, throwable); -<<<<<<< HEAD - - String target = responseMetadata.getMetadata().get(TARGET_METADATA_KEY); - System.out.println(String.format("Adding target to tracer: %s",target)); - //Record target - tracer.addTarget(target); -======= - if(responseMetadata.getMetadata() != null) { - String target = responseMetadata.getMetadata().get(TARGET_METADATA_KEY); - System.out.println(String.format("Adding target to tracer: %s",target)); - //Record target - tracer.addTarget(target); + if (responseMetadata.getMetadata() != null) { + Metadata.Key remoteAddressKey = + Metadata.Key.of( + "io.grpc.grpc.transport_attr_remote_addr", Metadata.ASCII_STRING_MARSHALLER); + String remoteAddr = responseMetadata.getMetadata().get(remoteAddressKey); + if (!StringUtils.isNullOrEmpty(remoteAddr)) { + tracer.addTarget(remoteAddr); + } } ->>>>>>> 20566b2b (Add target label to attributes, populated via new interceptor. Inteceptor pulls target from GRPC response header.) } /** diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTestUtils.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTestUtils.java index 09b7e1f663..36fcd7d09e 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTestUtils.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTestUtils.java @@ -94,8 +94,10 @@ public static void verifyAttributes(MetricData metricData, Attributes attributes case HISTOGRAM: List hd = metricData.getHistogramData().getPoints().stream() + .peek(item -> System.out.println(item)) .filter(pd -> pd.getAttributes().equals(attributes)) .collect(Collectors.toList()); + assertThat(hd).isNotEmpty(); break; case LONG_SUM: diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java index 22a3954563..f4e067aa27 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java @@ -131,6 +131,7 @@ public class BuiltinMetricsTracerTest { private static final long SERVER_LATENCY = 100; private static final long APPLICATION_LATENCY = 200; private static final long SLEEP_VARIABILITY = 15; + private static final String TARGET_ATTRIBUTE_VALUE = "localhost"; private static final String CLIENT_NAME = "java-bigtable/" + Version.VERSION; private static final long CHANNEL_BLOCKING_LATENCY = 75; @@ -186,7 +187,11 @@ public void sendHeaders(Metadata headers) { headers.put( Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER), String.format("gfet4t7; dur=%d", FAKE_SERVER_TIMING)); - headers.put(Metadata.Key.of("io.grpc.Grpc.TRANSPORT_ATTR_REMOTE_ADDR", Metadata.ASCII_STRING_MARSHALLER), "localhost"); + headers.put( + Metadata.Key.of( + "io.grpc.Grpc.TRANSPORT_ATTR_REMOTE_ADDR", + Metadata.ASCII_STRING_MARSHALLER), + "localhost"); ResponseParams params = ResponseParams.newBuilder().setZoneId(ZONE).setClusterId(CLUSTER).build(); @@ -298,13 +303,24 @@ public void testReadRowsOperationLatencies() { .put(METHOD_KEY, "Bigtable.ReadRows") .put(STREAMING_KEY, true) .put(CLIENT_NAME_KEY, CLIENT_NAME) - .put(TARGET_KEY, "") + .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); Collection allMetricData = metricReader.collectAllMetrics(); MetricData metricData = getMetricData(allMetricData, OPERATION_LATENCIES_NAME); - + metricData.getHistogramData().getPoints().stream() + .forEach( + histogramPointData -> + histogramPointData + .getAttributes() + .forEach( + (attributeKey, o) -> + System.out.println( + "point data attributes: " + + attributeKey.getKey().toString() + + " output: " + + o.toString()))); long value = getAggregatedValue(metricData, expectedAttributes); assertThat(value).isIn(Range.closed(SERVER_LATENCY, elapsed)); } @@ -327,6 +343,7 @@ public void testReadRowsOperationLatenciesOnAuthorizedView() { .put(METHOD_KEY, "Bigtable.ReadRows") .put(STREAMING_KEY, true) .put(CLIENT_NAME_KEY, CLIENT_NAME) + .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); Collection allMetricData = metricReader.collectAllMetrics(); @@ -349,6 +366,7 @@ public void testGfeMetrics() { .put(CLUSTER_ID_KEY, CLUSTER) .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(METHOD_KEY, "Bigtable.ReadRows") + .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); Collection allMetricData = metricReader.collectAllMetrics(); @@ -369,6 +387,7 @@ public void testGfeMetrics() { .put(CLUSTER_ID_KEY, "unspecified") .put(METHOD_KEY, "Bigtable.ReadRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) + .put(TARGET_KEY, new ArrayList()) .build(); Attributes expected2 = baseAttributes @@ -379,6 +398,7 @@ public void testGfeMetrics() { .put(CLUSTER_ID_KEY, CLUSTER) .put(METHOD_KEY, "Bigtable.ReadRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) + .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); verifyAttributes(connectivityErrorCountMetricData, expected1); @@ -435,6 +455,7 @@ public void onComplete() { .put(CLUSTER_ID_KEY, CLUSTER) .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(METHOD_KEY, "Bigtable.ReadRows") + .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); long value = getAggregatedValue(applicationLatency, expectedAttributes); @@ -472,6 +493,7 @@ public void testReadRowsApplicationLatencyWithManualFlowControl() throws Excepti .put(CLUSTER_ID_KEY, CLUSTER) .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(METHOD_KEY, "Bigtable.ReadRows") + .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); long value = getAggregatedValue(applicationLatency, expectedAttributes); @@ -504,6 +526,7 @@ public void testRetryCount() throws InterruptedException { .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(METHOD_KEY, "Bigtable.MutateRow") .put(STATUS_KEY, "OK") + .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); long value = getAggregatedValue(metricData, expectedAttributes); @@ -528,6 +551,7 @@ public void testMutateRowAttemptsTagValues() { .put(METHOD_KEY, "Bigtable.MutateRow") .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(STREAMING_KEY, false) + .put(TARGET_KEY, new ArrayList<>()) .build(); Attributes expected2 = @@ -540,6 +564,7 @@ public void testMutateRowAttemptsTagValues() { .put(METHOD_KEY, "Bigtable.MutateRow") .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(STREAMING_KEY, false) + .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); verifyAttributes(metricData, expected1); @@ -570,6 +595,7 @@ public void testMutateRowsPartialError() throws InterruptedException { .put(METHOD_KEY, "Bigtable.MutateRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(STREAMING_KEY, false) + .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); verifyAttributes(metricData, expected); @@ -600,6 +626,7 @@ public void testMutateRowsRpcError() { .put(METHOD_KEY, "Bigtable.MutateRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(STREAMING_KEY, false) + .put(TARGET_KEY, new ArrayList<>()) .build(); verifyAttributes(metricData, expected); @@ -622,6 +649,7 @@ public void testReadRowsAttemptsTagValues() { .put(METHOD_KEY, "Bigtable.ReadRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(STREAMING_KEY, true) + .put(TARGET_KEY, new ArrayList()) .build(); Attributes expected2 = @@ -634,6 +662,7 @@ public void testReadRowsAttemptsTagValues() { .put(METHOD_KEY, "Bigtable.ReadRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(STREAMING_KEY, true) + .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); verifyAttributes(metricData, expected1); @@ -663,6 +692,7 @@ public void testBatchBlockingLatencies() throws InterruptedException { .put(CLUSTER_ID_KEY, CLUSTER) .put(METHOD_KEY, "Bigtable.MutateRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) + .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); long value = getAggregatedValue(applicationLatency, expectedAttributes); @@ -689,6 +719,7 @@ public void testQueuedOnChannelServerStreamLatencies() { .put(ZONE_ID_KEY, ZONE) .put(METHOD_KEY, "Bigtable.ReadRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) + .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); long value = getAggregatedValue(clientLatency, attributes); @@ -711,6 +742,7 @@ public void testQueuedOnChannelUnaryLatencies() { .put(ZONE_ID_KEY, ZONE) .put(METHOD_KEY, "Bigtable.MutateRow") .put(CLIENT_NAME_KEY, CLIENT_NAME) + .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); long expected = CHANNEL_BLOCKING_LATENCY * 2 / 3; @@ -739,6 +771,7 @@ public void testPermanentFailure() { .put(STREAMING_KEY, true) .put(METHOD_KEY, "Bigtable.ReadRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) + .put(TARGET_KEY, new ArrayList<>()) .build(); verifyAttributes(attemptLatency, expected); From 10d19581b38689342200286679657ff587f94440 Mon Sep 17 00:00:00 2001 From: Meeral Date: Thu, 13 Jun 2024 21:14:51 +0000 Subject: [PATCH 15/18] remove uneecessary logging --- .../cloud/bigtable/data/v2/stub/metrics/Util.java | 5 +---- .../v2/stub/metrics/BuiltinMetricsTracerTest.java | 12 ------------ 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index 6e6a12bc2c..780fd8f569 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -65,10 +65,7 @@ public class Util { private static final Pattern SERVER_TIMING_HEADER_PATTERN = Pattern.compile(".*dur=(?\\d+)"); static final Metadata.Key LOCATION_METADATA_KEY = Metadata.Key.of("x-goog-ext-425905942-bin", Metadata.BINARY_BYTE_MARSHALLER); - - static final Metadata.Key TARGET_METADATA_KEY = - Metadata.Key.of("target", Metadata.ASCII_STRING_MARSHALLER); - + /** Convert an exception into a value that can be used to create an OpenCensus tag value. */ static String extractStatus(@Nullable Throwable error) { final String statusString; diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java index f4e067aa27..75019ed1a2 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java @@ -309,18 +309,6 @@ public void testReadRowsOperationLatencies() { Collection allMetricData = metricReader.collectAllMetrics(); MetricData metricData = getMetricData(allMetricData, OPERATION_LATENCIES_NAME); - metricData.getHistogramData().getPoints().stream() - .forEach( - histogramPointData -> - histogramPointData - .getAttributes() - .forEach( - (attributeKey, o) -> - System.out.println( - "point data attributes: " - + attributeKey.getKey().toString() - + " output: " - + o.toString()))); long value = getAggregatedValue(metricData, expectedAttributes); assertThat(value).isIn(Range.closed(SERVER_LATENCY, elapsed)); } From b2861dfab664a0642edd78b8b61e64788bd50517 Mon Sep 17 00:00:00 2001 From: Meeral Date: Wed, 19 Jun 2024 20:03:23 +0000 Subject: [PATCH 16/18] Inject target into tracer from new target interceptor, update tests --- .../data/v2/stub/EnhancedBigtableStub.java | 4 + .../v2/stub/EnhancedBigtableStubSettings.java | 12 +++ .../stub/metrics/BuiltinMetricsConstants.java | 2 +- .../v2/stub/metrics/BuiltinMetricsTracer.java | 12 +-- .../stub/metrics/TargetTracerInterceptor.java | 73 ++++++++++++++ .../bigtable/data/v2/stub/metrics/Util.java | 9 -- .../bigtable/data/v2/FakeServiceBuilder.java | 5 + .../stub/metrics/BuiltinMetricsTestUtils.java | 1 - .../metrics/BuiltinMetricsTracerTest.java | 98 +++++++++++-------- 9 files changed, 155 insertions(+), 61 deletions(-) create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetTracerInterceptor.java 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 7e0f54a928..5d602cbe52 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 @@ -112,6 +112,7 @@ import com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants; import com.google.cloud.bigtable.data.v2.stub.metrics.StatsHeadersServerStreamingCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.StatsHeadersUnaryCallable; +import com.google.cloud.bigtable.data.v2.stub.metrics.TargetTracerInterceptor; import com.google.cloud.bigtable.data.v2.stub.metrics.TracedBatcherUnaryCallable; import com.google.cloud.bigtable.data.v2.stub.mutaterows.BulkMutateRowsUserFacingCallable; import com.google.cloud.bigtable.data.v2.stub.mutaterows.MutateRowsAttemptResult; @@ -275,6 +276,9 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set } managedChannelBuilder.intercept(errorCountPerConnectionMetricTracker.getInterceptor()); + if(settings.getIsDirectpath()) { + managedChannelBuilder.intercept(new TargetTracerInterceptor()); + } if (oldChannelConfigurator != null) { managedChannelBuilder = oldChannelConfigurator.apply(managedChannelBuilder); } 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 79d0c37fea..d282a68939 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 @@ -234,6 +234,8 @@ public class EnhancedBigtableStubSettings extends StubSettings diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java index dcb1b32f02..b6e04a94a2 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java @@ -49,7 +49,7 @@ public class BuiltinMetricsConstants { public static final AttributeKey CLIENT_NAME_KEY = AttributeKey.stringKey("client_name"); static final AttributeKey METHOD_KEY = AttributeKey.stringKey("method"); static final AttributeKey STATUS_KEY = AttributeKey.stringKey("status"); - static AttributeKey> TARGET_KEY = AttributeKey.stringArrayKey("target"); + static AttributeKey TARGET_KEY = AttributeKey.stringKey("target"); static final AttributeKey CLIENT_UID_KEY = AttributeKey.stringKey("client_uid"); // Metric names diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index f6835e8bea..53b0d86090 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -32,11 +32,10 @@ import com.google.common.math.IntMath; import io.grpc.CallOptions; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.internal.StringUtils; import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.LongCounter; -import java.util.ArrayList; -import java.util.HashSet; import java.util.concurrent.CancellationException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -92,7 +91,7 @@ class BuiltinMetricsTracer extends BigtableTracer { private Long serverLatencies = null; - private HashSet targets = new HashSet<>(); + private String target_endpoint = "unspecified"; // OpenCensus (and server) histogram buckets use [start, end), however OpenTelemetry uses (start, // end]. To work around this, we measure all the latencies in nanoseconds and convert them @@ -186,7 +185,7 @@ public void attemptSucceeded() { public void addTarget(String target) { if (!StringUtils.isNullOrEmpty(target)) { - this.targets.add(target); + this.target_endpoint = target; } } @@ -308,7 +307,6 @@ private void recordOperationCompletion(@Nullable Throwable status) { .put(CLIENT_NAME_KEY, NAME) .put(STREAMING_KEY, isStreaming) .put(STATUS_KEY, statusStr) - .put(TARGET_KEY, new ArrayList<>(this.targets)) .build(); long operationLatencyNano = operationTimer.elapsed(TimeUnit.NANOSECONDS); @@ -364,13 +362,11 @@ private void recordAttemptCompletion(@Nullable Throwable status) { .put(CLIENT_NAME_KEY, NAME) .put(STREAMING_KEY, isStreaming) .put(STATUS_KEY, statusStr) - .put(TARGET_KEY, new ArrayList<>(this.targets)) .build(); clientBlockingLatenciesHistogram.record(convertToMs(totalClientBlockingTime.get()), attributes); - attemptLatenciesHistogram.record( - convertToMs(attemptTimer.elapsed(TimeUnit.NANOSECONDS)), attributes); + convertToMs(attemptTimer.elapsed(TimeUnit.NANOSECONDS)), attributes.toBuilder().put(TARGET_KEY,target_endpoint).build()); if (serverLatencies != null) { serverLatenciesHistogram.record(serverLatencies, attributes); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetTracerInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetTracerInterceptor.java new file mode 100644 index 0000000000..5a70586e6a --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/TargetTracerInterceptor.java @@ -0,0 +1,73 @@ +/* + * Copyright 2024 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.grpc.GrpcCallContext; +import com.google.api.gax.tracing.ApiTracer; +import io.grpc.CallOptions; +import io.grpc.Channel; +import io.grpc.ClientCall; +import io.grpc.ClientInterceptor; +import io.grpc.ForwardingClientCall; +import io.grpc.ForwardingClientCallListener; +import io.grpc.Grpc; +import io.grpc.Metadata; +import io.grpc.MethodDescriptor; +import io.grpc.Status; +import java.net.SocketAddress; +import java.util.concurrent.atomic.LongAdder; +import java.util.logging.Level; +import java.util.logging.Logger; + +/** An interceptor extracts remote target and appends metric. */ +public class TargetTracerInterceptor implements ClientInterceptor { + private static final Logger LOG = + Logger.getLogger(TargetTracerInterceptor.class.toString()); + + @Override + public ClientCall interceptCall( + MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { + ClientCall clientCall = channel.newCall(methodDescriptor,callOptions); + return new ForwardingClientCall.SimpleForwardingClientCall(clientCall) { + @Override + public void start(Listener responseListener, Metadata headers) { + super.start( + new ForwardingClientCallListener.SimpleForwardingClientCallListener( + responseListener) { + @Override + public void onHeaders(Metadata headers) { + // Connection accounting is non-critical, so we log the exception, but let normal + // processing proceed. + try { + SocketAddress remoteAddr = + clientCall.getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR); + ApiTracer bigtableTracer = callOptions.getOption(GrpcCallContext.TRACER_KEY); + if(bigtableTracer instanceof BuiltinMetricsTracer) { + ((BuiltinMetricsTracer)bigtableTracer).addTarget(String.valueOf(remoteAddr)); + } + } catch (Throwable t) { + LOG.log( + Level.WARNING, "Unexpected error while updating target label", t); + } + super.onHeaders(headers); + } + }, + headers); + } + }; + } + +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index 780fd8f569..b3f53cc4fc 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -213,15 +213,6 @@ static void recordMetricsFromMetadata( // Record gfe metrics tracer.recordGfeMetadata(latency, throwable); - if (responseMetadata.getMetadata() != null) { - Metadata.Key remoteAddressKey = - Metadata.Key.of( - "io.grpc.grpc.transport_attr_remote_addr", Metadata.ASCII_STRING_MARSHALLER); - String remoteAddr = responseMetadata.getMetadata().get(remoteAddressKey); - if (!StringUtils.isNullOrEmpty(remoteAddr)) { - tracer.addTarget(remoteAddr); - } - } } /** diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/FakeServiceBuilder.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/FakeServiceBuilder.java index 5edcca2f07..eeb3e9a4d0 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/FakeServiceBuilder.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/FakeServiceBuilder.java @@ -31,6 +31,7 @@ public class FakeServiceBuilder { private final List services = new ArrayList<>(); private final List transportFilters = new ArrayList<>(); + private int serverPort = 0; public static FakeServiceBuilder create(BindableService... services) { return new FakeServiceBuilder(services); } @@ -56,6 +57,9 @@ public FakeServiceBuilder addTransportFilter(ServerTransportFilter transportFilt return this; } + public int getServerPort() { + return serverPort; + } public Server start() throws IOException { IOException lastError = null; @@ -79,6 +83,7 @@ private Server startWithoutRetries() throws IOException { port = ss.getLocalPort(); } ServerBuilder builder = ServerBuilder.forPort(port); + serverPort = port; interceptors.forEach(builder::intercept); services.forEach(builder::addService); transportFilters.forEach(builder::addTransportFilter); diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTestUtils.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTestUtils.java index e59f679d13..120f93cd44 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTestUtils.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTestUtils.java @@ -122,7 +122,6 @@ public static void verifyAttributes(MetricData metricData, Attributes attributes case HISTOGRAM: List hd = metricData.getHistogramData().getPoints().stream() - .peek(item -> System.out.println(item)) .filter(pd -> pd.getAttributes().equals(attributes)) .collect(Collectors.toList()); diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java index ef4334b7fc..e0b9d66cd4 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java @@ -98,6 +98,7 @@ import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import java.nio.charset.Charset; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.Iterator; import java.util.List; @@ -130,7 +131,7 @@ public class BuiltinMetricsTracerTest { private static final long SERVER_LATENCY = 100; private static final long APPLICATION_LATENCY = 200; private static final long SLEEP_VARIABILITY = 15; - private static final String TARGET_ATTRIBUTE_VALUE = "localhost"; + private static final String TARGET_ENDPOINT_VALUE_FORMAT = "localhost/127.0.0.1:%s"; private static final String CLIENT_NAME = "java-bigtable/" + Version.VERSION; private static final long CHANNEL_BLOCKING_LATENCY = 75; @@ -186,11 +187,6 @@ public void sendHeaders(Metadata headers) { headers.put( Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER), String.format("gfet4t7; dur=%d", FAKE_SERVER_TIMING)); - headers.put( - Metadata.Key.of( - "io.grpc.Grpc.TRANSPORT_ATTR_REMOTE_ADDR", - Metadata.ASCII_STRING_MARSHALLER), - "localhost"); ResponseParams params = ResponseParams.newBuilder().setZoneId(ZONE).setClusterId(CLUSTER).build(); @@ -272,8 +268,10 @@ public void sendMessage(ReqT message) { if (oldConfigurator != null) { builder = oldConfigurator.apply(builder); } + builder.intercept(new TargetTracerInterceptor()); return builder.intercept(clientInterceptor); }); + stubSettingsBuilder.setTransportChannelProvider(channelProvider.build()); EnhancedBigtableStubSettings stubSettings = stubSettingsBuilder.build(); @@ -302,12 +300,12 @@ public void testReadRowsOperationLatencies() { .put(METHOD_KEY, "Bigtable.ReadRows") .put(STREAMING_KEY, true) .put(CLIENT_NAME_KEY, CLIENT_NAME) - .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); Collection allMetricData = metricReader.collectAllMetrics(); MetricData metricData = getMetricData(allMetricData, OPERATION_LATENCIES_NAME); + long value = getAggregatedValue(metricData, expectedAttributes); assertThat(value).isIn(Range.closed(SERVER_LATENCY, elapsed)); } @@ -330,10 +328,11 @@ public void testReadRowsOperationLatenciesOnAuthorizedView() { .put(METHOD_KEY, "Bigtable.ReadRows") .put(STREAMING_KEY, true) .put(CLIENT_NAME_KEY, CLIENT_NAME) - .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); - MetricData metricData = getMetricData(metricReader, OPERATION_LATENCIES_NAME); + Collection allMetricData = metricReader.collectAllMetrics(); + + MetricData metricData = getMetricData(allMetricData, OPERATION_LATENCIES_NAME); long value = getAggregatedValue(metricData, expectedAttributes); assertThat(value).isIn(Range.closed(SERVER_LATENCY, elapsed)); } @@ -351,16 +350,17 @@ public void testGfeMetrics() { .put(CLUSTER_ID_KEY, CLUSTER) .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(METHOD_KEY, "Bigtable.ReadRows") - .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); - MetricData serverLatenciesMetricData = getMetricData(metricReader, SERVER_LATENCIES_NAME); + Collection allMetricData = metricReader.collectAllMetrics(); + + MetricData serverLatenciesMetricData = getMetricData(allMetricData, SERVER_LATENCIES_NAME); long serverLatencies = getAggregatedValue(serverLatenciesMetricData, expectedAttributes); assertThat(serverLatencies).isEqualTo(FAKE_SERVER_TIMING); MetricData connectivityErrorCountMetricData = - getMetricData(metricReader, CONNECTIVITY_ERROR_COUNT_NAME); + getMetricData(allMetricData, CONNECTIVITY_ERROR_COUNT_NAME); Attributes expected1 = baseAttributes .toBuilder() @@ -370,7 +370,6 @@ public void testGfeMetrics() { .put(CLUSTER_ID_KEY, "unspecified") .put(METHOD_KEY, "Bigtable.ReadRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) - .put(TARGET_KEY, new ArrayList()) .build(); Attributes expected2 = baseAttributes @@ -381,7 +380,6 @@ public void testGfeMetrics() { .put(CLUSTER_ID_KEY, CLUSTER) .put(METHOD_KEY, "Bigtable.ReadRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) - .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); verifyAttributes(connectivityErrorCountMetricData, expected1); @@ -426,8 +424,9 @@ public void onComplete() { assertThat(counter.get()).isEqualTo(fakeService.getResponseCounter().get()); + Collection allMetricData = metricReader.collectAllMetrics(); MetricData applicationLatency = - getMetricData(metricReader, APPLICATION_BLOCKING_LATENCIES_NAME); + getMetricData(allMetricData, APPLICATION_BLOCKING_LATENCIES_NAME); Attributes expectedAttributes = baseAttributes @@ -437,13 +436,12 @@ public void onComplete() { .put(CLUSTER_ID_KEY, CLUSTER) .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(METHOD_KEY, "Bigtable.ReadRows") - .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); long value = getAggregatedValue(applicationLatency, expectedAttributes); assertThat(value).isAtLeast((APPLICATION_LATENCY - SLEEP_VARIABILITY) * counter.get()); - MetricData operationLatency = getMetricData(metricReader, OPERATION_LATENCIES_NAME); + MetricData operationLatency = getMetricData(allMetricData, OPERATION_LATENCIES_NAME); long operationLatencyValue = getAggregatedValue( operationLatency, @@ -463,8 +461,9 @@ public void testReadRowsApplicationLatencyWithManualFlowControl() throws Excepti rows.next(); } + Collection allMetricData = metricReader.collectAllMetrics(); MetricData applicationLatency = - getMetricData(metricReader, APPLICATION_BLOCKING_LATENCIES_NAME); + getMetricData(allMetricData, APPLICATION_BLOCKING_LATENCIES_NAME); Attributes expectedAttributes = baseAttributes @@ -474,7 +473,6 @@ public void testReadRowsApplicationLatencyWithManualFlowControl() throws Excepti .put(CLUSTER_ID_KEY, CLUSTER) .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(METHOD_KEY, "Bigtable.ReadRows") - .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); long value = getAggregatedValue(applicationLatency, expectedAttributes); @@ -483,7 +481,7 @@ public void testReadRowsApplicationLatencyWithManualFlowControl() throws Excepti assertThat(counter).isEqualTo(fakeService.getResponseCounter().get()); assertThat(value).isAtLeast(APPLICATION_LATENCY * (counter - 1) - SERVER_LATENCY); - MetricData operationLatency = getMetricData(metricReader, OPERATION_LATENCIES_NAME); + MetricData operationLatency = getMetricData(allMetricData, OPERATION_LATENCIES_NAME); long operationLatencyValue = getAggregatedValue( operationLatency, @@ -496,7 +494,8 @@ public void testRetryCount() throws InterruptedException { stub.mutateRowCallable() .call(RowMutation.create(TABLE, "random-row").setCell("cf", "q", "value")); - MetricData metricData = getMetricData(metricReader, RETRY_COUNT_NAME); + Collection allMetricData = metricReader.collectAllMetrics(); + MetricData metricData = getMetricData(allMetricData, RETRY_COUNT_NAME); Attributes expectedAttributes = baseAttributes .toBuilder() @@ -506,7 +505,6 @@ public void testRetryCount() throws InterruptedException { .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(METHOD_KEY, "Bigtable.MutateRow") .put(STATUS_KEY, "OK") - .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); long value = getAggregatedValue(metricData, expectedAttributes); @@ -518,7 +516,8 @@ public void testMutateRowAttemptsTagValues() { stub.mutateRowCallable() .call(RowMutation.create(TABLE, "random-row").setCell("cf", "q", "value")); - MetricData metricData = getMetricData(metricReader, ATTEMPT_LATENCIES_NAME); + Collection allMetricData = metricReader.collectAllMetrics(); + MetricData metricData = getMetricData(allMetricData, ATTEMPT_LATENCIES_NAME); Attributes expected1 = baseAttributes @@ -530,7 +529,7 @@ public void testMutateRowAttemptsTagValues() { .put(METHOD_KEY, "Bigtable.MutateRow") .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(STREAMING_KEY, false) - .put(TARGET_KEY, new ArrayList<>()) + .put(TARGET_KEY, "unspecified") .build(); Attributes expected2 = @@ -543,7 +542,7 @@ public void testMutateRowAttemptsTagValues() { .put(METHOD_KEY, "Bigtable.MutateRow") .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(STREAMING_KEY, false) - .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) + .put(TARGET_KEY, String.format(TARGET_ENDPOINT_VALUE_FORMAT,String.valueOf(server.getPort()))) .build(); verifyAttributes(metricData, expected1); @@ -561,7 +560,8 @@ public void testMutateRowsPartialError() throws InterruptedException { Assert.assertThrows(BatchingException.class, batcher::close); - MetricData metricData = getMetricData(metricReader, ATTEMPT_LATENCIES_NAME); + Collection allMetricData = metricReader.collectAllMetrics(); + MetricData metricData = getMetricData(allMetricData, ATTEMPT_LATENCIES_NAME); Attributes expected = baseAttributes @@ -573,7 +573,7 @@ public void testMutateRowsPartialError() throws InterruptedException { .put(METHOD_KEY, "Bigtable.MutateRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(STREAMING_KEY, false) - .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) + .put(TARGET_KEY, String.format(TARGET_ENDPOINT_VALUE_FORMAT,String.valueOf(server.getPort()))) .build(); verifyAttributes(metricData, expected); @@ -591,7 +591,8 @@ public void testMutateRowsRpcError() { Assert.assertThrows(BatchingException.class, batcher::close); - MetricData metricData = getMetricData(metricReader, ATTEMPT_LATENCIES_NAME); + Collection allMetricData = metricReader.collectAllMetrics(); + MetricData metricData = getMetricData(allMetricData, ATTEMPT_LATENCIES_NAME); Attributes expected = baseAttributes @@ -603,7 +604,7 @@ public void testMutateRowsRpcError() { .put(METHOD_KEY, "Bigtable.MutateRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(STREAMING_KEY, false) - .put(TARGET_KEY, new ArrayList<>()) + .put(TARGET_KEY, "unspecified") .build(); verifyAttributes(metricData, expected); @@ -613,7 +614,8 @@ public void testMutateRowsRpcError() { public void testReadRowsAttemptsTagValues() { Lists.newArrayList(stub.readRowsCallable().call(Query.create("fake-table")).iterator()); - MetricData metricData = getMetricData(metricReader, ATTEMPT_LATENCIES_NAME); + Collection allMetricData = metricReader.collectAllMetrics(); + MetricData metricData = getMetricData(allMetricData, ATTEMPT_LATENCIES_NAME); Attributes expected1 = baseAttributes @@ -625,7 +627,7 @@ public void testReadRowsAttemptsTagValues() { .put(METHOD_KEY, "Bigtable.ReadRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(STREAMING_KEY, true) - .put(TARGET_KEY, new ArrayList()) + .put(TARGET_KEY, "unspecified") .build(); Attributes expected2 = @@ -638,7 +640,7 @@ public void testReadRowsAttemptsTagValues() { .put(METHOD_KEY, "Bigtable.ReadRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(STREAMING_KEY, true) - .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) + .put(TARGET_KEY, String.format(TARGET_ENDPOINT_VALUE_FORMAT,String.valueOf(server.getPort()))) .build(); verifyAttributes(metricData, expected1); @@ -657,7 +659,8 @@ public void testBatchBlockingLatencies() throws InterruptedException { int expectedNumRequests = 6 / batchElementCount; - MetricData applicationLatency = getMetricData(metricReader, CLIENT_BLOCKING_LATENCIES_NAME); + Collection allMetricData = metricReader.collectAllMetrics(); + MetricData applicationLatency = getMetricData(allMetricData, CLIENT_BLOCKING_LATENCIES_NAME); Attributes expectedAttributes = baseAttributes @@ -667,7 +670,6 @@ public void testBatchBlockingLatencies() throws InterruptedException { .put(CLUSTER_ID_KEY, CLUSTER) .put(METHOD_KEY, "Bigtable.MutateRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) - .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); long value = getAggregatedValue(applicationLatency, expectedAttributes); @@ -683,7 +685,8 @@ public void testBatchBlockingLatencies() throws InterruptedException { public void testQueuedOnChannelServerStreamLatencies() { stub.readRowsCallable().all().call(Query.create(TABLE)); - MetricData clientLatency = getMetricData(metricReader, CLIENT_BLOCKING_LATENCIES_NAME); + Collection allMetricData = metricReader.collectAllMetrics(); + MetricData clientLatency = getMetricData(allMetricData, CLIENT_BLOCKING_LATENCIES_NAME); Attributes attributes = baseAttributes @@ -693,7 +696,6 @@ public void testQueuedOnChannelServerStreamLatencies() { .put(ZONE_ID_KEY, ZONE) .put(METHOD_KEY, "Bigtable.ReadRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) - .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); long value = getAggregatedValue(clientLatency, attributes); @@ -705,7 +707,8 @@ public void testQueuedOnChannelUnaryLatencies() { stub.mutateRowCallable().call(RowMutation.create(TABLE, "a-key").setCell("f", "q", "v")); - MetricData clientLatency = getMetricData(metricReader, CLIENT_BLOCKING_LATENCIES_NAME); + Collection allMetricData = metricReader.collectAllMetrics(); + MetricData clientLatency = getMetricData(allMetricData, CLIENT_BLOCKING_LATENCIES_NAME); Attributes attributes = baseAttributes @@ -715,7 +718,6 @@ public void testQueuedOnChannelUnaryLatencies() { .put(ZONE_ID_KEY, ZONE) .put(METHOD_KEY, "Bigtable.MutateRow") .put(CLIENT_NAME_KEY, CLIENT_NAME) - .put(TARGET_KEY, TARGET_ATTRIBUTE_VALUE) .build(); long expected = CHANNEL_BLOCKING_LATENCY * 2 / 3; @@ -731,7 +733,8 @@ public void testPermanentFailure() { } catch (NotFoundException e) { } - MetricData attemptLatency = getMetricData(metricReader, ATTEMPT_LATENCIES_NAME); + Collection allMetricData = metricReader.collectAllMetrics(); + MetricData attemptLatency = getMetricData(allMetricData, ATTEMPT_LATENCIES_NAME); Attributes expected = baseAttributes @@ -743,13 +746,24 @@ public void testPermanentFailure() { .put(STREAMING_KEY, true) .put(METHOD_KEY, "Bigtable.ReadRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) - .put(TARGET_KEY, new ArrayList<>()) + .put(TARGET_KEY, "unspecified") .build(); verifyAttributes(attemptLatency, expected); - MetricData opLatency = getMetricData(metricReader, OPERATION_LATENCIES_NAME); - verifyAttributes(opLatency, expected); + MetricData opLatency = getMetricData(allMetricData, OPERATION_LATENCIES_NAME); + Attributes expectedOperationLatencyAttributes = + baseAttributes + .toBuilder() + .put(STATUS_KEY, "NOT_FOUND") + .put(TABLE_ID_KEY, BAD_TABLE_ID) + .put(CLUSTER_ID_KEY, "unspecified") + .put(ZONE_ID_KEY, "global") + .put(STREAMING_KEY, true) + .put(METHOD_KEY, "Bigtable.ReadRows") + .put(CLIENT_NAME_KEY, CLIENT_NAME) + .build(); + verifyAttributes(opLatency, expectedOperationLatencyAttributes); } private static class FakeService extends BigtableGrpc.BigtableImplBase { From 31c4d21637f4695a81cbfa7348c1b9d2aa05da75 Mon Sep 17 00:00:00 2001 From: Meeral Date: Thu, 20 Jun 2024 15:21:05 +0000 Subject: [PATCH 17/18] fix unit test --- .../metrics/BuiltinMetricsTracerTest.java | 54 +++++++------------ 1 file changed, 18 insertions(+), 36 deletions(-) diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java index e0b9d66cd4..1a50e68478 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java @@ -302,9 +302,7 @@ public void testReadRowsOperationLatencies() { .put(CLIENT_NAME_KEY, CLIENT_NAME) .build(); - Collection allMetricData = metricReader.collectAllMetrics(); - - MetricData metricData = getMetricData(allMetricData, OPERATION_LATENCIES_NAME); + MetricData metricData = getMetricData(metricReader, OPERATION_LATENCIES_NAME); long value = getAggregatedValue(metricData, expectedAttributes); assertThat(value).isIn(Range.closed(SERVER_LATENCY, elapsed)); @@ -330,9 +328,7 @@ public void testReadRowsOperationLatenciesOnAuthorizedView() { .put(CLIENT_NAME_KEY, CLIENT_NAME) .build(); - Collection allMetricData = metricReader.collectAllMetrics(); - - MetricData metricData = getMetricData(allMetricData, OPERATION_LATENCIES_NAME); + MetricData metricData = getMetricData(metricReader, OPERATION_LATENCIES_NAME); long value = getAggregatedValue(metricData, expectedAttributes); assertThat(value).isIn(Range.closed(SERVER_LATENCY, elapsed)); } @@ -352,15 +348,13 @@ public void testGfeMetrics() { .put(METHOD_KEY, "Bigtable.ReadRows") .build(); - Collection allMetricData = metricReader.collectAllMetrics(); - - MetricData serverLatenciesMetricData = getMetricData(allMetricData, SERVER_LATENCIES_NAME); + MetricData serverLatenciesMetricData = getMetricData(metricReader, SERVER_LATENCIES_NAME); long serverLatencies = getAggregatedValue(serverLatenciesMetricData, expectedAttributes); assertThat(serverLatencies).isEqualTo(FAKE_SERVER_TIMING); MetricData connectivityErrorCountMetricData = - getMetricData(allMetricData, CONNECTIVITY_ERROR_COUNT_NAME); + getMetricData(metricReader, CONNECTIVITY_ERROR_COUNT_NAME); Attributes expected1 = baseAttributes .toBuilder() @@ -424,9 +418,8 @@ public void onComplete() { assertThat(counter.get()).isEqualTo(fakeService.getResponseCounter().get()); - Collection allMetricData = metricReader.collectAllMetrics(); MetricData applicationLatency = - getMetricData(allMetricData, APPLICATION_BLOCKING_LATENCIES_NAME); + getMetricData(metricReader, APPLICATION_BLOCKING_LATENCIES_NAME); Attributes expectedAttributes = baseAttributes @@ -441,7 +434,7 @@ public void onComplete() { assertThat(value).isAtLeast((APPLICATION_LATENCY - SLEEP_VARIABILITY) * counter.get()); - MetricData operationLatency = getMetricData(allMetricData, OPERATION_LATENCIES_NAME); + MetricData operationLatency = getMetricData(metricReader, OPERATION_LATENCIES_NAME); long operationLatencyValue = getAggregatedValue( operationLatency, @@ -461,9 +454,8 @@ public void testReadRowsApplicationLatencyWithManualFlowControl() throws Excepti rows.next(); } - Collection allMetricData = metricReader.collectAllMetrics(); MetricData applicationLatency = - getMetricData(allMetricData, APPLICATION_BLOCKING_LATENCIES_NAME); + getMetricData(metricReader, APPLICATION_BLOCKING_LATENCIES_NAME); Attributes expectedAttributes = baseAttributes @@ -481,7 +473,7 @@ public void testReadRowsApplicationLatencyWithManualFlowControl() throws Excepti assertThat(counter).isEqualTo(fakeService.getResponseCounter().get()); assertThat(value).isAtLeast(APPLICATION_LATENCY * (counter - 1) - SERVER_LATENCY); - MetricData operationLatency = getMetricData(allMetricData, OPERATION_LATENCIES_NAME); + MetricData operationLatency = getMetricData(metricReader, OPERATION_LATENCIES_NAME); long operationLatencyValue = getAggregatedValue( operationLatency, @@ -494,8 +486,7 @@ public void testRetryCount() throws InterruptedException { stub.mutateRowCallable() .call(RowMutation.create(TABLE, "random-row").setCell("cf", "q", "value")); - Collection allMetricData = metricReader.collectAllMetrics(); - MetricData metricData = getMetricData(allMetricData, RETRY_COUNT_NAME); + MetricData metricData = getMetricData(metricReader, RETRY_COUNT_NAME); Attributes expectedAttributes = baseAttributes .toBuilder() @@ -516,8 +507,7 @@ public void testMutateRowAttemptsTagValues() { stub.mutateRowCallable() .call(RowMutation.create(TABLE, "random-row").setCell("cf", "q", "value")); - Collection allMetricData = metricReader.collectAllMetrics(); - MetricData metricData = getMetricData(allMetricData, ATTEMPT_LATENCIES_NAME); + MetricData metricData = getMetricData(metricReader, ATTEMPT_LATENCIES_NAME); Attributes expected1 = baseAttributes @@ -560,8 +550,7 @@ public void testMutateRowsPartialError() throws InterruptedException { Assert.assertThrows(BatchingException.class, batcher::close); - Collection allMetricData = metricReader.collectAllMetrics(); - MetricData metricData = getMetricData(allMetricData, ATTEMPT_LATENCIES_NAME); + MetricData metricData = getMetricData(metricReader, ATTEMPT_LATENCIES_NAME); Attributes expected = baseAttributes @@ -591,8 +580,7 @@ public void testMutateRowsRpcError() { Assert.assertThrows(BatchingException.class, batcher::close); - Collection allMetricData = metricReader.collectAllMetrics(); - MetricData metricData = getMetricData(allMetricData, ATTEMPT_LATENCIES_NAME); + MetricData metricData = getMetricData(metricReader, ATTEMPT_LATENCIES_NAME); Attributes expected = baseAttributes @@ -604,7 +592,6 @@ public void testMutateRowsRpcError() { .put(METHOD_KEY, "Bigtable.MutateRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(STREAMING_KEY, false) - .put(TARGET_KEY, "unspecified") .build(); verifyAttributes(metricData, expected); @@ -614,8 +601,7 @@ public void testMutateRowsRpcError() { public void testReadRowsAttemptsTagValues() { Lists.newArrayList(stub.readRowsCallable().call(Query.create("fake-table")).iterator()); - Collection allMetricData = metricReader.collectAllMetrics(); - MetricData metricData = getMetricData(allMetricData, ATTEMPT_LATENCIES_NAME); + MetricData metricData = getMetricData(metricReader, ATTEMPT_LATENCIES_NAME); Attributes expected1 = baseAttributes @@ -659,8 +645,7 @@ public void testBatchBlockingLatencies() throws InterruptedException { int expectedNumRequests = 6 / batchElementCount; - Collection allMetricData = metricReader.collectAllMetrics(); - MetricData applicationLatency = getMetricData(allMetricData, CLIENT_BLOCKING_LATENCIES_NAME); + MetricData applicationLatency = getMetricData(metricReader, CLIENT_BLOCKING_LATENCIES_NAME); Attributes expectedAttributes = baseAttributes @@ -685,8 +670,7 @@ public void testBatchBlockingLatencies() throws InterruptedException { public void testQueuedOnChannelServerStreamLatencies() { stub.readRowsCallable().all().call(Query.create(TABLE)); - Collection allMetricData = metricReader.collectAllMetrics(); - MetricData clientLatency = getMetricData(allMetricData, CLIENT_BLOCKING_LATENCIES_NAME); + MetricData clientLatency = getMetricData(metricReader, CLIENT_BLOCKING_LATENCIES_NAME); Attributes attributes = baseAttributes @@ -707,8 +691,7 @@ public void testQueuedOnChannelUnaryLatencies() { stub.mutateRowCallable().call(RowMutation.create(TABLE, "a-key").setCell("f", "q", "v")); - Collection allMetricData = metricReader.collectAllMetrics(); - MetricData clientLatency = getMetricData(allMetricData, CLIENT_BLOCKING_LATENCIES_NAME); + MetricData clientLatency = getMetricData(metricReader, CLIENT_BLOCKING_LATENCIES_NAME); Attributes attributes = baseAttributes @@ -733,8 +716,7 @@ public void testPermanentFailure() { } catch (NotFoundException e) { } - Collection allMetricData = metricReader.collectAllMetrics(); - MetricData attemptLatency = getMetricData(allMetricData, ATTEMPT_LATENCIES_NAME); + MetricData attemptLatency = getMetricData(metricReader, ATTEMPT_LATENCIES_NAME); Attributes expected = baseAttributes @@ -751,7 +733,7 @@ public void testPermanentFailure() { verifyAttributes(attemptLatency, expected); - MetricData opLatency = getMetricData(allMetricData, OPERATION_LATENCIES_NAME); + MetricData opLatency = getMetricData(metricReader, OPERATION_LATENCIES_NAME); Attributes expectedOperationLatencyAttributes = baseAttributes .toBuilder() From ccf778589dcdf00be0706155ecda71edeffa8073 Mon Sep 17 00:00:00 2001 From: Meeral Date: Thu, 20 Jun 2024 15:28:37 +0000 Subject: [PATCH 18/18] addressed code comments --- .../data/v2/stub/metrics/BuiltinMetricsTracer.java | 8 ++------ .../google/cloud/bigtable/data/v2/stub/metrics/Util.java | 1 - 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 53b0d86090..b0450fa78b 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -29,11 +29,10 @@ import com.google.api.gax.tracing.SpanName; import com.google.cloud.bigtable.Version; import com.google.common.base.Stopwatch; +import com.google.common.base.Strings; import com.google.common.math.IntMath; import io.grpc.CallOptions; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.common.AttributesBuilder; -import io.opentelemetry.api.internal.StringUtils; import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.LongCounter; import java.util.concurrent.CancellationException; @@ -49,9 +48,6 @@ * bigtable.googleapis.com/client namespace */ class BuiltinMetricsTracer extends BigtableTracer { - - static final CallOptions.Key BUILTIN_METRICSTRACER_KEY = - CallOptions.Key.create("builtin-metrics-tracer"); private static final String NAME = "java-bigtable/" + Version.VERSION; private final OperationType operationType; private final SpanName spanName; @@ -184,7 +180,7 @@ public void attemptSucceeded() { } public void addTarget(String target) { - if (!StringUtils.isNullOrEmpty(target)) { + if (!Strings.isNullOrEmpty(target)) { this.target_endpoint = target; } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index b3f53cc4fc..0eca939901 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -39,7 +39,6 @@ import io.grpc.StatusException; import io.grpc.StatusRuntimeException; import io.opencensus.tags.TagValue; -import io.opentelemetry.api.internal.StringUtils; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Arrays;