Hibernate: set span name only on method entry (#3603)

open-telemetry · Jul 26, 2021 · 3555c25 · 3555c25
1 parent 26dc106
commit 3555c25
Show file tree

Hide file tree

Showing 18 changed files with 208 additions and 98 deletions.
diff --git a/...va/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/CriteriaInstrumentation.java b/...va/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/CriteriaInstrumentation.java
@@ -21,9 +21,9 @@
 import io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.implementation.bytecode.assign.Assigner;
 import net.bytebuddy.matcher.ElementMatcher;
 import org.hibernate.Criteria;
+import org.hibernate.impl.CriteriaImpl;
 
 public class CriteriaInstrumentation implements TypeInstrumentation {
 
@@ -63,7 +63,13 @@ public static void startMethod(
       ContextStore<Criteria, Context> contextStore =
           InstrumentationContext.get(Criteria.class, Context.class);
 
-      context = SessionMethodUtils.startSpanFrom(contextStore, criteria, "Criteria." + name, null);
+      String entityName = null;
+      if (criteria instanceof CriteriaImpl) {
+        entityName = ((CriteriaImpl) criteria).getEntityOrClassName();
+      }
+
+      context =
+          SessionMethodUtils.startSpanFrom(contextStore, criteria, "Criteria." + name, entityName);
       if (context != null) {
         scope = context.makeCurrent();
       }
@@ -72,7 +78,6 @@ public static void startMethod(
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void endMethod(
         @Advice.Thrown Throwable throwable,
-        @Advice.Return(typing = Assigner.Typing.DYNAMIC) Object entity,
         @Advice.Origin("#m") String name,
         @Advice.Local("otelCallDepth") CallDepth callDepth,
         @Advice.Local("otelContext") Context context,
@@ -84,7 +89,7 @@ public static void endMethod(
 
       if (scope != null) {
         scope.close();
-        SessionMethodUtils.end(context, throwable, "Criteria." + name, entity);
+        SessionMethodUtils.end(context, throwable);
       }
     }
   }

diff --git a/...c/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/EntityNameUtil.java b/...c/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/EntityNameUtil.java
@@ -0,0 +1,30 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.hibernate.v3_3;
+
+import java.util.function.Function;
+import org.hibernate.impl.AbstractSessionImpl;
+
+public final class EntityNameUtil {
+
+  private EntityNameUtil() {}
+
+  private static String bestGuessEntityName(Object session, Object entity) {
+    if (entity == null) {
+      return null;
+    }
+
+    if (session instanceof AbstractSessionImpl) {
+      return ((AbstractSessionImpl) session).bestGuessEntityName(entity);
+    }
+
+    return null;
+  }
+
+  public static Function<Object, String> bestGuessEntityName(Object session) {
+    return (entity) -> bestGuessEntityName(session, entity);
+  }
+}
diff --git a/.../java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/QueryInstrumentation.java b/.../java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/QueryInstrumentation.java
@@ -80,7 +80,7 @@ public static void endMethod(
 
       if (scope != null) {
         scope.close();
-        SessionMethodUtils.end(context, throwable, null, null);
+        SessionMethodUtils.end(context, throwable);
       }
     }
   }

diff --git a/...ava/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionInstrumentation.java b/...ava/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionInstrumentation.java
@@ -9,6 +9,7 @@
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface;
 import static io.opentelemetry.javaagent.instrumentation.hibernate.HibernateTracer.tracer;
 import static io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils.SCOPE_ONLY_METHODS;
+import static io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils.getEntityName;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 import static net.bytebuddy.matcher.ElementMatchers.namedOneOf;
@@ -26,7 +27,6 @@
 import io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.implementation.bytecode.assign.Assigner;
 import net.bytebuddy.matcher.ElementMatcher;
 import org.hibernate.Criteria;
 import org.hibernate.Query;
@@ -131,7 +131,9 @@ public static class SessionMethodAdvice {
     public static void startMethod(
         @Advice.This Object session,
         @Advice.Origin("#m") String name,
-        @Advice.Argument(0) Object entity,
+        @Advice.Origin("#d") String descriptor,
+        @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("otelScope") Scope scope) {
@@ -157,7 +159,9 @@ public static void startMethod(
       }
 
       if (!SCOPE_ONLY_METHODS.contains(name)) {
-        spanContext = tracer().startSpan(sessionContext, "Session." + name, entity);
+        String entityName =
+            getEntityName(descriptor, arg0, arg1, EntityNameUtil.bestGuessEntityName(session));
+        spanContext = tracer().startSpan(sessionContext, "Session." + name, entityName);
         scope = spanContext.makeCurrent();
       } else {
         scope = sessionContext.makeCurrent();
@@ -167,8 +171,6 @@ public static void startMethod(
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void endMethod(
         @Advice.Thrown Throwable throwable,
-        @Advice.Return(typing = Assigner.Typing.DYNAMIC) Object returned,
-        @Advice.Origin("#m") String name,
         @Advice.Local("otelCallDepth") CallDepth callDepth,
         @Advice.Local("otelContext") Context spanContext,
         @Advice.Local("otelScope") Scope scope) {
@@ -179,7 +181,7 @@ public static void endMethod(
 
       if (scope != null) {
         scope.close();
-        SessionMethodUtils.end(spanContext, throwable, "Session." + name, returned);
+        SessionMethodUtils.end(spanContext, throwable);
       }
     }
   }

diff --git a/...io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/TransactionInstrumentation.java b/...io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/TransactionInstrumentation.java
@@ -81,7 +81,7 @@ public static void endCommit(
 
       if (scope != null) {
         scope.close();
-        SessionMethodUtils.end(context, throwable, null, null);
+        SessionMethodUtils.end(context, 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
@@ -36,7 +36,7 @@ class CriteriaTest extends AbstractHibernateTest {
           }
         }
         span(1) {
-          name "Criteria.$methodName"
+          name "Criteria.$methodName Value"
           kind INTERNAL
           childOf span(0)
           attributes {

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
@@ -283,7 +283,7 @@ class SessionTest extends AbstractHibernateTest {
           }
         }
         span(1) {
-          name "Session.replicate"
+          name "Session.replicate java.lang.Long"
           kind INTERNAL
           childOf span(0)
           status ERROR

diff --git a/...va/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/CriteriaInstrumentation.java b/...va/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/CriteriaInstrumentation.java
@@ -21,9 +21,9 @@
 import io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.implementation.bytecode.assign.Assigner;
 import net.bytebuddy.matcher.ElementMatcher;
 import org.hibernate.Criteria;
+import org.hibernate.internal.CriteriaImpl;
 
 public class CriteriaInstrumentation implements TypeInstrumentation {
 
@@ -63,7 +63,13 @@ public static void startMethod(
       ContextStore<Criteria, Context> contextStore =
           InstrumentationContext.get(Criteria.class, Context.class);
 
-      context = SessionMethodUtils.startSpanFrom(contextStore, criteria, "Criteria." + name, null);
+      String entityName = null;
+      if (criteria instanceof CriteriaImpl) {
+        entityName = ((CriteriaImpl) criteria).getEntityOrClassName();
+      }
+
+      context =
+          SessionMethodUtils.startSpanFrom(contextStore, criteria, "Criteria." + name, entityName);
       if (context != null) {
         scope = context.makeCurrent();
       }
@@ -72,8 +78,6 @@ public static void startMethod(
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void endMethod(
         @Advice.Thrown Throwable throwable,
-        @Advice.Return(typing = Assigner.Typing.DYNAMIC) Object entity,
-        @Advice.Origin("#m") String name,
         @Advice.Local("otelCallDepth") CallDepth callDepth,
         @Advice.Local("otelContext") Context context,
         @Advice.Local("otelScope") Scope scope) {
@@ -84,7 +88,7 @@ public static void endMethod(
 
       if (scope != null) {
         scope.close();
-        SessionMethodUtils.end(context, throwable, "Criteria." + name, entity);
+        SessionMethodUtils.end(context, throwable);
       }
     }
   }

diff --git a/...c/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/EntityNameUtil.java b/...c/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/EntityNameUtil.java
@@ -0,0 +1,34 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.hibernate.v4_0;
+
+import java.util.function.Function;
+import org.hibernate.SharedSessionContract;
+import org.hibernate.internal.SessionImpl;
+import org.hibernate.internal.StatelessSessionImpl;
+
+public final class EntityNameUtil {
+
+  private EntityNameUtil() {}
+
+  private static String bestGuessEntityName(SharedSessionContract session, Object entity) {
+    if (entity == null) {
+      return null;
+    }
+
+    if (session instanceof SessionImpl) {
+      return ((SessionImpl) session).bestGuessEntityName(entity);
+    } else if (session instanceof StatelessSessionImpl) {
+      return ((StatelessSessionImpl) session).bestGuessEntityName(entity);
+    }
+
+    return null;
+  }
+
+  public static Function<Object, String> bestGuessEntityName(SharedSessionContract session) {
+    return (entity) -> bestGuessEntityName(session, entity);
+  }
+}
diff --git a/.../java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/QueryInstrumentation.java b/.../java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/QueryInstrumentation.java
@@ -80,7 +80,7 @@ public static void endMethod(
 
       if (scope != null) {
         scope.close();
-        SessionMethodUtils.end(context, throwable, null, null);
+        SessionMethodUtils.end(context, throwable);
       }
     }
   }

diff --git a/...ava/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/SessionInstrumentation.java b/...ava/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/SessionInstrumentation.java
@@ -9,6 +9,7 @@
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface;
 import static io.opentelemetry.javaagent.instrumentation.hibernate.HibernateTracer.tracer;
 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 net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -27,7 +28,6 @@
 import io.opentelemetry.javaagent.instrumentation.hibernate.SessionMethodUtils;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.implementation.bytecode.assign.Assigner;
 import net.bytebuddy.matcher.ElementMatcher;
 import org.hibernate.Criteria;
 import org.hibernate.Query;
@@ -125,7 +125,9 @@ public static class SessionMethodAdvice {
     public static void startMethod(
         @Advice.This SharedSessionContract session,
         @Advice.Origin("#m") String name,
-        @Advice.Argument(0) Object entity,
+        @Advice.Origin("#d") String descriptor,
+        @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("otelScope") Scope scope) {
@@ -144,7 +146,10 @@ public static void startMethod(
       }
 
       if (!SCOPE_ONLY_METHODS.contains(name)) {
-        spanContext = tracer().startSpan(sessionContext, getSessionMethodSpanName(name), entity);
+        String entityName =
+            getEntityName(descriptor, arg0, arg1, EntityNameUtil.bestGuessEntityName(session));
+        spanContext =
+            tracer().startSpan(sessionContext, getSessionMethodSpanName(name), entityName);
         scope = spanContext.makeCurrent();
       } else {
         scope = sessionContext.makeCurrent();
@@ -154,8 +159,6 @@ public static void startMethod(
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void endMethod(
         @Advice.Thrown Throwable throwable,
-        @Advice.Return(typing = Assigner.Typing.DYNAMIC) Object returned,
-        @Advice.Origin("#m") String name,
         @Advice.Local("otelCallDepth") CallDepth callDepth,
         @Advice.Local("otelContext") Context spanContext,
         @Advice.Local("otelScope") Scope scope) {
@@ -166,7 +169,7 @@ public static void endMethod(
 
       if (scope != null) {
         scope.close();
-        SessionMethodUtils.end(spanContext, throwable, getSessionMethodSpanName(name), returned);
+        SessionMethodUtils.end(spanContext, throwable);
       }
     }
   }

diff --git a/...io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/TransactionInstrumentation.java b/...io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/TransactionInstrumentation.java
@@ -81,7 +81,7 @@ public static void endCommit(
 
       if (scope != null) {
         scope.close();
-        SessionMethodUtils.end(context, throwable, null, null);
+        SessionMethodUtils.end(context, throwable);
       }
     }
   }

diff --git a/...umentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/AbstractHibernateTest.groovy b/...umentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/AbstractHibernateTest.groovy
@@ -23,7 +23,7 @@ abstract class AbstractHibernateTest extends AgentInstrumentationSpecification {
     Session writer = sessionFactory.openSession()
     writer.beginTransaction()
     prepopulated = new ArrayList<>()
-    for (int i = 0; i < 2; i++) {
+    for (int i = 0; i < 5; i++) {
       prepopulated.add(new Value("Hello :) " + i))
       writer.save(prepopulated.get(i))
     }

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
@@ -36,7 +36,7 @@ class CriteriaTest extends AbstractHibernateTest {
           }
         }
         span(1) {
-          name "Criteria.$methodName"
+          name "Criteria.$methodName Value"
           kind INTERNAL
           childOf span(0)
           attributes {