Address type erasure/reflection issue #36
Conversation
Describes the issue though: AWS can't deserialize the event if it can't tell what the input type is at runtime. Erasure is preventing that.
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`.
|
|
There was a problem hiding this comment.
@MattWhelan These changes look pretty good to me though I'd like to get input from at least one more person on the team given the scope of the changes.
To what extent have you done end to end testing with the refactored code?
Also, these changes are going to require that we update our usage examples in the tracer project as well as the example application that is provided there.
We're low on bandwidth at the moment but if I can eke out some time I might try to update the example app and do a bit of end to end testing myself, but if it is something you have cycles for please feel free to do so.
Thanks again for the overhaul!
|
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. |
|
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. |
|
@MattWhelan and I talked about his last comment offline. PR to update the example app and readme is here: newrelic/newrelic-lambda-tracer-java#10 |
src/main/java/com/newrelic/opentracing/aws/TracingRequestStreamHandler.java
Outdated
Show resolved
Hide resolved
README code streamlining, unnesting try blocks, exception error message.
This is a pretty broad rewrite, because the former approach cannot work.
TracingRequestHandler's type parameters erase to
Objectat runtime, whichdefeats 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.
StreamLambdaTracingaddresses this issue, while mirroring the design ofLambdaTracing.