From 2fc938a819f4a2da9cfd25d2d306b62f53fa1f91 Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Mon, 8 Jul 2024 09:46:32 -0400 Subject: [PATCH] fix: `BaseApiTracer` to noop on attemptFailed via overloaded method call (#3016) Fixes https://github.com/googleapis/sdk-platform-java/issues/3015 Fixes https://github.com/googleapis/sdk-platform-java/issues/3014 Gax tracing internally works with `attemptFailedDuration`, which defaults to a no-op. Downstream libraries use `attemptFailed`, which has a custom behavior. What happens when an attempt-failed event occurs is that `attemptFailedDuration` is called instead (in favor of using java.time methods internally). This fix makes `attemptFailedDuration`'s behavior to delegate the logic to `attemptFailed`. The downstreams will keep failing because the repos haven't got adapted to the new change in gax. See the follow ups below. ### Fixes in `java-spanner` ![image](https://github.com/googleapis/sdk-platform-java/assets/22083784/85e50341-fe8b-46c8-9743-8de1ca300058) ### Fixes in `java-bigtable` ![image](https://github.com/googleapis/sdk-platform-java/assets/22083784/026af401-1607-4465-abd5-1ee5ddc353d0) ### Follow ups in `java-bigtable` More failures in java-bigtable to be addressed in that repo: ``` Error: BigtableTableAdminSettingsTest.testToString:175 expected to contain: totalTimeout=PT13H32M but was : BigtableTableAdminSettings{projectId=our-project-85, instanceId=our-instance-06, ... ``` Fixed in https://github.com/googleapis/java-bigtable/pull/2274 ### Follow ups in `java-spanner` ``` Error: Failures: Error: CompositeTracerTest.testMethodsOverrideMetricsTracer:238 Method not found in compositeTracerMethods: public void com.google.api.gax.tracing.MetricsTracer.attemptFailedDuration(java.lang.Throwable,java.time.Duration) ``` Fixed in https://github.com/googleapis/java-spanner/pull/3200 --- .../java/com/google/api/gax/tracing/BaseApiTracer.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/BaseApiTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/BaseApiTracer.java index 73ead433ec..8feb1286d2 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/BaseApiTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/BaseApiTracer.java @@ -29,6 +29,8 @@ */ package com.google.api.gax.tracing; +import static com.google.api.gax.util.TimeConversionUtils.toThreetenDuration; + import com.google.api.core.InternalApi; import com.google.api.core.ObsoleteApi; @@ -105,7 +107,8 @@ public void attemptCancelled() { @Override public void attemptFailedDuration(Throwable error, java.time.Duration delay) { - // noop + // noop via attemptFailed(Throwable error, org.threeten.Duration) + attemptFailed(error, toThreetenDuration(delay)); } /** @@ -113,7 +116,7 @@ public void attemptFailedDuration(Throwable error, java.time.Duration delay) { * instead. */ @Override - @ObsoleteApi("Use attemptFailed(Throwable, java.time.Duration) instead") + @ObsoleteApi("Use attemptFailedDuration(Throwable, java.time.Duration) instead") public void attemptFailed(Throwable error, org.threeten.bp.Duration delay) { // noop }