From 33131a5bbbc51d0cfcc956d33a7c0472889b16db Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 23 Feb 2024 17:02:36 -0500 Subject: [PATCH 1/7] feat: migrate per connection error count metric to otel --- .../clirr-ignored-differences.xml | 6 ++ google-cloud-bigtable/pom.xml | 4 + .../data/v2/BigtableDataClientFactory.java | 22 +++-- .../data/v2/stub/EnhancedBigtableStub.java | 86 +++++++++---------- .../stub/metrics/BuiltinMetricsConstants.java | 64 +++++++++++--- .../v2/stub/metrics/BuiltinMetricsView.java | 2 +- .../CustomOpenTelemetryMetricsProvider.java | 2 +- .../stub/metrics/DefaultMetricsProvider.java | 19 ++++ .../ErrorCountPerConnectionMetricTracker.java | 37 ++++++-- .../v2/BigtableDataClientFactoryTest.java | 2 +- .../metrics/BigtableTracerCallableTest.java | 4 +- .../v2/stub/metrics/MetricsTracerTest.java | 2 +- 12 files changed, 177 insertions(+), 73 deletions(-) diff --git a/google-cloud-bigtable/clirr-ignored-differences.xml b/google-cloud-bigtable/clirr-ignored-differences.xml index e8e7f3ab0a..7bc32ca2be 100644 --- a/google-cloud-bigtable/clirr-ignored-differences.xml +++ b/google-cloud-bigtable/clirr-ignored-differences.xml @@ -186,4 +186,10 @@ com/google/cloud/bigtable/data/v2/models/MutateRowsException * + + + 7004 + com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker + * + diff --git a/google-cloud-bigtable/pom.xml b/google-cloud-bigtable/pom.xml index 31fbb6e0b6..45307e0c35 100644 --- a/google-cloud-bigtable/pom.xml +++ b/google-cloud-bigtable/pom.xml @@ -62,6 +62,10 @@ + + com.google.cloud + google-cloud-bigtable-stats + diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactory.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactory.java index e0b5ee398c..45ec5af814 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactory.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactory.java @@ -19,6 +19,7 @@ import com.google.api.gax.core.BackgroundResource; import com.google.api.gax.rpc.ClientContext; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStub; +import io.opentelemetry.api.OpenTelemetry; import java.io.IOException; import javax.annotation.Nonnull; @@ -64,6 +65,7 @@ public final class BigtableDataClientFactory implements AutoCloseable { private final BigtableDataSettings defaultSettings; private final ClientContext sharedClientContext; + private final OpenTelemetry openTelemetry; /** * Create a instance of this factory. @@ -75,13 +77,21 @@ public static BigtableDataClientFactory create(BigtableDataSettings defaultSetti throws IOException { ClientContext sharedClientContext = EnhancedBigtableStub.createClientContext(defaultSettings.getStubSettings()); - return new BigtableDataClientFactory(sharedClientContext, defaultSettings); + OpenTelemetry openTelemetry = + EnhancedBigtableStub.getOpenTelemetry( + defaultSettings.getProjectId(), + defaultSettings.getMetricsProvider(), + sharedClientContext.getCredentials()); + return new BigtableDataClientFactory(sharedClientContext, defaultSettings, openTelemetry); } private BigtableDataClientFactory( - ClientContext sharedClientContext, BigtableDataSettings defaultSettings) { + ClientContext sharedClientContext, + BigtableDataSettings defaultSettings, + OpenTelemetry openTelemetry) { this.sharedClientContext = sharedClientContext; this.defaultSettings = defaultSettings; + this.openTelemetry = openTelemetry; } /** @@ -112,7 +122,7 @@ public BigtableDataClient createDefault() { .toBuilder() .setTracerFactory( EnhancedBigtableStub.createBigtableTracerFactory( - defaultSettings.getStubSettings(), sharedClientContext)) + defaultSettings.getStubSettings(), openTelemetry)) .build(); return BigtableDataClient.createWithClientContext(defaultSettings, clientContext); @@ -141,7 +151,7 @@ public BigtableDataClient createForAppProfile(@Nonnull String appProfileId) thro .toBuilder() .setTracerFactory( EnhancedBigtableStub.createBigtableTracerFactory( - settings.getStubSettings(), sharedClientContext)) + settings.getStubSettings(), openTelemetry)) .build(); return BigtableDataClient.createWithClientContext(settings, clientContext); } @@ -170,7 +180,7 @@ public BigtableDataClient createForInstance(@Nonnull String projectId, @Nonnull .toBuilder() .setTracerFactory( EnhancedBigtableStub.createBigtableTracerFactory( - settings.getStubSettings(), sharedClientContext)) + settings.getStubSettings(), openTelemetry)) .build(); return BigtableDataClient.createWithClientContext(settings, clientContext); @@ -200,7 +210,7 @@ public BigtableDataClient createForInstance( .toBuilder() .setTracerFactory( EnhancedBigtableStub.createBigtableTracerFactory( - settings.getStubSettings(), sharedClientContext)) + settings.getStubSettings(), openTelemetry)) .build(); return BigtableDataClient.createWithClientContext(settings, clientContext); } 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 b39a9e7933..9fe2a56789 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 @@ -16,6 +16,7 @@ package com.google.cloud.bigtable.data.v2.stub; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.APP_PROFILE_KEY; +import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.CLIENT_NAME_KEY; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.INSTANCE_ID_KEY; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.PROJECT_ID_KEY; @@ -99,12 +100,11 @@ import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracerStreamingCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracerUnaryCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsTracerFactory; -import com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsView; import com.google.cloud.bigtable.data.v2.stub.metrics.CompositeTracerFactory; import com.google.cloud.bigtable.data.v2.stub.metrics.CustomOpenTelemetryMetricsProvider; import com.google.cloud.bigtable.data.v2.stub.metrics.DefaultMetricsProvider; -import com.google.cloud.bigtable.data.v2.stub.metrics.MetricsProvider; import com.google.cloud.bigtable.data.v2.stub.metrics.ErrorCountPerConnectionMetricTracker; +import com.google.cloud.bigtable.data.v2.stub.metrics.MetricsProvider; import com.google.cloud.bigtable.data.v2.stub.metrics.MetricsTracerFactory; import com.google.cloud.bigtable.data.v2.stub.metrics.NoopMetricsProvider; import com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants; @@ -138,9 +138,6 @@ import io.opencensus.tags.Tags; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.sdk.OpenTelemetrySdk; -import io.opentelemetry.sdk.metrics.SdkMeterProvider; -import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; @@ -194,10 +191,13 @@ public class EnhancedBigtableStub implements AutoCloseable { public static EnhancedBigtableStub create(EnhancedBigtableStubSettings settings) throws IOException { ClientContext clientContext = createClientContext(settings); + OpenTelemetry openTelemetry = + getOpenTelemetry( + settings.getProjectId(), settings.getMetricsProvider(), clientContext.getCredentials()); ClientContext contextWithTracer = clientContext .toBuilder() - .setTracerFactory(createBigtableTracerFactory(settings, clientContext)) + .setTracerFactory(createBigtableTracerFactory(settings, openTelemetry)) .build(); return new EnhancedBigtableStub(settings, contextWithTracer); @@ -219,15 +219,25 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set // workaround JWT audience issues patchCredentials(builder); + // Fix the credentials so that they can be shared + Credentials credentials = null; + if (builder.getCredentialsProvider() != null) { + credentials = builder.getCredentialsProvider().getCredentials(); + } + builder.setCredentialsProvider(FixedCredentialsProvider.create(credentials)); + InstantiatingGrpcChannelProvider.Builder transportProvider = builder.getTransportChannelProvider() instanceof InstantiatingGrpcChannelProvider ? ((InstantiatingGrpcChannelProvider) builder.getTransportChannelProvider()).toBuilder() : null; ErrorCountPerConnectionMetricTracker errorCountPerConnectionMetricTracker; + OpenTelemetry openTelemetry = + getOpenTelemetry(settings.getProjectId(), settings.getMetricsProvider(), credentials); if (transportProvider != null) { errorCountPerConnectionMetricTracker = - new ErrorCountPerConnectionMetricTracker(createBuiltinAttributes(builder)); + new ErrorCountPerConnectionMetricTracker( + openTelemetry, createBuiltinAttributes(settings)); ApiFunction oldChannelConfigurator = transportProvider.getChannelConfigurator(); transportProvider.setChannelConfigurator( @@ -249,12 +259,6 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set // Inject channel priming if (settings.isRefreshingChannel()) { - // Fix the credentials so that they can be shared - Credentials credentials = null; - if (builder.getCredentialsProvider() != null) { - credentials = builder.getCredentialsProvider().getCredentials(); - } - builder.setCredentialsProvider(FixedCredentialsProvider.create(credentials)); if (transportProvider != null) { transportProvider.setChannelPrimer( @@ -271,7 +275,7 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set } ClientContext clientContext = ClientContext.create(builder.build()); - if (errorCountPerConnectionMetricTracker != null) { + if (errorCountPerConnectionMetricTracker != null && openTelemetry != null) { errorCountPerConnectionMetricTracker.startConnectionErrorCountTracker( clientContext.getExecutor()); } @@ -279,9 +283,9 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set } public static ApiTracerFactory createBigtableTracerFactory( - EnhancedBigtableStubSettings settings, ClientContext clientContext) throws IOException { + EnhancedBigtableStubSettings settings, OpenTelemetry openTelemetry) throws IOException { return createBigtableTracerFactory( - settings, Tags.getTagger(), Stats.getStatsRecorder(), clientContext); + settings, Tags.getTagger(), Stats.getStatsRecorder(), openTelemetry); } @VisibleForTesting @@ -289,7 +293,7 @@ public static ApiTracerFactory createBigtableTracerFactory( EnhancedBigtableStubSettings settings, Tagger tagger, StatsRecorder stats, - ClientContext clientContext) + OpenTelemetry openTelemetry) throws IOException { String projectId = settings.getProjectId(); String instanceId = settings.getInstanceId(); @@ -302,8 +306,6 @@ public static ApiTracerFactory createBigtableTracerFactory( .put(RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID, TagValue.create(appProfileId)) .build(); - ImmutableMap builtinAttributes = createBuiltinAttributes(settings.toBuilder()); - ImmutableList.Builder tracerFactories = ImmutableList.builder(); tracerFactories .add( @@ -323,53 +325,47 @@ public static ApiTracerFactory createBigtableTracerFactory( .add(MetricsTracerFactory.create(tagger, stats, attributes)) // Add user configured tracer .add(settings.getTracerFactory()); - Attributes otelAttributes = - Attributes.of( - PROJECT_ID_KEY, projectId, INSTANCE_ID_KEY, instanceId, APP_PROFILE_KEY, appProfileId); BuiltinMetricsTracerFactory builtinMetricsTracerFactory = - createBuiltinMetricsTracerFactory( - projectId, settings.getMetricsProvider(), otelAttributes, clientContext); + openTelemetry != null + ? BuiltinMetricsTracerFactory.create(openTelemetry, createBuiltinAttributes(settings)) + : null; if (builtinMetricsTracerFactory != null) { tracerFactories.add(builtinMetricsTracerFactory); } return new CompositeTracerFactory(tracerFactories.build()); } - private static BuiltinMetricsTracerFactory createBuiltinMetricsTracerFactory( - String projectId, - MetricsProvider metricsProvider, - Attributes attributes, - ClientContext clientContext) + @Nullable + public static OpenTelemetry getOpenTelemetry( + String projectId, MetricsProvider metricsProvider, Credentials defaultCredentials) throws IOException { if (metricsProvider instanceof CustomOpenTelemetryMetricsProvider) { CustomOpenTelemetryMetricsProvider customMetricsProvider = (CustomOpenTelemetryMetricsProvider) metricsProvider; - return BuiltinMetricsTracerFactory.create( - customMetricsProvider.getOpenTelemetry(), attributes); + return customMetricsProvider.getOpenTelemetry(); } else if (metricsProvider instanceof DefaultMetricsProvider) { - SdkMeterProviderBuilder meterProvider = SdkMeterProvider.builder(); Credentials credentials = BigtableDataSettings.getMetricsCredentials() != null ? BigtableDataSettings.getMetricsCredentials() - : clientContext.getCredentials(); - BuiltinMetricsView.registerBuiltinMetrics(projectId, credentials, meterProvider); - OpenTelemetry openTelemetry = - OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build(); - return BuiltinMetricsTracerFactory.create(openTelemetry, attributes); + : defaultCredentials; + DefaultMetricsProvider defaultMetricsProvider = (DefaultMetricsProvider) metricsProvider; + return defaultMetricsProvider.getOpenTelemetry(projectId, credentials); } else if (metricsProvider instanceof NoopMetricsProvider) { return null; } throw new IOException("Invalid MetricsProvider type " + metricsProvider); } - private static ImmutableMap createBuiltinAttributes( - EnhancedBigtableStubSettings.Builder builder) { - return ImmutableMap.builder() - .put("project_id", builder.getProjectId()) - .put("instance", builder.getInstanceId()) - .put("app_profile", builder.getAppProfileId()) - .put("client_name", "bigtable-java/" + Version.VERSION) - .build(); + private static Attributes createBuiltinAttributes(EnhancedBigtableStubSettings settings) { + return Attributes.of( + PROJECT_ID_KEY, + settings.getProjectId(), + INSTANCE_ID_KEY, + settings.getInstanceId(), + APP_PROFILE_KEY, + settings.getAppProfileId(), + CLIENT_NAME_KEY, + "bigtable-java/" + Version.VERSION); } private static void patchCredentials(EnhancedBigtableStubSettings.Builder settings) 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 807191b7e2..b26557999a 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 @@ -44,9 +44,9 @@ public class BuiltinMetricsConstants { // IT tests, so they're public. public static final AttributeKey APP_PROFILE_KEY = AttributeKey.stringKey("app_profile"); public static final AttributeKey STREAMING_KEY = AttributeKey.booleanKey("streaming"); + 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 final AttributeKey CLIENT_NAME_KEY = AttributeKey.stringKey("client_name"); static final AttributeKey CLIENT_UID_KEY = AttributeKey.stringKey("client_uid"); // Metric names @@ -58,6 +58,7 @@ public class BuiltinMetricsConstants { static final String FIRST_RESPONSE_LATENCIES_NAME = "first_response_latencies"; static final String APPLICATION_BLOCKING_LATENCIES_NAME = "application_latencies"; static final String CLIENT_BLOCKING_LATENCIES_NAME = "throttling_latencies"; + static final String PER_CONNECTION_ERROR_COUNT_NAME = "per_connection_error_count"; // Buckets under 100,000 are identical to buckets for server side metrics handler_latencies. // Extending client side bucket to up to 3,200,000. @@ -69,6 +70,31 @@ public class BuiltinMetricsConstants { 800.0, 1000.0, 2000.0, 5000.0, 10000.0, 20000.0, 50000.0, 100000.0, 200000.0, 400000.0, 800000.0, 1600000.0, 3200000.0)); // max is 53.3 minutes + private static final Aggregation AGGREGATION_PER_CONNECTION_ERROR_COUNT_HISTOGRAM = + Aggregation.explicitBucketHistogram( + ImmutableList.of( + 1.0, + 2.0, + 4.0, + 8.0, + 16.0, + 32.0, + 64.0, + 125.0, + 250.0, + 500.0, + 1_000.0, + 2_000.0, + 4_000.0, + 8_000.0, + 16_000.0, + 32_000.0, + 64_000.0, + 128_000.0, + 250_000.0, + 500_000.0, + 1_000_000.0)); + public static final String METER_NAME = "bigtable.googleapis.com/internal/client/"; static final Set COMMON_ATTRIBUTES = @@ -88,7 +114,7 @@ static void defineView( Aggregation aggregation, InstrumentType type, String unit, - Set extraAttributes) { + Set attributes) { InstrumentSelector selector = InstrumentSelector.builder() .setName(id) @@ -100,7 +126,7 @@ static void defineView( ImmutableSet.builder() .addAll( COMMON_ATTRIBUTES.stream().map(AttributeKey::getKey).collect(Collectors.toSet())) - .addAll(extraAttributes.stream().map(AttributeKey::getKey).collect(Collectors.toSet())) + .addAll(attributes.stream().map(AttributeKey::getKey).collect(Collectors.toSet())) .build(); View view = View.builder() @@ -121,56 +147,72 @@ public static Map getAllViews() { AGGREGATION_WITH_MILLIS_HISTOGRAM, InstrumentType.HISTOGRAM, "ms", - ImmutableSet.of(STREAMING_KEY, STATUS_KEY)); + ImmutableSet.builder() + .addAll(COMMON_ATTRIBUTES) + .add(STREAMING_KEY, STATUS_KEY) + .build()); defineView( views, ATTEMPT_LATENCIES_NAME, AGGREGATION_WITH_MILLIS_HISTOGRAM, InstrumentType.HISTOGRAM, "ms", - ImmutableSet.of(STREAMING_KEY, STATUS_KEY)); + ImmutableSet.builder() + .addAll(COMMON_ATTRIBUTES) + .add(STREAMING_KEY, STATUS_KEY) + .build()); defineView( views, SERVER_LATENCIES_NAME, AGGREGATION_WITH_MILLIS_HISTOGRAM, InstrumentType.HISTOGRAM, "ms", - ImmutableSet.of(STATUS_KEY)); + ImmutableSet.builder().addAll(COMMON_ATTRIBUTES).add(STATUS_KEY).build()); defineView( views, FIRST_RESPONSE_LATENCIES_NAME, AGGREGATION_WITH_MILLIS_HISTOGRAM, InstrumentType.HISTOGRAM, "ms", - ImmutableSet.of(STATUS_KEY)); + ImmutableSet.builder().addAll(COMMON_ATTRIBUTES).add(STATUS_KEY).build()); defineView( views, APPLICATION_BLOCKING_LATENCIES_NAME, AGGREGATION_WITH_MILLIS_HISTOGRAM, InstrumentType.HISTOGRAM, "ms", - ImmutableSet.of()); + ImmutableSet.builder().addAll(COMMON_ATTRIBUTES).build()); defineView( views, CLIENT_BLOCKING_LATENCIES_NAME, AGGREGATION_WITH_MILLIS_HISTOGRAM, InstrumentType.HISTOGRAM, "ms", - ImmutableSet.of()); + ImmutableSet.builder().addAll(COMMON_ATTRIBUTES).build()); defineView( views, RETRY_COUNT_NAME, Aggregation.sum(), InstrumentType.COUNTER, "1", - ImmutableSet.of(STATUS_KEY)); + ImmutableSet.builder().addAll(COMMON_ATTRIBUTES).add(STATUS_KEY).build()); defineView( views, CONNECTIVITY_ERROR_COUNT_NAME, Aggregation.sum(), InstrumentType.COUNTER, "1", - ImmutableSet.of(STATUS_KEY)); + ImmutableSet.builder().addAll(COMMON_ATTRIBUTES).add(STATUS_KEY).build()); + + defineView( + views, + PER_CONNECTION_ERROR_COUNT_NAME, + AGGREGATION_PER_CONNECTION_ERROR_COUNT_HISTOGRAM, + InstrumentType.HISTOGRAM, + "1", + ImmutableSet.builder() + .add(PROJECT_ID_KEY, INSTANCE_ID_KEY, APP_PROFILE_KEY, CLIENT_NAME_KEY) + .build()); return views.build(); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java index 8c7c552841..1c2c3b9f65 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java @@ -26,7 +26,7 @@ import java.util.Map; /** - * A util class to register built-in metrics on a custom OpenTelemetry instance. This is for + * An util class to register built-in metrics on a custom OpenTelemetry instance. This is for * advanced usage, and is only necessary when wanting to write built-in metrics to cloud monitoring * and custom sinks. Please refer to {@link CustomOpenTelemetryMetricsProvider} for example usage. */ diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CustomOpenTelemetryMetricsProvider.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CustomOpenTelemetryMetricsProvider.java index bcd1fe488f..ceade9b2c0 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CustomOpenTelemetryMetricsProvider.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CustomOpenTelemetryMetricsProvider.java @@ -25,7 +25,7 @@ * 
{@code
  * SdkMeterProviderBuilder sdkMeterProvider = SdkMeterProvider.builder();
  *
- * // register Builtin metrics on your meter provider
+ * // register Builtin metrics on your meter provider with default credentials
  * BuiltinMetricsViews.registerBuiltinMetrics("project-id", sdkMeterProvider);
  *
  * // register other metrics reader and views
diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java
index 0f3ee0c98f..df2a18f431 100644
--- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java
+++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java
@@ -15,6 +15,13 @@
  */
 package com.google.cloud.bigtable.data.v2.stub.metrics;
 
+import com.google.auth.Credentials;
+import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.sdk.OpenTelemetrySdk;
+import io.opentelemetry.sdk.metrics.SdkMeterProvider;
+import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder;
+import java.io.IOException;
+
 /**
  * Set {@link
  * com.google.cloud.bigtable.data.v2.BigtableDataSettings.Builder#setMetricsProvider(MetricsProvider)},
@@ -26,8 +33,20 @@ public final class DefaultMetricsProvider implements MetricsProvider {
 
   public static DefaultMetricsProvider INSTANCE = new DefaultMetricsProvider();
 
+  private OpenTelemetry openTelemetry;
+
   private DefaultMetricsProvider() {}
 
+  public OpenTelemetry getOpenTelemetry(String projectId, Credentials credentials)
+      throws IOException {
+    if (openTelemetry == null) {
+      SdkMeterProviderBuilder meterProvider = SdkMeterProvider.builder();
+      BuiltinMetricsView.registerBuiltinMetrics(projectId, credentials, meterProvider);
+      openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build();
+    }
+    return openTelemetry;
+  }
+
   @Override
   public String toString() {
     return "DefaultMetricsProvider";
diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java
index cab3b0bbd0..9c9783dfc6 100644
--- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java
+++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java
@@ -15,22 +15,32 @@
  */
 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 com.google.cloud.bigtable.stats.StatsRecorderWrapperForConnection;
-import com.google.cloud.bigtable.stats.StatsWrapper;
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableMap;
 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.util.Collections;
 import java.util.Set;
 import java.util.WeakHashMap;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /* Background task that goes through all connections and updates the errors_per_connection metric. */
 @InternalApi("For internal use only")
 public class ErrorCountPerConnectionMetricTracker implements Runnable {
   private static final Integer PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS = 60;
+
+  private final LongHistogram perConnectionErrorCountHistogram;
+  private final Attributes attributes;
+
   private final Set connectionErrorCountInterceptors;
   private final Object interceptorsLock = new Object();
   // This is not final so that it can be updated and mocked during testing.
@@ -42,12 +52,25 @@ void setStatsRecorderWrapperForConnection(
     this.statsRecorderWrapperForConnection = statsRecorderWrapperForConnection;
   }
 
-  public ErrorCountPerConnectionMetricTracker(ImmutableMap builtinAttributes) {
+  public ErrorCountPerConnectionMetricTracker(
+      @Nullable OpenTelemetry openTelemetry, Attributes attributes) {
     connectionErrorCountInterceptors =
         Collections.synchronizedSet(Collections.newSetFromMap(new WeakHashMap<>()));
 
-    this.statsRecorderWrapperForConnection =
-        StatsWrapper.createRecorderForConnection(builtinAttributes);
+    if (openTelemetry != null) {
+      Meter meter = openTelemetry.getMeter(METER_NAME);
+
+      perConnectionErrorCountHistogram =
+          meter
+              .histogramBuilder(PER_CONNECTION_ERROR_COUNT_NAME)
+              .ofLongs()
+              .setDescription("Distribution of counts of channels per 'error count per minute'.")
+              .setUnit("1")
+              .build();
+    } else {
+      perConnectionErrorCountHistogram = null;
+    }
+    this.attributes = attributes;
   }
 
   public void startConnectionErrorCountTracker(ScheduledExecutorService scheduler) {
@@ -66,6 +89,9 @@ public ClientInterceptor getInterceptor() {
 
   @Override
   public void run() {
+    if (perConnectionErrorCountHistogram == null) {
+      return;
+    }
     synchronized (interceptorsLock) {
       for (ConnectionErrorCountInterceptor interceptor : connectionErrorCountInterceptors) {
         long errors = interceptor.getAndResetNumOfErrors();
@@ -76,6 +102,7 @@ public void run() {
           // TODO: add a metric to also keep track of the number of successful requests per each
           // connection.
           statsRecorderWrapperForConnection.putAndRecordPerConnectionErrorCount(errors);
+          perConnectionErrorCountHistogram.record(errors, attributes);
         }
       }
     }
diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java
index 9173e6f6af..fea66e82bf 100644
--- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java
+++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java
@@ -320,7 +320,7 @@ public void testFeatureFlags() throws Exception {
   @Test
   public void testBulkMutationFlowControllerConfigured() throws Exception {
     BigtableDataSettings settings =
-        BigtableDataSettings.newBuilder()
+        BigtableDataSettings.newBuilderForEmulator(server.getPort())
             .setProjectId("my-project")
             .setInstanceId("my-instance")
             .setCredentialsProvider(credentialsProvider)
diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerCallableTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerCallableTest.java
index 14f4c698b2..3e1d59b133 100644
--- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerCallableTest.java
+++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerCallableTest.java
@@ -135,7 +135,7 @@ public void sendHeaders(Metadata headers) {
                     settings.getStubSettings(),
                     Tags.getTagger(),
                     localStats.getStatsRecorder(),
-                    clientContext))
+                    null))
             .build();
     attempts = settings.getStubSettings().readRowsSettings().getRetrySettings().getMaxAttempts();
     stub = new EnhancedBigtableStub(settings.getStubSettings(), clientContext);
@@ -161,7 +161,7 @@ public void sendHeaders(Metadata headers) {
                     noHeaderSettings.getStubSettings(),
                     Tags.getTagger(),
                     localStats.getStatsRecorder(),
-                    noHeaderClientContext))
+                    null))
             .build();
     noHeaderStub =
         new EnhancedBigtableStub(noHeaderSettings.getStubSettings(), noHeaderClientContext);
diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracerTest.java
index a4e28703c8..ecf204d1e2 100644
--- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracerTest.java
+++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracerTest.java
@@ -130,7 +130,7 @@ public void setUp() throws Exception {
                     settings.getStubSettings(),
                     Tags.getTagger(),
                     localStats.getStatsRecorder(),
-                    clientContext))
+                    null))
             .build();
     stub = new EnhancedBigtableStub(settings.getStubSettings(), clientContext);
   }

From 4e94c98c13731ae9b5ad25c8406d526d03cb9e16 Mon Sep 17 00:00:00 2001
From: Mattie Fu 
Date: Mon, 26 Feb 2024 14:19:39 -0500
Subject: [PATCH 2/7] update test

---
 .../metrics/ErrorCountPerConnectionTest.java  | 171 ++++++++++++------
 1 file changed, 117 insertions(+), 54 deletions(-)

diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java
index a6670182b8..a2359711b0 100644
--- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java
+++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java
@@ -23,6 +23,7 @@
 import com.google.api.gax.grpc.ChannelPoolSettings;
 import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;
 import com.google.bigtable.v2.*;
+import com.google.cloud.bigtable.Version;
 import com.google.cloud.bigtable.data.v2.BigtableDataSettings;
 import com.google.cloud.bigtable.data.v2.FakeServiceBuilder;
 import com.google.cloud.bigtable.data.v2.models.*;
@@ -33,7 +34,18 @@
 import io.grpc.Status;
 import io.grpc.StatusRuntimeException;
 import io.grpc.stub.StreamObserver;
-import java.util.List;
+import io.opentelemetry.api.common.Attributes;
+import io.opentelemetry.sdk.OpenTelemetrySdk;
+import io.opentelemetry.sdk.metrics.InstrumentSelector;
+import io.opentelemetry.sdk.metrics.SdkMeterProvider;
+import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder;
+import io.opentelemetry.sdk.metrics.View;
+import io.opentelemetry.sdk.metrics.data.HistogramPointData;
+import io.opentelemetry.sdk.metrics.data.MetricData;
+import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Map;
 import java.util.concurrent.ScheduledExecutorService;
 import org.junit.After;
 import org.junit.Before;
@@ -53,17 +65,45 @@ public class ErrorCountPerConnectionTest {
   private ArgumentCaptor runnableCaptor;
   private StatsRecorderWrapperForConnection statsRecorderWrapperForConnection;
 
+  private InMemoryMetricReader metricReader;
+
+  private Attributes attributes;
+
   @Before
   public void setup() throws Exception {
     server = FakeServiceBuilder.create(fakeService).start();
 
     ScheduledExecutorService executors = Mockito.mock(ScheduledExecutorService.class);
+
+    attributes =
+        Attributes.builder()
+            .put(BuiltinMetricsConstants.PROJECT_ID_KEY, "fake-project")
+            .put(BuiltinMetricsConstants.INSTANCE_ID_KEY, "fake-instance")
+            .put(BuiltinMetricsConstants.APP_PROFILE_KEY, "")
+            .put(BuiltinMetricsConstants.CLIENT_NAME_KEY, "bigtable-java/" + Version.VERSION)
+            .build();
+
+    metricReader = InMemoryMetricReader.create();
+
+    SdkMeterProviderBuilder meterProvider =
+        SdkMeterProvider.builder().registerMetricReader(metricReader);
+
+    for (Map.Entry entry :
+        BuiltinMetricsConstants.getAllViews().entrySet()) {
+      meterProvider.registerView(entry.getKey(), entry.getValue());
+    }
+
+    OpenTelemetrySdk otel =
+        OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build();
+
     builder =
         BigtableDataSettings.newBuilderForEmulator(server.getPort())
             .stubSettings()
             .setBackgroundExecutorProvider(FixedExecutorProvider.create(executors))
             .setProjectId("fake-project")
-            .setInstanceId("fake-instance");
+            .setInstanceId("fake-instance")
+            .setMetricsProvider(CustomOpenTelemetryMetricsProvider.create(otel));
+
     runnableCaptor = ArgumentCaptor.forClass(Runnable.class);
     Mockito.when(
             executors.scheduleAtFixedRate(runnableCaptor.capture(), anyLong(), anyLong(), any()))
@@ -98,14 +138,28 @@ public void readWithOneChannel() throws Exception {
         // noop
       }
     }
-    ArgumentCaptor errorCountCaptor = ArgumentCaptor.forClass(long.class);
-    Mockito.doNothing()
-        .when(statsRecorderWrapperForConnection)
-        .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture());
+
     runInterceptorTasksAndAssertCount();
