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

Java wrapper swallows S3 event data #640

Closed
janfreymann opened this issue Apr 18, 2023 · 17 comments · Fixed by open-telemetry/opentelemetry-java-instrumentation#11868
Closed
Assignees
Labels
bug Something isn't working

Comments

@janfreymann
Copy link

Describe the bug
We use the Java wrapper with opentelemetry-lambda-layer.zip and the opt/otel-handler. The lambda is triggered via AWS event bridge whenever a new object is created in an S3 bucket. The event is processed via a Java class implementing RequestHandler<S3Event, String> .
When we add the opentelemetry layer, the event is gone, i.e. cannot be parsed.

Steps to reproduce

  • create Lambda with RequestHandler<S3Event, String>
  • create S3 bucket
  • create AWS EventBridge rule to trigger Lambda upon object creation
  • verify that S3 event contains data about new object
  • add opentelemetrey-lambda-layer.zip and configure with opt/otel-handler
  • S3 event is empty

What did you expect to see?
The S3 event should be visible to the Lambda's RequestHandler, there should be at least one record inside the S3 event.

What did you see instead?
The event was present, but did not contain any records.

What version of collector/language SDK version did you use?

  • latest version of opentelemetry-lambda: a2fa39178...
  • com.amazonaws:aws-lambda-java-core:1.2.2.
  • com.amazonaw:aws-lambda-java-events:3.11.0

What language layer did you use?
Java

@janfreymann janfreymann added the bug Something isn't working label Apr 18, 2023
@balajimaniv
Copy link

Is there any alternate workaround for this until this bug get fixed and released. ?

@hello-dotcom
Copy link

I have the same issue, i see the lambda is triggered when an object is inserted into s3 bucket, but there are no records in the s3Event. Is there any alternative?

@utr1903
Copy link

utr1903 commented Aug 3, 2023

I have the same issue as well... When I remove the wrapper (/opt/otel-handler), I can get the S3Event. However with the wrapper, I get an empty event object...

@PaurushGarg
Copy link
Member

Was able to replicate the issue for Java 11 runtime, however this issue is not present with Java 17 runtime.
For Java 11 and Java 8 (AL2) runtime, this issue require further triaging.

@Kausik-A
Copy link
Contributor

Bumping the thread so that I can be assigned to this issue.

@serkan-ozal
Copy link
Contributor

@janfreymann @Kausik-A @Aneurysm9

The reason is that deserialization of S3 event needs custom logic and therefore AWS Lambda runtime has its own specific deserializer for the S3 event: https://github.com/aws/aws-lambda-java-libs/blob/main/aws-lambda-java-serialization/src/main/java/com/amazonaws/services/lambda/runtime/serialization/events/serializers/S3EventSerializer.java

IMO, for a proper solution to this issue, aws-lambda-java-serialization needs to be added as 3rd party dependency (also needs to be relocated and shaded) and then com.amazonaws.services.lambda.runtime.serialization.events.LambdaEventSerializers can be used to handle the event deserialization logic.

If no one is working on this, I am happy to take this issue. Or can help the one who is already working on this.

@lacerz-cc
Copy link

Any update for this? I'm having the same issue right now, is the only solution to completely remove the otel layer?

@rapphil rapphil self-assigned this Jul 16, 2024
@rapphil
Copy link
Contributor

rapphil commented Jul 16, 2024

I think @serkan-ozal got the issue sorted out, specially based on comment comment in this code https://github.com/aws/aws-lambda-java-libs/blob/main/aws-lambda-java-serialization/src/main/java/com/amazonaws/services/lambda/runtime/serialization/events/serializers/S3EventSerializer.java#L22

@serkan-ozal do you still want to work on this? sorry for the late response. If not, I will take a look.

@rapphil
Copy link
Contributor

rapphil commented Jul 16, 2024

I wonder if we really need to add aws-lambda-java-serialization as dependency since this seems to be already part of the runtime environment. I think this can be added as a compile only dependency and in runtime it will use the one from the environment.

@serkan-ozal
Copy link
Contributor

Hi @rapphil, Yes, I can work on this if you have not started working yet.

About the adding aws-lambda-java-serialization as dependency, you might be right, it might be available in the classpath in AWS Lambda environment, but not sure. Need to check it first. But even though it is available, it might not be visible to user classloader. Anyway, I need to check this.

@serkan-ozal
Copy link
Contributor

serkan-ozal commented Jul 17, 2024

@rapphil, As far as I have checked, it is not available in the AWS Lambda runtime environment. Here are the available ones:

Screenshot 2024-07-17 at 17 44 08

So, seems that, we need to include aws-lambda-java-serialization as dependency. Is there any idea to use it without having it as a dependency in the bundle?

@rapphil
Copy link
Contributor

rapphil commented Jul 18, 2024

I believe the code from the serialization library is available by default in the runtime of the lambda. The same runtime code is responsible by calling the lambda handler.

If you follow through this https://github.com/aws/aws-lambda-java-libs/blob/258b00a546631a9b91cbb1b8c126c7c00a88354c/aws-lambda-java-runtime-interface-client/src/main/java/com/amazonaws/services/lambda/runtime/api/client/AWSLambda.java you are going to see how handlers are invoked and how the events are serialized and etc. This code is the one that is used for the case that you want to build a lambda from a container image. For the case of regular lambdas, I think it will be in the classpath by default.

I have run a small experiment that poke into a class that is present in the aws-lambda-java-serialization library. I added this dependency as compile only in the project and then in the code of the lambda I did:

import com.amazonaws.services.lambda.runtime.serialization.events.LambdaEventSerializers;
.....


      logger.log("Event supported: " + String.valueOf(LambdaEventSerializers.isLambdaSupportedEvent("com.amazonaws.services.lambda.runtime.events.S3Event")));

Then I got as output in cw logs:

Event supported: true

I think we can try to run a small experiment to check if it is possible to use the serializers from LambdaEventSerializers to serialize all objects.

@serkan-ozal
Copy link
Contributor

Hmm, that is interesting. I think, I got the jar files from Java 8 runtime so aws-lambda-java-serialization is not in the classpath, but it is available in the newer versions. I will check once I have time to look into deeper

@serkan-ozal
Copy link
Contributor

@rapphil

I have verified that aws-lambda-java-serialization is available in the java8.al2, java11, java17 and java21 runtimes, but not in the java8 runtime. So I didn't see LambdaEventSerializers class in my example, because it was using java8 runtime.

However, java8 runtime is deprecated and no new functions can be created with java8 runtime anymore, but existing ones can be updated to java8 until Feb 28, 2025: https://docs.aws.amazon.com/lambda/latest/dg/lambda-runtimes.html#runtimes-deprecated

So, seems that we don't need to put aws-lambda-java-serialization dependency into the bundle, but having it as a compile time dependency.

Additionally, not sure we are supporting java8 runtime currently or we should, but in any case, for the deserialization logic, we might catch the NoClassDefFoundError for the LambdaEventSerializers class and if it happens, fall back to the current deserialization logic (maybe with a one time warning log).

@rapphil
Copy link
Contributor

rapphil commented Jul 18, 2024

Nice findings!!!

we do currently support java8, but I wonder if should continue given that no new functions can be created with java8.

https://github.com/open-telemetry/opentelemetry-lambda/blob/main/.github/workflows/release-layer-java.yml#L115

I think we can remove support to the java8 runtime in the next release anyways since it is deprecated and there is an alternative to use java8.al2, which should work for the majority of the cases: https://aws.amazon.com/blogs/compute/announcing-migration-of-the-java-8-runtime-in-aws-lambda-to-amazon-corretto/

@serkan-ozal
Copy link
Contributor

@rapphil If no one is currently working on this, I can take this one. If so, could you assign this to me?

@serkan-ozal
Copy link
Contributor

@rapphil @tylerbenson Have just sent this PR: open-telemetry/opentelemetry-java-instrumentation#11868
PR is still WIP, going to add more tests, but I believe implementation is completed and ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
9 participants