fix(test): provide libraryMetadata for ApiTracerContext.newBuilder in tests

Author: westarle

sdk-platform-java/gax-java/gax/pom.xml

@@ -139,7 +139,7 @@
         <configuration>
           <argLine>@{argLine} -Djava.util.logging.SimpleFormatter.format="%1$tY %1$tl:%1$tM:%1$tS.%1$tL %2$s %4$s: %5$s%6$s%n"</argLine>
           <!-- These tests require an Env Var to be set. Use -PenvVarTest to ONLY run these tests -->
-          <test>!EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet+endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority,!LoggingEnabledTest</test>
+          <test>!EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet+endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority,!LoggingEnabledTest,!LoggingTracerTest</test>
         </configuration>
       </plugin>
     </plugins>
@@ -154,7 +154,7 @@
             <groupId>org.apache.maven.plugins</groupId>
             <artifactId>maven-surefire-plugin</artifactId>
             <configuration>
-              <test>EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet+endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority,LoggingEnabledTest</test>
+              <test>EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet+endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority,LoggingEnabledTest,LoggingTracerTest</test>
             </configuration>
           </plugin>
         </plugins>

sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java

@@ -54,11 +54,6 @@ public LoggingTracer(ApiTracerContext apiTracerContext) {
     this.apiTracerContext = apiTracerContext;
   }
 