-    List allErrorCounts = errorCountCaptor.getAllValues();
-    assertThat(allErrorCounts.size()).isEqualTo(1);
-    assertThat(allErrorCounts.get(0)).isEqualTo(errorCount);
+
+    Collection allMetrics = metricReader.collectAllMetrics();
+    MetricData metricData =
+        BuiltinMetricsTestUtils.getMetricData(
+            allMetrics, BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME);
+
+    // Make sure the correct bucket is updated with the correct number of data points
+    ArrayList histogramPointData =
+        new ArrayList<>(metricData.getHistogramData().getPoints());
+    assertThat(histogramPointData.size()).isEqualTo(1);
+    HistogramPointData point = histogramPointData.get(0);
+    int index = 0;
+    for (Double boundary : point.getBoundaries()) {
+      if (boundary < errorCount) {
+        index++;
+        continue;
+      }
+      break;
+    }
+    assertThat(point.getCounts().get(index)).isEqualTo(1);
   }
 
   @Test
@@ -131,28 +185,42 @@ public void readWithTwoChannels() throws Exception {
         // noop
       }
     }
-    ArgumentCaptor errorCountCaptor = ArgumentCaptor.forClass(long.class);
-    Mockito.doNothing()
-        .when(statsRecorderWrapperForConnection)
-        .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture());
     runInterceptorTasksAndAssertCount();
 
