From e0b1b19409ca6108aae1afe78c3ba34afa8946a8 Mon Sep 17 00:00:00 2001 From: Wes Tarle Date: Mon, 23 Mar 2026 15:47:53 -0400 Subject: [PATCH 1/4] feat(gax): implement actionable errors logging in ApiTracer framework --- .../google/api/gax/tracing/LoggingTracer.java | 127 ++++++++++++++++++ .../api/gax/tracing/LoggingTracerFactory.java | 69 ++++++++++ .../gax/tracing/LoggingTracerFactoryTest.java | 73 ++++++++++ .../api/gax/tracing/LoggingTracerTest.java | 60 +++++++++ 4 files changed, 329 insertions(+) create mode 100644 sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java create mode 100644 sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracerFactory.java create mode 100644 sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerFactoryTest.java create mode 100644 sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerTest.java diff --git a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java new file mode 100644 index 000000000000..6676bcdde0c5 --- /dev/null +++ b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java @@ -0,0 +1,127 @@ +/* + * Copyright 2026 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.google.api.gax.tracing; + +import com.google.api.core.BetaApi; +import com.google.api.core.InternalApi; +import com.google.api.gax.logging.LoggerProvider; +import com.google.api.gax.logging.LoggingUtils; +import com.google.api.gax.rpc.ApiException; +import com.google.rpc.ErrorInfo; +import java.util.HashMap; +import java.util.Map; + +/** + * An {@link ApiTracer} that logs actionable errors using {@link LoggingUtils} when an RPC attempt + * fails. + */ +@BetaApi +@InternalApi +public class LoggingTracer extends BaseApiTracer { + private static final LoggerProvider LOGGER_PROVIDER = + LoggerProvider.forClazz(LoggingTracer.class); + + private final ApiTracerContext apiTracerContext; + + 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); + } + + @Override + public void attemptFailedRetriesExhausted(Throwable error) { + recordActionableError(error); + } + + @Override + public void attemptPermanentFailure(Throwable error) { + recordActionableError(error); + } + + private void recordActionableError(Throwable error) { + Map 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) { + logContext.put( + ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE, + ObservabilityUtils.extractStatus(error)); + } + + if (error instanceof ApiException) { + ApiException apiException = (ApiException) error; + if (apiException.getErrorDetails() != null) { + ErrorInfo errorInfo = apiException.getErrorDetails().getErrorInfo(); + if (errorInfo != null) { + logContext.put("error.type", errorInfo.getReason()); + logContext.put("gcp.errors.domain", errorInfo.getDomain()); + for (Map.Entry entry : errorInfo.getMetadataMap().entrySet()) { + logContext.put("gcp.errors.metadata." + entry.getKey(), entry.getValue()); + } + } + } + } + + String message = "Unknown Error"; + if (error != null) { + message = error.getMessage() != null ? error.getMessage() : error.getClass().getName(); + } + LoggingUtils.logActionableError(logContext, LOGGER_PROVIDER, message); + } +} diff --git a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracerFactory.java b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracerFactory.java new file mode 100644 index 000000000000..08e50aa0bb8b --- /dev/null +++ b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracerFactory.java @@ -0,0 +1,69 @@ +/* + * Copyright 2026 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.google.api.gax.tracing; + +import com.google.api.core.BetaApi; +import com.google.api.core.InternalApi; + +/** A {@link ApiTracerFactory} that creates instances of {@link LoggingTracer}. */ +@BetaApi +@InternalApi +public class LoggingTracerFactory implements ApiTracerFactory { + private final ApiTracerContext apiTracerContext; + + public LoggingTracerFactory() { + this(ApiTracerContext.empty()); + } + + private LoggingTracerFactory(ApiTracerContext apiTracerContext) { + this.apiTracerContext = apiTracerContext; + } + + @Override + public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) { + return new LoggingTracer(apiTracerContext); + } + + @Override + public ApiTracer newTracer(ApiTracer parent, ApiTracerContext context) { + return new LoggingTracer(context); + } + + @Override + public ApiTracerContext getApiTracerContext() { + return apiTracerContext; + } + + @Override + public ApiTracerFactory withContext(ApiTracerContext context) { + return new LoggingTracerFactory(apiTracerContext.merge(context)); + } +} diff --git a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerFactoryTest.java b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerFactoryTest.java new file mode 100644 index 000000000000..fedfe7b8ecbc --- /dev/null +++ b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerFactoryTest.java @@ -0,0 +1,73 @@ +/* + * Copyright 2026 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.google.api.gax.tracing; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +class LoggingTracerFactoryTest { + + @Test + void testNewTracer_CreatesLoggingTracer() { + LoggingTracerFactory factory = new LoggingTracerFactory(); + ApiTracer tracer = + factory.newTracer( + BaseApiTracer.getInstance(), + SpanName.of("client", "method"), + ApiTracerFactory.OperationType.Unary); + + assertNotNull(tracer); + assertTrue(tracer instanceof LoggingTracer); + } + + @Test + void testNewTracer_WithContext_CreatesLoggingTracer() { + LoggingTracerFactory factory = new LoggingTracerFactory(); + ApiTracer tracer = factory.newTracer(BaseApiTracer.getInstance(), ApiTracerContext.empty()); + + assertNotNull(tracer); + assertTrue(tracer instanceof LoggingTracer); + } + + @Test + void testWithContext_ReturnsNewFactoryWithMergedContext() { + LoggingTracerFactory factory = new LoggingTracerFactory(); + ApiTracerContext context = ApiTracerContext.newBuilder().setServerAddress("address").build(); + ApiTracerFactory updatedFactory = factory.withContext(context); + + assertNotNull(updatedFactory); + assertTrue(updatedFactory instanceof LoggingTracerFactory); + assertEquals("address", updatedFactory.getApiTracerContext().serverAddress()); + } +} diff --git a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerTest.java b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerTest.java new file mode 100644 index 000000000000..ec0b86a25c4e --- /dev/null +++ b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerTest.java @@ -0,0 +1,60 @@ +/* + * Copyright 2026 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.google.api.gax.tracing; + +import com.google.api.gax.logging.TestLogger; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; + +class LoggingTracerTest { + + private TestLogger testLogger; + + @BeforeEach + void setUp() { + testLogger = (TestLogger) LoggerFactory.getLogger(LoggingTracer.class); + } + + @Test + void testAttemptFailed_LogsError() { + ApiTracerContext context = ApiTracerContext.empty(); + LoggingTracer tracer = new LoggingTracer(context); + + // Call attemptFailed with a generic exception + Exception error = new RuntimeException("generic failure"); + tracer.attemptFailed(error, org.threeten.bp.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. + } +} From 84e3b25ec18694c863a0d5e2879f23abc59d7cce Mon Sep 17 00:00:00 2001 From: Wes Tarle Date: Wed, 25 Mar 2026 17:29:16 -0400 Subject: [PATCH 2/4] refactor: use ObservabilityUtils.extractErrorInfo helper --- .../google/api/gax/tracing/LoggingTracer.java | 18 ++++++------------ .../api/gax/tracing/ObservabilityUtils.java | 13 +++++++++++++ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java index 6676bcdde0c5..cf8cc6397aa3 100644 --- a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java +++ b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java @@ -34,7 +34,6 @@ import com.google.api.core.InternalApi; import com.google.api.gax.logging.LoggerProvider; import com.google.api.gax.logging.LoggingUtils; -import com.google.api.gax.rpc.ApiException; import com.google.rpc.ErrorInfo; import java.util.HashMap; import java.util.Map; @@ -104,17 +103,12 @@ private void recordActionableError(Throwable error) { ObservabilityUtils.extractStatus(error)); } - if (error instanceof ApiException) { - ApiException apiException = (ApiException) error; - if (apiException.getErrorDetails() != null) { - ErrorInfo errorInfo = apiException.getErrorDetails().getErrorInfo(); - if (errorInfo != null) { - logContext.put("error.type", errorInfo.getReason()); - logContext.put("gcp.errors.domain", errorInfo.getDomain()); - for (Map.Entry entry : errorInfo.getMetadataMap().entrySet()) { - logContext.put("gcp.errors.metadata." + entry.getKey(), entry.getValue()); - } - } + ErrorInfo errorInfo = ObservabilityUtils.extractErrorInfo(error); + if (errorInfo != null) { + logContext.put("error.type", errorInfo.getReason()); + logContext.put("gcp.errors.domain", errorInfo.getDomain()); + for (Map.Entry entry : errorInfo.getMetadataMap().entrySet()) { + logContext.put("gcp.errors.metadata." + entry.getKey(), entry.getValue()); } } diff --git a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityUtils.java b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityUtils.java index 2487964370ca..f0062e38505a 100644 --- a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityUtils.java +++ b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityUtils.java @@ -31,6 +31,7 @@ import com.google.api.gax.rpc.ApiException; import com.google.api.gax.rpc.StatusCode; +import com.google.rpc.ErrorInfo; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; import java.util.Map; @@ -56,6 +57,18 @@ static String extractStatus(@Nullable Throwable error) { return statusString; } + /** Function to extract the ErrorInfo payload from the error, if available */ + @Nullable + static ErrorInfo extractErrorInfo(@Nullable Throwable error) { + if (error instanceof ApiException) { + ApiException apiException = (ApiException) error; + if (apiException.getErrorDetails() != null) { + return apiException.getErrorDetails().getErrorInfo(); + } + } + return null; + } + static Attributes toOtelAttributes(Map attributes) { AttributesBuilder attributesBuilder = Attributes.builder(); if (attributes == null) { From a73c78be318c9d89ef8870a7525a80b6b2442166 Mon Sep 17 00:00:00 2001 From: Wes Tarle Date: Wed, 25 Mar 2026 18:10:19 -0400 Subject: [PATCH 3/4] fix(test): provide libraryMetadata for ApiTracerContext.newBuilder in tests --- sdk-platform-java/gax-java/gax/pom.xml | 4 +- .../google/api/gax/tracing/LoggingTracer.java | 59 +++++------- .../api/gax/tracing/LoggingTracerFactory.java | 2 +- .../gax/tracing/ObservabilityAttributes.java | 9 ++ .../google/api/gax/logging/TestLogger.java | 12 +++ .../api/gax/logging/TestServiceProvider.java | 21 +++-- .../gax/tracing/LoggingTracerFactoryTest.java | 3 +- .../api/gax/tracing/LoggingTracerTest.java | 91 ++++++++++++++++++- 8 files changed, 149 insertions(+), 52 deletions(-) diff --git a/sdk-platform-java/gax-java/gax/pom.xml b/sdk-platform-java/gax-java/gax/pom.xml index 7a03fad54d29..2feb9c9aba81 100644 --- a/sdk-platform-java/gax-java/gax/pom.xml +++ b/sdk-platform-java/gax-java/gax/pom.xml @@ -139,7 +139,7 @@ @{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" - !EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet+endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority,!LoggingEnabledTest + !EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet+endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority,!LoggingEnabledTest,!LoggingTracerTest @@ -154,7 +154,7 @@ org.apache.maven.plugins maven-surefire-plugin - EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet+endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority,LoggingEnabledTest + EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet+endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority,LoggingEnabledTest,LoggingTracerTest diff --git a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java index cf8cc6397aa3..519902a4736d 100644 --- a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java +++ b/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,39 @@ public void attemptPermanentFailure(Throwable error) { } private void recordActionableError(Throwable error) { - Map 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 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()); - for (Map.Entry entry : errorInfo.getMetadataMap().entrySet()) { - logContext.put("gcp.errors.metadata." + entry.getKey(), entry.getValue()); + if (errorInfo.getReason() != null && !errorInfo.getReason().isEmpty()) { + logContext.put(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE, errorInfo.getReason()); + } + if (errorInfo.getDomain() != null && !errorInfo.getDomain().isEmpty()) { + logContext.put(ObservabilityAttributes.ERROR_DOMAIN_ATTRIBUTE, errorInfo.getDomain()); + } + if (errorInfo.getMetadataMap() != null) { + for (Map.Entry entry : errorInfo.getMetadataMap().entrySet()) { + 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); } } diff --git a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracerFactory.java b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracerFactory.java index 08e50aa0bb8b..101500aaf53b 100644 --- a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracerFactory.java +++ b/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 diff --git a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityAttributes.java b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityAttributes.java index 64095fde0099..6de87ca5758d 100644 --- a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityAttributes.java +++ b/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."; } diff --git a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/logging/TestLogger.java b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/logging/TestLogger.java index 3ee7e513cd8f..36e8d47e4827 100644 --- a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/logging/TestLogger.java +++ b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/logging/TestLogger.java @@ -48,8 +48,20 @@ public class TestLogger implements Logger, LoggingEventAware { List messageList = new ArrayList<>(); Level level; + public List getMessageList() { + return messageList; + } + Map keyValuePairsMap = new HashMap<>(); + public Map getMDCMap() { + return MDCMap; + } + + public Map getKeyValuePairsMap() { + return keyValuePairsMap; + } + private String loggerName; private boolean infoEnabled; private boolean debugEnabled; diff --git a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/logging/TestServiceProvider.java b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/logging/TestServiceProvider.java index 69cc43ba2a96..2b7f0f3b8268 100644 --- a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/logging/TestServiceProvider.java +++ b/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 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 diff --git a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerFactoryTest.java b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerFactoryTest.java index fedfe7b8ecbc..db36746ab56a 100644 --- a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerFactoryTest.java +++ b/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); diff --git a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerTest.java b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerTest.java index ec0b86a25c4e..8ae5d6577325 100644 --- a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerTest.java +++ b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerTest.java @@ -30,7 +30,18 @@ package com.google.api.gax.tracing; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + import com.google.api.gax.logging.TestLogger; +import com.google.api.gax.rpc.ApiExceptionFactory; +import com.google.api.gax.rpc.ErrorDetails; +import com.google.api.gax.rpc.StatusCode; +import com.google.api.gax.rpc.testing.FakeStatusCode; +import com.google.protobuf.Any; +import com.google.rpc.ErrorInfo; +import java.util.Collections; +import java.util.Map; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.slf4j.LoggerFactory; @@ -42,6 +53,9 @@ class LoggingTracerTest { @BeforeEach void setUp() { testLogger = (TestLogger) LoggerFactory.getLogger(LoggingTracer.class); + testLogger.getMessageList().clear(); + testLogger.getMDCMap().clear(); + testLogger.getKeyValuePairsMap().clear(); } @Test @@ -51,10 +65,79 @@ 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); + + assertEquals(1, testLogger.getMessageList().size()); + assertEquals("generic failure", testLogger.getMessageList().get(0)); + } + + @Test + void testAttemptFailed_LogsErrorAndAttributes() { + ApiTracerContext context = + ApiTracerContext.empty().toBuilder() + .setServiceName("test-service") + .setServerAddress("test.example.com") + .setServerPort(443) + .setFullMethodName("test.service.v1.Method") + .setTransport(ApiTracerContext.Transport.GRPC) + .build(); + LoggingTracer tracer = new LoggingTracer(context); + + ErrorInfo errorInfo = + ErrorInfo.newBuilder() + .setReason("TEST_REASON") + .setDomain("test.domain.com") + .putMetadata("test_key", "test_value") + .build(); + + ErrorDetails errorDetails = + ErrorDetails.builder() + .setRawErrorMessages(Collections.singletonList(Any.pack(errorInfo))) + .build(); + + Exception error = + ApiExceptionFactory.createException( + "test error message", + new RuntimeException("cause"), + FakeStatusCode.of(StatusCode.Code.INVALID_ARGUMENT), + false, + errorDetails); + + tracer.attemptFailedDuration(error, java.time.Duration.ZERO); + + assertEquals(1, testLogger.getMessageList().size()); + assertEquals("test error message", testLogger.getMessageList().get(0)); + + Map attributesMap; + // SLF4J 1.x logs using MDC, SLF4J 2.x logs using KeyValuePairs + // Depending on the classpath binding active, one will be populated by TestLogger + if (!testLogger.getMDCMap().isEmpty()) { + attributesMap = testLogger.getMDCMap(); + } else { + attributesMap = testLogger.getKeyValuePairsMap(); + } + + assertTrue(attributesMap != null && !attributesMap.isEmpty()); + assertEquals( + "test-service", attributesMap.get(ObservabilityAttributes.GCP_CLIENT_SERVICE_ATTRIBUTE)); + assertEquals( + "test.example.com", attributesMap.get(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE)); + + Object portValue = attributesMap.get(ObservabilityAttributes.SERVER_PORT_ATTRIBUTE); + assertTrue("443".equals(String.valueOf(portValue))); - // 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( + "test.service.v1.Method", + attributesMap.get(ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE)); + assertEquals("grpc", attributesMap.get(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE)); + assertEquals( + "INVALID_ARGUMENT", + attributesMap.get(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE)); + assertEquals("TEST_REASON", attributesMap.get(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE)); + assertEquals( + "test.domain.com", attributesMap.get(ObservabilityAttributes.ERROR_DOMAIN_ATTRIBUTE)); + assertEquals( + "test_value", + attributesMap.get(ObservabilityAttributes.ERROR_METADATA_ATTRIBUTE_PREFIX + "test_key")); } } From 457f88c32e9054372e6463ebb796fba0b0ec309d Mon Sep 17 00:00:00 2001 From: Wes Tarle Date: Fri, 27 Mar 2026 18:05:09 -0400 Subject: [PATCH 4/4] fix(gax): address PR3 feedback on actionable errors --- .../api/gax/tracing/ApiTracerContext.java | 3 + .../google/api/gax/tracing/LoggingTracer.java | 13 +- .../api/gax/tracing/LoggingTracerTest.java | 127 ++++++++++++------ 3 files changed, 96 insertions(+), 47 deletions(-) diff --git a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java index d6f7566c7461..49bce86cd1fe 100644 --- a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java +++ b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java @@ -205,6 +205,9 @@ public Map getAttemptAttributes() { attributes.put(ObservabilityAttributes.HTTP_URL_TEMPLATE_ATTRIBUTE, httpPathTemplate()); } } + if (!Strings.isNullOrEmpty(serviceName())) { + attributes.put(ObservabilityAttributes.GCP_CLIENT_SERVICE_ATTRIBUTE, serviceName()); + } if (!Strings.isNullOrEmpty(destinationResourceId())) { attributes.put( ObservabilityAttributes.DESTINATION_RESOURCE_ID_ATTRIBUTE, destinationResourceId()); diff --git a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java index 519902a4736d..ec1a39c33337 100644 --- a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java +++ b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java @@ -34,6 +34,7 @@ import com.google.api.core.InternalApi; import com.google.api.gax.logging.LoggerProvider; import com.google.api.gax.logging.LoggingUtils; +import com.google.common.annotations.VisibleForTesting; import com.google.rpc.ErrorInfo; import java.util.HashMap; import java.util.Map; @@ -44,13 +45,13 @@ */ @BetaApi @InternalApi -public class LoggingTracer extends BaseApiTracer { +class LoggingTracer extends BaseApiTracer { private static final LoggerProvider LOGGER_PROVIDER = LoggerProvider.forClazz(LoggingTracer.class); private final ApiTracerContext apiTracerContext; - public LoggingTracer(ApiTracerContext apiTracerContext) { + LoggingTracer(ApiTracerContext apiTracerContext) { this.apiTracerContext = apiTracerContext; } @@ -69,18 +70,14 @@ public void attemptPermanentFailure(Throwable error) { recordActionableError(error); } - private void recordActionableError(Throwable error) { + @VisibleForTesting + void recordActionableError(Throwable error) { if (error == null) { return; } Map logContext = new HashMap<>(apiTracerContext.getAttemptAttributes()); - if (apiTracerContext.serviceName() != null) { - logContext.put( - ObservabilityAttributes.GCP_CLIENT_SERVICE_ATTRIBUTE, apiTracerContext.serviceName()); - } - logContext.put( ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE, ObservabilityUtils.extractStatus(error)); diff --git a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerTest.java b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerTest.java index 8ae5d6577325..62e4506a5247 100644 --- a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerTest.java +++ b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerTest.java @@ -59,28 +59,94 @@ void setUp() { } @Test - void testAttemptFailed_LogsError() { + void testAttemptFailedDuration_LogsError() { ApiTracerContext context = ApiTracerContext.empty(); LoggingTracer tracer = new LoggingTracer(context); - // Call attemptFailed with a generic exception - Exception error = new RuntimeException("generic failure"); + Exception error = new RuntimeException("generic failure duration"); tracer.attemptFailedDuration(error, java.time.Duration.ZERO); assertEquals(1, testLogger.getMessageList().size()); - assertEquals("generic failure", testLogger.getMessageList().get(0)); + assertEquals("generic failure duration", testLogger.getMessageList().get(0)); + } + + @Test + void testAttemptFailedRetriesExhausted_LogsError() { + ApiTracerContext context = ApiTracerContext.empty(); + LoggingTracer tracer = new LoggingTracer(context); + + Exception error = new RuntimeException("generic failure retries exhausted"); + tracer.attemptFailedRetriesExhausted(error); + + assertEquals(1, testLogger.getMessageList().size()); + assertEquals("generic failure retries exhausted", testLogger.getMessageList().get(0)); + } + + @Test + void testAttemptPermanentFailure_LogsError() { + ApiTracerContext context = ApiTracerContext.empty(); + LoggingTracer tracer = new LoggingTracer(context); + + Exception error = new RuntimeException("generic permanent failure"); + tracer.attemptPermanentFailure(error); + + assertEquals(1, testLogger.getMessageList().size()); + assertEquals("generic permanent failure", testLogger.getMessageList().get(0)); + } + + @Test + void testRecordActionableError_logsErrorMessage() { + ApiTracerContext context = ApiTracerContext.empty(); + LoggingTracer tracer = new LoggingTracer(context); + + Exception error = new RuntimeException("test error message"); + tracer.recordActionableError(error); + + assertEquals(1, testLogger.getMessageList().size()); + assertEquals("test error message", testLogger.getMessageList().get(0)); + } + + @Test + void testRecordActionableError_logsStatus() { + ApiTracerContext context = ApiTracerContext.empty(); + LoggingTracer tracer = new LoggingTracer(context); + + Exception error = + ApiExceptionFactory.createException( + "test error message", + new RuntimeException("cause"), + FakeStatusCode.of(StatusCode.Code.INVALID_ARGUMENT), + false); + + tracer.recordActionableError(error); + + Map attributesMap = getAttributesMap(); + + assertTrue(attributesMap != null && !attributesMap.isEmpty()); + assertEquals( + "INVALID_ARGUMENT", + attributesMap.get(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE)); } @Test - void testAttemptFailed_LogsErrorAndAttributes() { + void testRecordActionableError_logsAttributes() { ApiTracerContext context = - ApiTracerContext.empty().toBuilder() - .setServiceName("test-service") - .setServerAddress("test.example.com") - .setServerPort(443) - .setFullMethodName("test.service.v1.Method") - .setTransport(ApiTracerContext.Transport.GRPC) - .build(); + ApiTracerContext.empty().toBuilder().setServiceName("test-service").build(); + LoggingTracer tracer = new LoggingTracer(context); + + Exception error = new RuntimeException("generic failure"); + tracer.recordActionableError(error); + + Map attributesMap = getAttributesMap(); + + assertTrue(attributesMap != null && !attributesMap.isEmpty()); + assertEquals( + "test-service", attributesMap.get(ObservabilityAttributes.GCP_CLIENT_SERVICE_ATTRIBUTE)); + } + + @Test + void testRecordActionableError_logsErrorInfo() { + ApiTracerContext context = ApiTracerContext.empty(); LoggingTracer tracer = new LoggingTracer(context); ErrorInfo errorInfo = @@ -103,36 +169,11 @@ void testAttemptFailed_LogsErrorAndAttributes() { false, errorDetails); - tracer.attemptFailedDuration(error, java.time.Duration.ZERO); + tracer.recordActionableError(error); - assertEquals(1, testLogger.getMessageList().size()); - assertEquals("test error message", testLogger.getMessageList().get(0)); - - Map attributesMap; - // SLF4J 1.x logs using MDC, SLF4J 2.x logs using KeyValuePairs - // Depending on the classpath binding active, one will be populated by TestLogger - if (!testLogger.getMDCMap().isEmpty()) { - attributesMap = testLogger.getMDCMap(); - } else { - attributesMap = testLogger.getKeyValuePairsMap(); - } + Map attributesMap = getAttributesMap(); assertTrue(attributesMap != null && !attributesMap.isEmpty()); - assertEquals( - "test-service", attributesMap.get(ObservabilityAttributes.GCP_CLIENT_SERVICE_ATTRIBUTE)); - assertEquals( - "test.example.com", attributesMap.get(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE)); - - Object portValue = attributesMap.get(ObservabilityAttributes.SERVER_PORT_ATTRIBUTE); - assertTrue("443".equals(String.valueOf(portValue))); - - assertEquals( - "test.service.v1.Method", - attributesMap.get(ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE)); - assertEquals("grpc", attributesMap.get(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE)); - assertEquals( - "INVALID_ARGUMENT", - attributesMap.get(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE)); assertEquals("TEST_REASON", attributesMap.get(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE)); assertEquals( "test.domain.com", attributesMap.get(ObservabilityAttributes.ERROR_DOMAIN_ATTRIBUTE)); @@ -140,4 +181,12 @@ void testAttemptFailed_LogsErrorAndAttributes() { "test_value", attributesMap.get(ObservabilityAttributes.ERROR_METADATA_ATTRIBUTE_PREFIX + "test_key")); } + + private Map getAttributesMap() { + if (!testLogger.getMDCMap().isEmpty()) { + return testLogger.getMDCMap(); + } else { + return testLogger.getKeyValuePairsMap(); + } + } }