-  @Override
-  public void attemptFailed(Throwable error, org.threeten.bp.Duration delay) {
-    recordActionableError(error);
-  }
-
   @Override
   public void attemptFailedDuration(Throwable error, java.time.Duration delay) {
     recordActionableError(error);
@@ -75,47 +70,37 @@ public void attemptPermanentFailure(Throwable error) {
   }
 
   private void recordActionableError(Throwable error) {
-    Map<String, Object> logContext = new HashMap<>();
-
-    if (apiTracerContext.rpcSystemName() != null) {
-      logContext.put(
-          ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, apiTracerContext.rpcSystemName());
-    }
-    if (apiTracerContext.fullMethodName() != null) {
-      logContext.put(
-          ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE, apiTracerContext.fullMethodName());
-    }
-    if (apiTracerContext.serverPort() != null) {
-      logContext.put(ObservabilityAttributes.SERVER_PORT_ATTRIBUTE, apiTracerContext.serverPort());
-    }
-    if (apiTracerContext.libraryMetadata() != null
-        && !apiTracerContext.libraryMetadata().isEmpty()) {
-      if (apiTracerContext.libraryMetadata().repository() != null) {
-        logContext.put(
-            ObservabilityAttributes.REPO_ATTRIBUTE,
-            apiTracerContext.libraryMetadata().repository());
-      }
+    if (error == null) {
+      return;
     }
 
-    if (error != null) {
+    Map<String, Object> logContext = new HashMap<>(apiTracerContext.getAttemptAttributes());
+
+    if (apiTracerContext.serviceName() != null) {
       logContext.put(
-          ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE,
-          ObservabilityUtils.extractStatus(error));
+          ObservabilityAttributes.GCP_CLIENT_SERVICE_ATTRIBUTE, apiTracerContext.serviceName());
     }
 
+    logContext.put(
+        ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE,
+        ObservabilityUtils.extractStatus(error));
+
     ErrorInfo errorInfo = ObservabilityUtils.extractErrorInfo(error);
     if (errorInfo != null) {
-      logContext.put("error.type", errorInfo.getReason());
-      logContext.put("gcp.errors.domain", errorInfo.getDomain());
+      if (!errorInfo.getReason().isEmpty()) {
+        logContext.put(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE, errorInfo.getReason());
+      }
+      if (!errorInfo.getDomain().isEmpty()) {
+        logContext.put(ObservabilityAttributes.ERROR_DOMAIN_ATTRIBUTE, errorInfo.getDomain());
+      }
       for (Map.Entry<String, String> entry : errorInfo.getMetadataMap().entrySet()) {
-        logContext.put("gcp.errors.metadata." + entry.getKey(), entry.getValue());
+        logContext.put(
+            ObservabilityAttributes.ERROR_METADATA_ATTRIBUTE_PREFIX + entry.getKey(),
+            entry.getValue());
       }
     }
 
-    String message = "Unknown Error";
-    if (error != null) {
-      message = error.getMessage() != null ? error.getMessage() : error.getClass().getName();
-    }
+    String message = error.getMessage() != null ? error.getMessage() : error.getClass().getName();
     LoggingUtils.logActionableError(logContext, LOGGER_PROVIDER, message);
   }
 }

sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracerFactory.java

@@ -54,7 +54,7 @@ public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType op
 
   @Override
   public ApiTracer newTracer(ApiTracer parent, ApiTracerContext context) {
-    return new LoggingTracer(context);
+    return new LoggingTracer(apiTracerContext.merge(context));
   }
 
   @Override

sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityAttributes.java

@@ -93,4 +93,13 @@ public class ObservabilityAttributes {
 
   /** The destination resource id of the request (e.g. projects/p/locations/l/topics/t). */
   public static final String DESTINATION_RESOURCE_ID_ATTRIBUTE = "gcp.resource.destination.id";
+
+  /** The type of error that occurred (e.g., from google.rpc.ErrorInfo.reason). */
+  public static final String ERROR_TYPE_ATTRIBUTE = "error.type";
+
+  /** The domain of the error (e.g., from google.rpc.ErrorInfo.domain). */
+  public static final String ERROR_DOMAIN_ATTRIBUTE = "gcp.errors.domain";
+
+  /** The prefix for error metadata (e.g., from google.rpc.ErrorInfo.metadata). */
+  public static final String ERROR_METADATA_ATTRIBUTE_PREFIX = "gcp.errors.metadata.";
 }

sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/logging/TestLogger.java

@@ -48,6 +48,10 @@ public class TestLogger implements Logger, LoggingEventAware {
   List<String> messageList = new ArrayList<>();
   Level level;
 
+  public List<String> getMessageList() {
+    return messageList;
+  }
+
   Map<String, Object> keyValuePairsMap = new HashMap<>();
 
   private String loggerName;

sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/logging/TestServiceProvider.java

@@ -30,12 +30,11 @@
 
 package com.google.api.gax.logging;
 
-import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 import org.slf4j.ILoggerFactory;
 import org.slf4j.IMarkerFactory;
+import org.slf4j.Logger;
 import org.slf4j.spi.MDCAdapter;
 import org.slf4j.spi.SLF4JServiceProvider;
 
@@ -45,12 +44,18 @@
  */
 public class TestServiceProvider implements SLF4JServiceProvider {
 
+  private final ConcurrentMap<String, Logger> loggers = new ConcurrentHashMap<>();
+  private final ILoggerFactory loggerFactory =
+      new ILoggerFactory() {
+        @Override
+        public Logger getLogger(String name) {
+          return loggers.computeIfAbsent(name, TestLogger::new);
+        }
+      };
+
   @Override
   public ILoggerFactory getLoggerFactory() {
-    // mock behavior when provider present
-    ILoggerFactory mockLoggerFactory = mock(ILoggerFactory.class);
-    when(mockLoggerFactory.getLogger(anyString())).thenReturn(new TestLogger("test-logger"));
-    return mockLoggerFactory;
+    return loggerFactory;
   }
 
   @Override

sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerFactoryTest.java

@@ -63,7 +63,8 @@ void testNewTracer_WithContext_CreatesLoggingTracer() {
   @Test
   void testWithContext_ReturnsNewFactoryWithMergedContext() {
     LoggingTracerFactory factory = new LoggingTracerFactory();
-    ApiTracerContext context = ApiTracerContext.newBuilder().setServerAddress("address").build();
+    ApiTracerContext context =
+        ApiTracerContext.empty().toBuilder().setServerAddress("address").build();
     ApiTracerFactory updatedFactory = factory.withContext(context);
 
     assertNotNull(updatedFactory);

sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerTest.java

@@ -30,6 +30,8 @@
 
 package com.google.api.gax.tracing;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
 import com.google.api.gax.logging.TestLogger;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -42,6 +44,7 @@ class LoggingTracerTest {
   @BeforeEach
   void setUp() {
     testLogger = (TestLogger) LoggerFactory.getLogger(LoggingTracer.class);
+    testLogger.getMessageList().clear();
   }
 
   @Test
@@ -51,10 +54,9 @@ void testAttemptFailed_LogsError() {
 
     // Call attemptFailed with a generic exception
     Exception error = new RuntimeException("generic failure");
-    tracer.attemptFailed(error, org.threeten.bp.Duration.ZERO);
+    tracer.attemptFailedDuration(error, java.time.Duration.ZERO);
 
-    // To prevent failing due to disabled logging or other missing context,
-    // we don't strictly assert the contents of the log here if the logger isn't enabled.
-    // The main verification is that calling attemptFailed doesn't throw.
+    assertEquals(1, testLogger.getMessageList().size());
+    assertEquals("generic failure", testLogger.getMessageList().get(0));
   }
 }