Skip to content
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

Jump to bottom

feat(spanner): allow custom lib name and version for telemetry purpose #4762

Merged
merged 4 commits into from Feb 13, 2020
Merged

feat(spanner): allow custom lib name and version for telemetry purpose #4762

merged 4 commits into from Feb 13, 2020

Conversation

jiren
Copy link
Member

@jiren jiren commented Feb 3, 2020

Towards: #4761

Allow user-agent to be passed in and not a hardcoded version. This will helpful for database connector i.e ActiveRecord connector.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 3, 2020
@jiren jiren requested a review from quartzmo February 3, 2020 03:54
@jiren jiren added the api: spanner Issues related to the Spanner API. label Feb 3, 2020
@jiren jiren self-assigned this Feb 3, 2020
@skuruppu skuruppu self-requested a review February 5, 2020 01:14
skuruppu
skuruppu reviewed Feb 5, 2020
View reviewed changes
Copy link
Contributor

@skuruppu skuruppu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable. But I wonder if the user agent should also contain which version of the client lib is being used by the caller. E.g. should it be:

x-goog-api-client':
'spanner-activerecord/0.0.1 google-cloud-spanner/1.13.1 gl-ruby/ grpc/<gRPC_VERSION> gax/<GAX_VERSION>'

Also, minor nit, please reformat the doc strings added in this PR as the line-wrapping seems a bit off.

google-cloud-spanner/lib/google-cloud-spanner.rb Outdated Show resolved Hide resolved
google-cloud-spanner/lib/google/cloud/spanner.rb Outdated Show resolved Hide resolved
skuruppu
skuruppu suggested changes Feb 6, 2020
View reviewed changes
Copy link
Contributor

@skuruppu skuruppu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doc strings added here still needs to be reformatted.

@jiren
Copy link
Member Author

jiren commented Feb 6, 2020

This seems reasonable. But I wonder if the user agent should also contain which version of the client lib is being used by the caller. E.g. should it be:

x-goog-api-client':
'spanner-activerecord/0.0.1 google-cloud-spanner/1.13.1 gl-ruby/ grpc/<gRPC_VERSION> gax/<GAX_VERSION>'

Also, minor nit, please reformat the doc strings added in this PR as the line-wrapping seems a bit off.

@skuruppu Updated code and added a prefix to lib name while creating underline gapic client instance.

skuruppu
skuruppu approved these changes Feb 7, 2020
View reviewed changes
Copy link
Contributor

@skuruppu skuruppu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jiren for updating the PR. LGTM.

@odeke-em
Copy link

Thank you @skuruppu for the review! @jiren please rebase from master as there is a merge conflict, but also @skuruppu gave you a LGTM.

@jiren jiren force-pushed the spanner_allow_user_agent_4761 branch from 03a0bee to 7fb2800 Compare February 12, 2020 08:51
@skuruppu skuruppu merged commit ade2581 into googleapis:master Feb 13, 2020
This was referenced Feb 13, 2020
Merged
Merged
Merged
Merged
quartzmo pushed a commit that referenced this pull request Feb 18, 2020
@yoshi-automation
Release google-cloud-spanner 1.14.0 (#4814)
578b5cf 
#### Features

* allow custom lib name and version for telemetry purpose ([#4762](https://www.github.com/googleapis/google-cloud-ruby/issues/4762))
  * allow custom lib name and version for telemetry purpose
  * format docs
  * added custom lib name and version prefix
  * update test cases for lib name and version
@skuruppu skuruppu mentioned this pull request Feb 20, 2020
Closed
This was referenced Feb 20, 2020
Merged
Closed
Merged
Merged
Closed
Merged
Merged
Merged
This was referenced Feb 21, 2020
Merged
Merged
Merged
Merged
Merged
Merged
Closed
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skuruppu skuruppu skuruppu approved these changes

@quartzmo quartzmo Awaiting requested review from quartzmo

Assignees

@jiren jiren

Labels
api: spanner Issues related to the Spanner API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jiren @odeke-em @skuruppu @googlebot