Fix LinkageError in apm-ecs-logging-plugin #2458

tobiasstadler · 2022-02-07T18:50:07Z

What does this PR do?

Fixes:

java.lang.IllegalAccessException: no such method: co.elastic.apm.agent.ecs_logging.Log4j2ServiceNameInstrumentation$AdviceClass.onEnter(Builder)void/invokeStatic
	at java.lang.invoke.MemberName.makeAccessException(MemberName.java:867) ~[?:1.8.0_322]
	at java.lang.invoke.MemberName$Factory.resolveOrFail(MemberName.java:1003) ~[?:1.8.0_322]
	at java.lang.invoke.MethodHandles$Lookup.resolveOrFail(MethodHandles.java:1386) ~[?:1.8.0_322]
	at java.lang.invoke.MethodHandles$Lookup.findStatic(MethodHandles.java:780) ~[?:1.8.0_322]
	at co.elastic.apm.agent.bci.IndyBootstrap.bootstrap(IndyBootstrap.java:408) [elastic-apm-agent-1.28.5-SNAPSHOT.jar:?]
	at sun.reflect.GeneratedMethodAccessor7.invoke(Unknown Source) ~[?:?]
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_322]
	at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_322]
	at java.lang.IndyBootstrapDispatcher.bootstrap(IndyBootstrapDispatcher.java:60) [?:1.8.0_322]
	at java.lang.invoke.CallSite.makeSite(CallSite.java:310) [?:1.8.0_322]
	at java.lang.invoke.MethodHandleNatives.linkCallSiteImpl(MethodHandleNatives.java:307) [?:1.8.0_322]
	at java.lang.invoke.MethodHandleNatives.linkCallSite(MethodHandleNatives.java:297) [?:1.8.0_322]
	at co.elastic.logging.log4j2.EcsLayout$Builder.build(EcsLayout.java:425) [log4j2-ecs-layout-1.3.2.jar:?]
	at co.elastic.logging.log4j2.EcsLayout$Builder.build(EcsLayout.java:328) [log4j2-ecs-layout-1.3.2.jar:?]
	at org.apache.logging.log4j.core.config.plugins.util.PluginBuilder.build(PluginBuilder.java:122) [log4j-core-2.17.1.jar:2.17.1]
	at org.apache.logging.log4j.core.config.AbstractConfiguration.createPluginObject(AbstractConfiguration.java:1120) [log4j-core-2.17.1.jar:2.17.1]
	at org.apache.logging.log4j.core.config.AbstractConfiguration.createConfiguration(AbstractConfiguration.java:1045) [log4j-core-2.17.1.jar:2.17.1]
	at org.apache.logging.log4j.core.config.AbstractConfiguration.createConfiguration(AbstractConfiguration.java:1037) [log4j-core-2.17.1.jar:2.17.1]
	at org.apache.logging.log4j.core.config.AbstractConfiguration.createConfiguration(AbstractConfiguration.java:1037) [log4j-core-2.17.1.jar:2.17.1]
	at org.apache.logging.log4j.core.config.AbstractConfiguration.doConfigure(AbstractConfiguration.java:651) [log4j-core-2.17.1.jar:2.17.1]
	at org.apache.logging.log4j.core.config.AbstractConfiguration.initialize(AbstractConfiguration.java:247) [log4j-core-2.17.1.jar:2.17.1]
	at org.apache.logging.log4j.core.config.AbstractConfiguration.start(AbstractConfiguration.java:293) [log4j-core-2.17.1.jar:2.17.1]
	at org.apache.logging.log4j.core.LoggerContext.setConfiguration(LoggerContext.java:626) [log4j-core-2.17.1.jar:2.17.1]
	at org.apache.logging.log4j.core.LoggerContext.reconfigure(LoggerContext.java:699) [log4j-core-2.17.1.jar:2.17.1]
	at org.apache.logging.log4j.core.LoggerContext.reconfigure(LoggerContext.java:716) [log4j-core-2.17.1.jar:2.17.1]
	at org.apache.logging.log4j.core.LoggerContext.start(LoggerContext.java:270) [log4j-core-2.17.1.jar:2.17.1]
	at org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:155) [log4j-core-2.17.1.jar:2.17.1]
	at org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:47) [log4j-core-2.17.1.jar:2.17.1]
	at org.apache.logging.log4j.LogManager.getContext(LogManager.java:196) [log4j-api-2.17.1.jar:2.17.1]
	at org.apache.logging.log4j.LogManager.getLogger(LogManager.java:599) [log4j-api-2.17.1.jar:2.17.1]
    ...
	at java.lang.Thread.run(Thread.java:750) [?:1.8.0_322]
Caused by: java.lang.LinkageError: bad method type alias: (Builder)void not visible from class co.elastic.apm.agent.ecs_logging.Log4j2ServiceNameInstrumentation$AdviceClass
	at java.lang.invoke.MemberName.checkForTypeAlias(MemberName.java:793) ~[?:1.8.0_322]
	at java.lang.invoke.MemberName$Factory.resolve(MemberName.java:976) ~[?:1.8.0_322]
	at java.lang.invoke.MemberName$Factory.resolveOrFail(MemberName.java:1000) ~[?:1.8.0_322]
	... 47 more

Checklist

This is a bugfix
- I have updated CHANGELOG.asciidoc
- I have added tests that would fail without this fix

java.lang.IllegalAccessException: no such method: co.elastic.apm.agent.ecs_logging.Log4j2ServiceNameInstrumentation$AdviceClass.onEnter(Builder)void/invokeStatic at java.lang.invoke.MemberName.makeAccessException(MemberName.java:867) ~[?:1.8.0_322] at java.lang.invoke.MemberName$Factory.resolveOrFail(MemberName.java:1003) ~[?:1.8.0_322] at java.lang.invoke.MethodHandles$Lookup.resolveOrFail(MethodHandles.java:1386) ~[?:1.8.0_322] at java.lang.invoke.MethodHandles$Lookup.findStatic(MethodHandles.java:780) ~[?:1.8.0_322] at co.elastic.apm.agent.bci.IndyBootstrap.bootstrap(IndyBootstrap.java:408) [elastic-apm-agent-1.28.5-SNAPSHOT.jar:?] at sun.reflect.GeneratedMethodAccessor7.invoke(Unknown Source) ~[?:?] at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_322] at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_322] at java.lang.IndyBootstrapDispatcher.bootstrap(IndyBootstrapDispatcher.java:60) [?:1.8.0_322] at java.lang.invoke.CallSite.makeSite(CallSite.java:310) [?:1.8.0_322] at java.lang.invoke.MethodHandleNatives.linkCallSiteImpl(MethodHandleNatives.java:307) [?:1.8.0_322] at java.lang.invoke.MethodHandleNatives.linkCallSite(MethodHandleNatives.java:297) [?:1.8.0_322] at co.elastic.logging.log4j2.EcsLayout$Builder.build(EcsLayout.java:425) [log4j2-ecs-layout-1.3.2.jar:?] at co.elastic.logging.log4j2.EcsLayout$Builder.build(EcsLayout.java:328) [log4j2-ecs-layout-1.3.2.jar:?] at org.apache.logging.log4j.core.config.plugins.util.PluginBuilder.build(PluginBuilder.java:122) [log4j-core-2.17.1.jar:2.17.1] at org.apache.logging.log4j.core.config.AbstractConfiguration.createPluginObject(AbstractConfiguration.java:1120) [log4j-core-2.17.1.jar:2.17.1] at org.apache.logging.log4j.core.config.AbstractConfiguration.createConfiguration(AbstractConfiguration.java:1045) [log4j-core-2.17.1.jar:2.17.1] at org.apache.logging.log4j.core.config.AbstractConfiguration.createConfiguration(AbstractConfiguration.java:1037) [log4j-core-2.17.1.jar:2.17.1] at org.apache.logging.log4j.core.config.AbstractConfiguration.createConfiguration(AbstractConfiguration.java:1037) [log4j-core-2.17.1.jar:2.17.1] at org.apache.logging.log4j.core.config.AbstractConfiguration.doConfigure(AbstractConfiguration.java:651) [log4j-core-2.17.1.jar:2.17.1] at org.apache.logging.log4j.core.config.AbstractConfiguration.initialize(AbstractConfiguration.java:247) [log4j-core-2.17.1.jar:2.17.1] at org.apache.logging.log4j.core.config.AbstractConfiguration.start(AbstractConfiguration.java:293) [log4j-core-2.17.1.jar:2.17.1] at org.apache.logging.log4j.core.LoggerContext.setConfiguration(LoggerContext.java:626) [log4j-core-2.17.1.jar:2.17.1] at org.apache.logging.log4j.core.LoggerContext.reconfigure(LoggerContext.java:699) [log4j-core-2.17.1.jar:2.17.1] at org.apache.logging.log4j.core.LoggerContext.reconfigure(LoggerContext.java:716) [log4j-core-2.17.1.jar:2.17.1] at org.apache.logging.log4j.core.LoggerContext.start(LoggerContext.java:270) [log4j-core-2.17.1.jar:2.17.1] at org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:155) [log4j-core-2.17.1.jar:2.17.1] at org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:47) [log4j-core-2.17.1.jar:2.17.1] at org.apache.logging.log4j.LogManager.getContext(LogManager.java:196) [log4j-api-2.17.1.jar:2.17.1] at org.apache.logging.log4j.LogManager.getLogger(LogManager.java:599) [log4j-api-2.17.1.jar:2.17.1] ... at java.lang.Thread.run(Thread.java:750) [?:1.8.0_322] Caused by: java.lang.LinkageError: bad method type alias: (Builder)void not visible from class co.elastic.apm.agent.ecs_logging.Log4j2ServiceNameInstrumentation$AdviceClass at java.lang.invoke.MemberName.checkForTypeAlias(MemberName.java:793) ~[?:1.8.0_322] at java.lang.invoke.MemberName$Factory.resolve(MemberName.java:976) ~[?:1.8.0_322] at java.lang.invoke.MemberName$Factory.resolveOrFail(MemberName.java:1000) ~[?:1.8.0_322] ... 47 more

github-actions · 2022-02-07T18:50:32Z

👋 @tobiasstadler Thanks a lot for your contribution!

It may take some time before we review a PR, so even if you don’t see activity for some time, it does not mean that we have forgotten about it.

Every once in a while we go through a process of prioritization, after which we are focussing on the tasks that were planned for the upcoming milestone. The prioritization status is typically reflected through the PR labels. It could be pending triage, a candidate for a future milestone, or have a target milestone set to it.

tobiasstadler · 2022-02-07T18:51:12Z

apm-agent-core/src/main/java/co/elastic/apm/agent/bci/classloading/IndyPluginClassLoader.java

            // As we're using a custom logging facade, plugins don't need to refer to the agent-bundled log4j2 or slf4j.
            return new DiscriminatingMultiParentClassLoader(
-                agentClassLoader, not(startsWith("org.apache.logging.log4j").and(not(startsWith("org.slf4j")))),
+                agentClassLoader, not(startsWith("org.apache.logging.log4j").and(not(startsWith("org.slf4j")))).and(not(startsWith("co.elastic.logging.log4j2"))),


not(startsWith("org.apache.logging.log4j").and(not(startsWith("org.slf4j")))) looks suspicious
to me, I would have expected not(startsWith("org.apache.logging.log4j")).and(not(startsWith("org.slf4j"))).

Yes, you rightfully think it is suspicious, but your proposal to exclude ECS logging is not something we want to do - we package ECS logging within the agent for the ECS-reformatting and we want to use our version rather than a different one provided as dependency.

We may exclude this package only for the IndyPluginClassLoader created for this specific plugin, but I have doubts about this plugin altogether. I think that going forward, the proper way to get ECS format when using the APM agent is through the ECS-reformating feature. There will still be a way to add it manually as a dependency for special cases, but in this case the service name can be manually configured. I am not too fond of this added complication coming from us instrumenting ECS logging.
@felixbarny WDYT?

The apm-log-shadding-plugins use PluginClassLoaderRootPackageCustomizer to add co.elastic.logging to the list of plugin packages. So shouldn't the apm-log-shadding-plugins always load co.elastic.logging.* from the agent (with or without this change)?

That's right! They are loaded from the plugin CL (not the agent CL).
OK, then let's go with that, but please verify manually that the ECS-reformating works as expected with this change in place.

Please confirm that everything works as expected ASAP, as we want to release 1.29.0 soon.

And please fix the improper matcher chaining as well

Is not(startsWith("org.slf4j")) still needed since you are using your own logging faced now?

I tested log_ecs_reformatting=SHADE, log_ecs_reformatting=REPLACE, log_ecs_reformatting=OVERRIDE. In case of SHADE and REPLACE a *.ecs.json file was created contain the logs in ECS format. In case of OVERRIDE the original of file contains the logs in ECS format.

I also noticed the message co.elastic.apm.agent.bci.bytebuddy.ErrorLoggingListener - co.elastic.logging.log4j2.EcsLayout$Builder refers to a missing class. in the agent logs, which I fixed in 7f7ce59

Is not(startsWith("org.slf4j")) still needed since you are using your own logging faced now?

Yes, we need slf4j to satisfy stagemonitor's logging needs.

Thanks for verifying

ghost · 2022-02-07T18:57:48Z

💚 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: 2022-02-09T05:41:51.855+0000
Duration: 50 min 1 sec

Test stats 🧪

Test	Results
Failed	0
Passed	2490
Skipped	16
Total	2506

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
run benchmark tests : Run the benchmark tests.
run jdk compatibility tests : Run the JDK Compatibility tests.
run integration tests : Run the Agent Integration tests.
run end-to-end tests : Run the APM-ITs.
run windows tests : Build & tests on windows.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

eyalkoren · 2022-02-08T08:00:12Z

apm-agent-core/src/main/java/co/elastic/apm/agent/bci/classloading/IndyPluginClassLoader.java

            // As we're using a custom logging facade, plugins don't need to refer to the agent-bundled log4j2 or slf4j.
            return new DiscriminatingMultiParentClassLoader(
-                agentClassLoader, not(startsWith("org.apache.logging.log4j").and(not(startsWith("org.slf4j")))),
+                agentClassLoader, not(startsWith("org.apache.logging.log4j").and(not(startsWith("org.slf4j")))).and(not(startsWith("co.elastic.logging.log4j2"))),


Yes, you rightfully think it is suspicious, but your proposal to exclude ECS logging is not something we want to do - we package ECS logging within the agent for the ECS-reformatting and we want to use our version rather than a different one provided as dependency.

We may exclude this package only for the IndyPluginClassLoader created for this specific plugin, but I have doubts about this plugin altogether. I think that going forward, the proper way to get ECS format when using the APM agent is through the ECS-reformating feature. There will still be a way to add it manually as a dependency for special cases, but in this case the service name can be manually configured. I am not too fond of this added complication coming from us instrumenting ECS logging.
@felixbarny WDYT?

…yout$Builder`

eyalkoren · 2022-02-08T12:01:53Z

/test

eyalkoren · 2022-02-08T12:03:38Z

run elasticsearch-ci/docs

tobiasstadler · 2022-02-08T12:25:11Z

The failed test co.elastic.apm.agent.util.CompletableFutureTest.testCancelBeforeStart seems to be unrelated.

eyalkoren · 2022-02-08T12:36:40Z

#2459

eyalkoren · 2022-02-08T13:30:33Z

run end-to-end tests

tobiasstadler · 2022-02-08T14:40:59Z

The Application Server integration tests failure seems to be not related to my changes (also fails on master).

tobiasstadler · 2022-02-09T07:45:01Z

Thank You!

github-actions bot added agent-java community Issues and PRs created by the community triage labels Feb 7, 2022

tobiasstadler commented Feb 7, 2022

View reviewed changes

eyalkoren suggested changes Feb 8, 2022

View reviewed changes

tobiasstadler added 2 commits February 8, 2022 11:30

Do not try to instrument the bundled `co.elastic.logging.log4j2.EcsLa…

7f7ce59

…yout$Builder`

Fix not loading org.slf4j* from the agent class loader

dfe0104

eyalkoren approved these changes Feb 8, 2022

View reviewed changes

eyalkoren merged commit 4992c02 into elastic:main Feb 9, 2022

tobiasstadler deleted the fix-log4j2-ecslayout-servicename branch February 9, 2022 07:45

Fix LinkageError in apm-ecs-logging-plugin #2458

Fix LinkageError in apm-ecs-logging-plugin #2458

Uh oh!

Conversation

tobiasstadler commented Feb 7, 2022

What does this PR do?

Checklist

Uh oh!

github-actions bot commented Feb 7, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Feb 7, 2022 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eyalkoren commented Feb 8, 2022

Uh oh!

eyalkoren commented Feb 8, 2022

Uh oh!

tobiasstadler commented Feb 8, 2022

Uh oh!

eyalkoren commented Feb 8, 2022

Uh oh!

eyalkoren commented Feb 8, 2022

Uh oh!

tobiasstadler commented Feb 8, 2022

Uh oh!

tobiasstadler commented Feb 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ghost commented Feb 7, 2022 •

edited by ghost

Loading