Addressed comments

Author: sakthivelmanii

google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java

@@ -25,6 +25,7 @@
 import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;
 import com.google.api.gax.rpc.PermissionDeniedException;
 import com.google.auth.Credentials;
+import com.google.cloud.NoCredentials;
 import com.google.cloud.monitoring.v3.MetricServiceClient;
 import com.google.cloud.monitoring.v3.MetricServiceSettings;
 import com.google.common.annotations.VisibleForTesting;
@@ -35,7 +36,7 @@
 import com.google.monitoring.v3.ProjectName;
 import com.google.monitoring.v3.TimeSeries;
 import com.google.protobuf.Empty;
-import io.grpc.inprocess.InProcessChannelBuilder;
+import io.grpc.ManagedChannelBuilder;
 import io.opentelemetry.sdk.common.CompletableResultCode;
 import io.opentelemetry.sdk.metrics.InstrumentType;
 import io.opentelemetry.sdk.metrics.data.AggregationTemporality;
@@ -94,12 +95,16 @@ static SpannerCloudMonitoringExporter create(
       settingsBuilder.setUniverseDomain(universeDomain);
     }
 
-    if (System.getProperty("jmh.monitoring-server") != null) {
+    if (System.getProperty("jmh.monitoring-server-port") != null) {
       settingsBuilder.setTransportChannelProvider(
           InstantiatingGrpcChannelProvider.newBuilder()
+              .setCredentials(NoCredentials.getInstance())
               .setChannelConfigurator(
                   managedChannelBuilder ->
-                      InProcessChannelBuilder.forName(System.getProperty("jmh.monitoring-server")))
+                      ManagedChannelBuilder.forAddress(
+                              "0.0.0.0",
+                              Integer.parseInt(System.getProperty("jmh.monitoring-server-port")))
+                          .usePlaintext())
               .build());
     }
 

google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java

@@ -2013,6 +2013,11 @@ public CallCredentialsProvider getCallCredentialsProvider() {
   }
 
   private boolean usesNoCredentials() {
+    // When JMH is enabled, we need to enable built-in metrics
+    if (System.getProperty("jmh.enabled") != null
+        && System.getProperty("jmh.enabled").equals("true")) {
+      return false;
+    }
     return Objects.equals(getCredentials(), NoCredentials.getInstance());
   }
 

google-cloud-spanner/src/test/java/com/google/cloud/spanner/benchmarking/BenchmarkValidator.java

@@ -55,8 +55,7 @@ void validate() {
       }
       Map<String, Double> actualPercentilesMap = actualResult.primaryMetric.scorePercentiles;
       // We will only be comparing the percentiles(p50, p90, p90) which are configured in the
-      // expected
-      // percentiles. This allows some checks to be disabled if required.
+      // expected percentiles. This allows some checks to be disabled if required.
       for (Percentile expectedPercentile : expectResult.scorePercentiles) {
         String percentile = expectedPercentile.percentile;
         double difference =
@@ -135,19 +134,23 @@ static class ValidationException extends RuntimeException {
     }
   }
 
