chore: Address PR comments

googleapis · Apr 23, 2024 · a8869fa · a8869fa
1 parent b1be846
commit a8869fa
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 70 deletions.
diff --git a/...java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/...java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java
@@ -32,7 +32,6 @@
 import com.google.api.core.ApiFunction;
 import com.google.api.core.BetaApi;
 import com.google.api.core.InternalApi;
-import com.google.api.core.InternalExtensionOnly;
 import com.google.api.gax.core.ExecutorProvider;
 import com.google.api.gax.rpc.FixedHeaderProvider;
 import com.google.api.gax.rpc.HeaderProvider;
@@ -82,7 +81,6 @@
  * <p>The client lib header and generator header values are used to form a value that goes into the
  * http header of requests to the service.
  */
-@InternalExtensionOnly
 public final class InstantiatingGrpcChannelProvider implements TransportChannelProvider {
 
   // Package-Private class used to read a file for the Product Name on Compute Engine.

diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java
@@ -61,7 +61,7 @@ public MetricsTracerFactory(MetricsRecorder metricsRecorder) {
    */
   public MetricsTracerFactory(MetricsRecorder metricsRecorder, Map<String, Object> attributes) {
     this.metricsRecorder = metricsRecorder;
-    this.attributes = attributes;
+    this.attributes = ImmutableMap.copyOf(attributes);
   }
 
   @Override

diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java
@@ -305,19 +305,19 @@ public void testGrpc_operationSucceeded_recordsMetrics() throws InterruptedExcep
         EchoRequest.newBuilder().setContent("test_grpc_operation_succeeded").build();
     grpcClient.echo(echoRequest);
 
-    List<MetricData> metricDataList = getMetricDataList();
-    verifyPointDataSum(metricDataList, attemptCount);
+    List<MetricData> actualMetricDataList = getMetricDataList();
+    verifyPointDataSum(actualMetricDataList, attemptCount);
 
-    Map<String, String> attributeMapping =
+    Map<String, String> expectedAttributes =
         ImmutableMap.of(
             MetricsTracer.METHOD_NAME_ATTRIBUTE,
             "Echo.Echo",
             MetricsTracer.LANGUAGE_ATTRIBUTE,
             MetricsTracer.DEFAULT_LANGUAGE);
-    verifyDefaultMetricsAttributes(metricDataList, attributeMapping);
+    verifyDefaultMetricsAttributes(actualMetricDataList, expectedAttributes);
 
     List<StatusCount> statusCountList = ImmutableList.of(new StatusCount(Code.OK));
-    verifyStatusAttribute(metricDataList, statusCountList);
+    verifyStatusAttribute(actualMetricDataList, statusCountList);
   }
 
   @Ignore("https://github.com/googleapis/sdk-platform-java/issues/2503")
@@ -328,19 +328,19 @@ public void testHttpJson_operationSucceeded_recordsMetrics() throws InterruptedE
         EchoRequest.newBuilder().setContent("test_http_operation_succeeded").build();
     httpClient.echo(echoRequest);
 
-    List<MetricData> metricDataList = getMetricDataList();
-    verifyPointDataSum(metricDataList, attemptCount);
+    List<MetricData> actualMetricDataList = getMetricDataList();
+    verifyPointDataSum(actualMetricDataList, attemptCount);
 
-    Map<String, String> attributeMapping =
+    Map<String, String> expectedAttributes =
         ImmutableMap.of(
             MetricsTracer.METHOD_NAME_ATTRIBUTE,
             "google.showcase.v1beta1.Echo/Echo",
             MetricsTracer.LANGUAGE_ATTRIBUTE,
             MetricsTracer.DEFAULT_LANGUAGE);
-    verifyDefaultMetricsAttributes(metricDataList, attributeMapping);
+    verifyDefaultMetricsAttributes(actualMetricDataList, expectedAttributes);
 
     List<StatusCount> statusCountList = ImmutableList.of(new StatusCount(Code.OK));
-    verifyStatusAttribute(metricDataList, statusCountList);
+    verifyStatusAttribute(actualMetricDataList, statusCountList);
   }
 
   @Test
@@ -358,19 +358,19 @@ public void testGrpc_operationCancelled_recordsMetrics() throws Exception {
     Thread.sleep(1000);
     blockResponseApiFuture.cancel(true);
 
-    List<MetricData> metricDataList = getMetricDataList();
-    verifyPointDataSum(metricDataList, attemptCount);
+    List<MetricData> actualMetricDataList = getMetricDataList();
+    verifyPointDataSum(actualMetricDataList, attemptCount);
 
-    Map<String, String> attributeMapping =
+    Map<String, String> expectedAttributes =
         ImmutableMap.of(
             MetricsTracer.METHOD_NAME_ATTRIBUTE,
             "Echo.Block",
             MetricsTracer.LANGUAGE_ATTRIBUTE,
             MetricsTracer.DEFAULT_LANGUAGE);
-    verifyDefaultMetricsAttributes(metricDataList, attributeMapping);
+    verifyDefaultMetricsAttributes(actualMetricDataList, expectedAttributes);
 
     List<StatusCount> statusCountList = ImmutableList.of(new StatusCount(Code.CANCELLED));
-    verifyStatusAttribute(metricDataList, statusCountList);
+    verifyStatusAttribute(actualMetricDataList, statusCountList);
   }
 
   @Ignore("https://github.com/googleapis/sdk-platform-java/issues/2503")
@@ -386,19 +386,19 @@ public void testHttpJson_operationCancelled_recordsMetrics() throws Exception {
     Thread.sleep(1000);
     blockResponseApiFuture.cancel(true);
 
-    List<MetricData> metricDataList = getMetricDataList();
-    verifyPointDataSum(metricDataList, attemptCount);
+    List<MetricData> actualMetricDataList = getMetricDataList();
+    verifyPointDataSum(actualMetricDataList, attemptCount);
 
-    Map<String, String> attributeMapping =
+    Map<String, String> expectedAttributes =
         ImmutableMap.of(
             MetricsTracer.METHOD_NAME_ATTRIBUTE,
             "google.showcase.v1beta1.Echo/Block",
             MetricsTracer.LANGUAGE_ATTRIBUTE,
             MetricsTracer.DEFAULT_LANGUAGE);
-    verifyDefaultMetricsAttributes(metricDataList, attributeMapping);
+    verifyDefaultMetricsAttributes(actualMetricDataList, expectedAttributes);
 
     List<StatusCount> statusCountList = ImmutableList.of(new StatusCount(Code.CANCELLED));
-    verifyStatusAttribute(metricDataList, statusCountList);
+    verifyStatusAttribute(actualMetricDataList, statusCountList);
   }
 
   @Test
@@ -415,19 +415,19 @@ public void testGrpc_operationFailed_recordsMetrics() throws InterruptedExceptio
     ApiFuture<BlockResponse> blockResponseApiFuture = blockCallable.futureCall(blockRequest);
     assertThrows(ExecutionException.class, blockResponseApiFuture::get);
 
-    List<MetricData> metricDataList = getMetricDataList();
-    verifyPointDataSum(metricDataList, attemptCount);
+    List<MetricData> actualMetricDataList = getMetricDataList();
+    verifyPointDataSum(actualMetricDataList, attemptCount);
 
-    Map<String, String> attributeMapping =
+    Map<String, String> expectedAttributes =
         ImmutableMap.of(
             MetricsTracer.METHOD_NAME_ATTRIBUTE,
             "Echo.Block",
             MetricsTracer.LANGUAGE_ATTRIBUTE,
             MetricsTracer.DEFAULT_LANGUAGE);
-    verifyDefaultMetricsAttributes(metricDataList, attributeMapping);
+    verifyDefaultMetricsAttributes(actualMetricDataList, expectedAttributes);
 
     List<StatusCount> statusCountList = ImmutableList.of(new StatusCount(statusCode));
-    verifyStatusAttribute(metricDataList, statusCountList);
+    verifyStatusAttribute(actualMetricDataList, statusCountList);
   }
 
   @Ignore("https://github.com/googleapis/sdk-platform-java/issues/2503")
@@ -445,19 +445,19 @@ public void testHttpJson_operationFailed_recordsMetrics() throws InterruptedExce
     ApiFuture<BlockResponse> blockResponseApiFuture = blockCallable.futureCall(blockRequest);
     assertThrows(ExecutionException.class, blockResponseApiFuture::get);
 
-    List<MetricData> metricDataList = getMetricDataList();
-    verifyPointDataSum(metricDataList, attemptCount);
+    List<MetricData> actualMetricDataList = getMetricDataList();
+    verifyPointDataSum(actualMetricDataList, attemptCount);
 
-    Map<String, String> attributeMapping =
+    Map<String, String> expectedAttributes =
         ImmutableMap.of(
             MetricsTracer.METHOD_NAME_ATTRIBUTE,
             "google.showcase.v1beta1.Echo/Block",
             MetricsTracer.LANGUAGE_ATTRIBUTE,
             MetricsTracer.DEFAULT_LANGUAGE);
-    verifyDefaultMetricsAttributes(metricDataList, attributeMapping);
+    verifyDefaultMetricsAttributes(actualMetricDataList, expectedAttributes);
 
     List<StatusCount> statusCountList = ImmutableList.of(new StatusCount(statusCode));
-    verifyStatusAttribute(metricDataList, statusCountList);
+    verifyStatusAttribute(actualMetricDataList, statusCountList);
   }
 
   @Test
@@ -508,19 +508,19 @@ public void testGrpc_attemptFailedRetriesExhausted_recordsMetrics() throws Excep
 
     assertThrows(UnavailableException.class, () -> grpcClient.echo(echoRequest));
 
-    List<MetricData> metricDataList = getMetricDataList();
-    verifyPointDataSum(metricDataList, attemptCount);
+    List<MetricData> actualMetricDataList = getMetricDataList();
+    verifyPointDataSum(actualMetricDataList, attemptCount);
 
-    Map<String, String> attributeMapping =
+    Map<String, String> expectedAttributes =
         ImmutableMap.of(
             MetricsTracer.METHOD_NAME_ATTRIBUTE,
             "Echo.Echo",
             MetricsTracer.LANGUAGE_ATTRIBUTE,
             MetricsTracer.DEFAULT_LANGUAGE);
-    verifyDefaultMetricsAttributes(metricDataList, attributeMapping);
+    verifyDefaultMetricsAttributes(actualMetricDataList, expectedAttributes);
 
     List<StatusCount> statusCountList = ImmutableList.of(new StatusCount(statusCode, 3));
-    verifyStatusAttribute(metricDataList, statusCountList);
+    verifyStatusAttribute(actualMetricDataList, statusCountList);
 
     grpcClient.close();
     grpcClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS);
@@ -576,19 +576,19 @@ public void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws E
 
     assertThrows(UnavailableException.class, () -> httpClient.echo(echoRequest));
 
-    List<MetricData> metricDataList = getMetricDataList();
-    verifyPointDataSum(metricDataList, attemptCount);
+    List<MetricData> actualMetricDataList = getMetricDataList();
+    verifyPointDataSum(actualMetricDataList, attemptCount);
 
-    Map<String, String> attributeMapping =
+    Map<String, String> expectedAttributes =
         ImmutableMap.of(
             MetricsTracer.METHOD_NAME_ATTRIBUTE,
             "google.showcase.v1beta1.Echo/Echo",
             MetricsTracer.LANGUAGE_ATTRIBUTE,
             MetricsTracer.DEFAULT_LANGUAGE);
-    verifyDefaultMetricsAttributes(metricDataList, attributeMapping);
+    verifyDefaultMetricsAttributes(actualMetricDataList, expectedAttributes);
 
     List<StatusCount> statusCountList = ImmutableList.of(new StatusCount(statusCode, 3));
-    verifyStatusAttribute(metricDataList, statusCountList);
+    verifyStatusAttribute(actualMetricDataList, statusCountList);
 
     httpClient.close();
     httpClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS);
@@ -606,19 +606,19 @@ public void testGrpc_attemptPermanentFailure_recordsMetrics() throws Interrupted
 
     assertThrows(InvalidArgumentException.class, () -> grpcClient.block(blockRequest));
 
-    List<MetricData> metricDataList = getMetricDataList();
-    verifyPointDataSum(metricDataList, attemptCount);
+    List<MetricData> actualMetricDataList = getMetricDataList();
+    verifyPointDataSum(actualMetricDataList, attemptCount);
 
-    Map<String, String> attributeMapping =
+    Map<String, String> expectedAttributes =
         ImmutableMap.of(
             MetricsTracer.METHOD_NAME_ATTRIBUTE,
             "Echo.Block",
             MetricsTracer.LANGUAGE_ATTRIBUTE,
             MetricsTracer.DEFAULT_LANGUAGE);
-    verifyDefaultMetricsAttributes(metricDataList, attributeMapping);
+    verifyDefaultMetricsAttributes(actualMetricDataList, expectedAttributes);
 
     List<StatusCount> statusCountList = ImmutableList.of(new StatusCount(statusCode));
-    verifyStatusAttribute(metricDataList, statusCountList);
+    verifyStatusAttribute(actualMetricDataList, statusCountList);
   }
 
   @Ignore("https://github.com/googleapis/sdk-platform-java/issues/2503")
@@ -634,19 +634,19 @@ public void testHttpJson_attemptPermanentFailure_recordsMetrics() throws Interru
 
     assertThrows(InvalidArgumentException.class, () -> httpClient.block(blockRequest));
 
-    List<MetricData> metricDataList = getMetricDataList();
-    verifyPointDataSum(metricDataList, attemptCount);
+    List<MetricData> actualMetricDataList = getMetricDataList();
+    verifyPointDataSum(actualMetricDataList, attemptCount);
 
-    Map<String, String> attributeMapping =
+    Map<String, String> expectedAttributes =
         ImmutableMap.of(
             MetricsTracer.METHOD_NAME_ATTRIBUTE,
             "google.showcase.v1beta1.Echo/Block",
             MetricsTracer.LANGUAGE_ATTRIBUTE,
             MetricsTracer.DEFAULT_LANGUAGE);
-    verifyDefaultMetricsAttributes(metricDataList, attributeMapping);
+    verifyDefaultMetricsAttributes(actualMetricDataList, expectedAttributes);
 
     List<StatusCount> statusCountList = ImmutableList.of(new StatusCount(statusCode));
-    verifyStatusAttribute(metricDataList, statusCountList);
+    verifyStatusAttribute(actualMetricDataList, statusCountList);
   }
 
   @Test
@@ -703,20 +703,20 @@ public void testGrpc_multipleFailedAttempts_successfulOperation() throws Excepti
 
     grpcClient.block(blockRequest);
 
-    List<MetricData> metricDataList = getMetricDataList();
-    verifyPointDataSum(metricDataList, attemptCount);
+    List<MetricData> actualMetricDataList = getMetricDataList();
+    verifyPointDataSum(actualMetricDataList, attemptCount);
 
-    Map<String, String> attributeMapping =
+    Map<String, String> expectedAttributes =
         ImmutableMap.of(
             MetricsTracer.METHOD_NAME_ATTRIBUTE,
             "Echo.Block",
             MetricsTracer.LANGUAGE_ATTRIBUTE,
             MetricsTracer.DEFAULT_LANGUAGE);
-    verifyDefaultMetricsAttributes(metricDataList, attributeMapping);
+    verifyDefaultMetricsAttributes(actualMetricDataList, expectedAttributes);
 
     List<StatusCount> statusCountList =
         ImmutableList.of(new StatusCount(Code.DEADLINE_EXCEEDED, 2), new StatusCount(Code.OK));
-    verifyStatusAttribute(metricDataList, statusCountList);
+    verifyStatusAttribute(actualMetricDataList, statusCountList);
 
     grpcClient.close();
     grpcClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS);
@@ -773,16 +773,16 @@ public void testHttpJson_multipleFailedAttempts_successfulOperation() throws Exc
 
     grpcClient.block(blockRequest);
 
-    List<MetricData> metricDataList = getMetricDataList();
-    verifyPointDataSum(metricDataList, attemptCount);
+    List<MetricData> actualMetricDataList = getMetricDataList();
+    verifyPointDataSum(actualMetricDataList, attemptCount);
 
-    Map<String, String> attributeMapping =
+    Map<String, String> expectedAttributes =
         ImmutableMap.of(
             MetricsTracer.METHOD_NAME_ATTRIBUTE,
             "google.showcase.v1beta1.Echo/Block",
             MetricsTracer.LANGUAGE_ATTRIBUTE,
             MetricsTracer.DEFAULT_LANGUAGE);
-    verifyDefaultMetricsAttributes(metricDataList, attributeMapping);
+    verifyDefaultMetricsAttributes(actualMetricDataList, expectedAttributes);
 
     httpClient.close();
     httpClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS);
@@ -797,8 +797,14 @@ public void recordsCustomAttributes() throws InterruptedException, IOException {
 
     // Add custom attributes to be added as client level attributes
     Map<String, Object> customAttributes = new HashMap<>();
-    customAttributes.put("directpath_enabled", channelProvider.canUseDirectPath());
-    customAttributes.put("testing", "showcase");
+    String directpathEnabled = "directpath_enabled";
+    customAttributes.put(directpathEnabled, channelProvider.canUseDirectPath());
+    String randomAttributeKey1 = "testing";
+    String randomAttributeValue1 = "showcase";
+    String randomAttributeKey2 = "hello";
+    String randomAttributeValue2 = "world";
+    customAttributes.put(randomAttributeKey1, randomAttributeValue1);
+    customAttributes.put(randomAttributeKey2, randomAttributeValue2);
 
     InMemoryMetricReader inMemoryMetricReader = InMemoryMetricReader.create();
     OpenTelemetryMetricsRecorder otelMetricsRecorder =
@@ -826,18 +832,20 @@ public void recordsCustomAttributes() throws InterruptedException, IOException {
     EchoRequest echoRequest = EchoRequest.newBuilder().setContent("content").build();
     grpcClient.echo(echoRequest);
 
-    List<MetricData> metricDataList = getMetricDataList(inMemoryMetricReader);
-    Map<String, String> attributeMapping =
+    List<MetricData> actualMetricDataList = getMetricDataList(inMemoryMetricReader);
+    Map<String, String> expectedAttributes =
         ImmutableMap.of(
             MetricsTracer.METHOD_NAME_ATTRIBUTE,
             "Echo.Echo",
             MetricsTracer.LANGUAGE_ATTRIBUTE,
             MetricsTracer.DEFAULT_LANGUAGE,
-            "directpath_enabled",
+                directpathEnabled,
             "false",
-            "testing",
-            "showcase");
-    verifyDefaultMetricsAttributes(metricDataList, attributeMapping);
+                randomAttributeKey1,
+                randomAttributeValue1,
+                randomAttributeKey2,
+                randomAttributeValue2);
+    verifyDefaultMetricsAttributes(actualMetricDataList, expectedAttributes);
 
     inMemoryMetricReader.close();
   }