To support java17 runtime, substitute wrapper handler for original in exec command line #944
Conversation
… exec command line
|
I notice that the CI didn't run any tests? I thought of switching the runtime of one of the java tests to |
|
(I don't think the java project here has any unit tests... Open to suggestions or contributions for how we might add useful tests without having AWS infrastructure to test against?) |
We (Splunk) test against real AWS infra; because of stuff like this (PR) I think it'd be best to focus on similar for upstream otel-lamba as well. I realize that's a major task, what with figuring out funding/credentials management/etc. but I think it would be a huge win for this component. Mocking things out just wouldn't uncover these kinds of issues. Regardless, it appears that's outside the scope for now. Please merge when ready. |
| exec "$@" | ||
| # java17 runtime puts the handler to run as a command line argument and seems to prefer | ||
| # this over _HANDLER, so apply a regex to replace the original handler name with our wrapper | ||
| exec "${@//$OTEL_INSTRUMENTATION_AWS_LAMBDA_HANDLER/$_HANDLER}" |
There was a problem hiding this comment.
@johnbley did you confirm this doesn't break anything with Java 11? I assume it doesn't but just want to be sure.
(I think it would still help to include an example string before/after in the PR description on both Java 11 and Java 17 -- unless they're the same.)
There was a problem hiding this comment.
java17:
sun.java.command=com.amazonaws.services.lambda.runtime.api.client.AWSLambda example.Hello::handleRequest
java11:
sun.java.command=lambdainternal.AWSLambda
And, yes, I verified with our internal tests that this regex works for both 11 and 17.
There was a problem hiding this comment.
For the java17 case you can see in their code that they clearly ignore _HANDLER and expect a single argument to be that handler string: https://github.com/aws/aws-lambda-java-libs/blob/90155b3db0f59f93d442eb533e735e9e0041b552/aws-lambda-java-runtime-interface-client/src/main/java/com/amazonaws/services/lambda/runtime/api/client/AWSLambda.java#L187
|
Ping! This has been here a while... |
|
sorry for the limbo, I just pinged @open-telemetry/technical-committee to request merging #968 and updating permissions |
The new java17 runtime appends the configured handler to the command line of the launched java process as an additional app-level argument, and appears to prefer that over the
_HANDLERenvironment variable. This means that our wrapper approach for Java instrumentation stops working since it overrides that variable. I've fixed this by applying some (gnarly looking) bash string substitution on the command line (preserving spaces, argument order, etc.) to swap out this extra argument, and verified that our wrapper gets called again.