New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: multiple calls to end of span #75
Conversation
@@ -892,7 +892,7 @@ private PooledSession take() throws SpannerException { | |||
return s.session; | |||
} | |||
} catch (Exception e) { | |||
TraceUtil.endSpanWithFailure(tracer.getCurrentSpan(), e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tracer.getCurrentSpan()
is called outside of scope, which means endSpanWithFailure
will execute on either READ_WRITE_TRANSACTION or READ_ONLY_TRANSACTION span instead of WAIT_FOR_SESSION span.
@rghetia Could you please review PR? |
span.setStatus(Status.INTERNAL.withDescription(e.getMessage())); | ||
} | ||
} | ||
|
||
static void endSpanWithFailure(Span span, Throwable e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is endSpanWithFailure used anywhere now? If not then please remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been used in a few places (ex: singleUse) where span does not end except when there is a failure.
See: https://github.com/googleapis/google-cloud-java/pull/2677/files#r157323309 for original reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much @mayurkale22 and @olavloite for reproducing the problem and working on the fix. I really appreciate it. Great work.
@olavloite could you please help me to fix the tests? Here is the issue: When I ran |
@mayurkale22 |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
Thanks for the help.
I managed to repro-d the issue in |
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
@olavloite I have added a few more tests and attempted to fix remaining issues, PTAL c0689bd when you get a chance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
I don't quite get where the dependency problem is coming from. It can be fixed by adding the following to the <ignoredDependencies>
tag in the pom.xml
: com.google.errorprone:error_prone_annotations
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
Great. Finally, build is passed. |
@rghetia would like to get your eyes on OpenCensus related changes. Please review when you get a chance. |
@skuruppu I think this is good to merge now. |
This PR contains the following updates: | Package | Update | Change | |---|---|---| | com.google.api.grpc:grpc-google-cloud-spanner-admin-database-v1 | minor | `1.49.2` -> `1.51.0` | | com.google.api.grpc:grpc-google-cloud-spanner-admin-instance-v1 | minor | `1.49.2` -> `1.51.0` | | com.google.api.grpc:grpc-google-cloud-spanner-v1 | minor | `1.49.2` -> `1.51.0` | | [com.google.cloud:google-cloud-spanner](https://togithub.com/googleapis/java-spanner) | minor | `1.49.2` -> `1.51.0` | | com.google.api.grpc:proto-google-cloud-spanner-admin-database-v1 | minor | `1.49.2` -> `1.51.0` | | com.google.api.grpc:proto-google-cloud-spanner-v1 | minor | `1.49.2` -> `1.51.0` | --- ### Release Notes <details> <summary>googleapis/java-spanner</summary> ### [`v1.51.0`](https://togithub.com/googleapis/java-spanner/blob/master/CHANGELOG.md#​1510httpswwwgithubcomgoogleapisjava-spannercomparev1500v1510-2020-03-13) [Compare Source](https://togithub.com/googleapis/java-spanner/compare/v1.50.0...v1.51.0) ##### Features - add backend query options ([#​90](https://www.github.com/googleapis/java-spanner/issues/90)) ([e96e172](https://www.github.com/googleapis/java-spanner/commit/e96e17246bee9691171b46857806d03d1f8e19b4)) - add QueryOptions proto ([#​84](https://www.github.com/googleapis/java-spanner/issues/84)) ([eb8fc37](https://www.github.com/googleapis/java-spanner/commit/eb8fc375bbd766f25966aa565e266ed972bbe818)) ##### Bug Fixes - never use credentials in combination with plain text ([#​98](https://www.github.com/googleapis/java-spanner/issues/98)) ([7eb8d49](https://www.github.com/googleapis/java-spanner/commit/7eb8d49cd6c35d7f757cb89009ad16be601b77c3)) ##### Dependencies - update dependency com.google.cloud:google-cloud-core-bom to v1.93.1 ([#​91](https://www.github.com/googleapis/java-spanner/issues/91)) ([29d8db8](https://www.github.com/googleapis/java-spanner/commit/29d8db8cfc9d12824b9264d0fb870049a58a9a03)) - update dependency io.opencensus:opencensus-api to v0.25.0 ([#​95](https://www.github.com/googleapis/java-spanner/issues/95)) ([57f5fd0](https://www.github.com/googleapis/java-spanner/commit/57f5fd0f3bee4b437f48b6a08ab3174f035c8cca)) ### [`v1.50.0`](https://togithub.com/googleapis/java-spanner/blob/master/CHANGELOG.md#​1510httpswwwgithubcomgoogleapisjava-spannercomparev1500v1510-2020-03-13) [Compare Source](https://togithub.com/googleapis/java-spanner/compare/v1.49.2...v1.50.0) ##### Features - add backend query options ([#​90](https://www.github.com/googleapis/java-spanner/issues/90)) ([e96e172](https://www.github.com/googleapis/java-spanner/commit/e96e17246bee9691171b46857806d03d1f8e19b4)) - add QueryOptions proto ([#​84](https://www.github.com/googleapis/java-spanner/issues/84)) ([eb8fc37](https://www.github.com/googleapis/java-spanner/commit/eb8fc375bbd766f25966aa565e266ed972bbe818)) ##### Bug Fixes - never use credentials in combination with plain text ([#​98](https://www.github.com/googleapis/java-spanner/issues/98)) ([7eb8d49](https://www.github.com/googleapis/java-spanner/commit/7eb8d49cd6c35d7f757cb89009ad16be601b77c3)) ##### Dependencies - update dependency com.google.cloud:google-cloud-core-bom to v1.93.1 ([#​91](https://www.github.com/googleapis/java-spanner/issues/91)) ([29d8db8](https://www.github.com/googleapis/java-spanner/commit/29d8db8cfc9d12824b9264d0fb870049a58a9a03)) - update dependency io.opencensus:opencensus-api to v0.25.0 ([#​95](https://www.github.com/googleapis/java-spanner/issues/95)) ([57f5fd0](https://www.github.com/googleapis/java-spanner/commit/57f5fd0f3bee4b437f48b6a08ab3174f035c8cca)) </details> --- ### Renovate configuration :date: **Schedule**: At any time (no schedule defined). :vertical_traffic_light: **Automerge**: Disabled by config. Please merge this manually once you are satisfied. :recycle: **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. :no_bell: **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#googleapis/java-spanner-jdbc).
🤖 I have created a release \*beep\* \*boop\* --- ## [1.14.0](https://www.github.com/googleapis/java-spanner-jdbc/compare/v1.13.0...v1.14.0) (2020-03-18) ### Features * add support for foreign keys ([googleapis#78](https://www.github.com/googleapis/java-spanner-jdbc/issues/78)) ([9e770f2](https://www.github.com/googleapis/java-spanner-jdbc/commit/9e770f281c03a1e9c034e5ff3ddee44fa20a7b30)), closes [googleapis#77](https://www.github.com/googleapis/java-spanner-jdbc/issues/77) ### Bug Fixes * add missing netty-shaded lib for über-jar ([googleapis#80](https://www.github.com/googleapis/java-spanner-jdbc/issues/80)) ([3d6f356](https://www.github.com/googleapis/java-spanner-jdbc/commit/3d6f35669671194e6772fe327ce48f27e5bf4643)) * fix deprecation warnings in JDBC (test) files ([googleapis#81](https://www.github.com/googleapis/java-spanner-jdbc/issues/81)) ([a5e031d](https://www.github.com/googleapis/java-spanner-jdbc/commit/a5e031d3183f8fe88a621500f235ca2b0242f50b)) * include Spanner gRPC test dependencies ([googleapis#63](https://www.github.com/googleapis/java-spanner-jdbc/issues/63)) ([a34bfc0](https://www.github.com/googleapis/java-spanner-jdbc/commit/a34bfc0ff1c2ddeef077dbfae4c56bdd53febcb2)) ### Dependencies * update core dependencies ([1ae098e](https://www.github.com/googleapis/java-spanner-jdbc/commit/1ae098e924c2a488cfddd0a3aee9511274b7a515)) * update core dependencies ([googleapis#40](https://www.github.com/googleapis/java-spanner-jdbc/issues/40)) ([18c3a1b](https://www.github.com/googleapis/java-spanner-jdbc/commit/18c3a1b069cb507a91d0320e64a8bf8ae8efe394)) * update core dependencies ([googleapis#73](https://www.github.com/googleapis/java-spanner-jdbc/issues/73)) ([cfa1539](https://www.github.com/googleapis/java-spanner-jdbc/commit/cfa153997599c36f1243e87f1ea0760694657dfe)) * update core dependencies to v1.27.1 ([googleapis#61](https://www.github.com/googleapis/java-spanner-jdbc/issues/61)) ([181991b](https://www.github.com/googleapis/java-spanner-jdbc/commit/181991bda1f66de707d27dad9658b9177626595a)) * update core dependencies to v1.27.2 ([googleapis#71](https://www.github.com/googleapis/java-spanner-jdbc/issues/71)) ([12425fc](https://www.github.com/googleapis/java-spanner-jdbc/commit/12425fcb4382449e4a7a0edad4c812b7ce15aa71)) * update core dependencies to v1.54.0 ([googleapis#72](https://www.github.com/googleapis/java-spanner-jdbc/issues/72)) ([5676021](https://www.github.com/googleapis/java-spanner-jdbc/commit/567602177e05fa198eaa011fbca05cfe4b72fb13)) * update core dependencies to v1.92.5 ([googleapis#53](https://www.github.com/googleapis/java-spanner-jdbc/issues/53)) ([604ee2b](https://www.github.com/googleapis/java-spanner-jdbc/commit/604ee2b75204ad52eaf724c3fb71e8c13540af7c)) * update core transport dependencies to v1.34.1 ([googleapis#43](https://www.github.com/googleapis/java-spanner-jdbc/issues/43)) ([2b6f04d](https://www.github.com/googleapis/java-spanner-jdbc/commit/2b6f04da3aeebac778fb664c4564fb8b58bf3be4)) * update core transport dependencies to v1.34.2 ([googleapis#62](https://www.github.com/googleapis/java-spanner-jdbc/issues/62)) ([8739015](https://www.github.com/googleapis/java-spanner-jdbc/commit/8739015f62289adb92fd55b19a5bff8762da20a9)) * update dependency com.google.api-client:google-api-client-bom to v1.30.8 ([googleapis#46](https://www.github.com/googleapis/java-spanner-jdbc/issues/46)) ([ef891b0](https://www.github.com/googleapis/java-spanner-jdbc/commit/ef891b000045d1f39f91b6a0ed3abaab19c5f05e)) * update dependency com.google.api-client:google-api-client-bom to v1.30.9 ([googleapis#74](https://www.github.com/googleapis/java-spanner-jdbc/issues/74)) ([3b62299](https://www.github.com/googleapis/java-spanner-jdbc/commit/3b622999b9f9645a6086e5efd3206f4d7b0806bc)) * update dependency com.google.truth:truth to v1.0.1 ([googleapis#32](https://www.github.com/googleapis/java-spanner-jdbc/issues/32)) ([5205863](https://www.github.com/googleapis/java-spanner-jdbc/commit/52058636e10951e883523204f0f161db8a972d62)) * update protobuf.version to v3.11.3 ([googleapis#48](https://www.github.com/googleapis/java-spanner-jdbc/issues/48)) ([0779fcb](https://www.github.com/googleapis/java-spanner-jdbc/commit/0779fcb0bfe935c3c302fa8442f733c7e3629761)) * update protobuf.version to v3.11.4 ([googleapis#64](https://www.github.com/googleapis/java-spanner-jdbc/issues/64)) ([f485cff](https://www.github.com/googleapis/java-spanner-jdbc/commit/f485cfffa0de27ce35f5d16c689c31c6ea22138e)) * update spanner.version to v1.51.0 ([googleapis#75](https://www.github.com/googleapis/java-spanner-jdbc/issues/75)) ([4fff168](https://www.github.com/googleapis/java-spanner-jdbc/commit/4fff168eae61fb55933cf3afd67f24ca65dfde54)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
Fixes #71