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

Support runtime attachment for OSGi containers #1085

Closed
felixbarny opened this issue Mar 18, 2020 · 4 comments
Closed

Support runtime attachment for OSGi containers #1085

felixbarny opened this issue Mar 18, 2020 · 4 comments
Assignees
Labels
enhancement Enhancement of an existing feature

Comments

@felixbarny
Copy link
Member

Background

We have experimental support for runtime attachment via a CLI tool and a self-attachment Java API. The reason that it's still in an experimental stage is that this doesn't work for OSGi containers.

Our current strategy of setting the org.osgi.framework.bootdelegation system property to allow the co.elastic.apm.agent package to be loaded by the bootstrap class loader only works when attaching the agent as a -javaagent. That is because when doing runtime attachment, the class OSGi loaders are already initialized and setting the bootdelegation doesn't have any effect.

Solutions

Instrument OSGi containers to allow loading the agent classes

Pros:

  • We can keep the current architecture of the agent

Cons:

  • It's quite a maintenance burden to support every version of every OSGi container
  • It only works for OSGi, there are other filtering class loaders

Inject a single dispatcher class into the java.lang namespace

This would require a major overhaul of our current class loading architecture. But it would change it for the better. It would even allow us to do fancy things like unloading/live-updating the agent down the road.

Currently, our whole agent is loaded by the bootstrap classloader. This exposes our classes to everyone. This slows down classpath scanning tools and can cause errors. With the new design, there would only be a handful of classes to be injected into the bootstrap classloader.

We would rely on the fact that all filtering classloaders have a whitelist for the java.lang package. That is because all classes should naturally have access to java.lang.String, for example. Otherwise, no Java application could run.
Therefore, we would inject a dispatcher class (for example java.lang.apm.Dispatcher) into the bootstrap classloader. The dispatcher is basically a globally-accessible ConcurrentHashMap.

The rest of the agent is loaded from a separate class loader that is a child of the bootstrap classloader.

The advices would only communicate with the rest of the agent via MethodHandles that are stored in the dispatcher map.

This is a diagram of how it would look like in a typical web application:

    Bootstrap CL <-------------------- Agent CL (created by AgentMain)
      ^  - apm-agent-bootstrap.jar     - agent.jar
      |    - Dispatcher                  - ElasticApmAgent
      |      Map<String, Object>         - ElasticApmTracer, Transaction, Span, ...
      |    - WeakConcurrentMap           - plugins, incl. advice classes
      |                                  - shaded dependencies
      |                                    (we could think about not shading dependencies)
    Ext/Platform CL                            ^
      ^                                        |
      |                                        |
    System CL                                  |
      ^  - elastic-apm-agent.jar               |
      |    - AgentMain                         |
      |    - bootstrap/apm-agent-bootstrap.jar |
      |    - agent/agent.jar                   |
      |                                        |
    Common - servlet-api                       |
   /      \                                    |
WebApp1  WebApp2 <---------------------------- Helper CL
          - spring-webmvc.jar                   * Have two parents
          - apache-httpclient.jar               * Helpers can access both the instrumented
          * inlined advice code can't access      library classes and the agent API
            anything within agent.jar,         - ApmAsyncListener
            including static fields in advice  - CallbackWrapperCreator
            classes
          * therefore, advices call helpers via
            a MethodHandle registered in the
            Dispatcher map

As this would mean quite a lot of changes, let's break it down to several steps:

  • Introduce a dispatcher class in the apm-agent-core module
    • The dispatcher class will be in the co.elastic.apm.agent namespace initially
    • Like we currently do, the elastic-apm-agent.jar will still be loaded by the bootstrap CL
    • Try out the new dispatcher with the servlet and JDBC instrumentation
      • The advice methods call helper methods via a MethodHandle
      • The helper may be in the bootstrap CL or in a Helper CL (child of the target CL), like today
    • Use the existing benchmarks to validate that the new approach does not negatively impact performance
  • Migrate all plugins to the dispatcher-based approach of calling helpers
  • Create separate boostrap jar for the dispatcher and move it to the java.lang namespace
  • Load the rest of the agent from a child of the bootstrap CL
@eyalkoren
Copy link
Contributor

eyalkoren commented Oct 13, 2020

The way to close this issue is update the integration tests to use runtime attach on all tested Servlet containers (basically - change AbstractServletContainerIntegrationTest#runtimeAttachSupported default implementation to return true).

@SylvainJuge SylvainJuge added the enhancement Enhancement of an existing feature label Feb 2, 2021
@SylvainJuge
Copy link
Member

Now that indy plugin migration + separate classloader are completed, we should natively support this.

@chetankokil
Copy link

@SylvainJuge since the classloader is separate now, how do we load the classes , so that we can write our own instrumentation. Currently i am in a situation where the .esclazz files are not been loaded by the classloader which avoids compiling the code. any help is appreciated.

@SylvainJuge
Copy link
Member

Hi @chetankokil, the agent classes must be loaded with an internal classloader, and using the .esclazz suffix in the agent jar is only a way to ensure that a generic URLClassLoader (or similar) will not be able to load the agent classes.

While the agent is packaged as a jar archive, it is not meant to be used as an application dependency because there is a complex class-loading strategy required to make the agent work properly, including in OSGi environments. It used to be possible to load agent classes from the jar previously, but it that never was something we officially supported.

In your case, you should be able to use one of the officially supported ways to setup the agent, and if you really need to extend agent features you should be able to create an external plugin.

If your use-case does not fit what external plugins currently allow, then let's discuss this in our forum, that will be a better place for conversation rather than in a github issue (just make sure to pose in Observability / APM + tag it with java).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement of an existing feature
Projects
None yet
Development

No branches or pull requests

4 participants