-  private static String parseCommandLineArg(String arg) {
-    if (arg == null || arg.isEmpty()) {
+  private static String parseCommandLineArgs(String[] args, String key) {
+    if (args == null) {
       return "";
     }
-    String[] args = arg.split("=");
-    if (args.length != 2) {
-      return "";
+    for (String arg : args) {
+      if (arg.startsWith("--" + key)) {
+        String[] splits = arg.split("=");
+        if (splits.length == 2) {
+          return splits[1].trim();
+        }
+      }
     }
-    return args[1];
+    return "";
   }
 
   public static void main(String[] args) {
-    String actualFile = parseCommandLineArg(args[0]);
+    String actualFile = parseCommandLineArgs(args, "file");
     new BenchmarkValidator("com/google/cloud/spanner/jmh/jmh-baseline.json", actualFile).validate();
   }
 }

google-cloud-spanner/src/test/java/com/google/cloud/spanner/benchmarking/MonitoringServiceImpl.java

@@ -19,6 +19,7 @@
 import com.google.monitoring.v3.CreateTimeSeriesRequest;
 import com.google.monitoring.v3.MetricServiceGrpc.MetricServiceImplBase;
 import com.google.protobuf.Empty;
+import io.grpc.Status;
 import io.grpc.stub.StreamObserver;
 
 class MonitoringServiceImpl extends MetricServiceImplBase {
@@ -31,7 +32,8 @@ public void createServiceTimeSeries(
       responseObserver.onNext(Empty.getDefaultInstance());
       responseObserver.onCompleted();
     } catch (InterruptedException e) {
-      responseObserver.onError(e);
+      responseObserver.onError(
+          Status.CANCELLED.withCause(e).withDescription(e.getMessage()).asException());
     }
   }
 }

google-cloud-spanner/src/test/java/com/google/cloud/spanner/benchmarking/ReadBenchmark.java

@@ -16,6 +16,7 @@
 
 package com.google.cloud.spanner.benchmarking;
 
+import com.google.cloud.NoCredentials;
 import com.google.cloud.spanner.DatabaseClient;
 import com.google.cloud.spanner.DatabaseId;
 import com.google.cloud.spanner.Key;
@@ -32,9 +33,9 @@
 import com.google.spanner.v1.StructType;
 import com.google.spanner.v1.StructType.Field;
 import com.google.spanner.v1.TypeCode;
+import io.grpc.ManagedChannelBuilder;
 import io.grpc.Server;
-import io.grpc.inprocess.InProcessChannelBuilder;
-import io.grpc.inprocess.InProcessServerBuilder;
+import io.grpc.ServerBuilder;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.List;
@@ -88,6 +89,9 @@ public static class BenchmarkState {
 
     @Setup(Level.Trial)
     public void setup() throws IOException {
+      // Enable JMH system property
+      System.setProperty("jmh.enabled", "true");
+
       // Initializing mock spanner service
       MockSpannerServiceImpl mockSpannerService = new MockSpannerServiceImpl();
       mockSpannerService.setAbortProbability(0.0D);
@@ -99,9 +103,8 @@ public void setup() throws IOException {
       gRPCServerExecutor = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors());
 
       // Creating Spanner Inprocess gRPC server
-      String serverName = InProcessServerBuilder.generateName();
       gRPCServer =
-          InProcessServerBuilder.forName(serverName)
+          ServerBuilder.forPort(0)
               .addService(mockSpannerService)
               .executor(gRPCServerExecutor)
               .build()
@@ -110,21 +113,21 @@ public void setup() throws IOException {
       registerMocks(mockSpannerService);
 
       // Creating Monitoring Inprocess gRPC server
-      String monitoringServerName = InProcessServerBuilder.generateName();
       gRPCMonitoringServer =
-          InProcessServerBuilder.forName(monitoringServerName)
-              .addService(mockMonitoringService)
-              .build()
-              .start();
+          ServerBuilder.forPort(0).addService(mockMonitoringService).build().start();
 
-      // Set the monitoring host for exporter to forward requests to inprocess gRPC server
-      System.setProperty("jmh.monitoring-server", monitoringServerName);
+      // Set the monitoring host port for exporter to forward requests to local netty gRPC server
+      System.setProperty(
+          "jmh.monitoring-server-port", String.valueOf(gRPCMonitoringServer.getPort()));
 
       spanner =
           SpannerOptions.newBuilder()
               .setProjectId("[PROJECT]")
+              .setCredentials(NoCredentials.getInstance())
               .setChannelConfigurator(
-                  managedChannelBuilder -> InProcessChannelBuilder.forName(serverName))
+                  managedChannelBuilder ->
+                      ManagedChannelBuilder.forAddress("0.0.0.0", gRPCServer.getPort())
+                          .usePlaintext())
               .build()
               .getService();
       databaseClient =
@@ -172,10 +175,14 @@ private void registerMocks(MockSpannerServiceImpl mockSpannerService) {
     }
 
     @TearDown(Level.Trial)
-    public void tearDown() {
+    public void tearDown() throws InterruptedException {
       spanner.close();
       gRPCServer.shutdown();
       gRPCServerExecutor.shutdown();
+
+      // awaiting termination for servers and executors
+      gRPCServer.awaitTermination(10, TimeUnit.SECONDS);
+      gRPCServerExecutor.awaitTermination(10, TimeUnit.SECONDS);
     }
   }
 

google-cloud-spanner/src/test/resources/com/google/cloud/spanner/jmh/jmh-baseline.json

@@ -1,10 +1,10 @@
 {
-  "baselineConfigurations": {
+  "benchmarkResultMap": {
     "com.google.cloud.spanner.benchmarking.ReadBenchmark.queryBenchmark": {
       "scorePercentiles": [
         {
           "percentile": "50.0",
-          "baseline": "120",
+          "baseline": "450",
           "difference": "15"
         }
       ]
@@ -13,7 +13,7 @@
       "scorePercentiles": [
         {
           "percentile": "50.0",
-          "baseline": "120",
+          "baseline": "450",
           "difference": "15"
         }
       ]