open-telemetry
/
opentelemetry-java-instrumentation
mirror of https://github.com/open-telemetry/opentelemetry-java-instrumentation.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Review comments: Replace un-needed instrumentation by just propagating scope not spans

This commit is contained in:
Will Gittoes 2019-03-06 10:05:21 +11:00
parent 9e3fda549d
commit 00865dab88
No known key found for this signature in database
GPG Key ID: 521026A02DB0BB42
13 changed files with 55 additions and 160 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AbstractAwsClientInstrumentation.java
View File

@ -6,7 +6,7 @@ public abstract class AbstractAwsClientInstrumentation extends Instrumenter.Defa
private static final String INSTRUMENTATION_NAME = "aws-sdk";
public AbstractAwsClientInstrumentation() {
super("hibernate-core");
super(INSTRUMENTATION_NAME);
}
@Override

4
dd-java-agent/instrumentation/hibernate/hibernate.gradle
View File

@ -9,7 +9,7 @@ muzzle {
pass {
group = "org.hibernate"
module = "hibernate-core"
versions = "[5.0.0.Final, 5.+]"
versions = "[5.0.0.Final,)"
}
}
@ -44,6 +44,6 @@ dependencies {
testCompile "com.sun.xml.bind:jaxb-impl:2.2.11"
testCompile "javax.activation:activation:1.1.1"
latestDepTestCompile group: 'org.hibernate', name: 'hibernate-core', version: '5.+'
latestDepTestCompile group: 'org.hibernate', name: 'hibernate-core', version: '+'
latestDepTestCompile group: 'com.h2database', name: 'h2', version: '1.4.197'
}

6
dd-java-agent/instrumentation/hibernate/src/main/java/datadog/trace/instrumentation/hibernate/CriteriaInstrumentation.java
View File

@ -34,8 +34,8 @@ public class CriteriaInstrumentation extends Instrumenter.Default {
@Override
public String[] helperClassNames() {
return new String[] {
"datadog.trace.instrumentation.hibernate.SessionMethodUtils",
"datadog.trace.instrumentation.hibernate.SessionState",
packageName + ".SessionMethodUtils",
packageName + ".SessionState",
"datadog.trace.agent.decorator.BaseDecorator",
"datadog.trace.agent.decorator.ClientDecorator",
"datadog.trace.agent.decorator.DatabaseClientDecorator",
@ -66,7 +66,7 @@ public class CriteriaInstrumentation extends Instrumenter.Default {
InstrumentationContext.get(Criteria.class, SessionState.class);
return SessionMethodUtils.startScopeFrom(
contextStore, criteria, "hibernate.criteria." + name, null);
contextStore, criteria, "hibernate.criteria." + name, null, true);
}
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)

81
dd-java-agent/instrumentation/hibernate/src/main/java/datadog/trace/instrumentation/hibernate/IteratorInstrumentation.java
View File

@ -1,81 +0,0 @@
package datadog.trace.instrumentation.hibernate;
import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.isInterface;
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;
import datadog.trace.bootstrap.ContextStore;
import datadog.trace.bootstrap.InstrumentationContext;
import java.util.Map;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.implementation.bytecode.assign.Assigner;
import net.bytebuddy.matcher.ElementMatcher;
import org.hibernate.engine.HibernateIterator;
@AutoService(Instrumenter.class)
public class IteratorInstrumentation extends Instrumenter.Default {
public IteratorInstrumentation() {
super("hibernate-core");
}
@Override
public Map<String, String> contextStore() {
return singletonMap("org.hibernate.engine.HibernateIterator", SessionState.class.getName());
}
@Override
public String[] helperClassNames() {
return new String[] {
"datadog.trace.instrumentation.hibernate.SessionMethodUtils",
"datadog.trace.instrumentation.hibernate.SessionState",
"datadog.trace.agent.decorator.BaseDecorator",
"datadog.trace.agent.decorator.ClientDecorator",
"datadog.trace.agent.decorator.DatabaseClientDecorator",
"datadog.trace.agent.decorator.OrmClientDecorator",
packageName + ".HibernateDecorator",
};
}
@Override
public ElementMatcher<TypeDescription> typeMatcher() {
return not(isInterface())
.and(safeHasSuperType(named("org.hibernate.engine.HibernateIterator")));
}
@Override
public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
return singletonMap(
isMethod().and(named("next").or(named("remove"))), IteratorAdvice.class.getName());
}
public static class IteratorAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static SessionState startMethod(
@Advice.This final HibernateIterator iterator, @Advice.Origin("#m") final String name) {
final ContextStore<HibernateIterator, SessionState> contextStore =
InstrumentationContext.get(HibernateIterator.class, SessionState.class);
return SessionMethodUtils.startScopeFrom(
contextStore, iterator, "hibernate.iterator." + name, null);
}
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void endMethod(
@Advice.Enter final SessionState state,
@Advice.Thrown final Throwable throwable,
@Advice.Return(typing = Assigner.Typing.DYNAMIC) final Object entity) {
SessionMethodUtils.closeScope(state, throwable, entity);
}
}
}

6
dd-java-agent/instrumentation/hibernate/src/main/java/datadog/trace/instrumentation/hibernate/ProcedureCallInstrumentation.java
View File

@ -33,8 +33,8 @@ public class ProcedureCallInstrumentation extends Instrumenter.Default {
@Override
public String[] helperClassNames() {
return new String[] {
"datadog.trace.instrumentation.hibernate.SessionMethodUtils",
"datadog.trace.instrumentation.hibernate.SessionState",
packageName + ".SessionMethodUtils",
packageName + ".SessionState",
"datadog.trace.agent.decorator.BaseDecorator",
"datadog.trace.agent.decorator.ClientDecorator",
"datadog.trace.agent.decorator.DatabaseClientDecorator",
@ -65,7 +65,7 @@ public class ProcedureCallInstrumentation extends Instrumenter.Default {
final SessionState state =
SessionMethodUtils.startScopeFrom(
contextStore, call, "hibernate.procedure." + name, call.getProcedureName());
contextStore, call, "hibernate.procedure." + name, call.getProcedureName(), true);
return state;
}

7
dd-java-agent/instrumentation/hibernate/src/main/java/datadog/trace/instrumentation/hibernate/QueryInstrumentation.java
View File

@ -36,8 +36,8 @@ public class QueryInstrumentation extends Instrumenter.Default {
@Override
public String[] helperClassNames() {
return new String[] {
"datadog.trace.instrumentation.hibernate.SessionMethodUtils",
"datadog.trace.instrumentation.hibernate.SessionState",
packageName + ".SessionMethodUtils",
packageName + ".SessionState",
"datadog.trace.agent.decorator.BaseDecorator",
"datadog.trace.agent.decorator.ClientDecorator",
"datadog.trace.agent.decorator.DatabaseClientDecorator",
@ -74,7 +74,8 @@ public class QueryInstrumentation extends Instrumenter.Default {
// Note: We don't know what the entity is until the method is returning.
final SessionState state =
SessionMethodUtils.startScopeFrom(contextStore, query, "hibernate.query." + name, null);
SessionMethodUtils.startScopeFrom(
contextStore, query, "hibernate.query." + name, null, true);
DECORATOR.onStatement(state.getMethodScope().span(), query.getQueryString());
return state;
}

2
dd-java-agent/instrumentation/hibernate/src/main/java/datadog/trace/instrumentation/hibernate/SessionFactoryInstrumentation.java
View File

@ -38,7 +38,7 @@ public class SessionFactoryInstrumentation extends Instrumenter.Default {
@Override
public String[] helperClassNames() {
return new String[] {
"datadog.trace.instrumentation.hibernate.SessionState",
packageName + ".SessionState",
"datadog.trace.agent.decorator.BaseDecorator",
"datadog.trace.agent.decorator.ClientDecorator",
"datadog.trace.agent.decorator.DatabaseClientDecorator",

29
dd-java-agent/instrumentation/hibernate/src/main/java/datadog/trace/instrumentation/hibernate/SessionInstrumentation.java
View File

@ -15,9 +15,12 @@ import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.bootstrap.ContextStore;
import datadog.trace.bootstrap.InstrumentationContext;
import io.opentracing.Span;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.description.type.TypeDescription;
@ -27,7 +30,6 @@ import org.hibernate.Criteria;
import org.hibernate.Query;
import org.hibernate.SharedSessionContract;
import org.hibernate.Transaction;
import org.hibernate.engine.HibernateIterator;
import org.hibernate.procedure.ProcedureCall;
@AutoService(Instrumenter.class)
@ -45,15 +47,14 @@ public class SessionInstrumentation extends Instrumenter.Default {
map.put("org.hibernate.Transaction", SessionState.class.getName());
map.put("org.hibernate.Criteria", SessionState.class.getName());
map.put("org.hibernate.procedure.ProcedureCall", SessionState.class.getName());
map.put("org.hibernate.engine.HibernateIterator", SessionState.class.getName());
return Collections.unmodifiableMap(map);
}
@Override
public String[] helperClassNames() {
return new String[] {
"datadog.trace.instrumentation.hibernate.SessionMethodUtils",
"datadog.trace.instrumentation.hibernate.SessionState",
packageName + ".SessionMethodUtils",
packageName + ".SessionState",
"datadog.trace.agent.decorator.BaseDecorator",
"datadog.trace.agent.decorator.ClientDecorator",
"datadog.trace.agent.decorator.DatabaseClientDecorator",
@ -150,15 +151,20 @@ public class SessionInstrumentation extends Instrumenter.Default {
public static class SessionMethodAdvice {
public static final Set<String> SCOPE_ONLY_METHODS =
new HashSet<>(Arrays.asList("immediateLoad", "internalLoad"));
@Advice.OnMethodEnter(suppress = Throwable.class)
public static SessionState startMethod(
@Advice.This final SharedSessionContract session,
@Advice.Origin("#m") final String name,
@Advice.Argument(0) final Object entity) {
final boolean startSpan = !SCOPE_ONLY_METHODS.contains(name);
final ContextStore<SharedSessionContract, SessionState> contextStore =
InstrumentationContext.get(SharedSessionContract.class, SessionState.class);
return SessionMethodUtils.startScopeFrom(contextStore, session, "hibernate." + name, entity);
return SessionMethodUtils.startScopeFrom(
contextStore, session, "hibernate." + name, entity, startSpan);
}
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
@ -169,19 +175,6 @@ public class SessionInstrumentation extends Instrumenter.Default {
@Advice.Return(typing = Assigner.Typing.DYNAMIC) final Object returned) {
SessionMethodUtils.closeScope(sessionState, throwable, returned);
// Attach instrumentation to any returned object (of eligible type).
if (returned == null) {
return;
}
final ContextStore<SharedSessionContract, SessionState> sessionContextStore =
InstrumentationContext.get(SharedSessionContract.class, SessionState.class);
if (returned instanceof HibernateIterator) {
final ContextStore<HibernateIterator, SessionState> iteratorContextStore =
InstrumentationContext.get(HibernateIterator.class, SessionState.class);
SessionMethodUtils.attachSpanFromStore(
sessionContextStore, session, iteratorContextStore, (HibernateIterator) returned);
}
}
}

37
dd-java-agent/instrumentation/hibernate/src/main/java/datadog/trace/instrumentation/hibernate/SessionMethodUtils.java
View File

@ -2,6 +2,7 @@ package datadog.trace.instrumentation.hibernate;
import static datadog.trace.instrumentation.hibernate.HibernateDecorator.DECORATOR;
import datadog.trace.bootstrap.CallDepthThreadLocalMap;
import datadog.trace.bootstrap.ContextStore;
import io.opentracing.Scope;
import io.opentracing.Span;
@ -15,28 +16,33 @@ public class SessionMethodUtils {
final ContextStore<TARGET, SessionState> contextStore,
final TARGET spanKey,
final String operationName,
final ENTITY entity) {
final ENTITY entity,
final boolean createSpan) {
final SessionState sessionState = contextStore.get(spanKey);
if (sessionState == null) {
// No state found. We aren't in a Session.
return null;
return null; // No state found. We aren't in a Session.
}
if (sessionState.getMethodScope() != null) {
// This method call was re-entrant. Do nothing, since it is being traced by the parent/first
// call.
return null;
final int depth = CallDepthThreadLocalMap.incrementCallDepth(SessionMethodUtils.class);
if (depth > 0) {
return null; // This method call is being traced already.
}
final Scope scope =
GlobalTracer.get()
.buildSpan(operationName)
.asChildOf(sessionState.getSessionSpan())
.startActive(true);
DECORATOR.afterStart(scope.span());
DECORATOR.onOperation(scope.span(), entity);
final Scope scope;
if (createSpan) {
scope =
GlobalTracer.get()
.buildSpan(operationName)
.asChildOf(sessionState.getSessionSpan())
.startActive(true);
DECORATOR.afterStart(scope.span());
DECORATOR.onOperation(scope.span(), entity);
} else {
scope = GlobalTracer.get().scopeManager().activate(sessionState.getSessionSpan(), false);
sessionState.setHasChildSpan(false);
}
sessionState.setMethodScope(scope);
return sessionState;
@ -52,9 +58,10 @@ public class SessionMethodUtils {
return;
}
CallDepthThreadLocalMap.reset(SessionMethodUtils.class);
final Scope scope = sessionState.getMethodScope();
final Span span = scope.span();
if (span != null) {
if (span != null && sessionState.hasChildSpan) {
DECORATOR.onError(span, throwable);
if (entity != null) {
DECORATOR.onOperation(span, entity);

1
dd-java-agent/instrumentation/hibernate/src/main/java/datadog/trace/instrumentation/hibernate/SessionState.java
View File

@ -9,4 +9,5 @@ import lombok.NonNull;
public class SessionState {
@NonNull public Span sessionSpan;
public Scope methodScope;
public boolean hasChildSpan = true;
}

6
dd-java-agent/instrumentation/hibernate/src/main/java/datadog/trace/instrumentation/hibernate/TransactionInstrumentation.java
View File

@ -34,8 +34,8 @@ public class TransactionInstrumentation extends Instrumenter.Default {
@Override
public String[] helperClassNames() {
return new String[] {
"datadog.trace.instrumentation.hibernate.SessionMethodUtils",
"datadog.trace.instrumentation.hibernate.SessionState",
packageName + ".SessionMethodUtils",
packageName + ".SessionState",
"datadog.trace.agent.decorator.BaseDecorator",
"datadog.trace.agent.decorator.ClientDecorator",
"datadog.trace.agent.decorator.DatabaseClientDecorator",
@ -65,7 +65,7 @@ public class TransactionInstrumentation extends Instrumenter.Default {
InstrumentationContext.get(Transaction.class, SessionState.class);
return SessionMethodUtils.startScopeFrom(
contextStore, transaction, "hibernate.transaction.commit", null);
contextStore, transaction, "hibernate.transaction.commit", null, true);
}
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)

32
dd-java-agent/instrumentation/hibernate/src/test/groovy/QueryTest.groovy
View File

@ -147,7 +147,7 @@ class QueryTest extends AbstractHibernateTest {
expect:
assertTraces(1) {
trace(0, 6) {
trace(0, 4) {
span(0) {
serviceName "hibernate"
resourceName "hibernate.session"
@ -175,32 +175,6 @@ class QueryTest extends AbstractHibernateTest {
}
}
span(2) {
serviceName "hibernate"
resourceName "hibernate.iterator.next"
operationName "hibernate.iterator.next"
spanType DDSpanTypes.HIBERNATE
childOf span(0)
tags {
"$Tags.COMPONENT.key" "java-hibernate"
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
"$DDTags.SPAN_TYPE" DDSpanTypes.HIBERNATE
defaultTags()
}
}
span(3) {
serviceName "hibernate"
resourceName "hibernate.iterator.next"
operationName "hibernate.iterator.next"
spanType DDSpanTypes.HIBERNATE
childOf span(0)
tags {
"$Tags.COMPONENT.key" "java-hibernate"
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
"$DDTags.SPAN_TYPE" DDSpanTypes.HIBERNATE
defaultTags()
}
}
span(4) {
serviceName "hibernate"
resourceName "from Value"
operationName "hibernate.iterate"
@ -213,9 +187,9 @@ class QueryTest extends AbstractHibernateTest {
defaultTags()
}
}
span(5) {
span(3) {
serviceName "h2"
childOf span(4)
childOf span(2)
}
}
}

2
dd-trace-api/src/main/java/datadog/trace/api/DDSpanTypes.java
View File

@ -14,7 +14,7 @@ public class DDSpanTypes {
public static final String REDIS = "redis";
public static final String MEMCACHED = "memcached";
public static final String ELASTICSEARCH = "elasticsearch";
public static final String HIBERNATE = "hibernate"; // TODO: Could this just be "db", or "orm"?
public static final String HIBERNATE = "hibernate";
public static final String MESSAGE_CLIENT = "queue";
public static final String MESSAGE_CONSUMER = "queue";