Don't instrument well behaved spring class loaders #1785
Conversation
bantonsson
force-pushed
the
ban/spring_class_loaders
branch
from
1f2637c
to
d2bfb3a
Compare
bantonsson
changed the title
| @@ -45,7 +45,12 @@ public ClassloadingInstrumentation() { | |||
| return namedNoneOf( | |||
| "java.lang.ClassLoader", | |||
| "com.ibm.oti.vm.BootstrapClassLoader", | |||
| "datadog.trace.bootstrap.AgentClassLoader") | |||
| "datadog.trace.bootstrap.AgentClassLoader", | |||
| if (name.startsWith("org.springframework.core.task.") | ||
| || name.equals("org.springframework.core.DecoratingClassLoader") | ||
| || name.equals("org.springframework.core.OverridingClassLoader")) { | ||
| if (name.startsWith("org.springframework.core.task.")) { |
There was a problem hiding this comment.
It's not new but this bugs me, I think we could implement a trie without much effort and probably do better here.
There was a problem hiding this comment.
Yes I completely agree.
| @@ -45,7 +45,12 @@ public ClassloadingInstrumentation() { | |||
| return namedNoneOf( | |||
| "java.lang.ClassLoader", | |||
| "com.ibm.oti.vm.BootstrapClassLoader", | |||
| "datadog.trace.bootstrap.AgentClassLoader") | |||
| "datadog.trace.bootstrap.AgentClassLoader", | |||
There was a problem hiding this comment.
datadog.trace.bootstrap.AgentClassLoader no longer exists. I wonder how long this has been like this for?
There was a problem hiding this comment.
I think it should be updated to this: datadog.trace.bootstrap.DatadogClassLoader
There was a problem hiding this comment.
Also datadog.trace.bootstrap.DatadogClassLoader$DelegateClassLoader
There was a problem hiding this comment.
Can we change this before merging?
There was a problem hiding this comment.
Turns out that they are already taken care of by datadog.trace.agent.tooling.ClassLoaderMatcher.SkipClassLoaderMatcher.canSkipClassLoaderByName.
There was a problem hiding this comment.
Do we need to rationalise this logic? We seem to match classloaders in so many places.
There was a problem hiding this comment.
I only count to 3 right now 😉
bantonsson
force-pushed
the
ban/spring_class_loaders
branch
from
d2bfb3a
to
fdb5482
Compare
...ng/src/main/java/datadog/trace/instrumentation/classloading/ClassloadingInstrumentation.java
Outdated
Show resolved
Hide resolved
...ooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/GlobalIgnoresMatcher.java
Outdated
Show resolved
Hide resolved
bantonsson
force-pushed
the
ban/spring_class_loaders
branch
from
6c6b7b5
to
d46d7ac
Compare
As part of #1293 we started instrumenting a couple of spring class loaders that all delegate class loading properly. This can have a big impact on application startup time in some cases.