fix: Fix the logic for updating connectivity metrics

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

@@ -19,6 +19,7 @@
 import static com.google.cloud.spanner.XGoogSpannerRequestId.REQUEST_ID;
 
 import com.google.api.core.InternalApi;
+import com.google.api.gax.rpc.StatusCode;
 import com.google.api.gax.tracing.OpenTelemetryMetricsRecorder;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -76,6 +77,10 @@ public class BuiltInMetricsConstant {
           "grpc.xds_client.resource_updates_invalid",
           "grpc.xds_client.resource_updates_valid");
 
+  static final Collection<String> CONNECTIVITY_ERROR_STATUSES =
+      ImmutableList.of(
+          StatusCode.Code.DEADLINE_EXCEEDED.toString(), StatusCode.Code.CANCELLED.toString());
+
   public static final String SPANNER_RESOURCE_TYPE = "spanner_instance_client";
 
   public static final AttributeKey<String> PROJECT_ID_KEY = AttributeKey.stringKey("project_id");

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

@@ -17,6 +17,7 @@
 package com.google.cloud.spanner;
 
 import com.google.api.gax.core.GaxProperties;
+import com.google.api.gax.tracing.MetricsTracer;
 import com.google.api.gax.tracing.OpenTelemetryMetricsRecorder;
 import com.google.common.base.Preconditions;
 import io.opentelemetry.api.OpenTelemetry;
@@ -106,13 +107,18 @@ void recordServerTimingHeaderMetrics(
     if (gfeLatency != null) {
       gfeLatencyRecorder.record(gfeLatency, otelAttributes);
     }
-    if (gfeHeaderMissingCount > 0) {
+
+    boolean isConnectivityErrorStatus =
+        BuiltInMetricsConstant.CONNECTIVITY_ERROR_STATUSES.contains(
+            attributes.get(MetricsTracer.STATUS_ATTRIBUTE));
+
+    if (gfeHeaderMissingCount > 0 && isConnectivityErrorStatus) {
       gfeHeaderMissingCountRecorder.add(gfeHeaderMissingCount, otelAttributes);
     }
     if (afeLatency != null) {
       afeLatencyRecorder.record(afeLatency, otelAttributes);
     }
-    if (afeHeaderMissingCount > 0) {
+    if (afeHeaderMissingCount > 0 && isConnectivityErrorStatus) {
       afeHeaderMissingCountRecorder.add(afeHeaderMissingCount, otelAttributes);
     }
   }

google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java

@@ -350,7 +350,7 @@ public void testNoNetworkConnection() {
   }
 
   @Test
-  public void testNoServerTimingHeader() throws IOException, InterruptedException {
+  public void testNoServerTimingHeaderWithSuccessRPC() throws IOException, InterruptedException {
     // Create Spanner Object without headers
     InetSocketAddress addressNoHeader = new InetSocketAddress("localhost", 0);
     Server serverNoHeader =
@@ -381,18 +381,12 @@ public void testNoServerTimingHeader() throws IOException, InterruptedException
         .readWriteTransaction()
         .run(transaction -> transaction.executeUpdate(UPDATE_RANDOM));
 
-    Attributes expectedAttributes =
-        expectedCommonBaseAttributes.toBuilder()
-            .putAll(expectedCommonRequestAttributes)
-            .put(BuiltInMetricsConstant.STATUS_KEY, "OK")
-            .put(BuiltInMetricsConstant.METHOD_KEY, "Spanner.ExecuteSql")
-            .build();
-
     assertFalse(checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME));
     assertFalse(checkIfMetricExists(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_NAME));
-    MetricData afeConnectivityMetricData =
-        getMetricData(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME);
-    assertThat(getAggregatedValue(afeConnectivityMetricData, expectedAttributes)).isEqualTo(1);
+    assertFalse(
+        checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME));
+    assertFalse(
+        checkIfMetricExists(metricReader, BuiltInMetricsConstant.GFE_CONNECTIVITY_ERROR_NAME));
 
     spannerNoHeader.close();
     serverNoHeader.shutdown();

google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBuiltInMetricsTest.java

@@ -126,9 +126,18 @@ public void testBuiltinMetricsWithDefaultOTEL() throws Exception {
         response = metricClient.listTimeSeriesCallable().call(request);
       }
 
-      assertWithMessage("Metric " + metric + " didn't return any data.")
-          .that(response.getTimeSeriesCount())
-          .isGreaterThan(0);
+      // afe_latencies metric currently does not return data as afe server-timing header is
+      // disabled.
+      // Keeping this check to enable this check in the future.
+      if (metric.equals("afe_latencies")) {
+        assertWithMessage("Metric " + metric + " returned data.")
+            .that(response.getTimeSeriesCount())
+            .isEqualTo(0);
+      } else {
+        assertWithMessage("Metric " + metric + " didn't return any data.")
+            .that(response.getTimeSeriesCount())
+            .isGreaterThan(0);
+      }
     }
   }
 }