refactor and fix tests

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java

@@ -533,11 +533,16 @@ public boolean isBulkMutationFlowControlEnabled() {
     }
 
     /**
-     * Sets the {@link MetricsProvider}. By default, this is set to {@link
-     * com.google.cloud.bigtable.data.v2.stub.metrics.DefaultMetricsProvider} which will collect and
-     * export built-in metrics. To disable built-in metrics, set it to {@link
-     * com.google.cloud.bigtable.data.v2.stub.metrics.NoopMetricsProvider}. To use a custom
-     * OpenTelemetry, refer to {@link
+     * Sets the {@link MetricsProvider}.
+     *
+     * <p>By default, this is set to {@link
+     * com.google.cloud.bigtable.data.v2.stub.metrics.DefaultMetricsProvider#INSTANCE} which will
+     * collect and export client side metrics.
+     *
+     * <p>To disable client side metrics, set it to {@link
+     * com.google.cloud.bigtable.data.v2.stub.metrics.NoopMetricsProvider#INSTANCE}.
+     *
+     * <p>To use a custom OpenTelemetry instance, refer to {@link
      * com.google.cloud.bigtable.data.v2.stub.metrics.CustomOpenTelemetryMetricsProvider} on how to
      * set it up.
      */

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java

@@ -102,6 +102,7 @@
 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.MetricsTracerFactory;
+import com.google.cloud.bigtable.data.v2.stub.metrics.NoopMetricsProvider;
 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;
@@ -275,13 +276,9 @@ public static ApiTracerFactory createBigtableTracerFactory(
             new OpencensusTracerFactory(
                 ImmutableMap.<String, String>builder()
                     // Annotate traces with the same tags as metrics
-                    .put(RpcMeasureConstants.BIGTABLE_PROJECT_ID.getName(), settings.getProjectId())
-                    .put(
-                        RpcMeasureConstants.BIGTABLE_INSTANCE_ID.getName(),
-                        settings.getInstanceId())
-                    .put(
-                        RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID.getName(),
-                        settings.getAppProfileId())
+                    .put(RpcMeasureConstants.BIGTABLE_PROJECT_ID.getName(), projectId)
+                    .put(RpcMeasureConstants.BIGTABLE_INSTANCE_ID.getName(), instanceId)
+                    .put(RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID.getName(), appProfileId)
                     // Also annotate traces with library versions
                     .put("gax", GaxGrpcProperties.getGaxGrpcVersion())
                     .put("grpc", GaxGrpcProperties.getGrpcVersion())
@@ -292,13 +289,7 @@ public static ApiTracerFactory createBigtableTracerFactory(
         // Add user configured tracer
         .add(settings.getTracerFactory());
     Attributes otelAttributes =
-        Attributes.of(
-            PROJECT_ID,
-            settings.getProjectId(),
-            INSTANCE_ID,
-            settings.getInstanceId(),
-            APP_PROFILE,
-            settings.getAppProfileId());
+        Attributes.of(PROJECT_ID, projectId, INSTANCE_ID, instanceId, APP_PROFILE, appProfileId);
     BuiltinMetricsTracerFactory builtinMetricsTracerFactory =
         setupBuiltinMetricsTracerFactory(settings.toBuilder(), otelAttributes);
     if (builtinMetricsTracerFactory != null) {
@@ -323,8 +314,11 @@ private static BuiltinMetricsTracerFactory setupBuiltinMetricsTracerFactory(
       OpenTelemetry openTelemetry =
           OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build();
       return BuiltinMetricsTracerFactory.create(openTelemetry, attributes);
+    } else if (settings.getMetricsProvider() instanceof NoopMetricsProvider) {
+      return null;
     }
-    return null;
+    throw new IOException(
+        "Invalid MetricsProvider type " + settings.getMetricsProvider().getClass());
   }
 
   private static void patchCredentials(EnhancedBigtableStubSettings.Builder settings)

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java

@@ -664,7 +664,7 @@ private Builder() {
       this.enableRoutingCookie = true;
       this.enableRetryInfo = true;
 
-      metricsProvider = new DefaultMetricsProvider();
+      metricsProvider = DefaultMetricsProvider.INSTANCE;
 
       // Defaults provider
       BigtableStubSettings.Builder baseDefaults = BigtableStubSettings.newBuilder();

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java

@@ -107,7 +107,7 @@ static List<TimeSeries> convertCollectionToListOfTimeSeries(
       if (!metricData
           .getInstrumentationScopeInfo()
           .getName()
-          .equals(BuiltinMetricsConstants.SCOPE)) {
+          .equals(BuiltinMetricsConstants.METER_NAME)) {
         continue;
       }
       metricData.getData().getPoints().stream()

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java

@@ -61,7 +61,7 @@ public class BuiltinMetricsConstants {
               300.0, 400.0, 500.0, 650.0, 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
 
-  static final String SCOPE = "bigtable.googleapis.com";
+  public static final String METER_NAME = "bigtable.googleapis.com/internal/client/";
 
   static final Set<String> COMMON_ATTRIBUTES =
       ImmutableSet.of(
@@ -84,15 +84,15 @@ static void defineView(
     InstrumentSelector selector =
         InstrumentSelector.builder()
             .setName(id)
-            .setMeterName(SCOPE)
+            .setMeterName(METER_NAME)
             .setType(type)
             .setUnit(unit)
             .build();
     Set<String> attributesFilter =
         ImmutableSet.<String>builder().addAll(COMMON_ATTRIBUTES).addAll(extraAttributes).build();
     View view =
         View.builder()
-            .setName(SCOPE + "/internal/client/" + id)
+            .setName(METER_NAME + id)
             .setAggregation(aggregation)
             .setAttributeFilter(attributesFilter)
             .build();

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java

@@ -299,9 +299,10 @@ private void recordOperationCompletion(@Nullable Throwable status) {
     operationLatenciesHistogram.record(
         operationLatency,
         attributes.toBuilder().put(STREAMING, isStreaming).put(STATUS, statusStr).build());
+
+    long applicationLatencyNano = operationLatencyNano - totalServerLatencyNano.get();
     applicationBlockingLatenciesHistogram.record(
-        Duration.ofNanos(operationLatencyNano - totalServerLatencyNano.get()).toMillis(),
-        attributes);
+        Duration.ofNanos(applicationLatencyNano).toMillis(), attributes);
 
     if (operationType == OperationType.ServerStreaming
         && spanName.getMethodName().equals("ReadRows")) {

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java

@@ -20,9 +20,9 @@
 import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.CLIENT_BLOCKING_LATENCIES_NAME;
 import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.CONNECTIVITY_ERROR_COUNT_NAME;
 import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.FIRST_RESPONSE_LATENCIES_NAME;
+import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.METER_NAME;
 import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.OPERATION_LATENCIES_NAME;
 import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.RETRY_COUNT_NAME;
-import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.SCOPE;
 import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.SERVER_LATENCIES_NAME;
 
 import com.google.api.core.InternalApi;
@@ -65,7 +65,7 @@ public static BuiltinMetricsTracerFactory create(
 
   BuiltinMetricsTracerFactory(OpenTelemetry openTelemetry, Attributes attributes) {
     this.attributes = attributes;
-    Meter meter = openTelemetry.getMeter(SCOPE);
+    Meter meter = openTelemetry.getMeter(METER_NAME);
 
     operationLatenciesHistogram =
         meter

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CustomOpenTelemetryMetricsProvider.java

@@ -37,7 +37,7 @@
  *     .builder()
  *     .setMeterProvider(sdkMeterProvider().build());
  *
- *
+ * // Override MetricsProvider in BigtableDataSettings
  * BigtableDataSettings settings = BigtableDataSettings.newBuilder()
  *   .setProjectId("my-project")
  *   .setInstanceId("my-instance-id")

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java

@@ -15,7 +15,16 @@
  */
 package com.google.cloud.bigtable.data.v2.stub.metrics;
 
+/**
+ * Set {@link
+ * com.google.cloud.bigtable.data.v2.BigtableDataSettings.Builder#setMetricsProvider(MetricsProvider)},
+ * to {@link this#INSTANCE} to enable collecting and export client side metrics
+ * https://cloud.google.com/bigtable/docs/client-side-metrics. This is the default setting in {@link
+ * com.google.cloud.bigtable.data.v2.BigtableDataSettings}.
+ */
 public final class DefaultMetricsProvider implements MetricsProvider {
 
-  public DefaultMetricsProvider() {}
+  public static DefaultMetricsProvider INSTANCE = new DefaultMetricsProvider();
+
+  private DefaultMetricsProvider() {}
 }

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/NoopMetricsProvider.java

@@ -15,4 +15,15 @@
  */
 package com.google.cloud.bigtable.data.v2.stub.metrics;
 
-public class NoopMetricsProvider implements MetricsProvider {}
+/**
+ * Set {@link
+ * com.google.cloud.bigtable.data.v2.BigtableDataSettings.Builder#setMetricsProvider(MetricsProvider)},
+ * to {@link this#INSTANCE} to disable collecting and export client side metrics
+ * https://cloud.google.com/bigtable/docs/client-side-metrics.
+ */
+public class NoopMetricsProvider implements MetricsProvider {
+
+  public static NoopMetricsProvider INSTANCE = new NoopMetricsProvider();
+
+  private NoopMetricsProvider() {}
+}

google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java

@@ -36,6 +36,7 @@
 import com.google.bigtable.v2.ReadRowsResponse;
 import com.google.cloud.bigtable.data.v2.internal.NameUtil;
 import com.google.cloud.bigtable.data.v2.models.RowMutation;
+import com.google.cloud.bigtable.data.v2.stub.metrics.NoopMetricsProvider;
 import com.google.common.base.Preconditions;
 import com.google.common.io.BaseEncoding;
 import io.grpc.Attributes;
@@ -175,7 +176,11 @@ public void testNewClientsShareTransportChannel() throws Exception {
     // FixedCredentialProvider.
     // So disabling in the test code it's fine.
     try (BigtableDataClientFactory factory =
-            BigtableDataClientFactory.create(defaultSettings.toBuilder().build());
+            BigtableDataClientFactory.create(
+                defaultSettings
+                    .toBuilder()
+                    .setMetricsProvider(NoopMetricsProvider.INSTANCE)
+                    .build());
         BigtableDataClient ignored1 = factory.createForInstance("project1", "instance1");
         BigtableDataClient ignored2 = factory.createForInstance("project2", "instance2");
         BigtableDataClient ignored3 = factory.createForInstance("project3", "instance3")) {
@@ -255,6 +260,7 @@ public void testCreateWithRefreshingChannel() throws Exception {
             // Builtin metrics will call getCredentialsProvider at which point it'll be a
             // FixedCredentialProvider.
             // So disabling in the test code it's fine.
+            .setMetricsProvider(NoopMetricsProvider.INSTANCE)
             .setRefreshingChannel(true);
     builder
         .stubSettings()

google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/StreamingMetricsMetadataIT.java

@@ -101,7 +101,7 @@ public void testSuccess() throws Exception {
 
     List<MetricData> metrics =
         metricReader.collectAllMetrics().stream()
-            .filter(m -> m.getName().equals(BuiltinMetricsConstants.OPERATION_LATENCIES_NAME))
+            .filter(m -> m.getName().contains(BuiltinMetricsConstants.OPERATION_LATENCIES_NAME))
             .collect(Collectors.toList());
 
     assertThat(metrics.size()).isEqualTo(1);
@@ -131,7 +131,7 @@ public void testFailure() {
 
     List<MetricData> metrics =
         metricReader.collectAllMetrics().stream()
-            .filter(m -> m.getName().equals(BuiltinMetricsConstants.OPERATION_LATENCIES_NAME))
+            .filter(m -> m.getName().contains(BuiltinMetricsConstants.OPERATION_LATENCIES_NAME))
             .collect(Collectors.toList());
 
     assertThat(metrics.size()).isEqualTo(1);

google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/UnaryMetricsMetadataIT.java

@@ -106,7 +106,7 @@ public void testSuccess() throws Exception {
 
     List<MetricData> metrics =
         metricReader.collectAllMetrics().stream()
-            .filter(m -> m.getName().equals(BuiltinMetricsConstants.OPERATION_LATENCIES_NAME))
+            .filter(m -> m.getName().contains(BuiltinMetricsConstants.OPERATION_LATENCIES_NAME))
             .collect(Collectors.toList());
 
     assertThat(metrics.size()).isEqualTo(1);
@@ -149,7 +149,7 @@ public void testFailure() throws Exception {
 
     List<MetricData> metrics =
         metricReader.collectAllMetrics().stream()
-            .filter(m -> m.getName().equals(BuiltinMetricsConstants.OPERATION_LATENCIES_NAME))
+            .filter(m -> m.getName().contains(BuiltinMetricsConstants.OPERATION_LATENCIES_NAME))
             .collect(Collectors.toList());
 
     assertThat(metrics.size()).isEqualTo(1);

google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java

@@ -884,8 +884,7 @@ public void enableRetryInfoFalseValueTest() throws IOException {
     "generateInitialChangeStreamPartitionsSettings",
     "readChangeStreamSettings",
     "pingAndWarmSettings",
-    "isBuiltinMetricsEnabled",
-    "openTelemetry",
+    "metricsProvider",
   };
 
   @Test

google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java

@@ -103,7 +103,7 @@ public void setUp() {
 
     resource = Resource.create(Attributes.empty());
 
-    scope = InstrumentationScopeInfo.create("bigtable.googleapis.com");
+    scope = InstrumentationScopeInfo.create(BuiltinMetricsConstants.METER_NAME);
   }
 
   @After

google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java

@@ -21,10 +21,10 @@
 import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.CLIENT_NAME;
 import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.CLUSTER_ID;
 import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.CONNECTIVITY_ERROR_COUNT_NAME;
+import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.METER_NAME;
 import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.METHOD;
 import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.OPERATION_LATENCIES_NAME;
 import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.RETRY_COUNT_NAME;
-import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.SCOPE;
 import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.SERVER_LATENCIES_NAME;
 import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.STATUS;
 import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.STREAMING;
@@ -159,7 +159,7 @@ public void setUp() throws Exception {
             .setResource(Resource.create(baseAttributes))
             .build();
 
-    Meter meter = meterProvider.get(SCOPE);
+    Meter meter = meterProvider.get(METER_NAME);
 
     OpenTelemetrySdk otel = OpenTelemetrySdk.builder().setMeterProvider(meterProvider).build();
     facotry = BuiltinMetricsTracerFactory.create(otel, baseAttributes);