Skip to content

Conversation

@diegomarquezp
Copy link
Contributor

@diegomarquezp diegomarquezp commented Jul 5, 2024

To be merged after gapic-generator-java >2.24.0 is merged.

This adapts CompositeTracer to also include attemptFailedDuration, which was recently introduced in googleapis/sdk-platform-java#1872

Confirmed it works by using gax 2.51.1-SNAPSHOT
image

@diegomarquezp diegomarquezp added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 5, 2024
@diegomarquezp diegomarquezp requested a review from a team as a code owner July 5, 2024 00:32
@product-auto-label product-auto-label bot added size: s Pull request size is small. api: spanner Issues related to the googleapis/java-spanner API. labels Jul 5, 2024
diegomarquezp added a commit to googleapis/sdk-platform-java that referenced this pull request Jul 8, 2024
…all (#3016) Fixes #3015 Fixes #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 googleapis/java-bigtable#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 googleapis/java-spanner#3200
@lqiu96 lqiu96 force-pushed the fix-javatime-updates branch from 7091d34 to abc3766 Compare August 6, 2024 20:23
@lqiu96 lqiu96 removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Aug 6, 2024
@lqiu96 lqiu96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 6, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 6, 2024
@lqiu96
Copy link
Member

lqiu96 commented Aug 7, 2024

Closing as this seems to have been fixed in #3243

@lqiu96 lqiu96 closed this Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: spanner Issues related to the googleapis/java-spanner API. size: s Pull request size is small.

3 participants