Skip to content

First step towards enabling external plugins#1271

Merged
felixbarny merged 9 commits intoelastic:masterfrom
felixbarny:plugin-sdk
Jul 17, 2020
Merged

First step towards enabling external plugins#1271
felixbarny merged 9 commits intoelastic:masterfrom
felixbarny:plugin-sdk

Conversation

@felixbarny
Copy link
Member

What does this PR do?

Implements #937 (comment)

Makes it possible to load external plugins in an isolated class loader. The plugins depend on the apm-agent-plugin-sdk and the apm-agent-api artifacts to create a jar that can be place it into the plugins_dir. An example for such a plugin is in integration-tests/external-plugin-test.

As some parts are still missing, this is not mentioned in the changelog yet and the corresponding configuration is marked internal (thus not included in the generated docs).

  • Creates apm-agent-plugin-sdk: used both by internal and external/community plugins
  • I decided against fractoring out a apm-agent-tracer-impl module from apm-agent-core for now. It was more tricky than expected and not needed for external plugins.

Missing bits before we can open this to the public:

  • Remove ElasticApmPlugin#indyPlugin, making indy plugins the only option
  • stop shading Byte Buddy so that users can depend on elastic-apm-agent in their tests to activate apm-agent-api
    • agent has to be loaded in its dedicated class loader first to avoid conflicts with the application
  • publish test module for apm-agent-core

@ghost
Copy link

ghost commented Jul 6, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #1271 updated]

  • Start Time: 2020-07-17T09:21:01.046+0000

  • Duration: 42 min 44 sec

Test stats 🧪

Test Results
Failed 0
Passed 1461
Skipped 11
Total 1472

@felixbarny felixbarny changed the title First step towards loading external plugins First step towards enabling external plugins Jul 6, 2020
@felixbarny
Copy link
Member Author

In a follow-up, we may want to include some of the CustomElementMatchers in the plugin SDK such as classLoaderCanLoadClass.

…mentation

so that we don't have to do a breaking change in the SDK.
External plugins are always indy-dispatched without an opt-out.
@felixbarny
Copy link
Member Author

@elastic/apm-agent-java any changes you want to do in the ElasticApmInstrumentation class? After we publish the SDK, we can only add but never remove or rename methods.

One thing we might want to consider, although being a very minor thing, is to remove the get prefixes, for example from getTypeMatcher. It feels a bit awkward when writing an instrumentation because a type matcher is created, not retrieved when the method is called.

I have also changed the method name getImplementationVersionPostFilter to getProtectionDomainPostFilter to give it a more generic name.

@SylvainJuge SylvainJuge self-requested a review July 8, 2020 11:33
@felixbarny felixbarny requested a review from eyalkoren July 8, 2020 15:07
Copy link
Member

@SylvainJuge SylvainJuge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass on the review, mostly minor comments and questions.

@felixbarny felixbarny requested a review from SylvainJuge July 15, 2020 14:35
@felixbarny felixbarny merged commit 48f01f8 into elastic:master Jul 17, 2020
@felixbarny felixbarny deleted the plugin-sdk branch July 17, 2020 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants