diff --git a/instrumentation/hibernate/hibernate-3.3/javaagent/build.gradle.kts b/instrumentation/hibernate/hibernate-3.3/javaagent/build.gradle.kts index 9884dbf598a8..e51b69972723 100644 --- a/instrumentation/hibernate/hibernate-3.3/javaagent/build.gradle.kts +++ b/instrumentation/hibernate/hibernate-3.3/javaagent/build.gradle.kts @@ -48,3 +48,8 @@ if (findProperty("testLatestDeps") as Boolean) { } } } + +tasks.withType().configureEach { + // TODO run tests both with and without experimental span attributes + jvmArgs("-Dotel.instrumentation.hibernate.experimental-span-attributes=true") +} diff --git a/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/CriteriaInstrumentation.java b/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/CriteriaInstrumentation.java index 6e2dec0bb6e4..de6c5f4ec905 100644 --- a/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/CriteriaInstrumentation.java +++ b/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/CriteriaInstrumentation.java @@ -7,6 +7,7 @@ import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static io.opentelemetry.javaagent.instrumentation.hibernate.HibernateSingletons.instrumenter; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; @@ -17,7 +18,9 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.CallDepth; -import io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; +import io.opentelemetry.javaagent.instrumentation.hibernate.HibernateOperation; +import io.opentelemetry.javaagent.instrumentation.hibernate.SessionInfo; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -51,33 +54,39 @@ public static void startMethod( @Advice.This Criteria criteria, @Advice.Origin("#m") String name, @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - callDepth = CallDepth.forClass(SessionMethodUtils.class); + callDepth = CallDepth.forClass(HibernateOperation.class); if (callDepth.getAndIncrement() > 0) { return; } - VirtualField virtualField = - VirtualField.find(Criteria.class, Context.class); - String entityName = null; if (criteria instanceof CriteriaImpl) { entityName = ((CriteriaImpl) criteria).getEntityOrClassName(); } - context = - SessionMethodUtils.startSpanFrom(virtualField, criteria, "Criteria." + name, entityName); - if (context != null) { - scope = context.makeCurrent(); + VirtualField criteriaVirtualField = + VirtualField.find(Criteria.class, SessionInfo.class); + SessionInfo sessionInfo = criteriaVirtualField.get(criteria); + + Context parentContext = Java8BytecodeBridge.currentContext(); + hibernateOperation = new HibernateOperation("Criteria." + name, entityName, sessionInfo); + if (!instrumenter().shouldStart(parentContext, hibernateOperation)) { + return; } + + context = instrumenter().start(parentContext, hibernateOperation); + scope = context.makeCurrent(); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void endMethod( @Advice.Thrown Throwable throwable, @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { @@ -87,7 +96,7 @@ public static void endMethod( if (scope != null) { scope.close(); - SessionMethodUtils.end(context, throwable); + instrumenter().end(context, hibernateOperation, null, throwable); } } } diff --git a/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/QueryInstrumentation.java b/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/QueryInstrumentation.java index e42909287551..7b69a2eb62df 100644 --- a/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/QueryInstrumentation.java +++ b/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/QueryInstrumentation.java @@ -7,6 +7,8 @@ import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static io.opentelemetry.javaagent.instrumentation.hibernate.HibernateSingletons.instrumenter; +import static io.opentelemetry.javaagent.instrumentation.hibernate.OperationNameUtil.getOperationNameForQuery; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; @@ -17,7 +19,9 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.CallDepth; -import io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; +import io.opentelemetry.javaagent.instrumentation.hibernate.HibernateOperation; +import io.opentelemetry.javaagent.instrumentation.hibernate.SessionInfo; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -49,26 +53,35 @@ public static class QueryMethodAdvice { public static void startMethod( @Advice.This Query query, @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - callDepth = CallDepth.forClass(SessionMethodUtils.class); + callDepth = CallDepth.forClass(HibernateOperation.class); if (callDepth.getAndIncrement() > 0) { return; } - VirtualField virtualField = VirtualField.find(Query.class, Context.class); + VirtualField criteriaVirtualField = + VirtualField.find(Query.class, SessionInfo.class); + SessionInfo sessionInfo = criteriaVirtualField.get(query); - context = SessionMethodUtils.startSpanFromQuery(virtualField, query, query.getQueryString()); - if (context != null) { - scope = context.makeCurrent(); + Context parentContext = Java8BytecodeBridge.currentContext(); + hibernateOperation = + new HibernateOperation(getOperationNameForQuery(query.getQueryString()), sessionInfo); + if (!instrumenter().shouldStart(parentContext, hibernateOperation)) { + return; } + + context = instrumenter().start(parentContext, hibernateOperation); + scope = context.makeCurrent(); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void endMethod( @Advice.Thrown Throwable throwable, @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { @@ -78,7 +91,7 @@ public static void endMethod( if (scope != null) { scope.close(); - SessionMethodUtils.end(context, throwable); + instrumenter().end(context, hibernateOperation, null, throwable); } } } diff --git a/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionFactoryInstrumentation.java b/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionFactoryInstrumentation.java index 428c91b959e9..52dfa3d09968 100644 --- a/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionFactoryInstrumentation.java +++ b/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionFactoryInstrumentation.java @@ -7,18 +7,16 @@ import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; -import static io.opentelemetry.javaagent.instrumentation.hibernate.HibernateSingletons.instrumenter; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; import static net.bytebuddy.matcher.ElementMatchers.returns; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; -import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.field.VirtualField; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; +import io.opentelemetry.javaagent.instrumentation.hibernate.SessionInfo; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -56,17 +54,14 @@ public static class SessionFactoryAdvice { @Advice.OnMethodExit(suppress = Throwable.class) public static void openSession(@Advice.Return Object session) { - Context parentContext = Java8BytecodeBridge.currentContext(); - Context context = instrumenter().start(parentContext, "Session"); - if (session instanceof Session) { - VirtualField virtualField = - VirtualField.find(Session.class, Context.class); - virtualField.set((Session) session, context); + VirtualField virtualField = + VirtualField.find(Session.class, SessionInfo.class); + virtualField.set((Session) session, new SessionInfo()); } else if (session instanceof StatelessSession) { - VirtualField virtualField = - VirtualField.find(StatelessSession.class, Context.class); - virtualField.set((StatelessSession) session, context); + VirtualField virtualField = + VirtualField.find(StatelessSession.class, SessionInfo.class); + virtualField.set((StatelessSession) session, new SessionInfo()); } } } diff --git a/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionInstrumentation.java b/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionInstrumentation.java index 8f1ed556a40e..352631c2b261 100644 --- a/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionInstrumentation.java +++ b/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionInstrumentation.java @@ -8,14 +8,13 @@ import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; import static io.opentelemetry.javaagent.instrumentation.hibernate.HibernateSingletons.instrumenter; -import static io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils.SCOPE_ONLY_METHODS; -import static io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils.getEntityName; +import static io.opentelemetry.javaagent.instrumentation.hibernate.OperationNameUtil.getEntityName; +import static io.opentelemetry.javaagent.instrumentation.hibernate.OperationNameUtil.getSessionMethodOperationName; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; import static net.bytebuddy.matcher.ElementMatchers.returns; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; -import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; @@ -23,14 +22,14 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.CallDepth; -import io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; +import io.opentelemetry.javaagent.instrumentation.hibernate.HibernateOperation; +import io.opentelemetry.javaagent.instrumentation.hibernate.SessionInfo; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; import org.hibernate.Criteria; import org.hibernate.Query; -import org.hibernate.Session; -import org.hibernate.StatelessSession; import org.hibernate.Transaction; public class SessionInstrumentation implements TypeInstrumentation { @@ -48,10 +47,6 @@ public ElementMatcher typeMatcher() { @Override public void transform(TypeTransformer transformer) { - transformer.applyAdviceToMethod( - isMethod().and(named("close")).and(takesArguments(0)), - SessionInstrumentation.class.getName() + "$SessionCloseAdvice"); - // Session synchronous methods we want to instrument. transformer.applyAdviceToMethod( isMethod() @@ -66,10 +61,7 @@ public void transform(TypeTransformer transformer) { "lock", "refresh", "insert", - "delete", - // Lazy-load methods. - "immediateLoad", - "internalLoad")), + "delete")), SessionInstrumentation.class.getName() + "$SessionMethodAdvice"); // Handle the non-generic 'get' separately. @@ -78,7 +70,7 @@ public void transform(TypeTransformer transformer) { SessionInstrumentation.class.getName() + "$SessionMethodAdvice"); // These methods return some object that we want to instrument, and so the Advice will pin the - // current Span to the returned object using a VirtualField. + // current SessionInfo to the returned object using a VirtualField. transformer.applyAdviceToMethod( isMethod() .and(namedOneOf("beginTransaction", "getTransaction")) @@ -94,31 +86,6 @@ public void transform(TypeTransformer transformer) { SessionInstrumentation.class.getName() + "$GetCriteriaAdvice"); } - @SuppressWarnings("unused") - public static class SessionCloseAdvice { - - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - public static void closeSession( - @Advice.This Object session, @Advice.Thrown Throwable throwable) { - - Context sessionContext = null; - if (session instanceof Session) { - VirtualField virtualField = - VirtualField.find(Session.class, Context.class); - sessionContext = virtualField.get((Session) session); - } else if (session instanceof StatelessSession) { - VirtualField virtualField = - VirtualField.find(StatelessSession.class, Context.class); - sessionContext = virtualField.get((StatelessSession) session); - } - - if (sessionContext == null) { - return; - } - instrumenter().end(sessionContext, null, null, throwable); - } - } - @SuppressWarnings("unused") public static class SessionMethodAdvice { @@ -130,45 +97,35 @@ public static void startMethod( @Advice.Argument(0) Object arg0, @Advice.Argument(value = 1, optional = true) Object arg1, @Advice.Local("otelCallDepth") CallDepth callDepth, - @Advice.Local("otelContext") Context spanContext, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, + @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - callDepth = CallDepth.forClass(SessionMethodUtils.class); + callDepth = CallDepth.forClass(HibernateOperation.class); if (callDepth.getAndIncrement() > 0) { return; } - Context sessionContext = null; - if (session instanceof Session) { - VirtualField virtualField = - VirtualField.find(Session.class, Context.class); - sessionContext = virtualField.get((Session) session); - } else if (session instanceof StatelessSession) { - VirtualField virtualField = - VirtualField.find(StatelessSession.class, Context.class); - sessionContext = virtualField.get((StatelessSession) session); - } - - if (sessionContext == null) { - return; // No state found. We aren't in a Session. + Context parentContext = Java8BytecodeBridge.currentContext(); + SessionInfo sessionInfo = SessionUtil.getSessionInfo(session); + String entityName = + getEntityName(descriptor, arg0, arg1, EntityNameUtil.bestGuessEntityName(session)); + hibernateOperation = + new HibernateOperation(getSessionMethodOperationName(name), entityName, sessionInfo); + if (!instrumenter().shouldStart(parentContext, hibernateOperation)) { + return; } - if (!SCOPE_ONLY_METHODS.contains(name)) { - String entityName = - getEntityName(descriptor, arg0, arg1, EntityNameUtil.bestGuessEntityName(session)); - spanContext = - SessionMethodUtils.startSpanFrom(sessionContext, "Session." + name, entityName); - scope = spanContext.makeCurrent(); - } else { - scope = sessionContext.makeCurrent(); - } + context = instrumenter().start(parentContext, hibernateOperation); + scope = context.makeCurrent(); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void endMethod( @Advice.Thrown Throwable throwable, @Advice.Local("otelCallDepth") CallDepth callDepth, - @Advice.Local("otelContext") Context spanContext, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, + @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { if (callDepth.decrementAndGet() > 0) { @@ -177,7 +134,7 @@ public static void endMethod( if (scope != null) { scope.close(); - SessionMethodUtils.end(spanContext, throwable); + instrumenter().end(context, hibernateOperation, null, throwable); } } } @@ -188,19 +145,10 @@ public static class GetQueryAdvice { @Advice.OnMethodExit(suppress = Throwable.class) public static void getQuery(@Advice.This Object session, @Advice.Return Query query) { - VirtualField queryVirtualField = - VirtualField.find(Query.class, Context.class); - if (session instanceof Session) { - VirtualField sessionVirtualField = - VirtualField.find(Session.class, Context.class); - SessionMethodUtils.attachSpanFromStore( - sessionVirtualField, (Session) session, queryVirtualField, query); - } else if (session instanceof StatelessSession) { - VirtualField sessionVirtualField = - VirtualField.find(StatelessSession.class, Context.class); - SessionMethodUtils.attachSpanFromStore( - sessionVirtualField, (StatelessSession) session, queryVirtualField, query); - } + SessionInfo sessionInfo = SessionUtil.getSessionInfo(session); + VirtualField queryVirtualField = + VirtualField.find(Query.class, SessionInfo.class); + queryVirtualField.set(query, sessionInfo); } } @@ -211,20 +159,10 @@ public static class GetTransactionAdvice { public static void getTransaction( @Advice.This Object session, @Advice.Return Transaction transaction) { - VirtualField transactionVirtualField = - VirtualField.find(Transaction.class, Context.class); - - if (session instanceof Session) { - VirtualField sessionVirtualField = - VirtualField.find(Session.class, Context.class); - SessionMethodUtils.attachSpanFromStore( - sessionVirtualField, (Session) session, transactionVirtualField, transaction); - } else if (session instanceof StatelessSession) { - VirtualField sessionVirtualField = - VirtualField.find(StatelessSession.class, Context.class); - SessionMethodUtils.attachSpanFromStore( - sessionVirtualField, (StatelessSession) session, transactionVirtualField, transaction); - } + SessionInfo sessionInfo = SessionUtil.getSessionInfo(session); + VirtualField transactionVirtualField = + VirtualField.find(Transaction.class, SessionInfo.class); + transactionVirtualField.set(transaction, sessionInfo); } } @@ -234,19 +172,10 @@ public static class GetCriteriaAdvice { @Advice.OnMethodExit(suppress = Throwable.class) public static void getCriteria(@Advice.This Object session, @Advice.Return Criteria criteria) { - VirtualField criteriaVirtualField = - VirtualField.find(Criteria.class, Context.class); - if (session instanceof Session) { - VirtualField sessionVirtualField = - VirtualField.find(Session.class, Context.class); - SessionMethodUtils.attachSpanFromStore( - sessionVirtualField, (Session) session, criteriaVirtualField, criteria); - } else if (session instanceof StatelessSession) { - VirtualField sessionVirtualField = - VirtualField.find(StatelessSession.class, Context.class); - SessionMethodUtils.attachSpanFromStore( - sessionVirtualField, (StatelessSession) session, criteriaVirtualField, criteria); - } + SessionInfo sessionInfo = SessionUtil.getSessionInfo(session); + VirtualField criteriaVirtualField = + VirtualField.find(Criteria.class, SessionInfo.class); + criteriaVirtualField.set(criteria, sessionInfo); } } } diff --git a/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionUtil.java b/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionUtil.java new file mode 100644 index 000000000000..8e3e6905df92 --- /dev/null +++ b/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionUtil.java @@ -0,0 +1,29 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.hibernate.v3_3; + +import io.opentelemetry.instrumentation.api.field.VirtualField; +import io.opentelemetry.javaagent.instrumentation.hibernate.SessionInfo; +import org.hibernate.Session; +import org.hibernate.StatelessSession; + +public final class SessionUtil { + private static final VirtualField sessionVirtualField = + VirtualField.find(Session.class, SessionInfo.class); + private static final VirtualField statelessSessionVirtualField = + VirtualField.find(StatelessSession.class, SessionInfo.class); + + private SessionUtil() {} + + public static SessionInfo getSessionInfo(Object session) { + if (session instanceof Session) { + return sessionVirtualField.get((Session) session); + } else if (session instanceof StatelessSession) { + return statelessSessionVirtualField.get((StatelessSession) session); + } + return null; + } +} diff --git a/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/TransactionInstrumentation.java b/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/TransactionInstrumentation.java index 83b9e17b0357..9bf9d3a6e7f9 100644 --- a/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/TransactionInstrumentation.java +++ b/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/TransactionInstrumentation.java @@ -7,6 +7,7 @@ import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static io.opentelemetry.javaagent.instrumentation.hibernate.HibernateSingletons.instrumenter; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; @@ -17,7 +18,9 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.CallDepth; -import io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; +import io.opentelemetry.javaagent.instrumentation.hibernate.HibernateOperation; +import io.opentelemetry.javaagent.instrumentation.hibernate.SessionInfo; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -49,28 +52,34 @@ public static class TransactionCommitAdvice { public static void startCommit( @Advice.This Transaction transaction, @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - callDepth = CallDepth.forClass(SessionMethodUtils.class); + callDepth = CallDepth.forClass(HibernateOperation.class); if (callDepth.getAndIncrement() > 0) { return; } - VirtualField virtualField = - VirtualField.find(Transaction.class, Context.class); + VirtualField transactionVirtualField = + VirtualField.find(Transaction.class, SessionInfo.class); + SessionInfo sessionInfo = transactionVirtualField.get(transaction); - context = - SessionMethodUtils.startSpanFrom(virtualField, transaction, "Transaction.commit", null); - if (context != null) { - scope = context.makeCurrent(); + Context parentContext = Java8BytecodeBridge.currentContext(); + hibernateOperation = new HibernateOperation("Transaction.commit", sessionInfo); + if (!instrumenter().shouldStart(parentContext, hibernateOperation)) { + return; } + + context = instrumenter().start(parentContext, hibernateOperation); + scope = context.makeCurrent(); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void endCommit( @Advice.Thrown Throwable throwable, @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { @@ -80,7 +89,7 @@ public static void endCommit( if (scope != null) { scope.close(); - SessionMethodUtils.end(context, throwable); + instrumenter().end(context, hibernateOperation, null, throwable); } } } diff --git a/instrumentation/hibernate/hibernate-3.3/javaagent/src/test/groovy/CriteriaTest.groovy b/instrumentation/hibernate/hibernate-3.3/javaagent/src/test/groovy/CriteriaTest.groovy index bab2536bccac..532f37bb045a 100644 --- a/instrumentation/hibernate/hibernate-3.3/javaagent/src/test/groovy/CriteriaTest.groovy +++ b/instrumentation/hibernate/hibernate-3.3/javaagent/src/test/groovy/CriteriaTest.groovy @@ -16,20 +16,23 @@ class CriteriaTest extends AbstractHibernateTest { def "test criteria.#methodName"() { setup: - Session session = sessionFactory.openSession() - session.beginTransaction() - Criteria criteria = session.createCriteria(Value) - .add(Restrictions.like("name", "Hello")) - .addOrder(Order.desc("name")) - interaction.call(criteria) - session.getTransaction().commit() - session.close() + runWithSpan("parent") { + Session session = sessionFactory.openSession() + session.beginTransaction() + Criteria criteria = session.createCriteria(Value) + .add(Restrictions.like("name", "Hello")) + .addOrder(Order.desc("name")) + interaction.call(criteria) + session.getTransaction().commit() + session.close() + } expect: + def sessionId assertTraces(1) { trace(0, 4) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -40,6 +43,10 @@ class CriteriaTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -61,6 +68,7 @@ class CriteriaTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } } diff --git a/instrumentation/hibernate/hibernate-3.3/javaagent/src/test/groovy/QueryTest.groovy b/instrumentation/hibernate/hibernate-3.3/javaagent/src/test/groovy/QueryTest.groovy index ab720c40ac4a..7f30dd4a4106 100644 --- a/instrumentation/hibernate/hibernate-3.3/javaagent/src/test/groovy/QueryTest.groovy +++ b/instrumentation/hibernate/hibernate-3.3/javaagent/src/test/groovy/QueryTest.groovy @@ -16,25 +16,30 @@ class QueryTest extends AbstractHibernateTest { setup: // With Transaction - Session session = sessionFactory.openSession() - session.beginTransaction() - queryInteraction(session) - session.getTransaction().commit() - session.close() + runWithSpan("parent") { + Session session = sessionFactory.openSession() + session.beginTransaction() + queryInteraction(session) + session.getTransaction().commit() + session.close() + } // Without Transaction if (!requiresTransaction) { - session = sessionFactory.openSession() - queryInteraction(session) - session.close() + runWithSpan("parent2") { + Session session = sessionFactory.openSession() + queryInteraction(session) + session.close() + } } expect: + def sessionId assertTraces(requiresTransaction ? 1 : 2) { // With Transaction trace(0, 4) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -45,6 +50,10 @@ class QueryTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -65,6 +74,7 @@ class QueryTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } } @@ -72,7 +82,7 @@ class QueryTest extends AbstractHibernateTest { // Without Transaction trace(1, 3) { span(0) { - name "Session" + name "parent2" kind INTERNAL hasNoParent() attributes { @@ -83,6 +93,7 @@ class QueryTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" String } } span(2) { @@ -132,21 +143,24 @@ class QueryTest extends AbstractHibernateTest { def "test hibernate query.iterate"() { setup: - Session session = sessionFactory.openSession() - session.beginTransaction() - Query q = session.createQuery("from Value") - Iterator it = q.iterate() - while (it.hasNext()) { - it.next() + runWithSpan("parent") { + Session session = sessionFactory.openSession() + session.beginTransaction() + Query q = session.createQuery("from Value") + Iterator iterator = q.iterate() + while (iterator.hasNext()) { + iterator.next() + } + session.getTransaction().commit() + session.close() } - session.getTransaction().commit() - session.close() expect: + def sessionId assertTraces(1) { trace(0, 4) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -157,6 +171,10 @@ class QueryTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -178,6 +196,7 @@ class QueryTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } } diff --git a/instrumentation/hibernate/hibernate-3.3/javaagent/src/test/groovy/SessionTest.groovy b/instrumentation/hibernate/hibernate-3.3/javaagent/src/test/groovy/SessionTest.groovy index 69fb26563927..c5d07a49109b 100644 --- a/instrumentation/hibernate/hibernate-3.3/javaagent/src/test/groovy/SessionTest.groovy +++ b/instrumentation/hibernate/hibernate-3.3/javaagent/src/test/groovy/SessionTest.groovy @@ -28,25 +28,28 @@ class SessionTest extends AbstractHibernateTest { // Test for each implementation of Session. for (def buildSession : sessionImplementations) { - def session = buildSession() - session.beginTransaction() + runWithSpan("parent") { + def session = buildSession() + session.beginTransaction() - try { - sessionMethodTest.call(session, prepopulated.get(0)) - } catch (Exception e) { - // We expected this, we should see the error field set on the span. - } + try { + sessionMethodTest.call(session, prepopulated.get(0)) + } catch (Exception e) { + // We expected this, we should see the error field set on the span. + } - session.getTransaction().commit() - session.close() + session.getTransaction().commit() + session.close() + } } expect: + def sessionId assertTraces(sessionImplementations.size()) { for (int i = 0; i < sessionImplementations.size(); i++) { trace(i, 4) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -57,6 +60,10 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -78,6 +85,7 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } } @@ -100,24 +108,27 @@ class SessionTest extends AbstractHibernateTest { def "test hibernate statless action #testName"() { setup: - // Test for each implementation of Session. - def session = statelessSessionBuilder() - session.beginTransaction() + runWithSpan("parent") { + // Test for each implementation of Session. + def session = statelessSessionBuilder() + session.beginTransaction() - try { - sessionMethodTest.call(session, prepopulated.get(0)) - } catch (Exception e) { - // We expected this, we should see the error field set on the span. - } + try { + sessionMethodTest.call(session, prepopulated.get(0)) + } catch (Exception e) { + // We expected this, we should see the error field set on the span. + } - session.getTransaction().commit() - session.close() + session.getTransaction().commit() + session.close() + } expect: + def sessionId assertTraces(1) { trace(0, 4) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -128,6 +139,10 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -135,6 +150,7 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } span(3) { @@ -174,24 +190,27 @@ class SessionTest extends AbstractHibernateTest { def "test hibernate replicate: #testName"() { setup: - // Test for each implementation of Session. - def session = sessionFactory.openSession() - session.beginTransaction() + runWithSpan("parent") { + // Test for each implementation of Session. + def session = sessionFactory.openSession() + session.beginTransaction() - try { - sessionMethodTest.call(session, prepopulated.get(0)) - } catch (Exception e) { - // We expected this, we should see the error field set on the span. - } + try { + sessionMethodTest.call(session, prepopulated.get(0)) + } catch (Exception e) { + // We expected this, we should see the error field set on the span. + } - session.getTransaction().commit() - session.close() + session.getTransaction().commit() + session.close() + } expect: + def sessionId assertTraces(1) { trace(0, 5) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -202,6 +221,10 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -223,6 +246,7 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } span(4) { @@ -259,24 +283,27 @@ class SessionTest extends AbstractHibernateTest { def "test hibernate failed replicate"() { setup: - // Test for each implementation of Session. - def session = sessionFactory.openSession() - session.beginTransaction() + runWithSpan("parent") { + // Test for each implementation of Session. + def session = sessionFactory.openSession() + session.beginTransaction() - try { - session.replicate(new Long(123) /* Not a valid entity */, ReplicationMode.OVERWRITE) - } catch (Exception e) { - // We expected this, we should see the error field set on the span. - } + try { + session.replicate(new Long(123) /* Not a valid entity */, ReplicationMode.OVERWRITE) + } catch (Exception e) { + // We expected this, we should see the error field set on the span. + } - session.getTransaction().commit() - session.close() + session.getTransaction().commit() + session.close() + } expect: + def sessionId assertTraces(1) { trace(0, 3) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -288,12 +315,19 @@ class SessionTest extends AbstractHibernateTest { childOf span(0) status ERROR errorEvent(MappingException, "Unknown entity: java.lang.Long") + attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } + } } span(2) { name "Transaction.commit" kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } } @@ -305,23 +339,26 @@ class SessionTest extends AbstractHibernateTest { def "test hibernate commit action #testName"() { setup: - def session = sessionBuilder() - session.beginTransaction() + runWithSpan("parent") { + def session = sessionBuilder() + session.beginTransaction() - try { - sessionMethodTest.call(session, prepopulated.get(0)) - } catch (Exception e) { - // We expected this, we should see the error field set on the span. - } + try { + sessionMethodTest.call(session, prepopulated.get(0)) + } catch (Exception e) { + // We expected this, we should see the error field set on the span. + } - session.getTransaction().commit() - session.close() + session.getTransaction().commit() + session.close() + } expect: + def sessionId assertTraces(1) { trace(0, 4) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -332,6 +369,10 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -339,6 +380,7 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } span(3) { @@ -391,18 +433,21 @@ class SessionTest extends AbstractHibernateTest { def "test attaches State to query created via #queryMethodName"() { setup: - Session session = sessionFactory.openSession() - session.beginTransaction() - Query query = queryBuildMethod(session) - query.list() - session.getTransaction().commit() - session.close() + runWithSpan("parent") { + Session session = sessionFactory.openSession() + session.beginTransaction() + Query query = queryBuildMethod(session) + query.list() + session.getTransaction().commit() + session.close() + } expect: + def sessionId assertTraces(1) { trace(0, 4) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -413,6 +458,10 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -433,6 +482,7 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } } @@ -468,45 +518,69 @@ class SessionTest extends AbstractHibernateTest { } expect: + def sessionId1 + def sessionId2 + def sessionId3 assertTraces(1) { - trace(0, 11) { + trace(0, 8) { span(0) { name "overlapping Sessions" attributes { } } span(1) { - name "Session" + name "Session.save Value" kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId1 = it + it instanceof String + } } } span(2) { - name "Session.save Value" + name "Session.insert Value" kind INTERNAL - childOf span(1) + childOf span(0) attributes { + "hibernate.session_id" { + sessionId2 = it + it instanceof String + } } } span(3) { - name "Session.delete Value" + name "Session.save Value" kind INTERNAL - childOf span(1) + childOf span(0) attributes { + "hibernate.session_id" { + sessionId3 = it + it instanceof String + } } } span(4) { - name "Transaction.commit" + name "Session.delete Value" kind INTERNAL - childOf span(1) + childOf span(0) attributes { + "hibernate.session_id" sessionId1 } } span(5) { + name "Transaction.commit" + kind INTERNAL + childOf span(0) + attributes { + "hibernate.session_id" sessionId1 + } + } + span(6) { name "INSERT db1.Value" kind CLIENT - childOf span(4) + childOf span(5) attributes { "${SemanticAttributes.DB_SYSTEM.key}" "h2" "${SemanticAttributes.DB_NAME.key}" "db1" @@ -517,10 +591,10 @@ class SessionTest extends AbstractHibernateTest { "${SemanticAttributes.DB_SQL_TABLE.key}" "Value" } } - span(6) { + span(7) { name "DELETE db1.Value" kind CLIENT - childOf span(4) + childOf span(5) attributes { "${SemanticAttributes.DB_SYSTEM.key}" "h2" "${SemanticAttributes.DB_NAME.key}" "db1" @@ -531,36 +605,9 @@ class SessionTest extends AbstractHibernateTest { "${SemanticAttributes.DB_SQL_TABLE.key}" "Value" } } - span(7) { - name "Session" - kind INTERNAL - childOf span(0) - attributes { - } - } - span(8) { - name "Session.insert Value" - kind INTERNAL - childOf span(7) - attributes { - } - } - span(9) { - name "Session" - kind INTERNAL - childOf span(0) - attributes { - } - } - span(10) { - name "Session.save Value" - kind INTERNAL - childOf span(9) - attributes { - } - } } } + sessionId1 != sessionId2 != sessionId3 } } diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/build.gradle.kts b/instrumentation/hibernate/hibernate-4.0/javaagent/build.gradle.kts index 74daf18ad445..10b7082f50dc 100644 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/build.gradle.kts +++ b/instrumentation/hibernate/hibernate-4.0/javaagent/build.gradle.kts @@ -16,9 +16,6 @@ testSets { create("version5Test") { dirName = "test" } - create("version6Test") { - dirName = "hibernate6Test" - } create("latestDepTest") { dirName = "test" @@ -27,17 +24,9 @@ testSets { tasks { val version5Test by existing(Test::class) - val version6Test by existing(Test::class) { - filter { - // version6 doesn't have all the tests that older versions have - // this here prevents failure when running a test that is missing from version6 from ide - isFailOnNoMatchingTests = false - } - } test { dependsOn(version5Test) - dependsOn(version6Test) } } @@ -67,12 +56,12 @@ dependencies { add("version5TestImplementation", "org.hibernate:hibernate-entitymanager:5.0.0.Final") add("version5TestImplementation", "org.springframework.data:spring-data-jpa:2.3.0.RELEASE") - add("version6TestImplementation", "org.hibernate:hibernate-core:6.0.0.Alpha6") - add("version6TestImplementation", "org.hibernate:hibernate-entitymanager:6.0.0.Alpha6") - add("version6TestImplementation", "org.springframework.data:spring-data-jpa:2.3.0.RELEASE") - - // hibernate 6 is alpha so use 5 as latest version add("latestDepTestImplementation", "org.hibernate:hibernate-core:5.+") add("latestDepTestImplementation", "org.hibernate:hibernate-entitymanager:5.+") add("latestDepTestImplementation", "org.springframework.data:spring-data-jpa:(2.4.0,)") } + +tasks.withType().configureEach { + // TODO run tests both with and without experimental span attributes + jvmArgs("-Dotel.instrumentation.hibernate.experimental-span-attributes=true") +} diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/src/hibernate6Test/groovy/SpringJpaTest.groovy b/instrumentation/hibernate/hibernate-4.0/javaagent/src/hibernate6Test/groovy/SpringJpaTest.groovy deleted file mode 100644 index 484647c7f30a..000000000000 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/src/hibernate6Test/groovy/SpringJpaTest.groovy +++ /dev/null @@ -1,198 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification -import io.opentelemetry.semconv.trace.attributes.SemanticAttributes -import org.springframework.context.annotation.AnnotationConfigApplicationContext -import spock.lang.Shared -import spring.jpa.Customer -import spring.jpa.CustomerRepository -import spring.jpa.PersistenceConfig - -import static io.opentelemetry.api.trace.SpanKind.CLIENT - -/** - * Unfortunately this test verifies that our hibernate instrumentation doesn't currently work with Spring Data Repositories. - */ -class SpringJpaTest extends AgentInstrumentationSpecification { - - @Shared - def context = new AnnotationConfigApplicationContext(PersistenceConfig) - - @Shared - def repo = context.getBean(CustomerRepository) - - def "test CRUD"() { - setup: - def customer = new Customer("Bob", "Anonymous") - - expect: - customer.id == null - !repo.findAll().iterator().hasNext() - - assertTraces(1) { - trace(0, 1) { - span(0) { - name "SELECT test.Customer" - kind CLIENT - attributes { - "${SemanticAttributes.DB_SYSTEM.key}" "hsqldb" - "${SemanticAttributes.DB_NAME.key}" "test" - "${SemanticAttributes.DB_USER.key}" "sa" - "${SemanticAttributes.DB_CONNECTION_STRING.key}" "hsqldb:mem:" - "${SemanticAttributes.DB_STATEMENT.key}" ~/select ([^.]+)\.id([^,]*), ([^.]+)\.firstName([^,]*), ([^.]+)\.lastName(.*)from Customer(.*)/ - "${SemanticAttributes.DB_OPERATION.key}" "SELECT" - "${SemanticAttributes.DB_SQL_TABLE.key}" "Customer" - } - } - } - } - clearExportedData() - - when: - repo.save(customer) - def savedId = customer.id - - then: - customer.id != null - // Behavior changed in new version: - def extraTrace = traces.size() == 2 - assertTraces(extraTrace ? 2 : 1) { - if (extraTrace) { - trace(0, 1) { - span(0) { - name "test" - kind CLIENT - attributes { - "${SemanticAttributes.DB_SYSTEM.key}" "hsqldb" - "${SemanticAttributes.DB_NAME.key}" "test" - "${SemanticAttributes.DB_USER.key}" "sa" - "${SemanticAttributes.DB_STATEMENT.key}" "call next value for hibernate_sequence" - "${SemanticAttributes.DB_CONNECTION_STRING.key}" "hsqldb:mem:" - } - } - } - } - trace(extraTrace ? 1 : 0, 1) { - span(0) { - name "INSERT test.Customer" - kind CLIENT - attributes { - "${SemanticAttributes.DB_SYSTEM.key}" "hsqldb" - "${SemanticAttributes.DB_NAME.key}" "test" - "${SemanticAttributes.DB_USER.key}" "sa" - "${SemanticAttributes.DB_CONNECTION_STRING.key}" "hsqldb:mem:" - "${SemanticAttributes.DB_STATEMENT.key}" ~/insert into Customer \(.*\) values \(.*, \?, \?\)/ - "${SemanticAttributes.DB_OPERATION.key}" "INSERT" - "${SemanticAttributes.DB_SQL_TABLE.key}" "Customer" - } - } - } - } - clearExportedData() - - when: - customer.firstName = "Bill" - repo.save(customer) - - then: - customer.id == savedId - assertTraces(2) { - trace(0, 1) { - span(0) { - name "SELECT test.Customer" - kind CLIENT - attributes { - "${SemanticAttributes.DB_SYSTEM.key}" "hsqldb" - "${SemanticAttributes.DB_NAME.key}" "test" - "${SemanticAttributes.DB_USER.key}" "sa" - "${SemanticAttributes.DB_CONNECTION_STRING.key}" "hsqldb:mem:" - "${SemanticAttributes.DB_STATEMENT.key}" ~/select ([^.]+)\.id([^,]*), ([^.]+)\.firstName([^,]*), ([^.]+)\.lastName (.*)from Customer (.*)where ([^.]+)\.id( ?)=( ?)\?/ - "${SemanticAttributes.DB_OPERATION.key}" "SELECT" - "${SemanticAttributes.DB_SQL_TABLE.key}" "Customer" - } - } - } - trace(1, 1) { - span(0) { - name "UPDATE test.Customer" - kind CLIENT - attributes { - "${SemanticAttributes.DB_SYSTEM.key}" "hsqldb" - "${SemanticAttributes.DB_NAME.key}" "test" - "${SemanticAttributes.DB_USER.key}" "sa" - "${SemanticAttributes.DB_CONNECTION_STRING.key}" "hsqldb:mem:" - "${SemanticAttributes.DB_STATEMENT.key}" "update Customer set firstName=?, lastName=? where id=?" - "${SemanticAttributes.DB_OPERATION.key}" "UPDATE" - "${SemanticAttributes.DB_SQL_TABLE.key}" "Customer" - } - } - } - } - clearExportedData() - - when: - customer = repo.findByLastName("Anonymous")[0] - - then: - customer.id == savedId - customer.firstName == "Bill" - assertTraces(1) { - trace(0, 1) { - span(0) { - name "SELECT test.Customer" - kind CLIENT - attributes { - "${SemanticAttributes.DB_SYSTEM.key}" "hsqldb" - "${SemanticAttributes.DB_NAME.key}" "test" - "${SemanticAttributes.DB_USER.key}" "sa" - "${SemanticAttributes.DB_CONNECTION_STRING.key}" "hsqldb:mem:" - "${SemanticAttributes.DB_STATEMENT.key}" ~/select ([^.]+)\.id([^,]*), ([^.]+)\.firstName([^,]*), ([^.]+)\.lastName (.*)from Customer (.*)(where ([^.]+)\.lastName( ?)=( ?)\?|)/ - "${SemanticAttributes.DB_OPERATION.key}" "SELECT" - "${SemanticAttributes.DB_SQL_TABLE.key}" "Customer" - } - } - } - } - clearExportedData() - - when: - repo.delete(customer) - - then: - assertTraces(2) { - trace(0, 1) { - span(0) { - name "SELECT test.Customer" - kind CLIENT - attributes { - "${SemanticAttributes.DB_SYSTEM.key}" "hsqldb" - "${SemanticAttributes.DB_NAME.key}" "test" - "${SemanticAttributes.DB_USER.key}" "sa" - "${SemanticAttributes.DB_CONNECTION_STRING.key}" "hsqldb:mem:" - "${SemanticAttributes.DB_STATEMENT.key}" ~/select ([^.]+)\.id([^,]*), ([^.]+)\.firstName([^,]*), ([^.]+)\.lastName (.*)from Customer (.*)where ([^.]+)\.id( ?)=( ?)\?/ - "${SemanticAttributes.DB_OPERATION.key}" "SELECT" - "${SemanticAttributes.DB_SQL_TABLE.key}" "Customer" - } - } - } - trace(1, 1) { - span(0) { - name "DELETE test.Customer" - kind CLIENT - attributes { - "${SemanticAttributes.DB_SYSTEM.key}" "hsqldb" - "${SemanticAttributes.DB_NAME.key}" "test" - "${SemanticAttributes.DB_USER.key}" "sa" - "${SemanticAttributes.DB_CONNECTION_STRING.key}" "hsqldb:mem:" - "${SemanticAttributes.DB_STATEMENT.key}" "delete from Customer where id=?" - "${SemanticAttributes.DB_OPERATION.key}" "DELETE" - "${SemanticAttributes.DB_SQL_TABLE.key}" "Customer" - } - } - } - } - } -} diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/src/hibernate6Test/java/Value.java b/instrumentation/hibernate/hibernate-4.0/javaagent/src/hibernate6Test/java/Value.java deleted file mode 100644 index f89b450965e6..000000000000 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/src/hibernate6Test/java/Value.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -import javax.persistence.Entity; -import javax.persistence.GeneratedValue; -import javax.persistence.Id; -import javax.persistence.Table; -import org.hibernate.annotations.GenericGenerator; -import org.hibernate.annotations.NamedQuery; - -@Entity -@Table -@NamedQuery(name = "TestNamedQuery", query = "FROM Value") -public class Value { - - private Long id; - private String name; - - public Value() {} - - public Value(String name) { - this.name = name; - } - - @Id - @GeneratedValue(generator = "increment") - @GenericGenerator(name = "increment", strategy = "increment") - public Long getId() { - return id; - } - - public void setId(Long id) { - this.id = id; - } - - public String getName() { - return name; - } - - public void setName(String title) { - name = title; - } -} diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/src/hibernate6Test/java/spring/jpa/Customer.java b/instrumentation/hibernate/hibernate-4.0/javaagent/src/hibernate6Test/java/spring/jpa/Customer.java deleted file mode 100644 index 27e7d6894055..000000000000 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/src/hibernate6Test/java/spring/jpa/Customer.java +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package spring.jpa; - -import java.util.Objects; -import javax.persistence.Entity; -import javax.persistence.GeneratedValue; -import javax.persistence.GenerationType; -import javax.persistence.Id; - -@Entity -public class Customer { - - @Id - @GeneratedValue(strategy = GenerationType.AUTO) - private Long id; - - private String firstName; - private String lastName; - - protected Customer() {} - - public Customer(String firstName, String lastName) { - this.firstName = firstName; - this.lastName = lastName; - } - - public Long getId() { - return id; - } - - public void setId(Long id) { - this.id = id; - } - - public String getFirstName() { - return firstName; - } - - public void setFirstName(String firstName) { - this.firstName = firstName; - } - - public String getLastName() { - return lastName; - } - - public void setLastName(String lastName) { - this.lastName = lastName; - } - - @Override - public String toString() { - return String.format("Customer[id=%d, firstName='%s', lastName='%s']", id, firstName, lastName); - } - - @Override - public boolean equals(Object obj) { - if (obj == this) { - return true; - } - if (!(obj instanceof Customer)) { - return false; - } - Customer other = (Customer) obj; - return Objects.equals(id, other.id) - && Objects.equals(firstName, other.firstName) - && Objects.equals(lastName, other.lastName); - } - - @Override - public int hashCode() { - return Objects.hash(id, firstName, lastName); - } -} diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/src/hibernate6Test/java/spring/jpa/CustomerRepository.java b/instrumentation/hibernate/hibernate-4.0/javaagent/src/hibernate6Test/java/spring/jpa/CustomerRepository.java deleted file mode 100644 index 9662f2a4b59c..000000000000 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/src/hibernate6Test/java/spring/jpa/CustomerRepository.java +++ /dev/null @@ -1,14 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package spring.jpa; - -import java.util.List; -import org.springframework.data.repository.CrudRepository; - -public interface CustomerRepository extends CrudRepository { - - List findByLastName(String lastName); -} diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/src/hibernate6Test/java/spring/jpa/PersistenceConfig.java b/instrumentation/hibernate/hibernate-4.0/javaagent/src/hibernate6Test/java/spring/jpa/PersistenceConfig.java deleted file mode 100644 index d161e7526a58..000000000000 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/src/hibernate6Test/java/spring/jpa/PersistenceConfig.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package spring.jpa; - -import java.util.Properties; -import javax.sql.DataSource; -import org.springframework.context.annotation.Bean; -import org.springframework.data.jpa.repository.config.EnableJpaRepositories; -import org.springframework.jdbc.datasource.DriverManagerDataSource; -import org.springframework.orm.jpa.JpaTransactionManager; -import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean; -import org.springframework.orm.jpa.vendor.Database; -import org.springframework.orm.jpa.vendor.HibernateJpaVendorAdapter; -import org.springframework.transaction.PlatformTransactionManager; - -@EnableJpaRepositories(basePackages = "spring/jpa") -public class PersistenceConfig { - - @Bean(name = "transactionManager") - public PlatformTransactionManager dbTransactionManager() { - JpaTransactionManager transactionManager = new JpaTransactionManager(); - transactionManager.setEntityManagerFactory(entityManagerFactory().getObject()); - return transactionManager; - } - - @Bean - public LocalContainerEntityManagerFactoryBean entityManagerFactory() { - - HibernateJpaVendorAdapter vendorAdapter = new HibernateJpaVendorAdapter(); - vendorAdapter.setDatabase(Database.HSQL); - vendorAdapter.setGenerateDdl(true); - - LocalContainerEntityManagerFactoryBean em = new LocalContainerEntityManagerFactoryBean(); - em.setDataSource(dataSource()); - em.setPackagesToScan("spring/jpa"); - em.setJpaVendorAdapter(vendorAdapter); - em.setJpaProperties(additionalProperties()); - - return em; - } - - @Bean - public DataSource dataSource() { - DriverManagerDataSource dataSource = new DriverManagerDataSource(); - dataSource.setDriverClassName("org.hsqldb.jdbcDriver"); - dataSource.setUrl("jdbc:hsqldb:mem:test"); - dataSource.setUsername("sa"); - dataSource.setPassword("1"); - return dataSource; - } - - private static Properties additionalProperties() { - Properties properties = new Properties(); - properties.setProperty("hibernate.show_sql", "true"); - properties.setProperty("hibernate.hbm2ddl.auto", "create"); - properties.setProperty("hibernate.dialect", "org.hibernate.dialect.HSQLDialect"); - return properties; - } -} diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/CriteriaInstrumentation.java b/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/CriteriaInstrumentation.java index ac278cb785c3..7ba304b52502 100644 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/CriteriaInstrumentation.java +++ b/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/CriteriaInstrumentation.java @@ -7,6 +7,7 @@ import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static io.opentelemetry.javaagent.instrumentation.hibernate.HibernateSingletons.instrumenter; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; @@ -17,7 +18,9 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.CallDepth; -import io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; +import io.opentelemetry.javaagent.instrumentation.hibernate.HibernateOperation; +import io.opentelemetry.javaagent.instrumentation.hibernate.SessionInfo; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -51,33 +54,39 @@ public static void startMethod( @Advice.This Criteria criteria, @Advice.Origin("#m") String name, @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - callDepth = CallDepth.forClass(SessionMethodUtils.class); + callDepth = CallDepth.forClass(HibernateOperation.class); if (callDepth.getAndIncrement() > 0) { return; } - VirtualField virtualField = - VirtualField.find(Criteria.class, Context.class); - String entityName = null; if (criteria instanceof CriteriaImpl) { entityName = ((CriteriaImpl) criteria).getEntityOrClassName(); } - context = - SessionMethodUtils.startSpanFrom(virtualField, criteria, "Criteria." + name, entityName); - if (context != null) { - scope = context.makeCurrent(); + VirtualField criteriaVirtualField = + VirtualField.find(Criteria.class, SessionInfo.class); + SessionInfo sessionInfo = criteriaVirtualField.get(criteria); + + Context parentContext = Java8BytecodeBridge.currentContext(); + hibernateOperation = new HibernateOperation("Criteria." + name, entityName, sessionInfo); + if (!instrumenter().shouldStart(parentContext, hibernateOperation)) { + return; } + + context = instrumenter().start(parentContext, hibernateOperation); + scope = context.makeCurrent(); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void endMethod( @Advice.Thrown Throwable throwable, @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { @@ -87,7 +96,7 @@ public static void endMethod( if (scope != null) { scope.close(); - SessionMethodUtils.end(context, throwable); + instrumenter().end(context, hibernateOperation, null, throwable); } } } diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/QueryInstrumentation.java b/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/QueryInstrumentation.java index 3d9888d11395..4b665f9dfa4a 100644 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/QueryInstrumentation.java +++ b/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/QueryInstrumentation.java @@ -7,6 +7,8 @@ import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static io.opentelemetry.javaagent.instrumentation.hibernate.HibernateSingletons.instrumenter; +import static io.opentelemetry.javaagent.instrumentation.hibernate.OperationNameUtil.getOperationNameForQuery; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; @@ -17,7 +19,9 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.CallDepth; -import io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; +import io.opentelemetry.javaagent.instrumentation.hibernate.HibernateOperation; +import io.opentelemetry.javaagent.instrumentation.hibernate.SessionInfo; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -49,26 +53,35 @@ public static class QueryMethodAdvice { public static void startMethod( @Advice.This Query query, @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - callDepth = CallDepth.forClass(SessionMethodUtils.class); + callDepth = CallDepth.forClass(HibernateOperation.class); if (callDepth.getAndIncrement() > 0) { return; } - VirtualField virtualField = VirtualField.find(Query.class, Context.class); + VirtualField criteriaVirtualField = + VirtualField.find(Query.class, SessionInfo.class); + SessionInfo sessionInfo = criteriaVirtualField.get(query); - context = SessionMethodUtils.startSpanFromQuery(virtualField, query, query.getQueryString()); - if (context != null) { - scope = context.makeCurrent(); + Context parentContext = Java8BytecodeBridge.currentContext(); + hibernateOperation = + new HibernateOperation(getOperationNameForQuery(query.getQueryString()), sessionInfo); + if (!instrumenter().shouldStart(parentContext, hibernateOperation)) { + return; } + + context = instrumenter().start(parentContext, hibernateOperation); + scope = context.makeCurrent(); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void endMethod( @Advice.Thrown Throwable throwable, @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { @@ -78,7 +91,7 @@ public static void endMethod( if (scope != null) { scope.close(); - SessionMethodUtils.end(context, throwable); + instrumenter().end(context, hibernateOperation, null, throwable); } } } diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/SessionFactoryInstrumentation.java b/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/SessionFactoryInstrumentation.java index 65a2c53754fe..be53cbae5c06 100644 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/SessionFactoryInstrumentation.java +++ b/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/SessionFactoryInstrumentation.java @@ -7,18 +7,16 @@ import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; -import static io.opentelemetry.javaagent.instrumentation.hibernate.HibernateSingletons.instrumenter; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; import static net.bytebuddy.matcher.ElementMatchers.returns; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; -import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.field.VirtualField; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; +import io.opentelemetry.javaagent.instrumentation.hibernate.SessionInfo; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -53,12 +51,9 @@ public static class SessionFactoryAdvice { @Advice.OnMethodExit(suppress = Throwable.class) public static void openSession(@Advice.Return SharedSessionContract session) { - Context parentContext = Java8BytecodeBridge.currentContext(); - Context context = instrumenter().start(parentContext, "Session"); - - VirtualField virtualField = - VirtualField.find(SharedSessionContract.class, Context.class); - virtualField.set(session, context); + VirtualField virtualField = + VirtualField.find(SharedSessionContract.class, SessionInfo.class); + virtualField.set(session, new SessionInfo()); } } } diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/SessionInstrumentation.java b/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/SessionInstrumentation.java index a2f780af8c89..00c6b50b1b0c 100644 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/SessionInstrumentation.java +++ b/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/SessionInstrumentation.java @@ -8,15 +8,13 @@ import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; import static io.opentelemetry.javaagent.instrumentation.hibernate.HibernateSingletons.instrumenter; -import static io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils.SCOPE_ONLY_METHODS; -import static io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils.getEntityName; -import static io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils.getSessionMethodSpanName; +import static io.opentelemetry.javaagent.instrumentation.hibernate.OperationNameUtil.getEntityName; +import static io.opentelemetry.javaagent.instrumentation.hibernate.OperationNameUtil.getSessionMethodOperationName; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; import static net.bytebuddy.matcher.ElementMatchers.returns; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; -import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; @@ -24,7 +22,9 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.CallDepth; -import io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; +import io.opentelemetry.javaagent.instrumentation.hibernate.HibernateOperation; +import io.opentelemetry.javaagent.instrumentation.hibernate.SessionInfo; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -47,9 +47,6 @@ public ElementMatcher typeMatcher() { @Override public void transform(TypeTransformer transformer) { - transformer.applyAdviceToMethod( - isMethod().and(named("close")).and(takesArguments(0)), - SessionInstrumentation.class.getName() + "$SessionCloseAdvice"); // Session synchronous methods we want to instrument. transformer.applyAdviceToMethod( @@ -66,10 +63,7 @@ public void transform(TypeTransformer transformer) { "fireLock", "refresh", "insert", - "delete", - // Lazy-load methods. - "immediateLoad", - "internalLoad")), + "delete")), SessionInstrumentation.class.getName() + "$SessionMethodAdvice"); // Handle the non-generic 'get' separately. transformer.applyAdviceToMethod( @@ -80,7 +74,7 @@ public void transform(TypeTransformer transformer) { SessionInstrumentation.class.getName() + "$SessionMethodAdvice"); // These methods return some object that we want to instrument, and so the Advice will pin the - // current Span to the returned object using a VirtualField. + // current SessionInfo to the returned object using a VirtualField. transformer.applyAdviceToMethod( isMethod() .and(namedOneOf("beginTransaction", "getTransaction")) @@ -96,23 +90,6 @@ public void transform(TypeTransformer transformer) { SessionInstrumentation.class.getName() + "$GetCriteriaAdvice"); } - @SuppressWarnings("unused") - public static class SessionCloseAdvice { - - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - public static void closeSession( - @Advice.This SharedSessionContract session, @Advice.Thrown Throwable throwable) { - - VirtualField virtualField = - VirtualField.find(SharedSessionContract.class, Context.class); - Context sessionContext = virtualField.get(session); - if (sessionContext == null) { - return; - } - instrumenter().end(sessionContext, null, null, throwable); - } - } - @SuppressWarnings("unused") public static class SessionMethodAdvice { @@ -124,39 +101,38 @@ public static void startMethod( @Advice.Argument(0) Object arg0, @Advice.Argument(value = 1, optional = true) Object arg1, @Advice.Local("otelCallDepth") CallDepth callDepth, - @Advice.Local("otelContext") Context spanContext, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, + @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - callDepth = CallDepth.forClass(SessionMethodUtils.class); + callDepth = CallDepth.forClass(HibernateOperation.class); if (callDepth.getAndIncrement() > 0) { return; } - VirtualField virtualField = - VirtualField.find(SharedSessionContract.class, Context.class); - Context sessionContext = virtualField.get(session); + VirtualField virtualField = + VirtualField.find(SharedSessionContract.class, SessionInfo.class); + SessionInfo sessionInfo = virtualField.get(session); - if (sessionContext == null) { - return; // No state found. We aren't in a Session. + Context parentContext = Java8BytecodeBridge.currentContext(); + String entityName = + getEntityName(descriptor, arg0, arg1, EntityNameUtil.bestGuessEntityName(session)); + hibernateOperation = + new HibernateOperation(getSessionMethodOperationName(name), entityName, sessionInfo); + if (!instrumenter().shouldStart(parentContext, hibernateOperation)) { + return; } - if (!SCOPE_ONLY_METHODS.contains(name)) { - String entityName = - getEntityName(descriptor, arg0, arg1, EntityNameUtil.bestGuessEntityName(session)); - spanContext = - SessionMethodUtils.startSpanFrom( - sessionContext, getSessionMethodSpanName(name), entityName); - scope = spanContext.makeCurrent(); - } else { - scope = sessionContext.makeCurrent(); - } + context = instrumenter().start(parentContext, hibernateOperation); + scope = context.makeCurrent(); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void endMethod( @Advice.Thrown Throwable throwable, @Advice.Local("otelCallDepth") CallDepth callDepth, - @Advice.Local("otelContext") Context spanContext, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, + @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { if (callDepth.decrementAndGet() > 0) { @@ -165,7 +141,7 @@ public static void endMethod( if (scope != null) { scope.close(); - SessionMethodUtils.end(spanContext, throwable); + instrumenter().end(context, hibernateOperation, null, throwable); } } } @@ -177,13 +153,12 @@ public static class GetQueryAdvice { public static void getQuery( @Advice.This SharedSessionContract session, @Advice.Return Query query) { - VirtualField sessionVirtualField = - VirtualField.find(SharedSessionContract.class, Context.class); - VirtualField queryVirtualField = - VirtualField.find(Query.class, Context.class); + VirtualField sessionVirtualField = + VirtualField.find(SharedSessionContract.class, SessionInfo.class); + VirtualField queryVirtualField = + VirtualField.find(Query.class, SessionInfo.class); - SessionMethodUtils.attachSpanFromStore( - sessionVirtualField, session, queryVirtualField, query); + queryVirtualField.set(query, sessionVirtualField.get(session)); } } @@ -194,13 +169,12 @@ public static class GetTransactionAdvice { public static void getTransaction( @Advice.This SharedSessionContract session, @Advice.Return Transaction transaction) { - VirtualField sessionVirtualField = - VirtualField.find(SharedSessionContract.class, Context.class); - VirtualField transactionVirtualField = - VirtualField.find(Transaction.class, Context.class); + VirtualField sessionVirtualField = + VirtualField.find(SharedSessionContract.class, SessionInfo.class); + VirtualField transactionVirtualField = + VirtualField.find(Transaction.class, SessionInfo.class); - SessionMethodUtils.attachSpanFromStore( - sessionVirtualField, session, transactionVirtualField, transaction); + transactionVirtualField.set(transaction, sessionVirtualField.get(session)); } } @@ -211,13 +185,12 @@ public static class GetCriteriaAdvice { public static void getCriteria( @Advice.This SharedSessionContract session, @Advice.Return Criteria criteria) { - VirtualField sessionVirtualField = - VirtualField.find(SharedSessionContract.class, Context.class); - VirtualField criteriaVirtualField = - VirtualField.find(Criteria.class, Context.class); + VirtualField sessionVirtualField = + VirtualField.find(SharedSessionContract.class, SessionInfo.class); + VirtualField criteriaVirtualField = + VirtualField.find(Criteria.class, SessionInfo.class); - SessionMethodUtils.attachSpanFromStore( - sessionVirtualField, session, criteriaVirtualField, criteria); + criteriaVirtualField.set(criteria, sessionVirtualField.get(session)); } } } diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/TransactionInstrumentation.java b/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/TransactionInstrumentation.java index 6f3e84b0c88f..afa220da4c4b 100644 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/TransactionInstrumentation.java +++ b/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/TransactionInstrumentation.java @@ -7,6 +7,7 @@ import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static io.opentelemetry.javaagent.instrumentation.hibernate.HibernateSingletons.instrumenter; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; @@ -17,7 +18,9 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.CallDepth; -import io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; +import io.opentelemetry.javaagent.instrumentation.hibernate.HibernateOperation; +import io.opentelemetry.javaagent.instrumentation.hibernate.SessionInfo; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -49,28 +52,34 @@ public static class TransactionCommitAdvice { public static void startCommit( @Advice.This Transaction transaction, @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - callDepth = CallDepth.forClass(SessionMethodUtils.class); + callDepth = CallDepth.forClass(HibernateOperation.class); if (callDepth.getAndIncrement() > 0) { return; } - VirtualField virtualField = - VirtualField.find(Transaction.class, Context.class); + VirtualField transactionVirtualField = + VirtualField.find(Transaction.class, SessionInfo.class); + SessionInfo sessionInfo = transactionVirtualField.get(transaction); - context = - SessionMethodUtils.startSpanFrom(virtualField, transaction, "Transaction.commit", null); - if (context != null) { - scope = context.makeCurrent(); + Context parentContext = Java8BytecodeBridge.currentContext(); + hibernateOperation = new HibernateOperation("Transaction.commit", sessionInfo); + if (!instrumenter().shouldStart(parentContext, hibernateOperation)) { + return; } + + context = instrumenter().start(parentContext, hibernateOperation); + scope = context.makeCurrent(); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void endCommit( @Advice.Thrown Throwable throwable, @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { @@ -80,7 +89,7 @@ public static void endCommit( if (scope != null) { scope.close(); - SessionMethodUtils.end(context, throwable); + instrumenter().end(context, hibernateOperation, null, throwable); } } } diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/CriteriaTest.groovy b/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/CriteriaTest.groovy index bab2536bccac..532f37bb045a 100644 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/CriteriaTest.groovy +++ b/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/CriteriaTest.groovy @@ -16,20 +16,23 @@ class CriteriaTest extends AbstractHibernateTest { def "test criteria.#methodName"() { setup: - Session session = sessionFactory.openSession() - session.beginTransaction() - Criteria criteria = session.createCriteria(Value) - .add(Restrictions.like("name", "Hello")) - .addOrder(Order.desc("name")) - interaction.call(criteria) - session.getTransaction().commit() - session.close() + runWithSpan("parent") { + Session session = sessionFactory.openSession() + session.beginTransaction() + Criteria criteria = session.createCriteria(Value) + .add(Restrictions.like("name", "Hello")) + .addOrder(Order.desc("name")) + interaction.call(criteria) + session.getTransaction().commit() + session.close() + } expect: + def sessionId assertTraces(1) { trace(0, 4) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -40,6 +43,10 @@ class CriteriaTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -61,6 +68,7 @@ class CriteriaTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } } diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/EntityManagerTest.groovy b/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/EntityManagerTest.groovy index 32fc11965f47..3aa56eb7789e 100644 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/EntityManagerTest.groovy +++ b/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/EntityManagerTest.groovy @@ -38,21 +38,24 @@ class EntityManagerTest extends AbstractHibernateTest { } when: - try { - sessionMethodTest.call(entityManager, entity) - } catch (Exception e) { - // We expected this, we should see the error field set on the span. - } + runWithSpan("parent") { + try { + sessionMethodTest.call(entityManager, entity) + } catch (Exception e) { + // We expected this, we should see the error field set on the span. + } - entityTransaction.commit() - entityManager.close() + entityTransaction.commit() + entityManager.close() + } then: boolean isPersistTest = "persist" == testName + def sessionId assertTraces(1) { trace(0, 4 + (isPersistTest ? 1 : 0)) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -63,6 +66,10 @@ class EntityManagerTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } @@ -105,6 +112,7 @@ class EntityManagerTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } } else { @@ -113,6 +121,7 @@ class EntityManagerTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } span(3 + offset) { @@ -158,19 +167,22 @@ class EntityManagerTest extends AbstractHibernateTest { @Unroll def "test attaches State to query created via #queryMethodName"() { setup: - EntityManager entityManager = entityManagerFactory.createEntityManager() - EntityTransaction entityTransaction = entityManager.getTransaction() - entityTransaction.begin() - Query query = queryBuildMethod(entityManager) - query.getResultList() - entityTransaction.commit() - entityManager.close() + runWithSpan("parent") { + EntityManager entityManager = entityManagerFactory.createEntityManager() + EntityTransaction entityTransaction = entityManager.getTransaction() + entityTransaction.begin() + Query query = queryBuildMethod(entityManager) + query.getResultList() + entityTransaction.commit() + entityManager.close() + } expect: + def sessionId assertTraces(1) { trace(0, 4) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -181,6 +193,10 @@ class EntityManagerTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -202,6 +218,7 @@ class EntityManagerTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } } diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/QueryTest.groovy b/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/QueryTest.groovy index f21847900866..b8e5939b746a 100644 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/QueryTest.groovy +++ b/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/QueryTest.groovy @@ -16,25 +16,30 @@ class QueryTest extends AbstractHibernateTest { setup: // With Transaction - Session session = sessionFactory.openSession() - session.beginTransaction() - queryInteraction(session) - session.getTransaction().commit() - session.close() + runWithSpan("parent") { + Session session = sessionFactory.openSession() + session.beginTransaction() + queryInteraction(session) + session.getTransaction().commit() + session.close() + } // Without Transaction if (!requiresTransaction) { - session = sessionFactory.openSession() - queryInteraction(session) - session.close() + runWithSpan("parent2") { + Session session = sessionFactory.openSession() + queryInteraction(session) + session.close() + } } expect: + def sessionId assertTraces(requiresTransaction ? 1 : 2) { // With Transaction trace(0, 4) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -45,6 +50,10 @@ class QueryTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -65,6 +74,7 @@ class QueryTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } } @@ -72,7 +82,7 @@ class QueryTest extends AbstractHibernateTest { // Without Transaction trace(1, 3) { span(0) { - name "Session" + name "parent2" kind INTERNAL hasNoParent() attributes { @@ -83,6 +93,7 @@ class QueryTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" String } } span(2) { @@ -132,21 +143,24 @@ class QueryTest extends AbstractHibernateTest { def "test hibernate query.iterate"() { setup: - Session session = sessionFactory.openSession() - session.beginTransaction() - Query q = session.createQuery("from Value") - Iterator it = q.iterate() - while (it.hasNext()) { - it.next() + runWithSpan("parent") { + Session session = sessionFactory.openSession() + session.beginTransaction() + Query q = session.createQuery("from Value") + Iterator iterator = q.iterate() + while (iterator.hasNext()) { + iterator.next() + } + session.getTransaction().commit() + session.close() } - session.getTransaction().commit() - session.close() expect: + def sessionId assertTraces(1) { trace(0, 4) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -157,6 +171,10 @@ class QueryTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -178,6 +196,7 @@ class QueryTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } } diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/SessionTest.groovy b/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/SessionTest.groovy index d55c9e34ee6b..643c8bec5990 100644 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/SessionTest.groovy +++ b/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/SessionTest.groovy @@ -29,25 +29,28 @@ class SessionTest extends AbstractHibernateTest { // Test for each implementation of Session. for (def buildSession : sessionImplementations) { - def session = buildSession() - session.beginTransaction() + runWithSpan("parent") { + def session = buildSession() + session.beginTransaction() + + try { + sessionMethodTest.call(session, prepopulated.get(0)) + } catch (Exception e) { + // We expected this, we should see the error field set on the span. + } - try { - sessionMethodTest.call(session, prepopulated.get(0)) - } catch (Exception e) { - // We expected this, we should see the error field set on the span. + session.getTransaction().commit() + session.close() } - - session.getTransaction().commit() - session.close() } expect: + def sessionId assertTraces(sessionImplementations.size()) { for (int i = 0; i < sessionImplementations.size(); i++) { trace(i, 4) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -58,6 +61,10 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -78,6 +85,7 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } } @@ -142,23 +150,26 @@ class SessionTest extends AbstractHibernateTest { setup: // Test for each implementation of Session. - def session = sessionFactory.openSession() - session.beginTransaction() + runWithSpan("parent") { + def session = sessionFactory.openSession() + session.beginTransaction() - try { - sessionMethodTest.call(session, prepopulated.get(0)) - } catch (Exception e) { - // We expected this, we should see the error field set on the span. - } + try { + sessionMethodTest.call(session, prepopulated.get(0)) + } catch (Exception e) { + // We expected this, we should see the error field set on the span. + } - session.getTransaction().commit() - session.close() + session.getTransaction().commit() + session.close() + } expect: + def sessionId assertTraces(1) { trace(0, 5) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -169,6 +180,10 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -190,6 +205,7 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } span(4) { @@ -227,23 +243,26 @@ class SessionTest extends AbstractHibernateTest { setup: // Test for each implementation of Session. - def session = sessionFactory.openSession() - session.beginTransaction() + runWithSpan("parent") { + def session = sessionFactory.openSession() + session.beginTransaction() - try { - session.replicate(new Long(123) /* Not a valid entity */, ReplicationMode.OVERWRITE) - } catch (Exception e) { - // We expected this, we should see the error field set on the span. - } + try { + session.replicate(new Long(123) /* Not a valid entity */, ReplicationMode.OVERWRITE) + } catch (Exception e) { + // We expected this, we should see the error field set on the span. + } - session.getTransaction().commit() - session.close() + session.getTransaction().commit() + session.close() + } expect: + def sessionId assertTraces(1) { trace(0, 3) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -255,12 +274,19 @@ class SessionTest extends AbstractHibernateTest { childOf span(0) status ERROR errorEvent(MappingException, "Unknown entity: java.lang.Long") + attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } + } } span(2) { name "Transaction.commit" kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } } @@ -272,23 +298,26 @@ class SessionTest extends AbstractHibernateTest { def "test hibernate commit action #testName"() { setup: - def session = sessionBuilder() - session.beginTransaction() + runWithSpan("parent") { + def session = sessionBuilder() + session.beginTransaction() - try { - sessionMethodTest.call(session, prepopulated.get(0)) - } catch (Exception e) { - // We expected this, we should see the error field set on the span. - } + try { + sessionMethodTest.call(session, prepopulated.get(0)) + } catch (Exception e) { + // We expected this, we should see the error field set on the span. + } - session.getTransaction().commit() - session.close() + session.getTransaction().commit() + session.close() + } expect: + def sessionId assertTraces(1) { trace(0, 4) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -299,6 +328,10 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -306,6 +339,7 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } span(3) { @@ -378,18 +412,21 @@ class SessionTest extends AbstractHibernateTest { def "test attaches State to query created via #queryMethodName"() { setup: - Session session = sessionFactory.openSession() - session.beginTransaction() - Query query = queryBuildMethod(session) - query.list() - session.getTransaction().commit() - session.close() + runWithSpan("parent") { + Session session = sessionFactory.openSession() + session.beginTransaction() + Query query = queryBuildMethod(session) + query.list() + session.getTransaction().commit() + session.close() + } expect: + def sessionId assertTraces(1) { trace(0, 4) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -400,6 +437,10 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -420,6 +461,7 @@ class SessionTest extends AbstractHibernateTest { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } } @@ -455,45 +497,42 @@ class SessionTest extends AbstractHibernateTest { } expect: + def sessionId1 + def sessionId2 + def sessionId3 assertTraces(1) { - trace(0, 12) { + trace(0, 9) { span(0) { name "overlapping Sessions" attributes { } } span(1) { - name "Session" + name "Session.save Value" kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId1 = it + it instanceof String + } } } span(2) { - name "Session.save Value" + name "Session.insert Value" kind INTERNAL - childOf span(1) + childOf span(0) attributes { + "hibernate.session_id" { + sessionId2 = it + it instanceof String + } } } span(3) { - name "Session.delete Value" - kind INTERNAL - childOf span(1) - attributes { - } - } - span(4) { - name "Transaction.commit" - kind INTERNAL - childOf span(1) - attributes { - } - } - span(5) { name "INSERT db1.Value" kind CLIENT - childOf span(4) + childOf span(2) attributes { "${SemanticAttributes.DB_SYSTEM.key}" "h2" "${SemanticAttributes.DB_NAME.key}" "db1" @@ -504,38 +543,37 @@ class SessionTest extends AbstractHibernateTest { "${SemanticAttributes.DB_SQL_TABLE.key}" "Value" } } - span(6) { - name "DELETE db1.Value" - kind CLIENT - childOf span(4) + span(4) { + name "Session.save Value" + kind INTERNAL + childOf span(0) attributes { - "${SemanticAttributes.DB_SYSTEM.key}" "h2" - "${SemanticAttributes.DB_NAME.key}" "db1" - "${SemanticAttributes.DB_USER.key}" "sa" - "${SemanticAttributes.DB_CONNECTION_STRING.key}" "h2:mem:" - "${SemanticAttributes.DB_STATEMENT.key}" ~/^delete / - "${SemanticAttributes.DB_OPERATION.key}" "DELETE" - "${SemanticAttributes.DB_SQL_TABLE.key}" "Value" + "hibernate.session_id" { + sessionId3 = it + it instanceof String + } } } - span(7) { - name "Session" + span(5) { + name "Session.delete Value" kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId1 } } - span(8) { - name "Session.insert Value" + span(6) { + name "Transaction.commit" kind INTERNAL - childOf span(7) + childOf span(0) attributes { + "hibernate.session_id" sessionId1 } } - span(9) { + span(7) { name "INSERT db1.Value" kind CLIENT - childOf span(8) + childOf span(6) attributes { "${SemanticAttributes.DB_SYSTEM.key}" "h2" "${SemanticAttributes.DB_NAME.key}" "db1" @@ -546,22 +584,23 @@ class SessionTest extends AbstractHibernateTest { "${SemanticAttributes.DB_SQL_TABLE.key}" "Value" } } - span(10) { - name "Session" - kind INTERNAL - childOf span(0) - attributes { - } - } - span(11) { - name "Session.save Value" - kind INTERNAL - childOf span(10) + span(8) { + name "DELETE db1.Value" + kind CLIENT + childOf span(6) attributes { + "${SemanticAttributes.DB_SYSTEM.key}" "h2" + "${SemanticAttributes.DB_NAME.key}" "db1" + "${SemanticAttributes.DB_USER.key}" "sa" + "${SemanticAttributes.DB_CONNECTION_STRING.key}" "h2:mem:" + "${SemanticAttributes.DB_STATEMENT.key}" ~/^delete / + "${SemanticAttributes.DB_OPERATION.key}" "DELETE" + "${SemanticAttributes.DB_SQL_TABLE.key}" "Value" } } } } + sessionId1 != sessionId2 != sessionId3 } } diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/SpringJpaTest.groovy b/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/SpringJpaTest.groovy index 1ac531502361..804d8d0353e3 100644 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/SpringJpaTest.groovy +++ b/instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/SpringJpaTest.groovy @@ -30,12 +30,15 @@ class SpringJpaTest extends AgentInstrumentationSpecification { expect: customer.id == null - !repo.findAll().iterator().hasNext() + !runWithSpan("parent") { + repo.findAll().iterator().hasNext() + } + def sessionId assertTraces(1) { trace(0, 4) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -46,6 +49,10 @@ class SpringJpaTest extends AgentInstrumentationSpecification { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -67,6 +74,7 @@ class SpringJpaTest extends AgentInstrumentationSpecification { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } } @@ -74,15 +82,18 @@ class SpringJpaTest extends AgentInstrumentationSpecification { clearExportedData() when: - repo.save(customer) + runWithSpan("parent") { + repo.save(customer) + } def savedId = customer.id then: customer.id != null + def sessionId2 assertTraces(1) { trace(0, 4 + (isHibernate4 ? 0 : 1)) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -93,6 +104,10 @@ class SpringJpaTest extends AgentInstrumentationSpecification { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId2 = it + it instanceof String + } } } if (!isHibernate4) { @@ -113,6 +128,7 @@ class SpringJpaTest extends AgentInstrumentationSpecification { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId2 } } span(4) { @@ -149,6 +165,7 @@ class SpringJpaTest extends AgentInstrumentationSpecification { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId2 } } } @@ -158,14 +175,17 @@ class SpringJpaTest extends AgentInstrumentationSpecification { when: customer.firstName = "Bill" - repo.save(customer) + runWithSpan("parent") { + repo.save(customer) + } then: customer.id == savedId + def sessionId3 assertTraces(1) { trace(0, 5) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -176,6 +196,10 @@ class SpringJpaTest extends AgentInstrumentationSpecification { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId3 = it + it instanceof String + } } } span(2) { @@ -196,6 +220,7 @@ class SpringJpaTest extends AgentInstrumentationSpecification { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId3 } } span(4) { @@ -216,7 +241,9 @@ class SpringJpaTest extends AgentInstrumentationSpecification { clearExportedData() when: - customer = repo.findByLastName("Anonymous")[0] + customer = runWithSpan("parent") { + repo.findByLastName("Anonymous")[0] + } then: customer.id == savedId @@ -224,7 +251,7 @@ class SpringJpaTest extends AgentInstrumentationSpecification { assertTraces(1) { trace(0, 3) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -235,6 +262,7 @@ class SpringJpaTest extends AgentInstrumentationSpecification { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" String } } span(2) { @@ -256,13 +284,15 @@ class SpringJpaTest extends AgentInstrumentationSpecification { clearExportedData() when: - repo.delete(customer) + runWithSpan("parent") { + repo.delete(customer) + } then: assertTraces(1) { trace(0, 6 + (isHibernate4 ? 0 : 1)) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -276,6 +306,7 @@ class SpringJpaTest extends AgentInstrumentationSpecification { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" String } } span(2) { @@ -298,6 +329,7 @@ class SpringJpaTest extends AgentInstrumentationSpecification { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" String } } if (isHibernate4) { @@ -322,6 +354,7 @@ class SpringJpaTest extends AgentInstrumentationSpecification { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" String } } span(3 + offset) { @@ -329,6 +362,7 @@ class SpringJpaTest extends AgentInstrumentationSpecification { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" String } } span(4 + offset) { diff --git a/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/HibernateExperimentalAttributesExtractor.java b/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/HibernateExperimentalAttributesExtractor.java new file mode 100644 index 000000000000..e576ca0f3c9f --- /dev/null +++ b/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/HibernateExperimentalAttributesExtractor.java @@ -0,0 +1,29 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.hibernate; + +import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import javax.annotation.Nullable; + +class HibernateExperimentalAttributesExtractor + implements AttributesExtractor { + + @Override + public void onStart(AttributesBuilder attributes, HibernateOperation hibernateOperation) { + String sessionId = hibernateOperation.getSessionId(); + if (sessionId != null) { + attributes.put("hibernate.session_id", sessionId); + } + } + + @Override + public void onEnd( + AttributesBuilder attributes, + HibernateOperation hibernateOperation, + @Nullable Void unused, + @Nullable Throwable error) {} +} diff --git a/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/HibernateOperation.java b/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/HibernateOperation.java new file mode 100644 index 000000000000..13adea2bcd12 --- /dev/null +++ b/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/HibernateOperation.java @@ -0,0 +1,35 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.hibernate; + +public class HibernateOperation { + private final String spanName; + private final String sessionId; + + public HibernateOperation(String operation, String entityName, SessionInfo sessionInfo) { + this(spanNameForOperation(operation, entityName), sessionInfo); + } + + public HibernateOperation(String operation, SessionInfo sessionInfo) { + this.spanName = operation; + this.sessionId = sessionInfo != null ? sessionInfo.getSessionId() : null; + } + + public String getName() { + return spanName; + } + + public String getSessionId() { + return sessionId; + } + + private static String spanNameForOperation(String operationName, String entityName) { + if (entityName != null) { + return operationName + " " + entityName; + } + return operationName; + } +} diff --git a/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/HibernateSingletons.java b/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/HibernateSingletons.java index 3ed4b05fedfb..65c44eb669fe 100644 --- a/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/HibernateSingletons.java +++ b/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/HibernateSingletons.java @@ -6,20 +6,32 @@ package io.opentelemetry.javaagent.instrumentation.hibernate; import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.instrumentation.api.config.Config; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder; public class HibernateSingletons { - private static final Instrumenter INSTANCE; + static final boolean CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES = + Config.get().getBoolean("otel.instrumentation.hibernate.experimental-span-attributes", false); + + private static final Instrumenter INSTANCE; static { - INSTANCE = - Instrumenter.builder( - GlobalOpenTelemetry.get(), "io.opentelemetry.hibernate-common", s -> s) - .newInstrumenter(); + InstrumenterBuilder instrumenterBuilder = + Instrumenter.builder( + GlobalOpenTelemetry.get(), + "io.opentelemetry.hibernate-common", + HibernateOperation::getName); + + if (CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES) { + instrumenterBuilder.addAttributesExtractor(new HibernateExperimentalAttributesExtractor()); + } + + INSTANCE = instrumenterBuilder.newInstrumenter(); } - public static Instrumenter instrumenter() { + public static Instrumenter instrumenter() { return INSTANCE; } } diff --git a/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/OperationNameUtil.java b/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/OperationNameUtil.java new file mode 100644 index 000000000000..199eeeb4aa97 --- /dev/null +++ b/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/OperationNameUtil.java @@ -0,0 +1,58 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.hibernate; + +import io.opentelemetry.instrumentation.api.db.SqlStatementInfo; +import io.opentelemetry.instrumentation.api.db.SqlStatementSanitizer; +import java.util.function.Function; + +public final class OperationNameUtil { + + public static String getOperationNameForQuery(String query) { + // set operation to default value that is used when sql sanitizer fails to extract + // operation name + String operation = "Hibernate Query"; + SqlStatementInfo info = SqlStatementSanitizer.sanitize(query); + if (info.getOperation() != null) { + operation = info.getOperation(); + if (info.getTable() != null) { + operation += " " + info.getTable(); + } + } + return operation; + } + + public static String getSessionMethodOperationName(String methodName) { + if ("fireLock".equals(methodName)) { + return "Session.lock"; + } + return "Session." + methodName; + } + + public static String getEntityName( + String descriptor, Object arg0, Object arg1, Function nameFromEntity) { + String entityName = null; + // methods like save(String entityName, Object object) + // that take entity name as first argument and entity as second + // if given entity name is null compute it from entity object + if (descriptor.startsWith("(Ljava/lang/String;Ljava/lang/Object;")) { + entityName = arg0 == null ? nameFromEntity.apply(arg1) : (String) arg0; + // methods like save(Object obj) + } else if (descriptor.startsWith("(Ljava/lang/Object;")) { + entityName = nameFromEntity.apply(arg0); + // methods like get(String entityName, Serializable id) + } else if (descriptor.startsWith("(Ljava/lang/String;")) { + entityName = (String) arg0; + // methods like get(Class entityClass, Serializable id) + } else if (descriptor.startsWith("(Ljava/lang/Class;") && arg0 != null) { + entityName = ((Class) arg0).getName(); + } + + return entityName; + } + + private OperationNameUtil() {} +} diff --git a/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/SessionInfo.java b/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/SessionInfo.java new file mode 100644 index 000000000000..e363e5e10ec4 --- /dev/null +++ b/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/SessionInfo.java @@ -0,0 +1,30 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.hibernate; + +import static io.opentelemetry.javaagent.instrumentation.hibernate.HibernateSingletons.CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES; + +import java.util.UUID; + +public class SessionInfo { + private final String sessionId; + + public SessionInfo() { + sessionId = generateSessionId(); + } + + public String getSessionId() { + return sessionId; + } + + private static String generateSessionId() { + if (!CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES) { + return null; + } + + return UUID.randomUUID().toString(); + } +} diff --git a/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/SessionMethodUtils.java b/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/SessionMethodUtils.java deleted file mode 100644 index 6b59e9b1d211..000000000000 --- a/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/SessionMethodUtils.java +++ /dev/null @@ -1,133 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.hibernate; - -import static io.opentelemetry.javaagent.instrumentation.hibernate.HibernateSingletons.instrumenter; - -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.db.SqlStatementInfo; -import io.opentelemetry.instrumentation.api.db.SqlStatementSanitizer; -import io.opentelemetry.instrumentation.api.field.VirtualField; -import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; -import java.util.function.Function; -import java.util.function.Supplier; -import javax.annotation.Nullable; - -public final class SessionMethodUtils { - - public static final Set SCOPE_ONLY_METHODS = - new HashSet<>(Arrays.asList("immediateLoad", "internalLoad")); - - public static Context startSpanFrom( - VirtualField virtualField, - TARGET spanKey, - String operationName, - String entityName) { - return startSpanFrom(virtualField, spanKey, () -> operationName, entityName); - } - - private static Context startSpanFrom( - VirtualField virtualField, - TARGET spanKey, - Supplier operationNameSupplier, - String entityName) { - - Context sessionContext = virtualField.get(spanKey); - if (sessionContext == null) { - return null; // No state found. We aren't in a Session. - } - - return startSpanFrom(sessionContext, operationNameSupplier.get(), entityName); - } - - public static Context startSpanFrom( - Context sessionContext, String operationName, String entityName) { - return instrumenter().start(sessionContext, spanNameForOperation(operationName, entityName)); - } - - private static String spanNameForOperation(String operationName, String entityName) { - if (entityName != null) { - return operationName + " " + entityName; - } - return operationName; - } - - public static Context startSpanFromQuery( - VirtualField virtualField, TARGET spanKey, String query) { - Supplier operationNameSupplier = - () -> { - // set operation to default value that is used when sql sanitizer fails to extract - // operation name - String operation = "Hibernate Query"; - SqlStatementInfo info = SqlStatementSanitizer.sanitize(query); - if (info.getOperation() != null) { - operation = info.getOperation(); - if (info.getTable() != null) { - operation += " " + info.getTable(); - } - } - return operation; - }; - return startSpanFrom(virtualField, spanKey, operationNameSupplier, null); - } - - public static void end(@Nullable Context context, Throwable throwable) { - if (context == null) { - return; - } - - instrumenter().end(context, null, null, throwable); - } - - // Copies a span from the given Session VirtualField into the targetVirtualField. Used to - // propagate a Span from a Session to transient Session objects such as Transaction and Query. - public static void attachSpanFromStore( - VirtualField sourceVirtualField, - S source, - VirtualField targetVirtualField, - T target) { - - Context sessionContext = sourceVirtualField.get(source); - if (sessionContext == null) { - return; - } - - targetVirtualField.set(target, sessionContext); - } - - public static String getSessionMethodSpanName(String methodName) { - if ("fireLock".equals(methodName)) { - return "Session.lock"; - } - return "Session." + methodName; - } - - public static String getEntityName( - String descriptor, Object arg0, Object arg1, Function nameFromEntity) { - String entityName = null; - // methods like save(String entityName, Object object) - // that take entity name as first argument and entity as second - // if given entity name is null compute it from entity object - if (descriptor.startsWith("(Ljava/lang/String;Ljava/lang/Object;")) { - entityName = arg0 == null ? nameFromEntity.apply(arg1) : (String) arg0; - // methods like save(Object obj) - } else if (descriptor.startsWith("(Ljava/lang/Object;")) { - entityName = nameFromEntity.apply(arg0); - // methods like get(String entityName, Serializable id) - } else if (descriptor.startsWith("(Ljava/lang/String;")) { - entityName = (String) arg0; - // methods like get(Class entityClass, Serializable id) - } else if (descriptor.startsWith("(Ljava/lang/Class;") && arg0 != null) { - entityName = ((Class) arg0).getName(); - } - - return entityName; - } - - private SessionMethodUtils() {} -} diff --git a/instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/build.gradle.kts b/instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/build.gradle.kts index 2d401c846c3b..314c2478f9ec 100644 --- a/instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/build.gradle.kts +++ b/instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/build.gradle.kts @@ -31,3 +31,8 @@ dependencies { latestDepTestLibrary("org.hibernate:hibernate-core:5.+") latestDepTestLibrary("org.hibernate:hibernate-entitymanager:5.+") } + +tasks.withType().configureEach { + // TODO run tests both with and without experimental span attributes + jvmArgs("-Dotel.instrumentation.hibernate.experimental-span-attributes=true") +} diff --git a/instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_3/ProcedureCallInstrumentation.java b/instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_3/ProcedureCallInstrumentation.java index 1002ead1a691..4ecc2ae3574d 100644 --- a/instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_3/ProcedureCallInstrumentation.java +++ b/instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_3/ProcedureCallInstrumentation.java @@ -7,6 +7,7 @@ import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static io.opentelemetry.javaagent.instrumentation.hibernate.HibernateSingletons.instrumenter; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -16,7 +17,9 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.CallDepth; -import io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; +import io.opentelemetry.javaagent.instrumentation.hibernate.HibernateOperation; +import io.opentelemetry.javaagent.instrumentation.hibernate.SessionInfo; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -49,29 +52,35 @@ public static void startMethod( @Advice.This ProcedureCall call, @Advice.Origin("#m") String name, @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - callDepth = CallDepth.forClass(SessionMethodUtils.class); + callDepth = CallDepth.forClass(HibernateOperation.class); if (callDepth.getAndIncrement() > 0) { return; } - VirtualField virtualField = - VirtualField.find(ProcedureCall.class, Context.class); + VirtualField criteriaVirtualField = + VirtualField.find(ProcedureCall.class, SessionInfo.class); + SessionInfo sessionInfo = criteriaVirtualField.get(call); - context = - SessionMethodUtils.startSpanFrom( - virtualField, call, "ProcedureCall." + name, call.getProcedureName()); - if (context != null) { - scope = context.makeCurrent(); + Context parentContext = Java8BytecodeBridge.currentContext(); + hibernateOperation = + new HibernateOperation("ProcedureCall." + name, call.getProcedureName(), sessionInfo); + if (!instrumenter().shouldStart(parentContext, hibernateOperation)) { + return; } + + context = instrumenter().start(parentContext, hibernateOperation); + scope = context.makeCurrent(); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void endMethod( @Advice.Thrown Throwable throwable, @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelHibernateOperation") HibernateOperation hibernateOperation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { @@ -81,7 +90,7 @@ public static void endMethod( if (scope != null) { scope.close(); - SessionMethodUtils.end(context, throwable); + instrumenter().end(context, hibernateOperation, null, throwable); } } } diff --git a/instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_3/SessionInstrumentation.java b/instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_3/SessionInstrumentation.java index 0e77d3c4095a..ca27740a0a5d 100644 --- a/instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_3/SessionInstrumentation.java +++ b/instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_3/SessionInstrumentation.java @@ -11,11 +11,10 @@ import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.returns; -import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.field.VirtualField; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils; +import io.opentelemetry.javaagent.instrumentation.hibernate.SessionInfo; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -49,13 +48,12 @@ public static class GetProcedureCallAdvice { public static void getProcedureCall( @Advice.This SharedSessionContract session, @Advice.Return ProcedureCall returned) { - VirtualField sessionVirtualField = - VirtualField.find(SharedSessionContract.class, Context.class); - VirtualField returnedVirtualField = - VirtualField.find(ProcedureCall.class, Context.class); + VirtualField sessionVirtualField = + VirtualField.find(SharedSessionContract.class, SessionInfo.class); + VirtualField returnedVirtualField = + VirtualField.find(ProcedureCall.class, SessionInfo.class); - SessionMethodUtils.attachSpanFromStore( - sessionVirtualField, session, returnedVirtualField, returned); + returnedVirtualField.set(returned, sessionVirtualField.get(session)); } } } diff --git a/instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/src/test/groovy/ProcedureCallTest.groovy b/instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/src/test/groovy/ProcedureCallTest.groovy index 272695456466..a8a36a588ae5 100644 --- a/instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/src/test/groovy/ProcedureCallTest.groovy +++ b/instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/src/test/groovy/ProcedureCallTest.groovy @@ -70,20 +70,23 @@ class ProcedureCallTest extends AgentInstrumentationSpecification { def "test ProcedureCall"() { setup: - Session session = sessionFactory.openSession() - session.beginTransaction() + runWithSpan("parent") { + Session session = sessionFactory.openSession() + session.beginTransaction() - ProcedureCall call = session.createStoredProcedureCall("TEST_PROC") - callProcedure(call) + ProcedureCall call = session.createStoredProcedureCall("TEST_PROC") + callProcedure(call) - session.getTransaction().commit() - session.close() + session.getTransaction().commit() + session.close() + } expect: + def sessionId assertTraces(1) { trace(0, 4) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -94,6 +97,10 @@ class ProcedureCallTest extends AgentInstrumentationSpecification { kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } } } span(2) { @@ -113,6 +120,7 @@ class ProcedureCallTest extends AgentInstrumentationSpecification { name "Transaction.commit" childOf span(0) attributes { + "hibernate.session_id" sessionId } } } @@ -122,27 +130,30 @@ class ProcedureCallTest extends AgentInstrumentationSpecification { def "test failing ProcedureCall"() { setup: - Session session = sessionFactory.openSession() - session.beginTransaction() + runWithSpan("parent") { + Session session = sessionFactory.openSession() + session.beginTransaction() + + ProcedureCall call = session.createStoredProcedureCall("TEST_PROC") + def parameterRegistration = call.registerParameter("nonexistent", Long, ParameterMode.IN) + Assumptions.assumeTrue(parameterRegistration.metaClass.getMetaMethod("bindValue", Object) != null) + parameterRegistration.bindValue(420L) + try { + callProcedure(call) + } catch (Exception e) { + // We expected this. + } - ProcedureCall call = session.createStoredProcedureCall("TEST_PROC") - def parameterRegistration = call.registerParameter("nonexistent", Long, ParameterMode.IN) - Assumptions.assumeTrue(parameterRegistration.metaClass.getMetaMethod("bindValue", Object) != null) - parameterRegistration.bindValue(420L) - try { - callProcedure(call) - } catch (Exception e) { - // We expected this. + session.getTransaction().commit() + session.close() } - session.getTransaction().commit() - session.close() - expect: + def sessionId assertTraces(1) { trace(0, 3) { span(0) { - name "Session" + name "parent" kind INTERNAL hasNoParent() attributes { @@ -154,12 +165,19 @@ class ProcedureCallTest extends AgentInstrumentationSpecification { childOf span(0) status ERROR errorEvent(SQLGrammarException, "could not prepare statement") + attributes { + "hibernate.session_id" { + sessionId = it + it instanceof String + } + } } span(2) { name "Transaction.commit" kind INTERNAL childOf span(0) attributes { + "hibernate.session_id" sessionId } } }