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

fix: add reflection configurations for com.google.rpc classes #2617

Merged
merged 3 commits into from
Sep 12, 2023

Conversation

mpeddada1
Copy link
Contributor

@mpeddada1 mpeddada1 commented Sep 10, 2023

For test failures in #2615

This PR adds native image metadata for the classes that are being invoked reflectively. The call to ProtoUtils.keyForProto in SpannerInterceptorProvider calls GeneratedMessageV3#getMethodOrDie down the line which then makes use of reflection to invoke methods in the google.rpc classes.

https://github.com/protocolbuffers/protobuf/blob/2bea49218587ddb9cb2f76b75618ba70b1e11fa5/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java#L1999-L2008

Stacktrace from IntelliJ debugger:
Screenshot 2023-09-11 at 5 42 48 PM

@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: spanner Issues related to the googleapis/java-spanner API. labels Sep 10, 2023
@mpeddada1 mpeddada1 marked this pull request as ready for review September 11, 2023 14:40
@mpeddada1 mpeddada1 requested review from a team as code owners September 11, 2023 14:40
@blakeli0
Copy link
Contributor

For future improvement, if we don't worry too much about extra reflection configs, we can generate a common json for every common proto message in java-common-protos. Otherwise we may have to add these files manually for every handwritten library as we don't know what exactly common proto messages they are using.

Comment on lines +1 to +9
[ {
"name": "com.google.rpc.LocalizedMessage",
"queryAllDeclaredConstructors": true,
"queryAllPublicConstructors": true,
"queryAllDeclaredMethods": true,
"allPublicMethods": true,
"allDeclaredClasses": true,
"allPublicClasses": true
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: fix indentation

Copy link
Collaborator

Choose a reason for hiding this comment

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

(Let's do this in a separate PR, and merge this now, so we can proceed with the releases)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied change in follow-up PR #2621. Thanks.

@olavloite olavloite added the owlbot:run Add this label to trigger the Owlbot post processor. label Sep 12, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Sep 12, 2023
@olavloite olavloite merged commit c42460a into main Sep 12, 2023
24 checks passed
@olavloite olavloite deleted the reachability-fix branch September 12, 2023 08:54
gcf-merge-on-green bot pushed a commit that referenced this pull request Sep 12, 2023
🤖 I have created a release *beep* *boop*
---


## [6.47.0](https://togithub.com/googleapis/java-spanner/compare/v6.46.0...v6.47.0) (2023-09-12)


### Features

* Add devcontainers for enabling github codespaces usage. ([#2605](https://togithub.com/googleapis/java-spanner/issues/2605)) ([a7d60f1](https://togithub.com/googleapis/java-spanner/commit/a7d60f13781f87054a1631ca511492c5c8334751))
* Disable dynamic code loading properties by default ([#2606](https://togithub.com/googleapis/java-spanner/issues/2606)) ([d855ebb](https://togithub.com/googleapis/java-spanner/commit/d855ebbd2dec11cdd6cdbe326de81115632598cd))


### Bug Fixes

* Add reflection configurations for com.google.rpc classes ([#2617](https://togithub.com/googleapis/java-spanner/issues/2617)) ([c42460a](https://togithub.com/googleapis/java-spanner/commit/c42460ae7b6bb5874cc18c7aecff34186dcbff2a))
* Avoid unbalanced session pool creation ([#2442](https://togithub.com/googleapis/java-spanner/issues/2442)) ([db751ce](https://togithub.com/googleapis/java-spanner/commit/db751ceebc8b6981d00cd07ce4742196cc1dd50d))


### Dependencies

* Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.15.0 ([#2615](https://togithub.com/googleapis/java-spanner/issues/2615)) ([ac762fb](https://togithub.com/googleapis/java-spanner/commit/ac762fbf079db79eab5f2ebee971b850ac89eb11))

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
@mpeddada1
Copy link
Contributor Author

For future improvement, if we don't worry too much about extra reflection configs, we can generate a common json for every common proto message in java-common-protos. Otherwise we may have to add these files manually for every handwritten library as we don't know what exactly common proto messages they are using.

Agreed! This is something that we should we consider + discuss further. Additionally, we rely quite a bit tests to detect native image incompatibilities so if a handwritten library adds new code that explicitly uses com.google.rpc classes but doesn't test for that code path (or uses canary tests) then we may risk pushing incompatible code.

surbhigarg92 pushed a commit to surbhigarg92/java-spanner that referenced this pull request Oct 5, 2023
…apis#2617)

* fix: add native image configurations for com.google.rpc classes

* relocate config to separate reflect-config.json files

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
surbhigarg92 pushed a commit to surbhigarg92/java-spanner that referenced this pull request Oct 5, 2023
🤖 I have created a release *beep* *boop*
---


## [6.47.0](https://togithub.com/googleapis/java-spanner/compare/v6.46.0...v6.47.0) (2023-09-12)


### Features

* Add devcontainers for enabling github codespaces usage. ([googleapis#2605](https://togithub.com/googleapis/java-spanner/issues/2605)) ([a7d60f1](https://togithub.com/googleapis/java-spanner/commit/a7d60f13781f87054a1631ca511492c5c8334751))
* Disable dynamic code loading properties by default ([googleapis#2606](https://togithub.com/googleapis/java-spanner/issues/2606)) ([d855ebb](https://togithub.com/googleapis/java-spanner/commit/d855ebbd2dec11cdd6cdbe326de81115632598cd))


### Bug Fixes

* Add reflection configurations for com.google.rpc classes ([googleapis#2617](https://togithub.com/googleapis/java-spanner/issues/2617)) ([c42460a](https://togithub.com/googleapis/java-spanner/commit/c42460ae7b6bb5874cc18c7aecff34186dcbff2a))
* Avoid unbalanced session pool creation ([googleapis#2442](https://togithub.com/googleapis/java-spanner/issues/2442)) ([db751ce](https://togithub.com/googleapis/java-spanner/commit/db751ceebc8b6981d00cd07ce4742196cc1dd50d))


### Dependencies

* Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.15.0 ([googleapis#2615](https://togithub.com/googleapis/java-spanner/issues/2615)) ([ac762fb](https://togithub.com/googleapis/java-spanner/commit/ac762fbf079db79eab5f2ebee971b850ac89eb11))

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.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. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants