newrelic / java-aws-lambda

AWS Lambda OpenTracing Java SDK
Apache License 2.0
11 stars 6 forks source link

Address type erasure/reflection issue #36

Closed MattWhelan closed 4 years ago

MattWhelan commented 4 years ago

This is a pretty broad rewrite, because the former approach cannot work. TracingRequestHandler's type parameters erase to Object at runtime, which defeats AWS's reflection-based automatic deserialization mechanism.

The new approach uses composition over inheritance: the user provides their own event handler, using any mechanism AWS supports, and wraps their business logic with our instrumentation. This allows reflection to work as intended.

In addition, the TracingRequestStreamHandler class's doHandler method didn't allow for IOException to be thrown, despite it being more or less inevitable. StreamLambdaTracing addresses this issue, while mirroring the design of LambdaTracing.

CLAassistant commented 4 years ago

CLA assistant check
All committers have signed the CLA.

MattWhelan commented 4 years ago

I've verified that if you work around the reflection issues, the AWS platform's event deserialization operates correctly. But I haven't actually run a Lambda in AWS with this code. I've just unit tested it.

I may have some cycles later this week to look at the traces docs/examples; at the moment I'm fully committed elsewhere.

MattWhelan commented 4 years ago

I did a bit more end to end testing with this (in example code, in yet another repo), and it seems to work well.

I should have some time tomorrow to revise the example code in the tracer. Do you mind if I integrate the API Gateway setup with the SAM template? It's generally best to manage those things together, and keep your API Gateway config under version control.

jasonjkeller commented 4 years ago

@MattWhelan and I talked about his last comment offline. PR to update the example app and readme is here: https://github.com/newrelic/newrelic-lambda-tracer-java/pull/10