Added basic support for com.sun.net.httpserver.HttpServer (#1853)

[tobiasstadler]

What does this PR do?

Fixes #1853

Checklist

• This is a new plugin
  • I have updated CHANGELOG.asciidoc
  • My code follows the style guidelines of this project
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have updated supported-technologies.asciidoc
  • Added an API method or config option? Document in which version this will be introduced
  • Added an instrumentation plugin? Describe how you made sure that old, non-supported versions are not instrumented by accident.

[ghost]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2021-07-07T05:42:31.179+0000

• Duration: 53 min 23 sec

• Commit: f1da7c6

Test stats 🧪

Test	Results
Failed	0
Passed	2341
Skipped	19
Total	2360

Trends 🧪

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	2341
Skipped	19
Total	2360

[tobiasstadler]

Can somebody please do a review?

[tobiasstadler]

@SylvainJuge Maybe you could have a look please?

[SylvainJuge]

Here it's relevant to override getTypeMatcherPreFilter to ensure that it's only applied within package com.sun.net.httpserver, otherwise that makes hasSuperType matcher expensive. This will make it work only with the implementations that are part of the JDK.

---

Alternatively, if you aim to extend support to implementations that might be provided through com.sun.net.httpserver.spi.HttpServerProvider, this would require to apply instrumentation at runtime when instances are returned by HttpServerProvider. This could easily be done in a follow-up PR unless required for your use-case.

[tobiasstadler]

Thank you for your review.

I am fine with only instrumenting com.sun.net.httpserver Handler. But then the HttpHandlerTest passes no longer because of java.lang.NoClassDefFoundError: org/slf4j/LoggerFactory. The reason for the exception is that com.sun.net.httpserver.* is loaded by the platform classloader which cannot load org/slf4j/LoggerFactory and neither can the agent class loader, because org/slf4j is not listed in co.elastic.apm.agent.bci.classloading.IndyPluginClassLoaderParent#agentPackages. This is not an issue in the release build because org/slf4jis shaded.

How should I proceed? Remove the logger? Remove the test?

[SylvainJuge]

I've modified a bit the implementation to instrument the HttpHandler implementation at runtime, which allows to cover the nominal case where the handler class is not part of the com.sun.net nor sun.net packages. There are two ways to set the HttpHandler on the HttpContext and both are covered.

I also managed to reproduce the issue with ClassNotFoundError, but it does not appear with the current state of the code, thus I'd say that we can get with it for now and investigate further only if required. The underlying issue seemed related to trying to load one of the advice classes directly within the PlatformClassLoader, whereas those advice classes should rather be loaded within the agent PluginClassLoader which is here a direct child of PlatformClassLoader (the instrumentation target classloader) and can also access most agent classes.

[tobiasstadler]

Thank You for working on this.

[SylvainJuge]

For the record, the ClassNotFoundError is triggered when we move the HttpHandlerHelper#ensureInstrumented method to HttpHandlerAdvice class and then call it directly from within another advice method body. While direct advice-to-advice is not common as we usually use intermediate or helper classes, this might be convenient to share code between advices.

[tobiasstadler]

Thank You!

[SylvainJuge]

Thank for your contributions! if you haven't subscribed already you might be interesting by the Elastic Contributor program : https://www.elastic.co/community/contributor