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.

Sign up for GitHub

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(gax): add protobuf version tracking to headers #3199

Merged
merged 28 commits into from
Oct 15, 2024
Merged

feat(gax): add protobuf version tracking to headers #3199

merged 28 commits into from
Oct 15, 2024

Conversation

ldetmer
Copy link
Contributor

@ldetmer ldetmer commented Sep 13, 2024 •
edited
Loading

Update the Java client libraries to report the runtime version of Protobuf as part of the existing x-goog-api-client request header.

Tested: java-cloud-library api (billing) and hand written api (storage)

@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Sep 13, 2024
@ldetmer ldetmer changed the title Protobuf metric feat(gax): add protobuf version to headers to track Sep 13, 2024
@ldetmer ldetmer changed the title feat(gax): add protobuf version to headers to track feat(gax): add protobuf version tracking to headers Sep 16, 2024
@ldetmer ldetmer marked this pull request as ready for review September 16, 2024 13:39
@burkedavison
Copy link
Contributor

LGTM w/ addition of Showcase tests showing gRPC + httpjson clients are both providing the new headers.

Looking for approval from @blakeli0

burkedavison
burkedavison reviewed Sep 18, 2024
View reviewed changes
showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITVersionHeaders.java Outdated
Pattern.compile("gl-java/.+ gapic/.+--protobuf-.+ gax/.+ rest/ protobuf/.*");
httpJsonClient.echo(EchoRequest.newBuilder().build());
ArrayList headerValues =
(ArrayList) httpJsonInterceptor.metadata.getHeaders().get(HTTP_CLIENT_API_HEADER_KEY);
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than using raw-types, use ArrayList<?> if you don't know the type. In this case, we're casting to String in the next line so we might as well cast to ArrayList<String>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@burkedavison
Copy link
Contributor

Thank you for the showcase tests - LGTM.

@blakeli0 blakeli0 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 19, 2024
@blakeli0
Copy link
Collaborator

Adding Do Not Merge for now since we plan to release this together with the protobuf runtime upgrade.

@ldetmer
Copy link
Contributor Author

ldetmer commented Sep 20, 2024

Adding Do Not Merge for now since we plan to release this together with the protobuf runtime upgrade.

Understood about DNM, but is this verbally LGTM?

@blakeli0 blakeli0 removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 15, 2024
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Oct 15, 2024

Quality Gate Passed Quality Gate passed for 'gapic-generator-java-root'

Issues
10 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Oct 15, 2024

Quality Gate Passed Quality Gate passed for 'java_showcase_integration_tests'

Issues
10 New issues
0 Accepted issues

Measures
0 Security Hotspots
84.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@blakeli0 blakeli0 merged commit 40c19b1 into main Oct 15, 2024
49 of 50 checks passed
@blakeli0 blakeli0 deleted the protobuf-metric branch October 15, 2024 22:03
@release-please release-please bot mentioned this pull request Oct 16, 2024
Merged
mpeddada1 pushed a commit that referenced this pull request Oct 22, 2024
@release-please
chore(main): release 2.48.0 (#3295)
d5e74d9 
🤖 I have created a release *beep* *boop*
---


<details><summary>2.48.0</summary>

##
[2.48.0](v2.47.0...v2.48.0)
(2024-10-22)


### Features

* **gax:** add protobuf version tracking to headers
([#3199](#3199))
([40c19b1](40c19b1))
* selectively generate libraries
([#3290](#3290))
([dfe1a50](dfe1a50))


### Bug Fixes

* generator setting incorrect name/class for sample due to region tag
(2nd attempt)
([#3293](#3293))
([771bd0e](771bd0e))


### Dependencies

* update dependency com.google.errorprone:error_prone_annotations to
v2.34.0
([#3303](#3303))
([5b01274](5b01274))
* update dependency com.google.errorprone:error_prone_annotations to
v2.34.0
([#3304](#3304))
([5bd6c9c](5bd6c9c))
* update google api dependencies
([#3282](#3282))
([a9eac85](a9eac85))
* update google auth library dependencies to v1.29.0
([#3302](#3302))
([e64eda2](e64eda2))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blakeli0 blakeli0 blakeli0 approved these changes

@burkedavison burkedavison burkedavison approved these changes

Assignees
No one assigned
Labels
size: m Pull request size is medium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ldetmer @burkedavison @blakeli0