Review comments and additional testing

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

@@ -1,6 +1,8 @@
 package datadog.trace.instrumentation.hibernate;
 
 import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
+import static datadog.trace.instrumentation.hibernate.HibernateInstrumentation.INSTRUMENTATION_NAME;
+import static java.util.Collections.singletonMap;
 import static net.bytebuddy.matcher.ElementMatchers.isInterface;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -10,8 +12,6 @@ import com.google.auto.service.AutoService;
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.bootstrap.ContextStore;
 import datadog.trace.bootstrap.InstrumentationContext;
-import java.util.Collections;
-import java.util.HashMap;
 import java.util.Map;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.method.MethodDescription;
@@ -24,14 +24,12 @@ import org.hibernate.Criteria;
 public class CriteriaInstrumentation extends Instrumenter.Default {
 
   public CriteriaInstrumentation() {
-    super("hibernate");
+    super(INSTRUMENTATION_NAME);
   }
 
   @Override
   public Map<String, String> contextStore() {
-    final Map<String, String> map = new HashMap<>();
+    return singletonMap("org.hibernate.Criteria", SessionState.class.getName());
-    map.put("org.hibernate.Criteria", SessionState.class.getName());
-    return Collections.unmodifiableMap(map);
   }
 
   @Override
@@ -54,12 +52,9 @@ public class CriteriaInstrumentation extends Instrumenter.Default {
 
   @Override
   public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
-    final Map<ElementMatcher<? super MethodDescription>, String> transformers = new HashMap<>();
+    return singletonMap(
-    transformers.put(
         isMethod().and(named("list").or(named("uniqueResult")).or(named("scroll"))),
         CriteriaMethodAdvice.class.getName());
-
-    return transformers;
   }
 
   public static class CriteriaMethodAdvice {
@@ -77,7 +72,6 @@ public class CriteriaInstrumentation extends Instrumenter.Default {
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void endMethod(
-        @Advice.This final Criteria criteria,
         @Advice.Enter final SessionState state,
         @Advice.Thrown final Throwable throwable,
         @Advice.Return(typing = Assigner.Typing.DYNAMIC) final Object entity) {

dd-java-agent/instrumentation/hibernate/src/main/java/datadog/trace/instrumentation/hibernate/HibernateDecorator.java

@@ -1,5 +1,7 @@
 package datadog.trace.instrumentation.hibernate;
 
+import static datadog.trace.instrumentation.hibernate.HibernateInstrumentation.INSTRUMENTATION_NAME;
+
 import datadog.trace.agent.decorator.OrmClientDecorator;
 import datadog.trace.api.DDSpanTypes;
 import java.util.List;
@@ -15,7 +17,7 @@ public class HibernateDecorator extends OrmClientDecorator {
 
   @Override
   protected String[] instrumentationNames() {
-    return new String[] {"hibernate"};
+    return new String[] {INSTRUMENTATION_NAME};
   }
 
   @Override

dd-java-agent/instrumentation/hibernate/src/main/java/datadog/trace/instrumentation/hibernate/HibernateInstrumentation.java

@@ -0,0 +1,5 @@
+package datadog.trace.instrumentation.hibernate;
+
+public class HibernateInstrumentation {
+  public static final String INSTRUMENTATION_NAME = "hibernate-core";
+}

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

@@ -1,6 +1,8 @@
 package datadog.trace.instrumentation.hibernate;
 
 import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
+import static datadog.trace.instrumentation.hibernate.HibernateInstrumentation.INSTRUMENTATION_NAME;
+import static java.util.Collections.singletonMap;
 import static net.bytebuddy.matcher.ElementMatchers.isInterface;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -10,8 +12,6 @@ import com.google.auto.service.AutoService;
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.bootstrap.ContextStore;
 import datadog.trace.bootstrap.InstrumentationContext;
-import java.util.Collections;
-import java.util.HashMap;
 import java.util.Map;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.method.MethodDescription;
@@ -24,14 +24,12 @@ import org.hibernate.engine.HibernateIterator;
 public class IteratorInstrumentation extends Instrumenter.Default {
 
   public IteratorInstrumentation() {
-    super("hibernate");
+    super(INSTRUMENTATION_NAME);
   }
 
   @Override
   public Map<String, String> contextStore() {
-    final Map<String, String> map = new HashMap<>();
+    return singletonMap("org.hibernate.engine.HibernateIterator", SessionState.class.getName());
-    map.put("org.hibernate.engine.HibernateIterator", SessionState.class.getName());
-    return Collections.unmodifiableMap(map);
   }
 
   @Override
@@ -55,10 +53,8 @@ public class IteratorInstrumentation extends Instrumenter.Default {
 
   @Override
   public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
-    final Map<ElementMatcher<? super MethodDescription>, String> transformers = new HashMap<>();
+    return singletonMap(
-    transformers.put(
         isMethod().and(named("next").or(named("remove"))), IteratorAdvice.class.getName());
-    return transformers;
   }
 
   public static class IteratorAdvice {
@@ -76,7 +72,6 @@ public class IteratorInstrumentation extends Instrumenter.Default {
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void endMethod(
-        @Advice.This final HibernateIterator iterator,
         @Advice.Enter final SessionState state,
         @Advice.Thrown final Throwable throwable,
         @Advice.Return(typing = Assigner.Typing.DYNAMIC) final Object entity) {

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

@@ -1,6 +1,8 @@
 package datadog.trace.instrumentation.hibernate;
 
 import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
+import static datadog.trace.instrumentation.hibernate.HibernateInstrumentation.INSTRUMENTATION_NAME;
+import static java.util.Collections.singletonMap;
 import static net.bytebuddy.matcher.ElementMatchers.isInterface;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -10,8 +12,6 @@ import com.google.auto.service.AutoService;
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.bootstrap.ContextStore;
 import datadog.trace.bootstrap.InstrumentationContext;
-import java.util.Collections;
-import java.util.HashMap;
 import java.util.Map;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.method.MethodDescription;
@@ -23,14 +23,12 @@ import org.hibernate.procedure.ProcedureCall;
 public class ProcedureCallInstrumentation extends Instrumenter.Default {
 
   public ProcedureCallInstrumentation() {
-    super("hibernate");
+    super(INSTRUMENTATION_NAME);
   }
 
   @Override
   public Map<String, String> contextStore() {
-    final Map<String, String> map = new HashMap<>();
+    return singletonMap("org.hibernate.procedure.ProcedureCall", SessionState.class.getName());
-    map.put("org.hibernate.procedure.ProcedureCall", SessionState.class.getName());
-    return Collections.unmodifiableMap(map);
   }
 
   @Override
@@ -53,11 +51,8 @@ public class ProcedureCallInstrumentation extends Instrumenter.Default {
 
   @Override
   public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
-    final Map<ElementMatcher<? super MethodDescription>, String> transformers = new HashMap<>();
+    return singletonMap(
-    transformers.put(
         isMethod().and(named("getOutputs")), ProcedureCallMethodAdvice.class.getName());
-
-    return transformers;
   }
 
   public static class ProcedureCallMethodAdvice {

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

@@ -2,6 +2,8 @@ package datadog.trace.instrumentation.hibernate;
 
 import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
 import static datadog.trace.instrumentation.hibernate.HibernateDecorator.DECORATOR;
+import static datadog.trace.instrumentation.hibernate.HibernateInstrumentation.INSTRUMENTATION_NAME;
+import static java.util.Collections.singletonMap;
 import static net.bytebuddy.matcher.ElementMatchers.isInterface;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -11,8 +13,6 @@ import com.google.auto.service.AutoService;
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.bootstrap.ContextStore;
 import datadog.trace.bootstrap.InstrumentationContext;
-import java.util.Collections;
-import java.util.HashMap;
 import java.util.Map;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.method.MethodDescription;
@@ -26,14 +26,12 @@ import org.hibernate.SQLQuery;
 public class QueryInstrumentation extends Instrumenter.Default {
 
   public QueryInstrumentation() {
-    super("hibernate");
+    super(INSTRUMENTATION_NAME);
   }
 
   @Override
   public Map<String, String> contextStore() {
-    final Map<String, String> map = new HashMap<>();
+    return singletonMap("org.hibernate.Query", SessionState.class.getName());
-    map.put("org.hibernate.Query", SessionState.class.getName());
-    return Collections.unmodifiableMap(map);
   }
 
   @Override
@@ -56,8 +54,7 @@ public class QueryInstrumentation extends Instrumenter.Default {
 
   @Override
   public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
-    final Map<ElementMatcher<? super MethodDescription>, String> transformers = new HashMap<>();
+    return singletonMap(
-    transformers.put(
         isMethod()
             .and(
                 named("list")
@@ -65,8 +62,6 @@ public class QueryInstrumentation extends Instrumenter.Default {
                     .or(named("uniqueResult"))
                     .or(named("scroll"))),
         QueryMethodAdvice.class.getName());
-
-    return transformers;
   }
 
   public static class QueryMethodAdvice {

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

@@ -2,6 +2,8 @@ package datadog.trace.instrumentation.hibernate;
 
 import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
 import static datadog.trace.instrumentation.hibernate.HibernateDecorator.DECORATOR;
+import static datadog.trace.instrumentation.hibernate.HibernateInstrumentation.INSTRUMENTATION_NAME;
+import static java.util.Collections.singletonMap;
 import static net.bytebuddy.matcher.ElementMatchers.isInterface;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -15,8 +17,6 @@ import datadog.trace.bootstrap.ContextStore;
 import datadog.trace.bootstrap.InstrumentationContext;
 import io.opentracing.Span;
 import io.opentracing.util.GlobalTracer;
-import java.util.Collections;
-import java.util.HashMap;
 import java.util.Map;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.method.MethodDescription;
@@ -28,14 +28,12 @@ import org.hibernate.SharedSessionContract;
 public class SessionFactoryInstrumentation extends Instrumenter.Default {
 
   public SessionFactoryInstrumentation() {
-    super("hibernate");
+    super(INSTRUMENTATION_NAME);
   }
 
   @Override
   public Map<String, String> contextStore() {
-    final Map<String, String> map = new HashMap<>();
+    return singletonMap("org.hibernate.SharedSessionContract", SessionState.class.getName());
-    map.put("org.hibernate.SharedSessionContract", SessionState.class.getName());
-    return Collections.unmodifiableMap(map);
   }
 
   @Override
@@ -57,10 +55,7 @@ public class SessionFactoryInstrumentation extends Instrumenter.Default {
 
   @Override
   public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
-    final Map<ElementMatcher<? super MethodDescription>, String> transformers = new HashMap<>();
+    return singletonMap(
-    // Session lifecycle. A span will cover openSession->Session.close, but no scope will be
-    // generated.
-    transformers.put(
         isMethod()
             .and(named("openSession").or(named("openStatelessSession")))
             .and(takesArguments(0))
@@ -68,8 +63,6 @@ public class SessionFactoryInstrumentation extends Instrumenter.Default {
                 returns(
                     named("org.hibernate.Session").or(named("org.hibernate.StatelessSession")))),
         SessionFactoryAdvice.class.getName());
-
-    return transformers;
   }
 
   public static class SessionFactoryAdvice {

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

@@ -2,6 +2,7 @@ package datadog.trace.instrumentation.hibernate;
 
 import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
 import static datadog.trace.instrumentation.hibernate.HibernateDecorator.DECORATOR;
+import static datadog.trace.instrumentation.hibernate.HibernateInstrumentation.INSTRUMENTATION_NAME;
 import static net.bytebuddy.matcher.ElementMatchers.isInterface;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -34,7 +35,7 @@ import org.hibernate.procedure.ProcedureCall;
 public class SessionInstrumentation extends Instrumenter.Default {
 
   public SessionInstrumentation() {
-    super("hibernate");
+    super(INSTRUMENTATION_NAME);
   }
 
   @Override
@@ -93,7 +94,7 @@ public class SessionInstrumentation extends Instrumenter.Default {
                     .or(named("immediateLoad"))
                     .or(named("internalLoad"))),
         SessionMethodAdvice.class.getName());
-    // Handle the generic and non-generic 'get' separately.
+    // Handle the non-generic 'get' separately.
     transformers.put(
         isMethod()
             .and(named("get"))
@@ -106,7 +107,6 @@ public class SessionInstrumentation extends Instrumenter.Default {
     transformers.put(
         isMethod()
             .and(named("beginTransaction").or(named("getTransaction")))
-            .and(takesArguments(0))
             .and(returns(named("org.hibernate.Transaction"))),
         GetTransactionAdvice.class.getName());
 
@@ -139,7 +139,7 @@ public class SessionInstrumentation extends Instrumenter.Default {
         return;
       }
       if (state.getMethodScope() != null) {
-        System.err.println("THIS IS WRONG"); // TODO: proper warning/logging.
+        state.getMethodScope().close();
       }
 
       final Span span = state.getSessionSpan();

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

@@ -1,7 +1,8 @@
 package datadog.trace.instrumentation.hibernate;
 
 import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
-import static net.bytebuddy.matcher.ElementMatchers.isDeclaredBy;
+import static datadog.trace.instrumentation.hibernate.HibernateInstrumentation.INSTRUMENTATION_NAME;
+import static java.util.Collections.singletonMap;
 import static net.bytebuddy.matcher.ElementMatchers.isInterface;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -12,8 +13,6 @@ import com.google.auto.service.AutoService;
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.bootstrap.ContextStore;
 import datadog.trace.bootstrap.InstrumentationContext;
-import java.util.Collections;
-import java.util.HashMap;
 import java.util.Map;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.method.MethodDescription;
@@ -25,14 +24,12 @@ import org.hibernate.Transaction;
 public class TransactionInstrumentation extends Instrumenter.Default {
 
   public TransactionInstrumentation() {
-    super("hibernate");
+    super(INSTRUMENTATION_NAME);
   }
 
   @Override
   public Map<String, String> contextStore() {
-    final Map<String, String> map = new HashMap<>();
+    return singletonMap("org.hibernate.Transaction", SessionState.class.getName());
-    map.put("org.hibernate.Transaction", SessionState.class.getName());
-    return Collections.unmodifiableMap(map);
   }
 
   @Override
@@ -55,14 +52,9 @@ public class TransactionInstrumentation extends Instrumenter.Default {
 
   @Override
   public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
-    final Map<ElementMatcher<? super MethodDescription>, String> transformers = new HashMap<>();
+    return singletonMap(
-    transformers.put(
+        isMethod().and(named("commit")).and(takesArguments(0)),
-        isMethod()
-            .and(named("commit"))
-            .and(isDeclaredBy(safeHasSuperType(named("org.hibernate.Transaction"))))
-            .and(takesArguments(0)),
         TransactionCommitAdvice.class.getName());
-    return transformers;
   }
 
   public static class TransactionCommitAdvice {

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

@@ -9,14 +9,23 @@ class QueryTest extends AbstractHibernateTest {
   def "test hibernate query.#queryMethodName single call"() {
     setup:
 
+    // With Transaction
     Session session = sessionFactory.openSession()
     session.beginTransaction()
     queryInteraction(session)
     session.getTransaction().commit()
     session.close()
 
+    // Without Transaction
+    if (!requiresTransaction) {
+      session = sessionFactory.openSession()
+      queryInteraction(session)
+      session.close()
+    }
+
     expect:
-    assertTraces(1) {
+    assertTraces(requiresTransaction ? 1 : 2) {
+      // With Transaction
       trace(0, 4) {
         span(0) {
           serviceName "hibernate"
@@ -62,27 +71,62 @@ class QueryTest extends AbstractHibernateTest {
           childOf span(2)
         }
       }
+      if (!requiresTransaction) {
+        // Without Transaction
+        trace(1, 3) {
+          span(0) {
+            serviceName "hibernate"
+            resourceName "hibernate.session"
+            operationName "hibernate.session"
+            spanType DDSpanTypes.HIBERNATE
+            parent()
+            tags {
+              "$Tags.COMPONENT.key" "java-hibernate"
+              "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+              "$DDTags.SPAN_TYPE" DDSpanTypes.HIBERNATE
+              defaultTags()
+            }
+          }
+          span(1) {
+            serviceName "hibernate"
+            resourceName "$resource"
+            operationName "hibernate.$queryMethodName"
+            spanType DDSpanTypes.HIBERNATE
+            childOf span(0)
+            tags {
+              "$Tags.COMPONENT.key" "java-hibernate"
+              "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+              "$DDTags.SPAN_TYPE" DDSpanTypes.HIBERNATE
+              defaultTags()
+            }
+          }
+          span(2) {
+            serviceName "h2"
+            childOf span(1)
+          }
+        }
+      }
     }
 
     where:
-    queryMethodName       | isError | resource                         | queryInteraction
+    queryMethodName       | resource                         | requiresTransaction | queryInteraction
-    "query.list"          | false   | "Value"                          | { sess ->
+    "query.list"          | "Value"                          | false               | { sess ->
       Query q = sess.createQuery("from Value")
       q.list()
     }
-    "query.executeUpdate" | false   | "update Value set name = 'alyx'" | { sess ->
+    "query.executeUpdate" | "update Value set name = 'alyx'" | true                | { sess ->
       Query q = sess.createQuery("update Value set name = 'alyx'")
       q.executeUpdate()
     }
-    "query.uniqueResult"  | false   | "Value"                          | { sess ->
+    "query.uniqueResult"  | "Value"                          | false               | { sess ->
       Query q = sess.createQuery("from Value where id = 1")
       q.uniqueResult()
     }
-    "iterate"             | false   | "from Value"                     | { sess ->
+    "iterate"             | "from Value"                     | false               | { sess ->
       Query q = sess.createQuery("from Value")
       q.iterate()
     }
-    "query.scroll"        | false   | "from Value"                     | { sess ->
+    "query.scroll"        | "from Value"                     | false               | { sess ->
       Query q = sess.createQuery("from Value")
       q.scroll()
     }

dd-java-agent/instrumentation/hibernate/src/test/groovy/SessionTest.groovy

@@ -1,6 +1,9 @@
 import datadog.trace.api.DDSpanTypes
 import datadog.trace.api.DDTags
+import io.opentracing.Scope
+import io.opentracing.Tracer
 import io.opentracing.tag.Tags
+import io.opentracing.util.GlobalTracer
 import org.hibernate.*
 import spock.lang.Shared
 
@@ -72,13 +75,7 @@ class SessionTest extends AbstractHibernateTest {
             operationName "hibernate.$methodName"
             spanType DDSpanTypes.HIBERNATE
             childOf span(0)
-            if (isError) {
-              errored true
-            }
             tags {
-              if (isError) {
-                errorTags(MappingException, "Unknown entity: java.lang.Long")
-              }
               "$Tags.COMPONENT.key" "java-hibernate"
               "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
               "$DDTags.SPAN_TYPE" DDSpanTypes.HIBERNATE
@@ -94,28 +91,28 @@ class SessionTest extends AbstractHibernateTest {
     }
 
     where:
-    testName                                  | methodName | resource | isError | sessionImplementations          | sessionMethodTest
+    testName                                  | methodName | resource | sessionImplementations          | sessionMethodTest
-    "lock"                                    | "lock"     | "Value"  | false   | ["Session"]                     | { sesh, val ->
+    "lock"                                    | "lock"     | "Value"  | ["Session"]                     | { sesh, val ->
       sesh.lock(val, LockMode.READ)
     }
-    "refresh"                                 | "refresh"  | "Value"  | false   | ["Session", "StatelessSession"] | { sesh, val ->
+    "refresh"                                 | "refresh"  | "Value"  | ["Session", "StatelessSession"] | { sesh, val ->
       sesh.refresh(val)
     }
-    "get"                                     | "get"      | "Value"  | false   | ["Session", "StatelessSession"] | { sesh, val ->
+    "get"                                     | "get"      | "Value"  | ["Session", "StatelessSession"] | { sesh, val ->
       sesh.get("Value", val.getId())
     }
-    "insert"                                  | "insert"   | "Value"  | false   | ["StatelessSession"]            | { sesh, val ->
+    "insert"                                  | "insert"   | "Value"  | ["StatelessSession"]            | { sesh, val ->
       sesh.insert("Value", new Value("insert me"))
     }
-    "update (StatelessSession)"               | "update"   | "Value"  | false   | ["StatelessSession"]            | { sesh, val ->
+    "update (StatelessSession)"               | "update"   | "Value"  | ["StatelessSession"]            | { sesh, val ->
       val.setName("New name")
       sesh.update(val)
     }
-    "update by entityName (StatelessSession)" | "update"   | "Value"  | false   | ["StatelessSession"]            | { sesh, val ->
+    "update by entityName (StatelessSession)" | "update"   | "Value"  | ["StatelessSession"]            | { sesh, val ->
       val.setName("New name")
       sesh.update("Value", val)
     }
-    "delete (Session)"                        | "delete"   | "Value"  | false   | ["StatelessSession"]            | { sesh, val ->
+    "delete (Session)"                        | "delete"   | "Value"  | ["StatelessSession"]            | { sesh, val ->
       sesh.delete(val)
     }
   }
@@ -175,13 +172,7 @@ class SessionTest extends AbstractHibernateTest {
           operationName "hibernate.$methodName"
           spanType DDSpanTypes.HIBERNATE
           childOf span(0)
-          if (isError) {
-            errored true
-          }
           tags {
-            if (isError) {
-              errorTags(MappingException, "Unknown entity: java.lang.Long")
-            }
             "$Tags.COMPONENT.key" "java-hibernate"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
             "$DDTags.SPAN_TYPE" DDSpanTypes.HIBERNATE
@@ -197,13 +188,13 @@ class SessionTest extends AbstractHibernateTest {
     }
 
     where:
-    testName                  | methodName  | resource | isError | sessionMethodTest
+    testName                  | methodName  | resource | sessionMethodTest
-    "replicate"               | "replicate" | "Value"  | false   | { sesh, val ->
+    "replicate"               | "replicate" | "Value"  | { sesh, val ->
       Value replicated = new Value(val.getName() + " replicated")
       replicated.setId(val.getId())
       sesh.replicate(replicated, ReplicationMode.OVERWRITE)
     }
-    "replicate by entityName" | "replicate" | "Value"  | false   | { sesh, val ->
+    "replicate by entityName" | "replicate" | "Value"  | { sesh, val ->
       Value replicated = new Value(val.getName() + " replicated")
       replicated.setId(val.getId())
       sesh.replicate("Value", replicated, ReplicationMode.OVERWRITE)
@@ -334,13 +325,7 @@ class SessionTest extends AbstractHibernateTest {
             operationName "hibernate.$methodName"
             spanType DDSpanTypes.HIBERNATE
             childOf span(0)
-            if (isError) {
-              errored true
-            }
             tags {
-              if (isError) {
-                errorTags(MappingException, "Unknown entity: java.lang.Long")
-              }
               "$Tags.COMPONENT.key" "java-hibernate"
               "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
               "$DDTags.SPAN_TYPE" DDSpanTypes.HIBERNATE
@@ -352,32 +337,32 @@ class SessionTest extends AbstractHibernateTest {
     }
 
     where:
-    testName                         | methodName     | resource | isError | sessionImplementations | sessionMethodTest
+    testName                         | methodName     | resource | sessionImplementations | sessionMethodTest
-    "save"                           | "save"         | "Value"  | false   | ["Session"]            | { sesh, val ->
+    "save"                           | "save"         | "Value"  | ["Session"]            | { sesh, val ->
       sesh.save(new Value("Another value"))
     }
-    "saveOrUpdate save"              | "saveOrUpdate" | "Value"  | false   | ["Session"]            | { sesh, val ->
+    "saveOrUpdate save"              | "saveOrUpdate" | "Value"  | ["Session"]            | { sesh, val ->
       sesh.saveOrUpdate(new Value("Value"))
     }
-    "saveOrUpdate update"            | "saveOrUpdate" | "Value"  | false   | ["Session"]            | { sesh, val ->
+    "saveOrUpdate update"            | "saveOrUpdate" | "Value"  | ["Session"]            | { sesh, val ->
       val.setName("New name")
       sesh.saveOrUpdate(val)
     }
-    "merge"                          | "merge"        | "Value"  | false   | ["Session"]            | { sesh, val ->
+    "merge"                          | "merge"        | "Value"  | ["Session"]            | { sesh, val ->
       sesh.merge(new Value("merge me in"))
     }
-    "persist"                        | "persist"      | "Value"  | false   | ["Session"]            | { sesh, val ->
+    "persist"                        | "persist"      | "Value"  | ["Session"]            | { sesh, val ->
       sesh.persist(new Value("merge me in"))
     }
-    "update (Session)"               | "update"       | "Value"  | false   | ["Session"]            | { sesh, val ->
+    "update (Session)"               | "update"       | "Value"  | ["Session"]            | { sesh, val ->
       val.setName("New name")
       sesh.update(val)
     }
-    "update by entityName (Session)" | "update"       | "Value"  | false   | ["Session"]            | { sesh, val ->
+    "update by entityName (Session)" | "update"       | "Value"  | ["Session"]            | { sesh, val ->
       val.setName("New name")
       sesh.update("Value", val)
     }
-    "delete (Session)"               | "delete"       | "Value"  | false   | ["Session"]            | { sesh, val ->
+    "delete (Session)"               | "delete"       | "Value"  | ["Session"]            | { sesh, val ->
       sesh.delete(val)
     }
   }
@@ -447,5 +432,159 @@ class SessionTest extends AbstractHibernateTest {
     "getNamedQuery"  | "Value"               | { sess -> sess.getNamedQuery("TestNamedQuery") }
     "createSQLQuery" | "SELECT * FROM Value" | { sess -> sess.createSQLQuery("SELECT * FROM Value") }
   }
+
+
+  def "test hibernate overlapping Sessions"() {
+    setup:
+
+    Tracer tracer = GlobalTracer.get()
+
+    Scope scope = tracer.buildSpan("overlapping Sessions").startActive(true)
+
+    def session1 = sessionFactory.openSession()
+    session1.beginTransaction()
+    def session2 = sessionFactory.openStatelessSession()
+    def session3 = sessionFactory.openSession()
+
+    def value1 = new Value("Value 1")
+    session1.save(value1)
+    session2.insert(new Value("Value 2"))
+    session3.save(new Value("Value 3"))
+    session1.delete(value1)
+
+    session2.close()
+    session1.getTransaction().commit()
+    session1.close()
+    session3.close()
+
+    scope.close();
+
+
+    expect:
+    assertTraces(1) {
+      trace(0, 12) {
+        span(0) {
+          serviceName "unnamed-java-app"
+          operationName "overlapping Sessions"
+        }
+        span(1) {
+          serviceName "hibernate"
+          resourceName "hibernate.session"
+          operationName "hibernate.session"
+          spanType DDSpanTypes.HIBERNATE
+          childOf span(0)
+          tags {
+            "$Tags.COMPONENT.key" "java-hibernate"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HIBERNATE
+            defaultTags()
+          }
+        }
+        span(2) {
+          serviceName "hibernate"
+          resourceName "hibernate.session"
+          operationName "hibernate.session"
+          spanType DDSpanTypes.HIBERNATE
+          childOf span(0)
+          tags {
+            "$Tags.COMPONENT.key" "java-hibernate"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HIBERNATE
+            defaultTags()
+          }
+        }
+        span(3) {
+          serviceName "hibernate"
+          resourceName "hibernate.transaction.commit"
+          operationName "hibernate.transaction.commit"
+          spanType DDSpanTypes.HIBERNATE
+          childOf span(2)
+          tags {
+            "$Tags.COMPONENT.key" "java-hibernate"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HIBERNATE
+            defaultTags()
+          }
+        }
+        span(4) {
+          serviceName "h2"
+          childOf span(3)
+        }
+        span(5) {
+          serviceName "h2"
+          childOf span(3)
+        }
+        span(6) {
+          serviceName "hibernate"
+          resourceName "hibernate.session"
+          operationName "hibernate.session"
+          spanType DDSpanTypes.HIBERNATE
+          childOf span(0)
+          tags {
+            "$Tags.COMPONENT.key" "java-hibernate"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HIBERNATE
+            defaultTags()
+          }
+        }
+        span(7) {
+          serviceName "hibernate"
+          resourceName "Value"
+          operationName "hibernate.delete"
+          spanType DDSpanTypes.HIBERNATE
+          childOf span(2)
+          tags {
+            "$Tags.COMPONENT.key" "java-hibernate"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HIBERNATE
+            defaultTags()
+          }
+        }
+        span(8) {
+          serviceName "hibernate"
+          resourceName "Value"
+          operationName "hibernate.save"
+          spanType DDSpanTypes.HIBERNATE
+          childOf span(1)
+          tags {
+            "$Tags.COMPONENT.key" "java-hibernate"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HIBERNATE
+            defaultTags()
+          }
+        }
+        span(9) {
+          serviceName "hibernate"
+          resourceName "Value"
+          operationName "hibernate.insert"
+          spanType DDSpanTypes.HIBERNATE
+          childOf span(6)
+          tags {
+            "$Tags.COMPONENT.key" "java-hibernate"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HIBERNATE
+            defaultTags()
+          }
+        }
+        span(10) {
+          serviceName "h2"
+          childOf span(9)
+        }
+        span(11) {
+          serviceName "hibernate"
+          resourceName "Value"
+          operationName "hibernate.save"
+          spanType DDSpanTypes.HIBERNATE
+          childOf span(2)
+          tags {
+            "$Tags.COMPONENT.key" "java-hibernate"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HIBERNATE
+            defaultTags()
+          }
+        }
+      }
+    }
+  }
 }
 

dd-java-agent/instrumentation/hibernate/src/test/resources/hibernate.cfg.xml

@@ -14,7 +14,7 @@
     <property name="connection.password"/>
     <property name="dialect">org.hibernate.dialect.H2Dialect</property>
 
-    <property name="connection.pool_size">1</property>
+    <property name="connection.pool_size">3</property>
     <property name="cache.provider_class">org.hibernate.cache.internal.NoCacheProvider</property>
     <property name="show_sql">true</property>