-    List allErrorCounts = errorCountCaptor.getAllValues();
-    assertThat(allErrorCounts.size()).isEqualTo(2);
-    // Requests get assigned to channels using a Round Robin algorithm, so half to each.
-    assertThat(allErrorCounts).containsExactly(totalErrorCount / 2, totalErrorCount / 2);
+    long errorCountPerChannel = totalErrorCount / 2;
+
+    Collection allMetrics = metricReader.collectAllMetrics();
+    MetricData metricData =
+        BuiltinMetricsTestUtils.getMetricData(
+            allMetrics, BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME);
+
+    // The 2 channels should get equal amount of errors, so the totalErrorCount / 2 bucket is
+    // updated twice.
+    ArrayList histogramPointData =
+        new ArrayList<>(metricData.getHistogramData().getPoints());
+    assertThat(histogramPointData.size()).isEqualTo(1);
+    HistogramPointData point = histogramPointData.get(0);
+    int index = 0;
+    for (Double boundary : point.getBoundaries()) {
+      if (boundary < errorCountPerChannel) {
+        index++;
+        continue;
+      }
+      break;
+    }
+    assertThat(point.getCounts().get(index)).isEqualTo(2);
   }
 
   @Test
   public void readOverTwoPeriods() throws Exception {
     EnhancedBigtableStub stub = EnhancedBigtableStub.create(builder.build());
-    long errorCount = 0;
+    long errorCount1 = 0;
 
     for (int i = 0; i < 20; i++) {
       Query query;
       if (i % 3 == 0) {
         query = Query.create(ERROR_TABLE_NAME);
-        errorCount += 1;
+        errorCount1 += 1;
       } else {
         query = Query.create(SUCCESS_TABLE_NAME);
       }
@@ -162,16 +230,9 @@ public void readOverTwoPeriods() throws Exception {
         // noop
       }
     }
-    ArgumentCaptor errorCountCaptor = ArgumentCaptor.forClass(long.class);
-    Mockito.doNothing()
-        .when(statsRecorderWrapperForConnection)
-        .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture());
-    runInterceptorTasksAndAssertCount();
-    List allErrorCounts = errorCountCaptor.getAllValues();
-    assertThat(allErrorCounts.size()).isEqualTo(1);
-    assertThat(allErrorCounts.get(0)).isEqualTo(errorCount);
 
-    errorCount = 0;
+    runInterceptorTasksAndAssertCount();
+    long errorCount2 = 0;
 
     for (int i = 0; i < 20; i++) {
       Query query;
@@ -179,7 +240,7 @@ public void readOverTwoPeriods() throws Exception {
         query = Query.create(SUCCESS_TABLE_NAME);
       } else {
         query = Query.create(ERROR_TABLE_NAME);
-        errorCount += 1;
+        errorCount2 += 1;
       }
       try {
         stub.readRowsCallable().call(query).iterator().hasNext();
@@ -187,27 +248,30 @@ public void readOverTwoPeriods() throws Exception {
         // noop
       }
     }
-    errorCountCaptor = ArgumentCaptor.forClass(long.class);
-    Mockito.doNothing()
-        .when(statsRecorderWrapperForConnection)
-        .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture());
+
     runInterceptorTasksAndAssertCount();
-    allErrorCounts = errorCountCaptor.getAllValues();
-    assertThat(allErrorCounts.size()).isEqualTo(1);
-    assertThat(allErrorCounts.get(0)).isEqualTo(errorCount);
-  }
 
-  @Test
-  public void ignoreInactiveConnection() throws Exception {
-    EnhancedBigtableStub stub = EnhancedBigtableStub.create(builder.build());
+    Collection allMetrics = metricReader.collectAllMetrics();
+    MetricData metricData =
+        BuiltinMetricsTestUtils.getMetricData(
+            allMetrics, BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME);
 
-    ArgumentCaptor errorCountCaptor = ArgumentCaptor.forClass(long.class);
-    Mockito.doNothing()
-        .when(statsRecorderWrapperForConnection)
-        .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture());
-    runInterceptorTasksAndAssertCount();
-    List allErrorCounts = errorCountCaptor.getAllValues();
-    assertThat(allErrorCounts).isEmpty();
+    ArrayList histogramPointData =
+        new ArrayList<>(metricData.getHistogramData().getPoints());
+    assertThat(histogramPointData.size()).isEqualTo(1);
+    HistogramPointData point = histogramPointData.get(0);
+    int index1 = 0;
+    int index2 = 0;
+    for (Double boundary : point.getBoundaries()) {
+      if (boundary < errorCount1) {
+        index1++;
+      }
+      if (boundary < errorCount2) {
+        index2++;
+      }
+    }
+    assertThat(point.getCounts().get(index1)).isEqualTo(1);
+    assertThat(point.getCounts().get(index2)).isEqualTo(1);
   }
 
   @Test
@@ -221,14 +285,13 @@ public void noFailedRequests() throws Exception {
         // noop
       }
     }
-    ArgumentCaptor errorCountCaptor = ArgumentCaptor.forClass(long.class);
-    Mockito.doNothing()
-        .when(statsRecorderWrapperForConnection)
-        .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture());
     runInterceptorTasksAndAssertCount();
-    List allErrorCounts = errorCountCaptor.getAllValues();
-    assertThat(allErrorCounts.size()).isEqualTo(1);
-    assertThat(allErrorCounts.get(0)).isEqualTo(0);
+    Collection allMetrics = metricReader.collectAllMetrics();
+    MetricData metricData =
+        BuiltinMetricsTestUtils.getMetricData(
+            allMetrics, BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME);
+    long value = BuiltinMetricsTestUtils.getAggregatedValue(metricData, attributes);
+    assertThat(value).isEqualTo(0);
   }
 
   private void runInterceptorTasksAndAssertCount() {

From 89f9aedf59d3a4dcb404a5b77920a3d600ff6eb2 Mon Sep 17 00:00:00 2001
From: Mattie Fu 
Date: Tue, 27 Feb 2024 10:38:53 -0500
Subject: [PATCH 3/7] address comments

---
 .../data/v2/stub/EnhancedBigtableStub.java        |  9 +++++----
 .../metrics/BigtableCloudMonitoringExporter.java  | 15 ++++++++++++---
 .../data/v2/stub/metrics/BuiltinMetricsView.java  |  5 +++--
 .../v2/stub/metrics/DefaultMetricsProvider.java   |  3 ++-
 4 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java
index 9fe2a56789..172152b318 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
@@ -231,10 +231,11 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set
             ? ((InstantiatingGrpcChannelProvider) builder.getTransportChannelProvider()).toBuilder()
             : null;
 
-    ErrorCountPerConnectionMetricTracker errorCountPerConnectionMetricTracker;
     OpenTelemetry openTelemetry =
         getOpenTelemetry(settings.getProjectId(), settings.getMetricsProvider(), credentials);
-    if (transportProvider != null) {
+    ErrorCountPerConnectionMetricTracker errorCountPerConnectionMetricTracker;
+    // Skip setting up ErrorCountPerConnectionMetricTracker if openTelemetry is null
+    if (openTelemetry != null && transportProvider != null) {
       errorCountPerConnectionMetricTracker =
           new ErrorCountPerConnectionMetricTracker(
               openTelemetry, createBuiltinAttributes(settings));
@@ -275,7 +276,7 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set
     }
 
     ClientContext clientContext = ClientContext.create(builder.build());
-    if (errorCountPerConnectionMetricTracker != null && openTelemetry != null) {
+    if (errorCountPerConnectionMetricTracker != null) {
       errorCountPerConnectionMetricTracker.startConnectionErrorCountTracker(
           clientContext.getExecutor());
     }
@@ -337,7 +338,7 @@ public static ApiTracerFactory createBigtableTracerFactory(
 
   @Nullable
   public static OpenTelemetry getOpenTelemetry(
-      String projectId, MetricsProvider metricsProvider, Credentials defaultCredentials)
+      String projectId, MetricsProvider metricsProvider, @Nullable Credentials defaultCredentials)
       throws IOException {
     if (metricsProvider instanceof CustomOpenTelemetryMetricsProvider) {
       CustomOpenTelemetryMetricsProvider customMetricsProvider =
diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
index 019fe88a50..72d4432c44 100644
--- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
+++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
@@ -20,7 +20,9 @@
 import com.google.api.core.ApiFutureCallback;
 import com.google.api.core.ApiFutures;
 import com.google.api.core.InternalApi;
+import com.google.api.gax.core.CredentialsProvider;
 import com.google.api.gax.core.FixedCredentialsProvider;
+import com.google.api.gax.core.NoCredentialsProvider;
 import com.google.auth.Credentials;
 import com.google.cloud.monitoring.v3.MetricServiceClient;
 import com.google.cloud.monitoring.v3.MetricServiceSettings;
@@ -43,6 +45,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.annotation.Nullable;
 import org.threeten.bp.Duration;
 
 /**
@@ -75,10 +78,16 @@ public final class BigtableCloudMonitoringExporter implements MetricExporter {
 
   private CompletableResultCode lastExportCode;
 
-  public static BigtableCloudMonitoringExporter create(String projectId, Credentials credentials)
-      throws IOException {
+  public static BigtableCloudMonitoringExporter create(
+      String projectId, @Nullable Credentials credentials) throws IOException {
     MetricServiceSettings.Builder settingsBuilder = MetricServiceSettings.newBuilder();
-    settingsBuilder.setCredentialsProvider(FixedCredentialsProvider.create(credentials));
+    CredentialsProvider credentialsProvider;
+    if (credentials == null) {
+      credentialsProvider = NoCredentialsProvider.create();
+    } else {
+      credentialsProvider = FixedCredentialsProvider.create(credentials);
+    }
+    settingsBuilder.setCredentialsProvider(credentialsProvider);
     settingsBuilder.setEndpoint(MONITORING_ENDPOINT);
 
     org.threeten.bp.Duration timeout = Duration.ofMinutes(1);
diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java
index 1c2c3b9f65..445160a146 100644
--- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java
+++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java
@@ -24,9 +24,10 @@
 import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader;
 import java.io.IOException;
 import java.util.Map;
+import javax.annotation.Nullable;
 
 /**
- * An util class to register built-in metrics on a custom OpenTelemetry instance. This is for
+ * A util class to register built-in metrics on a custom OpenTelemetry instance. This is for
  * advanced usage, and is only necessary when wanting to write built-in metrics to cloud monitoring
  * and custom sinks. Please refer to {@link CustomOpenTelemetryMetricsProvider} for example usage.
  */
@@ -46,7 +47,7 @@ public static void registerBuiltinMetrics(String projectId, SdkMeterProviderBuil
 
   /** Register built-in metrics on the {@link SdkMeterProviderBuilder} with credentials. */
   public static void registerBuiltinMetrics(
-      String projectId, Credentials credentials, SdkMeterProviderBuilder builder)
+      String projectId, @Nullable Credentials credentials, SdkMeterProviderBuilder builder)
       throws IOException {
     MetricExporter metricExporter = BigtableCloudMonitoringExporter.create(projectId, credentials);
     for (Map.Entry entry :
diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java
index df2a18f431..68e37a8f3c 100644
--- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java
+++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java
@@ -21,6 +21,7 @@
 import io.opentelemetry.sdk.metrics.SdkMeterProvider;
 import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder;
 import java.io.IOException;
+import javax.annotation.Nullable;
 
 /**
  * Set {@link
@@ -37,7 +38,7 @@ public final class DefaultMetricsProvider implements MetricsProvider {
 
   private DefaultMetricsProvider() {}
 
-  public OpenTelemetry getOpenTelemetry(String projectId, Credentials credentials)
+  public OpenTelemetry getOpenTelemetry(String projectId, @Nullable Credentials credentials)
       throws IOException {
     if (openTelemetry == null) {
       SdkMeterProviderBuilder meterProvider = SdkMeterProvider.builder();

From f3abda3a6275fe57b695666af6d40bc0808b8433 Mon Sep 17 00:00:00 2001
From: Mattie Fu 
Date: Wed, 28 Feb 2024 10:57:56 -0500
Subject: [PATCH 4/7] remove unnecessary check

---
 .../ErrorCountPerConnectionMetricTracker.java | 28 +++++++------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java
index 9c9783dfc6..c2094d8589 100644
--- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java
+++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java
@@ -31,7 +31,6 @@
 import java.util.WeakHashMap;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
-import org.checkerframework.checker.nullness.qual.Nullable;
 
 /* Background task that goes through all connections and updates the errors_per_connection metric. */
 @InternalApi("For internal use only")
@@ -52,24 +51,20 @@ void setStatsRecorderWrapperForConnection(
     this.statsRecorderWrapperForConnection = statsRecorderWrapperForConnection;
   }
 
-  public ErrorCountPerConnectionMetricTracker(
-      @Nullable OpenTelemetry openTelemetry, Attributes attributes) {
+  public ErrorCountPerConnectionMetricTracker(OpenTelemetry openTelemetry, Attributes attributes) {
     connectionErrorCountInterceptors =
         Collections.synchronizedSet(Collections.newSetFromMap(new WeakHashMap<>()));
 
-    if (openTelemetry != null) {
-      Meter meter = openTelemetry.getMeter(METER_NAME);
+    Meter meter = openTelemetry.getMeter(METER_NAME);
+
+    perConnectionErrorCountHistogram =
+        meter
+            .histogramBuilder(PER_CONNECTION_ERROR_COUNT_NAME)
+            .ofLongs()
+            .setDescription("Distribution of counts of channels per 'error count per minute'.")
+            .setUnit("1")
+            .build();
 
-      perConnectionErrorCountHistogram =
-          meter
-              .histogramBuilder(PER_CONNECTION_ERROR_COUNT_NAME)
-              .ofLongs()
-              .setDescription("Distribution of counts of channels per 'error count per minute'.")
-              .setUnit("1")
-              .build();
-    } else {
-      perConnectionErrorCountHistogram = null;
-    }
     this.attributes = attributes;
   }
 
@@ -89,9 +84,6 @@ public ClientInterceptor getInterceptor() {
 
   @Override
   public void run() {
-    if (perConnectionErrorCountHistogram == null) {
-      return;
-    }
     synchronized (interceptorsLock) {
       for (ConnectionErrorCountInterceptor interceptor : connectionErrorCountInterceptors) {
         long errors = interceptor.getAndResetNumOfErrors();

From 6eb366ad90875d93b62e5f5eae44cbf82281a074 Mon Sep 17 00:00:00 2001
From: Mattie Fu 
Date: Thu, 29 Feb 2024 11:51:54 -0500
Subject: [PATCH 5/7] clean up statsRecorder

---
 .../metrics/ErrorCountPerConnectionMetricTracker.java | 11 -----------
 .../v2/stub/metrics/ErrorCountPerConnectionTest.java  |  6 ------
 2 files changed, 17 deletions(-)

diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java
index c2094d8589..df37ee3e9d 100644
--- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java
+++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java
@@ -19,8 +19,6 @@
 import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME;
 
 import com.google.api.core.InternalApi;
-import com.google.cloud.bigtable.stats.StatsRecorderWrapperForConnection;
-import com.google.common.annotations.VisibleForTesting;
 import io.grpc.ClientInterceptor;
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.common.Attributes;
@@ -42,14 +40,6 @@ public class ErrorCountPerConnectionMetricTracker implements Runnable {
 
   private final Set connectionErrorCountInterceptors;
   private final Object interceptorsLock = new Object();
-  // This is not final so that it can be updated and mocked during testing.
-  private StatsRecorderWrapperForConnection statsRecorderWrapperForConnection;
-
-  @VisibleForTesting
-  void setStatsRecorderWrapperForConnection(
-      StatsRecorderWrapperForConnection statsRecorderWrapperForConnection) {
-    this.statsRecorderWrapperForConnection = statsRecorderWrapperForConnection;
-  }
 
   public ErrorCountPerConnectionMetricTracker(OpenTelemetry openTelemetry, Attributes attributes) {
     connectionErrorCountInterceptors =
@@ -93,7 +83,6 @@ public void run() {
         if (errors > 0 || successes > 0) {
           // TODO: add a metric to also keep track of the number of successful requests per each
           // connection.
-          statsRecorderWrapperForConnection.putAndRecordPerConnectionErrorCount(errors);
           perConnectionErrorCountHistogram.record(errors, attributes);
         }
       }
diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java
index a2359711b0..2de5b98069 100644
--- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java
+++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java
@@ -29,7 +29,6 @@
 import com.google.cloud.bigtable.data.v2.models.*;
 import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStub;
 import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings;
-import com.google.cloud.bigtable.stats.StatsRecorderWrapperForConnection;
 import io.grpc.Server;
 import io.grpc.Status;
 import io.grpc.StatusRuntimeException;
@@ -63,7 +62,6 @@ public class ErrorCountPerConnectionTest {
   private final FakeService fakeService = new FakeService();
   private EnhancedBigtableStubSettings.Builder builder;
   private ArgumentCaptor runnableCaptor;
-  private StatsRecorderWrapperForConnection statsRecorderWrapperForConnection;
 
   private InMemoryMetricReader metricReader;
 
@@ -108,8 +106,6 @@ public void setup() throws Exception {
     Mockito.when(
             executors.scheduleAtFixedRate(runnableCaptor.capture(), anyLong(), anyLong(), any()))
         .thenReturn(null);
-
-    statsRecorderWrapperForConnection = Mockito.mock(StatsRecorderWrapperForConnection.class);
   }
 
   @After
@@ -298,8 +294,6 @@ private void runInterceptorTasksAndAssertCount() {
     int actualNumOfTasks = 0;
     for (Runnable runnable : runnableCaptor.getAllValues()) {
       if (runnable instanceof ErrorCountPerConnectionMetricTracker) {
-        ((ErrorCountPerConnectionMetricTracker) runnable)
-            .setStatsRecorderWrapperForConnection(statsRecorderWrapperForConnection);
         runnable.run();
         actualNumOfTasks++;
       }

From 240527f3f1e35985106581191fc7f7ad4dc6989c Mon Sep 17 00:00:00 2001
From: Mattie Fu 
Date: Thu, 29 Feb 2024 12:08:23 -0500
Subject: [PATCH 6/7] remove dependency

---
 google-cloud-bigtable/pom.xml | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/google-cloud-bigtable/pom.xml b/google-cloud-bigtable/pom.xml
index 45307e0c35..31fbb6e0b6 100644
--- a/google-cloud-bigtable/pom.xml
+++ b/google-cloud-bigtable/pom.xml
@@ -62,10 +62,6 @@
   
 
   
-    
-      com.google.cloud
-      google-cloud-bigtable-stats
-    
     
     

From 97482b582c382725cce652dd90b1218143a0f4d8 Mon Sep 17 00:00:00 2001
From: Mattie Fu 
Date: Mon, 4 Mar 2024 11:49:24 -0500
Subject: [PATCH 7/7] address comments

---
 .../metrics/ErrorCountPerConnectionTest.java  | 41 +++++++------------
 1 file changed, 15 insertions(+), 26 deletions(-)

diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java
index 2de5b98069..dec5120b1c 100644
--- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java
+++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java
@@ -44,6 +44,7 @@
 import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ScheduledExecutorService;
 import org.junit.After;
@@ -147,14 +148,7 @@ public void readWithOneChannel() throws Exception {
         new ArrayList<>(metricData.getHistogramData().getPoints());
     assertThat(histogramPointData.size()).isEqualTo(1);
     HistogramPointData point = histogramPointData.get(0);
-    int index = 0;
-    for (Double boundary : point.getBoundaries()) {
-      if (boundary < errorCount) {
-        index++;
-        continue;
-      }
-      break;
-    }
+    int index = findDataPointIndex(point.getBoundaries(), errorCount);
     assertThat(point.getCounts().get(index)).isEqualTo(1);
   }
 
@@ -196,14 +190,7 @@ public void readWithTwoChannels() throws Exception {
         new ArrayList<>(metricData.getHistogramData().getPoints());
     assertThat(histogramPointData.size()).isEqualTo(1);
     HistogramPointData point = histogramPointData.get(0);
-    int index = 0;
-    for (Double boundary : point.getBoundaries()) {
-      if (boundary < errorCountPerChannel) {
-        index++;
-        continue;
-      }
-      break;
-    }
+    int index = findDataPointIndex(point.getBoundaries(), errorCountPerChannel);
     assertThat(point.getCounts().get(index)).isEqualTo(2);
   }
 
@@ -256,16 +243,8 @@ public void readOverTwoPeriods() throws Exception {
         new ArrayList<>(metricData.getHistogramData().getPoints());
     assertThat(histogramPointData.size()).isEqualTo(1);
     HistogramPointData point = histogramPointData.get(0);
-    int index1 = 0;
-    int index2 = 0;
-    for (Double boundary : point.getBoundaries()) {
-      if (boundary < errorCount1) {
-        index1++;
-      }
-      if (boundary < errorCount2) {
-        index2++;
-      }
-    }
+    int index1 = findDataPointIndex(point.getBoundaries(), errorCount1);
+    int index2 = findDataPointIndex(point.getBoundaries(), errorCount2);
     assertThat(point.getCounts().get(index1)).isEqualTo(1);
     assertThat(point.getCounts().get(index2)).isEqualTo(1);
   }
@@ -301,6 +280,16 @@ private void runInterceptorTasksAndAssertCount() {
     assertThat(actualNumOfTasks).isEqualTo(1);
   }
 
+  private int findDataPointIndex(List boundaries, long dataPoint) {
+    int index = 0;
+    for (; index < boundaries.size(); index++) {
+      if (boundaries.get(index) >= dataPoint) {
+        break;
+      }
+    }
+    return index;
+  }
+
   static class FakeService extends BigtableGrpc.BigtableImplBase {
     @Override
     public void readRows(