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

feat: include User agent #747

merged 4 commits into from Dec 15, 2020

Conversation

mohanli-ml
Copy link
Contributor

@mohanli-ml mohanli-ml commented Dec 15, 2020

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

@mohanli-ml mohanli-ml requested a review from as a code owner Dec 15, 2020
@product-auto-label product-auto-label bot added the api: spanner label Dec 15, 2020
@google-cla google-cla bot added the cla: yes label Dec 15, 2020
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()

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 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

@thiagotnunes thiagotnunes Dec 15, 2020

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

@mohanli-ml mohanli-ml Dec 15, 2020

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
21 checks passed
thiagotnunes pushed a commit that referenced this issue May 6, 2021
* chore: add DirectPath fallback integration test

* feat: include User agent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants