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

feat: include User agent #747

Merged
merged 4 commits into from Dec 15, 2020
Merged

Conversation

mohanli-ml
Copy link
Contributor

Configure the UserAgent in addition to x-goog-api-client. This will be needed for DirectPath.

@mohanli-ml mohanli-ml requested a review from a team as a code owner December 15, 2020 03:14
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/java-spanner API. label Dec 15, 2020
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Dec 15, 2020
Comment on lines 384 to 388
options.getSpannerStubSettings().toBuilder()
.setTransportChannelProvider(channelProvider)

Choose a reason for hiding this comment

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

Suggested change
options.getSpannerStubSettings().toBuilder()
.setTransportChannelProvider(channelProvider)
options
.getSpannerStubSettings()
.toBuilder()
options
.getSpannerStubSettings()
.executeSqlSettings()
.getRetrySettings()
.toBuilder()

Comment on lines 423 to 433
options.getInstanceAdminStubSettings().toBuilder()
.setTransportChannelProvider(channelProvider)

Choose a reason for hiding this comment

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

Suggested change
options.getInstanceAdminStubSettings().toBuilder()
.setTransportChannelProvider(channelProvider)
options
.getInstanceAdminStubSettings()
.toBuilder()
options
.getDatabaseAdminStubSettings()
.toBuilder()

options
.getInstanceAdminStubSettings()
.toBuilder()
options.getInstanceAdminStubSettings().toBuilder()

Choose a reason for hiding this comment

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

Suggested change
options.getInstanceAdminStubSettings().toBuilder()
options
.getInstanceAdminStubSettings()
.toBuilder()

@codecov
Copy link

codecov bot commented Dec 15, 2020

Codecov Report

Merging #747 (a075cff) into master (0fd859d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #747   +/-   ##
=========================================
  Coverage     85.05%   85.06%           
  Complexity     2556     2556           
=========================================
  Files           142      142           
  Lines         13930    13938    +8     
  Branches       1326     1326           
=========================================
+ Hits          11848    11856    +8     
  Misses         1526     1526           
  Partials        556      556           
Impacted Files Coverage Δ Complexity Δ
...m/google/cloud/spanner/spi/v1/GapicSpannerRpc.java 83.39% <100.00%> (+0.17%) 81.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0fd859d...a075cff. Read the comment docs.

.setHeaderProvider(mergedHeaderProvider)

// Inject client library version to `user-agent`
.setHeaderProvider(
Copy link
Contributor

Choose a reason for hiding this comment

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

We are overriding the header provider here, instead of adding to it. We have to manually construct a new provider that will encapsulate the existing one the the new one:

private static final String USER_AGENT_KEY = "user-agent";
private static final String CLIENT_LIBRARY_LANGUAGE = "spanner-java";

Map<String, String> headersWithUserAgent = ImmutableMap.<String, String>builder()
    .put(USER_AGENT_KEY, CLIENT_LIBRARY_LANGUAGE + "/" + GaxProperties.getLibraryVersion(GapicSpannerRpc.class)))
    .putAll(mergedHeaderProvider.getHeaders())
    .build();
final HeaderProvider headerProviderWithUserAgent = FixedHeaderProvider.create(headersWithUserAgent);

...
.setHeaderProvider(headerProviderWithUserAgent);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Thanks!

@thiagotnunes thiagotnunes merged commit fc63bc3 into googleapis:master Dec 15, 2020
thiagotnunes pushed a commit that referenced this pull request May 6, 2021
* chore: add DirectPath fallback integration test

* feat: include User agent
rajatbhatta pushed a commit to rajatbhatta/java-spanner that referenced this pull request Nov 17, 2022
🤖 I have created a release *beep* *boop*
---


### Updating meta-information for bleeding-edge SNAPSHOT release.

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
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. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants