Skip to content
This repository was archived by the owner on Apr 7, 2026. It is now read-only.

fix: adapt to introduction of java.time in gax - #3200

Closed
diegomarquezp wants to merge 4 commits into
mainfrom
fix-javatime-updates
Closed

fix: adapt to introduction of java.time in gax#3200
diegomarquezp wants to merge 4 commits into
mainfrom
fix-javatime-updates

Conversation

@diegomarquezp

@diegomarquezpdiegomarquezp commented Jul 5, 2024

Copy link
Copy Markdown
Contributor

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

@diegomarquezpdiegomarquezp 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 teamJuly 5, 2024 00:32
@product-auto-labelproduct-auto-labelBot 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#3015Fixes#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
lqiu96force-pushed the fix-javatime-updates branch from 7091d34 to abc3766CompareAugust 6, 2024 20:23
@lqiu96lqiu96 removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Aug 6, 2024
@lqiu96lqiu96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 6, 2024
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 6, 2024
@lqiu96lqiu96 mentioned this pull request Aug 6, 2024
@lqiu96

Copy link
Copy Markdown
Member

Closing as this seems to have been fixed in #3243

@lqiu96lqiu96 closed this Aug 7, 2024
chingor13 pushed a commit to googleapis/google-cloud-java that referenced this pull request Mar 24, 2026
…all (#3016)
Fixesgoogleapis/sdk-platform-java#3015Fixesgoogleapis/sdk-platform-java#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
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: spannerIssues related to the googleapis/java-spanner API.size: sPull request size is small.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@diegomarquezp@lqiu96@yoshi-kokoro