fix: add client id to metrics to avoid collisions #117
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
googlebot
added
the
cla: yes
mayurkale22
reviewed
Mar 19, 2020
There was a problem hiding this comment.
I have verified these changes locally and seems to be working fine.
If possible, please include unit test to validate the logic.
- Multiple database clients for the same database w/ different
SessionPoolOptions - Multiple database clients for different databases.
Thanks for verifying it locally 👍 . I've added a test case for the generated client id. |
skuruppu
approved these changes
Mar 20, 2020
skuruppu
added
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
skuruppu
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
|
I don't understand why cla/google is getting stuck. This has happened each time I've run the presubmits. It's run fine on other PRs. |
skuruppu
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
skuruppu
added
kokoro:force-run
googlebot
added
cla: yes
yoshi-kokoro
removed
the
kokoro:force-run
skuruppu
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
gcf-merge-on-green bot
pushed a commit
that referenced
this pull request
Mar 20, 2020
🤖 I have created a release \*beep\* \*boop\* --- ## [1.52.0](https://www.github.com/googleapis/java-spanner/compare/v1.51.0...v1.52.0) (2020-03-20) ### Features * add backup support ([#100](https://www.github.com/googleapis/java-spanner/issues/100)) ([ed3874a](https://www.github.com/googleapis/java-spanner/commit/ed3874afcf55fe7381354e03dab3a3b97d7eb520)) * add Backups protos and APIs ([#97](https://www.github.com/googleapis/java-spanner/issues/97)) ([5643c22](https://www.github.com/googleapis/java-spanner/commit/5643c22a4531dac75b9fac5b128eb714a27920a0)) ### Bug Fixes * add client id to metrics to avoid collisions ([#117](https://www.github.com/googleapis/java-spanner/issues/117)) ([338e136](https://www.github.com/googleapis/java-spanner/commit/338e136508edc6745f9371e8a5d66638021bc8d7)), closes [#106](https://www.github.com/googleapis/java-spanner/issues/106) * ignore added interface methods for generated code ([#101](https://www.github.com/googleapis/java-spanner/issues/101)) ([402cfa1](https://www.github.com/googleapis/java-spanner/commit/402cfa1e1e2994f7bb1b783cf823021b54fb175e)), closes [#99](https://www.github.com/googleapis/java-spanner/issues/99) * use grpc 1.27.2 to prevent version conflicts ([#105](https://www.github.com/googleapis/java-spanner/issues/105)) ([37b7c88](https://www.github.com/googleapis/java-spanner/commit/37b7c8859e5f35d85bd14ef72662614fd185c020)) ### Dependencies * update core dependencies ([#94](https://www.github.com/googleapis/java-spanner/issues/94)) ([f3ca4c9](https://www.github.com/googleapis/java-spanner/commit/f3ca4c99c3d54f64c5eda11e4a4c076140fdbc6a)) * update opencensus.version to v0.26.0 ([#116](https://www.github.com/googleapis/java-spanner/issues/116)) ([1b8db0b](https://www.github.com/googleapis/java-spanner/commit/1b8db0b407429e02bb1e4c9af839afeed21dac5d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
yoshi-automation
added a commit
that referenced
this pull request
Mar 25, 2020
338e136 commit 338e136 Author: Knut Olav Løite <koloite@gmail.com> Date: Fri Mar 20 04:32:04 2020 +0100 fix: add client id to metrics to avoid collisions (#117) This PR adds an automatically generated identifier to all database clients that are created by the client library. This avoids collisions of the same metrics being registered multiple times, and makes it possible to distinguish different clients from each other in the monitoring. This PR does not allow the user to specify the id, but this could be added in a future change. That would need an API change by adding an overload to the method `getDatabaseClient(DatabaseId)` with an additional `clientId` parameter. This should also fix the build error in GoogleCloudPlatform/java-docs-samples#2323. Fixes #106 google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClientImpl.java google-cloud-spanner/src/main/java/com/google/cloud/spanner/MetricRegistryConstants.java google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestWithClosedSessionsEnv.java google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java
yoshi-automation
added a commit
that referenced
this pull request
Mar 25, 2020
1d3c4ac commit 1d3c4ac Author: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Date: Fri Mar 20 04:10:05 2020 +0000 chore: release 1.52.0 (#110) 🤖 I have created a release \*beep\* \*boop\* --- ## [1.52.0](https://www.github.com/googleapis/java-spanner/compare/v1.51.0...v1.52.0) (2020-03-20) ### Features * add backup support ([#100](https://www.github.com/googleapis/java-spanner/issues/100)) ([ed3874a](https://www.github.com/googleapis/java-spanner/commit/ed3874afcf55fe7381354e03dab3a3b97d7eb520)) * add Backups protos and APIs ([#97](https://www.github.com/googleapis/java-spanner/issues/97)) ([5643c22](https://www.github.com/googleapis/java-spanner/commit/5643c22a4531dac75b9fac5b128eb714a27920a0)) ### Bug Fixes * add client id to metrics to avoid collisions ([#117](https://www.github.com/googleapis/java-spanner/issues/117)) ([338e136](https://www.github.com/googleapis/java-spanner/commit/338e136508edc6745f9371e8a5d66638021bc8d7)), closes [#106](https://www.github.com/googleapis/java-spanner/issues/106) * ignore added interface methods for generated code ([#101](https://www.github.com/googleapis/java-spanner/issues/101)) ([402cfa1](https://www.github.com/googleapis/java-spanner/commit/402cfa1e1e2994f7bb1b783cf823021b54fb175e)), closes [#99](https://www.github.com/googleapis/java-spanner/issues/99) * use grpc 1.27.2 to prevent version conflicts ([#105](https://www.github.com/googleapis/java-spanner/issues/105)) ([37b7c88](https://www.github.com/googleapis/java-spanner/commit/37b7c8859e5f35d85bd14ef72662614fd185c020)) ### Dependencies * update core dependencies ([#94](https://www.github.com/googleapis/java-spanner/issues/94)) ([f3ca4c9](https://www.github.com/googleapis/java-spanner/commit/f3ca4c99c3d54f64c5eda11e4a4c076140fdbc6a)) * update opencensus.version to v0.26.0 ([#116](https://www.github.com/googleapis/java-spanner/issues/116)) ([1b8db0b](https://www.github.com/googleapis/java-spanner/commit/1b8db0b407429e02bb1e4c9af839afeed21dac5d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). CHANGELOG.md README.md google-cloud-spanner-bom/pom.xml google-cloud-spanner/pom.xml grpc-google-cloud-spanner-admin-database-v1/pom.xml grpc-google-cloud-spanner-admin-instance-v1/pom.xml grpc-google-cloud-spanner-v1/pom.xml pom.xml proto-google-cloud-spanner-admin-database-v1/pom.xml proto-google-cloud-spanner-admin-instance-v1/pom.xml proto-google-cloud-spanner-v1/pom.xml versions.txt
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds an automatically generated identifier to all database clients that are created by the client library. This avoids collisions of the same metrics being registered multiple times, and makes it possible to distinguish different clients from each other in the monitoring.
This PR does not allow the user to specify the id, but this could be added in a future change. That would need an API change by adding an overload to the method
getDatabaseClient(DatabaseId)with an additionalclientIdparameter.This should also fix the build error in GoogleCloudPlatform/java-docs-samples#2323.
Fixes #106