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(generator): add "generator" to API client header "gccl" semver #7383

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

devbww
Copy link
Contributor

@devbww devbww commented Sep 30, 2021

  1. Strip the leading "v" from the value of the "gccl" component in the
    X-Goog-Api-Client header so that what remains is a valid semantic
    version string (see https://semver.org/).

  2. Add a "generator" build identifier to the "gccl" semver for requests
    sent from generated clients.

The upshot is that the library-version component of the client header
will be gccl/<major>.<minor>.<patch>+<git-hash>.generator, allowing
us to identify requests from generated clients.


This change is Reviewable

1. Strip the leading "v" from the value of the "gccl" component in the
   `X-Goog-Api-Client` header so that what remains is a valid semantic
   version string (see https://semver.org/).

2. Add a "generator" build identifier to the "gccl" semvar for requests
   sent from generated clients.

The upshot is that the library-version component of the client header
will be "gccl/<major>.<minor>.<patch>+<git-hash>.generator", allowing
us to identify requests from generated clients.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 30, 2021
@google-cloud-cpp-bot
Copy link
Collaborator

Google Cloud Build Logs
For commit: 48da85504e5bf1dabf3acbb39ad0edd3801a0676

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

@codecov
Copy link

codecov bot commented Sep 30, 2021

Codecov Report

Merging #7383 (48da855) into main (92e742f) will increase coverage by 0.00%.
The diff coverage is 97.14%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #7383   +/-   ##
=======================================
  Coverage   93.64%   93.65%           
=======================================
  Files        1351     1351           
  Lines      117315   117334   +19     
=======================================
+ Hits       109865   109885   +20     
+ Misses       7450     7449    -1     
Impacted Files Coverage Δ
generator/internal/metadata_decorator_generator.cc 100.00% <ø> (ø)
google/cloud/internal/api_client_header.cc 93.75% <91.66%> (-6.25%) ⬇️
...internal/golden_kitchen_sink_metadata_decorator.cc 100.00% <100.00%> (ø)
.../internal/golden_thing_admin_metadata_decorator.cc 100.00% <100.00%> (ø)
...sts/golden_kitchen_sink_metadata_decorator_test.cc 100.00% <100.00%> (ø)
...ests/golden_thing_admin_metadata_decorator_test.cc 100.00% <100.00%> (ø)
...query/internal/bigquery_read_metadata_decorator.cc 100.00% <100.00%> (ø)
...iam/internal/iam_credentials_metadata_decorator.cc 100.00% <100.00%> (ø)
...oogle/cloud/iam/internal/iam_metadata_decorator.cc 91.95% <100.00%> (+0.09%) ⬆️
google/cloud/internal/api_client_header_test.cc 100.00% <100.00%> (ø)
... and 8 more

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 92e742f...48da855. Read the comment docs.

@devbww devbww marked this pull request as ready for review September 30, 2021 22:25
@devbww devbww requested a review from a team as a code owner September 30, 2021 22:25
@coryan coryan changed the title feat(generator): add "generator" to API client header "gccl" semvar feat(generator): add "generator" to API client header "gccl" semver Sep 30, 2021
Copy link
Contributor

@coryan coryan left a comment

Choose a reason for hiding this comment

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

Make sure you edit the commit message to s/semvar/semver/

@devbww devbww merged commit 39d87d4 into googleapis:main Oct 1, 2021
@devbww devbww deleted the generated-api-client-header branch October 1, 2021 01:12
devbww added a commit to devbww/google-cloud-cpp that referenced this pull request Oct 15, 2021
Resume sending the  leading "v" in the value of the "gccl" component
of the `X-Goog-Api-Client` header (see googleapis#7383).  The Cloud Spanner
frontend's route lookup service uses the version string in a semver
comparison to determine how the request should be routed, and it
currently expects to see a literal "v".  (For the record, the
minimal version supported by the route lookup service is "v1.17.1".)

Changing the route lookup service to not care about the leading "v"
is non-trivial (it uses a single-value map, indexed by language, to
store the minimum supported version, and the value comes from a flag),
and even then the fix will take some time to roll out, so, in the
meantime, we revert to sending the "v".

Eventually eliminating the "v" is still a goal because it makes the
header conform to the specification, and it also makes C++ behave
like the other languages.
devbww added a commit that referenced this pull request Oct 15, 2021
)

Resume sending the  leading "v" in the value of the "gccl" component
of the `X-Goog-Api-Client` header (see #7383).  The Cloud Spanner
frontend's route lookup service uses the version string in a semver
comparison to determine how the request should be routed, and it
currently expects to see a literal "v".  (For the record, the
minimal version supported by the route lookup service is "v1.17.1".)

Changing the route lookup service to not care about the leading "v"
is non-trivial (it uses a single-value map, indexed by language, to
store the minimum supported version, and the value comes from a flag),
and even then the fix will take some time to roll out, so, in the
meantime, we revert to sending the "v".

Eventually eliminating the "v" is still a goal because it makes the
header conform to the specification, and it also makes C++ behave
like the other languages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants