-
Notifications
You must be signed in to change notification settings - Fork 336
Add Classloader precheck to expensive matchers. #1283
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,11 @@ | ||
| package datadog.trace.instrumentation.aws.v0; | ||
|
|
||
| import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasNoResources; | ||
| import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.extendsClass; | ||
| import static java.util.Collections.singletonMap; | ||
| import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; | ||
| import static net.bytebuddy.matcher.ElementMatchers.named; | ||
| import static net.bytebuddy.matcher.ElementMatchers.not; | ||
| import static net.bytebuddy.matcher.ElementMatchers.takesArgument; | ||
|
|
||
| import com.amazonaws.AmazonWebServiceRequest; | ||
|
|
@@ -25,6 +27,12 @@ public RequestInstrumentation() { | |
| super("aws-sdk"); | ||
| } | ||
|
|
||
| @Override | ||
| public ElementMatcher<ClassLoader> classLoaderMatcher() { | ||
| // Optimization for expensive typeMatcher. | ||
| return not(classLoaderHasNoResources("com/amazonaws/AmazonWebServiceRequest.class")); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The double negative is a bit hard to read. I think a utility method would help clarity greatly. |
||
| } | ||
|
|
||
| @Override | ||
| public ElementMatcher<TypeDescription> typeMatcher() { | ||
| return nameStartsWith("com.amazonaws.") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,11 @@ | ||
| package datadog.trace.instrumentation.hibernate.core.v4_3; | ||
|
|
||
| import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasNoResources; | ||
| import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.implementsInterface; | ||
| import static java.util.Collections.singletonMap; | ||
| import static net.bytebuddy.matcher.ElementMatchers.isMethod; | ||
| import static net.bytebuddy.matcher.ElementMatchers.named; | ||
| import static net.bytebuddy.matcher.ElementMatchers.not; | ||
|
|
||
| import com.google.auto.service.AutoService; | ||
| import datadog.trace.agent.tooling.Instrumenter; | ||
|
|
@@ -43,6 +45,12 @@ public String[] helperClassNames() { | |
| }; | ||
| } | ||
|
|
||
| @Override | ||
| public ElementMatcher<ClassLoader> classLoaderMatcher() { | ||
| // Optimization for expensive typeMatcher. | ||
| return not(classLoaderHasNoResources("org/hibernate/Session.class")); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One observation from the field injection optimization. Often our classLoaderMatchers are really more aim at matching a library or an API, but we don't really have that as a concept in the code. I think this change also suggest that we should have some notion of a Library go which Instrumenters refer. |
||
| } | ||
|
|
||
| @Override | ||
| public ElementMatcher<TypeDescription> typeMatcher() { | ||
| return implementsInterface(named("org.hibernate.procedure.ProcedureCall")); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double negatives are somewhat hard to read