newrelic / newrelic-java-agent

The New Relic Java agent
Apache License 2.0
202 stars 143 forks source link

Add support for scala2.13 akka-http-core 10.1.X #102

Closed junder31 closed 3 years ago

junder31 commented 3 years ago

Overview

This change adds instrumentation for akka-http-core 10.1.X with scala 2.13. The existing scala 2.11 module does not instrument 2.13 apps because it references classes from the scala library that were removed in scala 2.13 (scala/Serializable,scala/collection/JavaConversions$, and possibly others). This is just a copy of the old akka-http-core_10.0.11 instrumentation with some minimal refactoring required to compile against scala 2.13.

Play Framework 2.8 introduced support for scala 2.13. The default HTTP server implementation for play2.8 is the 10.1.X version of akka-http-core. This change allows proper reporting of response times and HTTP status codes for scala2.13/play2.8 applications.

Checks

[ ] Are your contributions backwards compatible with relevant frameworks and APIs? Yes [ ] Does your code contain any breaking changes? No [ ] Does your code introduce any new dependencies? Only the scala2.13 versions of akka-http-core that are being instrumented.

XiXiaPdx commented 3 years ago

@junder31 Thank you for another fantastic contribution! We are in the middle of working through a new release of the agent this week and appreciate your patience to give us a bit of time to review your PR.

Just curious, may I ask if you have manually tested this locally with a sample app and have seen data reported to New Relic?

Thanks again for your time and effort on this!

junder31 commented 3 years ago

@XiXiaPdx Yes. I've been deploying this to instrument a play2.8 scala 2.13 application on my companies development environment. It is correctly reporting response times and http statuses. There are definitely other issues with scala2.13 support in the java-agent, so none of the transaction segments get reported. This at least allows proper reporting of the appdex score for play2.8/scala2.13 applications which I think is a pretty good improvement by itself. It is likely that other frameworks that use akka-http-server would also see proper appdex reporting as well.

tspring commented 3 years ago

Thanks @junder31 . I'm off this week so will let someone else merge this (@XiXiaPdx ), but it all looks good to me.

XiXiaPdx commented 3 years ago

@junder31 May I ask if you would share a NR1 link to the development application you instrumented? I'm curious to take a quick look at what the UI is reporting and what is missing. I understand if you can't and thanks for considering it.

junder31 commented 3 years ago

@XiXiaPdx I'd be happy to share over some other private channel. I'm making these contributions in order to better support monitoring my company's proprietary software, so I'd be reluctant to post a link here.

XiXiaPdx commented 3 years ago

Totally understandable! Would my work email be acceptable? xxia@newrelic.com

matwojcik commented 3 years ago

Hi, would it work in Akka http 10.2 as well?

junder31 commented 3 years ago

@matwojcik I think the 10.2 instrumentation is a different package. This PR probably won't support that version with scala 2.13, but it would be a very similar process to this PR in order to fix the 10.2 support for scala2.13