feat(gax): Actionable Errors Logging API Tracer#12202
feat(gax): Actionable Errors Logging API Tracer#12202westarle merged 5 commits intogoogleapis:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the GAX tracing mechanism by introducing a dedicated Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.gh.lumenfield.work/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new LoggingTracer and LoggingTracerFactory to provide actionable error logging for RPC failures, along with their corresponding unit tests. The LoggingTracer collects various context attributes and error details for logging. Feedback includes suggestions to use constants for log context keys in LoggingTracer for better maintainability, to ensure consistent context merging in one of the newTracer overloads in LoggingTracerFactory, and to improve the LoggingTracerTest to assert actual logged messages.
sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java
Outdated
Show resolved
Hide resolved
...latform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracerFactory.java
Outdated
Show resolved
Hide resolved
sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerTest.java
Outdated
Show resolved
Hide resolved
westarle
force-pushed
the
feat/actionable-errors-api-tracer
branch
5 times, most recently
from
2de91cf to
d5cc997
Compare
sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java
Outdated
Show resolved
Hide resolved
sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java
Outdated
Show resolved
Hide resolved
sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java
Outdated
Show resolved
Hide resolved
sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerTest.java
Show resolved
Hide resolved
westarle
force-pushed
the
feat/actionable-errors-api-tracer
branch
from
d5cc997 to
60569e8
Compare
westarle
force-pushed
the
feat/actionable-errors-api-tracer
branch
from
60569e8 to
a73c78b
Compare
| */ | ||
| @BetaApi | ||
| @InternalApi | ||
| public class LoggingTracer extends BaseApiTracer { |
There was a problem hiding this comment.
Can we change the scope of this class and the constructor from public to package private?
|
|
||
| Map<String, Object> logContext = new HashMap<>(apiTracerContext.getAttemptAttributes()); | ||
|
|
||
| if (apiTracerContext.serviceName() != null) { |
There was a problem hiding this comment.
This is probably a miss. I think getAttemptAttributes should include serviceName as well, since it is listed as a common attribute in the requirement.
There was a problem hiding this comment.
added to getAttemptAttributes()
| } | ||
|
|
||
| @Override | ||
| public void attemptFailedRetriesExhausted(Throwable error) { |
There was a problem hiding this comment.
Can we add unit tests for this method attemptFailedRetriesExhausted and method below attemptPermanentFailure?
| } | ||
|
|
||
| @Test | ||
| void testAttemptFailed_LogsErrorAndAttributes() { |
There was a problem hiding this comment.
I see that most of the logics are in recordActionableError, we could test it directly by making it package private and @VisibleForTesting.
Then we can have a simple test for each method that calls it.
| void testAttemptFailed_LogsErrorAndAttributes() { | ||
| ApiTracerContext context = | ||
| ApiTracerContext.empty().toBuilder() | ||
| .setServiceName("test-service") |
There was a problem hiding this comment.
Since all these 5 attributes are all from getAttemptAttributes(), maybe we don't have to add all them, using one or two should be good enough? I think it also blurred the focus of the test which is extracting errors info.
| "test.service.v1.Method", | ||
| attributesMap.get(ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE)); | ||
| assertEquals("grpc", attributesMap.get(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE)); | ||
| assertEquals( |
There was a problem hiding this comment.
Can we try to test these attributes in its own test case? This is inline with the best practices. Maybe we can group all the error info ones together, but I feel status should be its own test case.
Same comment for testing the error message as well.
|
|
||
| /** Function to extract the ErrorInfo payload from the error, if available */ | ||
| @Nullable | ||
| static ErrorInfo extractErrorInfo(@Nullable Throwable error) { |
There was a problem hiding this comment.
I'm adding similar logic in #12189.
Let me know if you'd prefer a different surface for my util.
There was a problem hiding this comment.
happy to move to your logic when it's ready.
blakeli0
left a comment
blakeli0
left a comment
There was a problem hiding this comment.
LGTM, thanks!
Do you mind adding some descriptions to the PR before merging?
🤖 I have created a release *beep* *boop* --- <details><summary>1.83.0</summary> ## [1.83.0](v1.82.0...v1.83.0) (2026-04-07) ### Features * [aiplatform] [Memorystore for Redis Cluster] Add support for ([0bd7666](0bd7666)) * [aiplatform] Add container_spec to Reasoning Engine public protos ([0bd7666](0bd7666)) * [aiplatform] Add container_spec to Reasoning Engine public protos ([0bd7666](0bd7666)) * [aiplatform] Add container_spec to Reasoning Engine public protos ([3ba3854](3ba3854)) * [aiplatform] Add container_spec to Reasoning Engine public protos ([3ba3854](3ba3854)) * [aiplatform] add evaluation metrics and autorater configuration to ([0bd7666](0bd7666)) * [backupdr] Adding new workload specific fields for AlloyDB ([6344cb0](6344cb0)) * [ces] update public libraries for CES v1 ([6344cb0](6344cb0)) * [ces] update public libraries for CES v1beta ([0bd7666](0bd7666)) * [ces] update public libraries for CES v1beta ([0bd7666](0bd7666)) * [chat] Addition of Section and SectionItem APIs ([0bd7666](0bd7666)) * [chat] Support app authentication with admin-consent scopes for ([0bd7666](0bd7666)) * [databasecenter] A new value `SUB_RESOURCE_TYPE_READ_POOL` is ([6344cb0](6344cb0)) * [dataflow] Add Pausing/Yaml capabilities to public protos ([3ba3854](3ba3854)) * [dataflow] add sha256 field to Package proto ([0bd7666](0bd7666)) * [dataflow] add sha256 field to Package proto ([3ba3854](3ba3854)) * [dataform] add folders and teamFolders related changes to v1 ([6344cb0](6344cb0)) * [datalineage] add configmanagement v1 module ([#12355](#12355)) ([2def625](2def625)) * [datamanager] add INVALID_MERCHANT_ID to the ErrorReason enum for ([6344cb0](6344cb0)) * [dialogflow-cx] updated v3 dialogflow client libraries with ([6344cb0](6344cb0)) * [dialogflow] updated v2 dialogflow client libraries ([6344cb0](6344cb0)) * [dialogflow] updated v2beta1 dialogflow client libraries ([6344cb0](6344cb0)) * [dlp] added support for detecting key-value pairs in client ([e5e22ed](e5e22ed)) * [document-ai] Added a fields for image and table annotation output ([0bd7666](0bd7666)) * [geocode] new module for geocode ([#12343](#12343)) ([474efb1](474efb1)) * [netapp] Add ONTAP passthrough APIs ([6344cb0](6344cb0)) * [network-security] Publish proto definitions for AuthzPolicy, ([6344cb0](6344cb0)) * [redis-cluster] [Memorystore for Redis Cluster] Add support for ([0bd7666](0bd7666)) * [redis-cluster] [Memorystore for Redis Cluster] Add support for ([3ba3854](3ba3854)) * [redis-cluster] [Memorystore for Redis Cluster] Add support for ([3ba3854](3ba3854)) * [securesourcemanager] Add CustomHostConfig to configure custom ([6344cb0](6344cb0)) * [storage] populate the `persisted_data_checksums` field with ([e5e22ed](e5e22ed)) * [texttospeech] Support safety settings for Gemini voices and ([0bd7666](0bd7666)) * [texttospeech] Support safety settings for Gemini voices and ([0bd7666](0bd7666)) * [texttospeech] Support safety settings for Gemini voices and ([0bd7666](0bd7666)) * [texttospeech] Support safety settings for Gemini voices and ([0bd7666](0bd7666)) * [translate] A new field `mime_type` is added to message ([e5e22ed](e5e22ed)) * [valkey] [Memorystore for Valkey] Add support for Flexible CA ([0bd7666](0bd7666)) * [valkey] [Memorystore for Valkey] Add support for Flexible CA ([0bd7666](0bd7666)) * [valkey] [Memorystore for Valkey] Add support for Flexible CA ([3ba3854](3ba3854)) * Add getProjectId getter for ComputeEngineCredentials ([#1833](#1833)) ([0a7895a](0a7895a)) * **bigguery:** add url.domain to span tracing ([#12208](#12208)) ([6f79c2d](6f79c2d)) * **bigquery observability:** add version attribute to span tracing ([#12132](#12132)) ([95c3eb8](95c3eb8)) * **bigquery:** add gcp.resource.destination.id for span tracing ([#12134](#12134)) ([5f31ded](5f31ded)) * **bigquery:** add opentelemetry W3C Trace Context to headers ([#12203](#12203)) ([965761a](965761a)) * **bigquery:** add resend attribute to span tracing + integration tests ([#12313](#12313)) ([167722d](167722d)) * **bigquery:** add url.full attribute to span tracing ([#12176](#12176)) ([7fdf9ff](7fdf9ff)) * **bigquery:** add url.template to span tracing ([#12181](#12181)) ([30f8afb](30f8afb)) * **bigquery:** added error attributes to span tracing ([#12115](#12115)) ([863d23b](863d23b)) * Extract resource name from unary requests for tracing ([#4159](#4159)) ([23b16b7](23b16b7)) * **gapic-generator-java:** Extract resource name heuristicly ([#12207](#12207)) ([f46480a](f46480a)) * **gax:** Actionable Errors Logging API Tracer ([#12202](#12202)) ([8d23279](8d23279)) * **gax:** Add error attributes to golden signal metrics. ([#12564](#12564)) ([063dfe5](063dfe5)) * **gax:** add utility for logging actionable errors ([#4144](#4144)) ([54fb8a5](54fb8a5)) * **gax:** Implement trace context extraction and injection with integration test ([#12625](#12625)) ([6675310](6675310)) * **observability:** Implement gcp.client.service attribute ([#12315](#12315)) ([e99812f](e99812f)) * **observability:** implement url.domain attribute ([#12316](#12316)) ([0a865bf](0a865bf)) * **sdk-platform-java:** Add CompositeTracer and CompositeTracerFactory. ([#12321](#12321)) ([4b5e8af](4b5e8af)) * Switch Eef metrics to using built in open telemetry ([#4385](#4385)) ([759bb22](759bb22)) ### Bug Fixes * Add error attributes to logging ([#12685](#12685)) ([a9198ee](a9198ee)) * **bq jdbc:** allow & ignore unknown parameters ([#12352](#12352)) ([2332ff1](2332ff1)) * **bq jdbc:** ensure getMoreResults() always moves the cursor ([#12353](#12353)) ([ac1f0f4](ac1f0f4)) * **ci:** consolidate duplicate yaml keys in github actions workflows ([#12306](#12306)) ([f644a19](f644a19)) * Clean up attributes for traces and metrics ([#12677](#12677)) ([914f97f](914f97f)) * fix getLong on NUMERIC ([#2420](#2420)) ([75ec5c2](75ec5c2)) * **gax:** Implement lazy resource name evaluation in ApiTracerContext ([#12618](#12618)) ([5e47749](5e47749)) * Handle null server address ([#12184](#12184)) ([435dd8c](435dd8c)) * **hermetic-build:** do not add release please comments on vertexai poms ([#12559](#12559)) ([5e161a7](5e161a7)) * **o11y:** create noop tracer when artifact ID is not set ([#12307](#12307)) ([630d83d](630d83d)) * **o11y:** do not record error.type in successful runs ([#12620](#12620)) ([28eebf0](28eebf0)) * **o11y:** remove `gpc.client.language` attribute ([#12621](#12621)) ([40d2e43](40d2e43)) * **oauth2:** mask sensitive tokens in HTTP logs ([#1900](#1900)) ([3e4ccb7](3e4ccb7)) * **release:** add Version.java as extra files in release-please ([#12617](#12617)) ([f5101d9](f5101d9)) * **spanner:** enforce READY-only location aware routing and add endpoint lifecycle management ([ecb86fd](ecb86fd)) * **spanner:** enforce READY-only location aware routing and add endpoint lifecycle management ([#12678](#12678)) ([ca9edb9](ca9edb9)) * **spanner:** improve grpc-gcp affinity cleanup and location-aware retries ([a157c2f](a157c2f)) * **spanner:** improve grpc-gcp affinity cleanup and location-aware retries ([#12682](#12682)) ([aca0428](aca0428)) * use dynamic tracer name instead of hardcoded gax-java ([#12190](#12190)) ([dea24db](dea24db)) ### Dependencies * bump jackson version to 2.18.3 ([#12351](#12351)) ([50304c1](50304c1)) * update dependencies.txt for grpc-gcp to 1.9.2 ([#4164](#4164)) ([f336fdc](f336fdc)) * update dependency com.google.apis:google-api-services-storage to v1-rev20260204-2.0.0 ([#1750](#1750)) ([340ecbe](340ecbe)) * update dependency com.google.apis:google-api-services-storage to v1-rev20260204-2.0.0 ([#3519](#3519)) ([1531107](1531107)) * update dependency com.google.cloud:google-cloud-storage to v2.64.1 ([#1752](#1752)) ([8fb6935](8fb6935)) * update dependency com.google.cloud:sdk-platform-java-config to v3.58.0 ([#1751](#1751)) ([9cc3e22](9cc3e22)) * update dependency com.google.cloud:sdk-platform-java-config to v3.58.0 ([#3523](#3523)) ([26d772a](26d772a)) * update dependency node to v24 ([#3509](#3509)) ([f308477](f308477)) * update gcr.io/cloud-devrel-public-resources/storage-testbench docker tag to v0.62.0 ([#3526](#3526)) ([ca29d5e](ca29d5e)) * update googleapis/sdk-platform-java action to v2.68.0 ([#3522](#3522)) ([abae1ac](abae1ac)) ### Reverts * ci: only run default list of graalvm tests if too many modules are touched ([#12292](#12292)) ([92bcdf4](92bcdf4)) ### Documentation * [dataplex] Change Dataplex library from `ALPHA` to `GA` ([6344cb0](6344cb0)) * [run] An existing repeated string field custom_audiences is marked ([015d9a1](015d9a1)) * **hermetic-build:** improve usability of development guide ([#12362](#12362)) ([5944127](5944127)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: chingor13 <chingor@google.com>
3 participants
This PR implements the "Actionable Errors" logging framework in the Java client libraries by hooking directly into the
ApiTracerframework. By capturing errors at the tracer level(specifically tied to T4 spans), we ensure that developers receive detailed, actionable context (like
google.rpc.ErrorInfopayloads including reason, domain, and metadata) directly intheir SLF4J logs.
Introduced
LoggingTracerandLoggingTracerFactory: Created a new tracer incom.google.api.gax.tracingextendingBaseApiTracer.If the thrown error is an
ApiExceptioncontainingErrorInfo, the tracer builds a context map (flattening themetadatamap and includingreason/domain) and emits a structured SLF4J log record viaLoggingUtils.logActionableErrorat theDEBUGlevel.Testing
TestServiceProviderto act as a properILoggerFactorythat caches and returns identicalTestLoggerinstances. This eliminates the needfor reflection to override
LOGGER_PROVIDERcaches, keeping tests clean and isolated.gax-java/gax/pom.xmlto excludeLoggingTracerTestfrom default surefire executions and explicitly bind it to theenvVarTestprofile,adhering to our environment-variable test constraints.