Skip to content

feat(v2): GraalVM support for parameters module #1824

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

Open
wants to merge 3 commits into
base: v2
Choose a base branch
from

Conversation

jreijn
Copy link
Contributor

@jreijn jreijn commented May 2, 2025

Issue #, if available: #1833

Description of changes:

Introduced GraalVM metadata for all submodules of the parameters module:

  • powertools-parameters
  • powertools-parameters-ssm
  • powertools-parameters-secrets
  • powertools-parameters-appconfig
  • powertools-parameters-dynamodb

Checklist

Breaking change checklist

RFC issue #:

  • Migration process documented
  • Implement warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jreijn
Copy link
Contributor Author

jreijn commented May 2, 2025

I tried to follow the guide as mentioned by https://github.com/aws-powertools/powertools-lambda-java/blob/v2/GraalVM.md which helped a lot with generating the meta data.

However I do wonder if we need all the metadata generated by the GraalVM agent. In on of my projects where I used v1 of Powertools I only needed the following snippet to get the SecretsProvider to work:

[
  {
    "name" : "software.amazon.lambda.powertools.parameters.SecretsProvider",
    "allDeclaredConstructors" : true,
    "allPublicConstructors" : true,
    "allDeclaredMethods" : true,
    "allPublicMethods" : true,
    "allDeclaredFields" : true,
    "allPublicFields" : true
  }
]

Now we have all sorts of java internal classes or classes coming from other libraries or the JVM, which I don't think we need? Shouldn't they be handled by the corresponding projects? I'm by no means a GraalVM expert, but I always understood it in such a way that metadata in a provided library only contains metadata about its own classes and resources which otherwise can't be reached.

@phipag phipag self-requested a review May 5, 2025 09:47
@phipag phipag assigned phipag and jreijn and unassigned phipag May 5, 2025
@phipag phipag moved this to Pending review in Powertools for AWS Lambda (Java) May 5, 2025
@phipag phipag linked an issue May 5, 2025 that may be closed by this pull request
7 tasks
@phipag phipag linked an issue May 8, 2025 that may be closed by this pull request
@phipag phipag linked an issue May 8, 2025 that may be closed by this pull request
@phipag
Copy link
Contributor

phipag commented May 8, 2025

Hi @jreijn,

I just review your changes and you did an amazing job 🚀 – especially cleaning up the test scoped dependencies in the generated metadata files. This helped us a lot.

I just verified that unit tests against the native image pass, which they do ✅

Before we go ahead merging this PR, I would love to do two things:

  1. Update the roadmap documentation page with the sub-issues and the progress made in the parameters module thanks to you (I can take care of this).
  2. Test this at runtime using the Parameters example we have and update the example to also include GraalVM -> https://github.com/aws-powertools/powertools-lambda-java/tree/v2/examples/powertools-examples-parameters
    • For this I would take this graalvm example as inspiration and provide the user with a template-native.yaml using provided.al2023 as runtime for the Lambda.

I am happy to also update the example and don't want to cause more work on your end. But let me know if you like to do it.


Regarding your comment:

Now we have all sorts of java internal classes or classes coming from other libraries or the JVM, which I don't think we need? Shouldn't they be handled by the corresponding projects? I'm by no means a GraalVM expert, but I always understood it in such a way that metadata in a provided library only contains metadata about its own classes and resources which otherwise can't be reached.

You are absolutely right here. Since we run the metadata generation via unit tests (which is uncommon) there is a risk of including "too much" metadata and this is something we can optimize in the future. This is also why the manual cleanup is needed. The unit tests are good candidate for now since they have pretty good coverage.

One way to address this which I tested was including a filter on the GraalVM tracing agent to only generate metadata for the library itself and not dependencies. However, this led to two issues:

  1. It requires customers to include metadata files for transitive dependencies that do not support GraalVM. For example, for X-RAY in the case of the tracing utility. We felt that this is a bad customer experience because the customer did not explicitly choose to include that transitive dependency.
  2. In some edge cases when we access something reflectively ourselves that is outside of the filter it would not be included. Hence, a lot of manual fine-tuning on the filter is necessary which we didn't do (yet).

Having said that, there is a lot of potential on improving this and before going to production with v2 we will do some improvements (e.g. running integ tests for both JVM and GraalVM native images #1805).

It seems like in the parameters example that you also tested in v1 using GraalVM, we can definitely reduce the generated metadata files. For now, I would like to be consistent with the other modules and improve this consistently in the future. What are your thoughts on this? Happy to have a discussion, also in a short meeting if you like.

@jreijn
Copy link
Contributor Author

jreijn commented May 8, 2025

@phipag thanks for the feedback!

With regards to the two open items:

  1. it would be great if you can do that

  2. I will pick that up and update the example. I'll see if I can find some time over the next couple of days.

With regard to the reflection metadata I see your point. I'm fine with leaving it for now and improving it later. Once we get to that point I would love to brainstorm a bit about it and see how I can contribute.

@phipag
Copy link
Contributor

phipag commented May 9, 2025

Thanks @jreijn, this sounds like a good plan. Awesome that you would like to update the example as well! I'll be waiting for your update here and try to review it promptly.

Regarding the roadmap update, I will take care of this as part of a separate PR since I am already working on a different documentation update.


Next steps: Wait for powertools-examples-parameters update and review it to close this PR.

Copy link

sonarqubecloud bot commented May 9, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Pending review
Development

Successfully merging this pull request may close these issues.

feat(v2): GraalVM support for Parameters utility
2 participants