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: Allow user-agents to be specified by both internal headers and user headers #1190

Merged
merged 2 commits into from Sep 24, 2020

Conversation

igorbernstein2
Copy link
Collaborator

@igorbernstein2 igorbernstein2 commented Sep 23, 2020

This is primarily meant to address the googleapis/java-bigtable#404 (review). Where both the hbase adapter and the core client needs to set the UserAgent. To reconcile the conflict this PR takes grpc's approach and concatenates the values. The reason this needs to happen now is that for DirectPath we need to ship clients that send a UserAgent, however Bigtable needs to keep stats to differentiate client usage.

…user headers

This is primarily meant to address the googleapis/java-bigtable#404 (review). Where both the hbase adapter and the core client needs to set the UserAgent. To reconcile the conflict this PR takes grpc's approach and concatenates the values. The reason this needs to happen now is that for DirectPath we need to ship clients that send a UserAgent, however Bigtable needs to keep stats to differentiate client usage.
@igorbernstein2 igorbernstein2 requested a review from as a code owner Sep 23, 2020
@google-cla google-cla bot added the cla: yes label Sep 23, 2020
@igorbernstein2
Copy link
Collaborator Author

@igorbernstein2 igorbernstein2 commented Sep 23, 2020

@codecov
Copy link

@codecov codecov bot commented Sep 23, 2020

Codecov Report

Merging #1190 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1190   +/-   ##
=========================================
  Coverage     58.91%   58.91%           
  Complexity      115      115           
=========================================
  Files            20       20           
  Lines           589      589           
  Branches         60       60           
=========================================
  Hits            347      347           
  Misses          213      213           
  Partials         29       29           

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 287cada...429a580. Read the comment docs.

elharo
elharo approved these changes Sep 23, 2020
@igorbernstein2 igorbernstein2 added kokoro:force-run kokoro:run labels Sep 23, 2020
@igorbernstein2
Copy link
Collaborator Author

@igorbernstein2 igorbernstein2 commented Sep 23, 2020

I think kokoro is broken?

@WeiranFang
Copy link
Contributor

@WeiranFang WeiranFang commented Sep 23, 2020

Thanks Igor for making this change!

@igorbernstein2 igorbernstein2 merged commit 266329e into googleapis:master Sep 24, 2020
8 checks passed
@igorbernstein2 igorbernstein2 deleted the user-agent-merge branch Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes kokoro:force-run kokoro:run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants