Hibernate query span naming

instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/QueryInstrumentation.java

@@ -53,7 +53,7 @@ public static void startMethod(
       ContextStore<Query, Context> contextStore =
           InstrumentationContext.get(Query.class, Context.class);
 
-      context = SessionMethodUtils.startSpanFrom(contextStore, query, query.getQueryString(), null);
+      context = SessionMethodUtils.startSpanFromQuery(contextStore, query, query.getQueryString());
       if (context != null) {
         scope = context.makeCurrent();
       }

instrumentation/hibernate/hibernate-3.3/javaagent/src/test/groovy/QueryTest.groovy

@@ -105,25 +105,25 @@ class QueryTest extends AbstractHibernateTest {
 
     where:
     queryMethodName       | expectedSpanName            | requiresTransaction | queryInteraction
-    "Query.list"          | "from Value"                | false               | { sess ->
+    "Query.list"          | "SELECT Value"              | false               | { sess ->
       Query q = sess.createQuery("from Value")
       q.list()
     }
-    "Query.executeUpdate" | "update Value set name = ?" | true                | { sess ->
+    "Query.executeUpdate" | "UPDATE Value"              | true                | { sess ->
       Query q = sess.createQuery("update Value set name = ?")
       q.setParameter(0, "alyx")
       q.executeUpdate()
     }
-    "Query.uniqueResult"  | "from Value where id = ?"   | false               | { sess ->
+    "Query.uniqueResult"  | "SELECT Value"              | false               | { sess ->
       Query q = sess.createQuery("from Value where id = ?")
       q.setParameter(0, 1L)
       q.uniqueResult()
     }
-    "Query.iterate"       | "from Value"                | false               | { sess ->
+    "Query.iterate"       | "SELECT Value"              | false               | { sess ->
       Query q = sess.createQuery("from Value")
       q.iterate()
     }
-    "Query.scroll"        | "from Value"                | false               | { sess ->
+    "Query.scroll"        | "SELECT Value"              | false               | { sess ->
       Query q = sess.createQuery("from Value")
       q.scroll()
     }
@@ -153,7 +153,7 @@ class QueryTest extends AbstractHibernateTest {
           }
         }
         span(1) {
-          name "from Value"
+          name "SELECT Value"
           kind INTERNAL
           childOf span(0)
           attributes {

instrumentation/hibernate/hibernate-3.3/javaagent/src/test/groovy/SessionTest.groovy

@@ -440,10 +440,10 @@ class SessionTest extends AbstractHibernateTest {
     }
 
     where:
-    queryMethodName  | expectedSpanName      | queryBuildMethod
-    "createQuery"    | "from Value"          | { sess -> sess.createQuery("from Value") }
-    "getNamedQuery"  | "from Value"          | { sess -> sess.getNamedQuery("TestNamedQuery") }
-    "createSQLQuery" | "SELECT * FROM Value" | { sess -> sess.createSQLQuery("SELECT * FROM Value") }
+    queryMethodName  | expectedSpanName | queryBuildMethod
+    "createQuery"    | "SELECT Value"   | { sess -> sess.createQuery("from Value") }
+    "getNamedQuery"  | "SELECT Value"   | { sess -> sess.getNamedQuery("TestNamedQuery") }
+    "createSQLQuery" | "SELECT Value"   | { sess -> sess.createSQLQuery("SELECT * FROM Value") }
   }
 
 

instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/QueryInstrumentation.java

@@ -53,7 +53,7 @@ public static void startMethod(
       ContextStore<Query, Context> contextStore =
           InstrumentationContext.get(Query.class, Context.class);
 
-      context = SessionMethodUtils.startSpanFrom(contextStore, query, query.getQueryString(), null);
+      context = SessionMethodUtils.startSpanFromQuery(contextStore, query, query.getQueryString());
       if (context != null) {
         scope = context.makeCurrent();
       }

instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/QueryTest.groovy

@@ -105,25 +105,25 @@ class QueryTest extends AbstractHibernateTest {
 
     where:
     queryMethodName       | expectedSpanName            | requiresTransaction | queryInteraction
-    "query/list"          | "from Value"                | false               | { sess ->
+    "query/list"          | "SELECT Value"              | false               | { sess ->
       Query q = sess.createQuery("from Value")
       q.list()
     }
-    "query/executeUpdate" | "update Value set name = ?" | true                | { sess ->
+    "query/executeUpdate" | "UPDATE Value"              | true                | { sess ->
       Query q = sess.createQuery("update Value set name = ?")
       q.setParameter(0, "alyx")
       q.executeUpdate()
     }
-    "query/uniqueResult"  | "from Value where id = ?"   | false               | { sess ->
+    "query/uniqueResult"  | "SELECT Value"              | false               | { sess ->
       Query q = sess.createQuery("from Value where id = ?")
       q.setParameter(0, 1L)
       q.uniqueResult()
     }
-    "iterate"             | "from Value"                | false               | { sess ->
+    "iterate"             | "SELECT Value"              | false               | { sess ->
       Query q = sess.createQuery("from Value")
       q.iterate()
     }
-    "query/scroll"        | "from Value"                | false               | { sess ->
+    "query/scroll"        | "SELECT Value"              | false               | { sess ->
       Query q = sess.createQuery("from Value")
       q.scroll()
     }
@@ -153,7 +153,7 @@ class QueryTest extends AbstractHibernateTest {
           }
         }
         span(1) {
-          name "from Value"
+          name "SELECT Value"
           kind INTERNAL
           childOf span(0)
           attributes {

instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/SessionTest.groovy

@@ -379,10 +379,10 @@ class SessionTest extends AbstractHibernateTest {
     }
 
     where:
-    queryMethodName  | resource              | queryBuildMethod
-    "createQuery"    | "from Value"          | { sess -> sess.createQuery("from Value") }
-    "getNamedQuery"  | "from Value"          | { sess -> sess.getNamedQuery("TestNamedQuery") }
-    "createSQLQuery" | "SELECT * FROM Value" | { sess -> sess.createSQLQuery("SELECT * FROM Value") }
+    queryMethodName  | resource       | queryBuildMethod
+    "createQuery"    | "SELECT Value" | { sess -> sess.createQuery("from Value") }
+    "getNamedQuery"  | "SELECT Value" | { sess -> sess.getNamedQuery("TestNamedQuery") }
+    "createSQLQuery" | "SELECT Value" | { sess -> sess.createSQLQuery("SELECT * FROM Value") }
   }
 
 

instrumentation/hibernate/hibernate-4.3/javaagent/hibernate-4.3-javaagent.gradle

@@ -15,7 +15,7 @@ testSets {
     dirName = 'test'
   }
   version6Test {
-    dirName = 'test'
+    dirName = 'hibernate6Test'
   }
 
   latestDepTest {
@@ -38,6 +38,7 @@ dependencies {
   testImplementation "org.hibernate:hibernate-core:4.3.0.Final"
   testImplementation "org.hibernate:hibernate-entitymanager:4.3.0.Final"
   testImplementation "org.hsqldb:hsqldb:2.0.0"
+  testImplementation "com.h2database:h2:1.4.197"
   //First version to work with Java 14
   testImplementation "org.springframework.data:spring-data-jpa:1.8.0.RELEASE"
 

instrumentation/hibernate/hibernate-4.3/javaagent/src/hibernate6Test/groovy/SpringJpaTest.groovy

@@ -0,0 +1,198 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import static io.opentelemetry.api.trace.SpanKind.CLIENT
+
+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
+
+/**
+ * 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"
+          }
+        }
+      }
+    }
+  }
+}

instrumentation/hibernate/hibernate-4.3/javaagent/src/hibernate6Test/java/Value.java

@@ -0,0 +1,45 @@
+/*
+ * 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;
+  }
+
+  private void setId(Long id) {
+    this.id = id;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public void setName(String title) {
+    name = title;
+  }
+}