Update SQL span name for procedures

Author: akats7

Diff for: instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/db/SqlStatementInfo.java

@@ -6,19 +6,14 @@
 package io.opentelemetry.instrumentation.api.db;
 
 import com.google.auto.value.AutoValue;
-import java.util.function.Function;
 import javax.annotation.Nullable;
 
 @AutoValue
 public abstract class SqlStatementInfo {
 
   public static SqlStatementInfo create(
-      @Nullable String fullStatement, @Nullable String operation, @Nullable String table) {
-    return new AutoValue_SqlStatementInfo(fullStatement, operation, table);
-  }
-
-  public SqlStatementInfo mapTable(Function<String, String> mapper) {
-    return SqlStatementInfo.create(getFullStatement(), getOperation(), mapper.apply(getTable()));
+      @Nullable String fullStatement, @Nullable String operation, @Nullable String identifier) {
+    return new AutoValue_SqlStatementInfo(fullStatement, operation, identifier);
   }
 
   @Nullable
@@ -28,5 +23,5 @@ public SqlStatementInfo mapTable(Function<String, String> mapper) {
   public abstract String getOperation();
 
   @Nullable
-  public abstract String getTable();
+  public abstract String getMainIdentifier();
 }

Diff for: instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/db/DbClientSpanNameExtractor.java

@@ -25,11 +25,12 @@ public static <REQUEST> SpanNameExtractor<REQUEST> create(
 
   /**
    * Returns a {@link SpanNameExtractor} that constructs the span name according to DB semantic
-   * conventions: {@code <db.operation> <db.name>.<table>}.
+   * conventions: {@code <db.operation> <db.name>.<identifier>}.
    *
    * @see SqlStatementInfo#getOperation() used to extract {@code <db.operation>}.
    * @see DbClientAttributesGetter#name(Object) used to extract {@code <db.name>}.
-   * @see SqlStatementInfo#getTable() used to extract {@code <db.table>}.
+   * @see SqlStatementInfo#getMainIdentifier() used to extract {@code <db.table>} or stored
+   *     procedure name.
    */
   public static <REQUEST> SpanNameExtractor<REQUEST> create(
       SqlClientAttributesGetter<REQUEST> getter) {
@@ -40,24 +41,24 @@ public static <REQUEST> SpanNameExtractor<REQUEST> create(
 
   private DbClientSpanNameExtractor() {}
 
-  protected String computeSpanName(String dbName, String operation, String table) {
+  protected String computeSpanName(String dbName, String operation, String identifier) {
     if (operation == null) {
       return dbName == null ? DEFAULT_SPAN_NAME : dbName;
     }
 
     StringBuilder name = new StringBuilder(operation);
-    if (dbName != null || table != null) {
+    if (dbName != null || identifier != null) {
       name.append(' ');
     }
-    // skip db name if table already has a db name prefixed to it
-    if (dbName != null && (table == null || table.indexOf('.') == -1)) {
+    // skip db name if identifier already has a db name prefixed to it
+    if (dbName != null && (identifier == null || identifier.indexOf('.') == -1)) {
       name.append(dbName);
-      if (table != null) {
+      if (identifier != null) {
         name.append('.');
       }
     }
-    if (table != null) {
-      name.append(table);
+    if (identifier != null) {
+      name.append(identifier);
     }
     return name.toString();
   }
@@ -82,7 +83,7 @@ public String extract(REQUEST request) {
   private static final class SqlClientSpanNameExtractor<REQUEST>
       extends DbClientSpanNameExtractor<REQUEST> {
 
-    // a dedicated sanitizer just for extracting the operation and table name
+    // a dedicated sanitizer just for extracting the operation and identifier name
     private static final SqlStatementSanitizer sanitizer = SqlStatementSanitizer.create(true);
 
     private final SqlClientAttributesGetter<REQUEST> getter;
@@ -96,7 +97,7 @@ public String extract(REQUEST request) {
       String dbName = getter.name(request);
       SqlStatementInfo sanitizedStatement = sanitizer.sanitize(getter.rawStatement(request));
       return computeSpanName(
-          dbName, sanitizedStatement.getOperation(), sanitizedStatement.getTable());
+          dbName, sanitizedStatement.getOperation(), sanitizedStatement.getMainIdentifier());
     }
   }
 }

Diff for: instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/db/SqlClientAttributesExtractor.java

@@ -43,6 +43,7 @@ public static <REQUEST, RESPONSE> SqlClientAttributesExtractorBuilder<REQUEST, R
     return new SqlClientAttributesExtractorBuilder<>(getter);
   }
 
+  private static final String SQL_CALL = "CALL";
   private final AttributeKey<String> dbTableAttribute;
   private final SqlStatementSanitizer sanitizer;
 
@@ -60,8 +61,11 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
     super.onStart(attributes, parentContext, request);
 
     SqlStatementInfo sanitizedStatement = sanitizer.sanitize(getter.rawStatement(request));
+    String operation = sanitizedStatement.getOperation();
     internalSet(attributes, SemanticAttributes.DB_STATEMENT, sanitizedStatement.getFullStatement());
-    internalSet(attributes, SemanticAttributes.DB_OPERATION, sanitizedStatement.getOperation());
-    internalSet(attributes, dbTableAttribute, sanitizedStatement.getTable());
+    internalSet(attributes, SemanticAttributes.DB_OPERATION, operation);
+    if (!SQL_CALL.equals(operation)) {
+      internalSet(attributes, dbTableAttribute, sanitizedStatement.getMainIdentifier());
+    }
   }
 }

Diff for: instrumentation-api-semconv/src/main/jflex/SqlSanitizer.jflex

@@ -63,13 +63,13 @@ WHITESPACE          = [ \t\r\n]+
   }
 
   /** @return text matched by current token without enclosing double quotes or backticks */
-  private String readTableName() {
-    String tableName = yytext();
-    if (tableName != null && ((tableName.startsWith("\"") && tableName.endsWith("\""))
-        || (tableName.startsWith("`") && tableName.endsWith("`")))) {
-      tableName = tableName.substring(1, tableName.length() - 1);
+  private String readIdentifierName() {
+    String identifierName = yytext();
+    if (identifierName != null && ((identifierName.startsWith("\"") && identifierName.endsWith("\""))
+        || (identifierName.startsWith("`") && identifierName.endsWith("`")))) {
+      identifierName = identifierName.substring(1, identifierName.length() - 1);
     }
-    return tableName;
+    return identifierName;
   }
 
   // you can reference a table in the FROM clause in one of the following ways:
@@ -92,7 +92,7 @@ WHITESPACE          = [ \t\r\n]+
   }
 
   private static abstract class Operation {
-    String mainTable = null;
+    String mainIdentifier = null;
 
     /** @return true if all statement info is gathered */
     boolean handleFrom() {
@@ -119,8 +119,13 @@ WHITESPACE          = [ \t\r\n]+
       return false;
     }
 
+    /** @return true if all statement info is gathered */
+    boolean handleNext() {
+      return false;
+     }
+
     SqlStatementInfo getResult(String fullStatement) {
-      return SqlStatementInfo.create(fullStatement, getClass().getSimpleName().toUpperCase(java.util.Locale.ROOT), mainTable);
+      return SqlStatementInfo.create(fullStatement, getClass().getSimpleName().toUpperCase(java.util.Locale.ROOT), mainIdentifier);
     }
   }
 
@@ -152,13 +157,13 @@ WHITESPACE          = [ \t\r\n]+
       }
 
       // subquery in WITH or SELECT clause, before main FROM clause; skipping
-      mainTable = null;
+      mainIdentifier = null;
       return true;
     }
 
     boolean handleJoin() {
       // for SELECT statements with joined tables there's no main table
-      mainTable = null;
+      mainIdentifier = null;
       return true;
     }
 
@@ -173,17 +178,17 @@ WHITESPACE          = [ \t\r\n]+
 
       // SELECT FROM (subquery) case
       if (parenLevel != 0) {
-        mainTable = null;
+        mainIdentifier = null;
         return true;
       }
 
       // whenever >1 table is used there is no main table (e.g. unions)
       if (mainTableSetAlready) {
-        mainTable = null;
+        mainIdentifier = null;
         return true;
       }
 
-      mainTable = readTableName();
+      mainIdentifier = readIdentifierName();
       mainTableSetAlready = true;
       expectingTableName = false;
       // start counting identifiers after encountering main from clause
@@ -199,7 +204,7 @@ WHITESPACE          = [ \t\r\n]+
       // any other list that can appear later needs at least 4 idents)
       if (identifiersAfterMainFromClause > 0
           && identifiersAfterMainFromClause <= FROM_TABLE_REF_MAX_IDENTIFIERS) {
-        mainTable = null;
+        mainIdentifier = null;
         return true;
       }
       return false;
@@ -219,7 +224,7 @@ WHITESPACE          = [ \t\r\n]+
         return false;
       }
 
-      mainTable = readTableName();
+      mainIdentifier = readIdentifierName();
       return true;
     }
   }
@@ -237,21 +242,33 @@ WHITESPACE          = [ \t\r\n]+
         return false;
       }
 
-      mainTable = readTableName();
+      mainIdentifier = readIdentifierName();
       return true;
     }
   }
 
   private class Update extends Operation {
     boolean handleIdentifier() {
-      mainTable = readTableName();
+      mainIdentifier = readIdentifierName();
+      return true;
+    }
+  }
+
+  private class Call extends Operation {
+    boolean handleIdentifier() {
+      mainIdentifier = readIdentifierName();
+      return true;
+    }
+
+    boolean handleNext() {
+      mainIdentifier = null;
       return true;
     }
   }
 
   private class Merge extends Operation {
     boolean handleIdentifier() {
-      mainTable = readTableName();
+      mainIdentifier = readIdentifierName();
       return true;
     }
   }
@@ -298,14 +315,20 @@ WHITESPACE          = [ \t\r\n]+
           appendCurrentFragment();
           if (isOverLimit()) return YYEOF;
       }
+  "CALL" {
+          if (!insideComment) {
+            setOperation(new Call());
+          }
+          appendCurrentFragment();
+          if (isOverLimit()) return YYEOF;
+      }
   "MERGE" {
           if (!insideComment) {
             setOperation(new Merge());
           }
           appendCurrentFragment();
           if (isOverLimit()) return YYEOF;
       }
-
   "FROM" {
           if (!insideComment && !extractionDone) {
             if (operation == NoOp.INSTANCE) {
@@ -332,6 +355,13 @@ WHITESPACE          = [ \t\r\n]+
           appendCurrentFragment();
           if (isOverLimit()) return YYEOF;
       }
+  "NEXT" {
+          if (!insideComment && !extractionDone) {
+              extractionDone = operation.handleNext();
+            }
+          appendCurrentFragment();
+          if (isOverLimit()) return YYEOF;
+      }
   {COMMA} {
           if (!insideComment && !extractionDone) {
             extractionDone = operation.handleComma();
@@ -407,4 +437,4 @@ WHITESPACE          = [ \t\r\n]+
           appendCurrentFragment();
           if (isOverLimit()) return YYEOF;
       }
-}
+}

Diff for: instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/db/SqlStatementSanitizerTest.java

@@ -232,12 +232,13 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) th
 
   static class SimplifyArgs implements ArgumentsProvider {
 
-    static Function<String, SqlStatementInfo> expect(String operation, String table) {
-      return sql -> SqlStatementInfo.create(sql, operation, table);
+    static Function<String, SqlStatementInfo> expect(String operation, String identifier) {
+      return sql -> SqlStatementInfo.create(sql, operation, identifier);
     }
 
-    static Function<String, SqlStatementInfo> expect(String sql, String operation, String table) {
-      return ignored -> SqlStatementInfo.create(sql, operation, table);
+    static Function<String, SqlStatementInfo> expect(
+        String sql, String operation, String identifier) {
+      return ignored -> SqlStatementInfo.create(sql, operation, identifier);
     }
 
     @Override
@@ -275,6 +276,7 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) th
           Arguments.of("/* update comment */ select * from table1", expect("SELECT", "table1")),
           Arguments.of("select /*((*/abc from table", expect("SELECT", "table")),
           Arguments.of("SeLeCT * FrOm TAblE", expect("SELECT", "table")),
+          Arguments.of("select next value in hibernate_sequence", expect("SELECT", null)),
 
           // hibernate/jpa
           Arguments.of("FROM schema.table", expect("SELECT", "schema.table")),
@@ -308,6 +310,12 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) th
               expect("update \"my table\" set answer=?", "UPDATE", "my table")),
           Arguments.of("update /*table", expect("UPDATE", null)),
 
+          // Call
+          Arguments.of("call test_proc()", expect("CALL", "test_proc")),
+          Arguments.of("call test_proc", expect("CALL", "test_proc")),
+          Arguments.of("call next value in hibernate_sequence", expect("CALL", null)),
+          Arguments.of("call db.test_proc", expect("CALL", "db.test_proc")),
+
           // Merge
           Arguments.of("merge into table", expect("MERGE", "table")),
           Arguments.of("merge into `my table`", expect("MERGE", "my table")),

Diff for: instrumentation/hibernate/hibernate-4.0/javaagent/src/test/groovy/SpringJpaTest.groovy

@@ -112,7 +112,7 @@ class SpringJpaTest extends AgentInstrumentationSpecification {
         }
         if (!isHibernate4) {
           span(2) {
-            name "test"
+            name "CALL test"
             kind CLIENT
             childOf span(1)
             attributes {
@@ -121,6 +121,7 @@ class SpringJpaTest extends AgentInstrumentationSpecification {
               "$SemanticAttributes.DB_USER" "sa"
               "$SemanticAttributes.DB_STATEMENT" "call next value for hibernate_sequence"
               "$SemanticAttributes.DB_CONNECTION_STRING" "hsqldb:mem:"
+              "$SemanticAttributes.DB_OPERATION" "CALL"
             }
           }
           span(3) {

Diff for: instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/OperationNameUtil.java

@@ -22,8 +22,8 @@ public static String getOperationNameForQuery(String query) {
     SqlStatementInfo info = sanitizer.sanitize(query);
     if (info.getOperation() != null) {
       operation = info.getOperation();
-      if (info.getTable() != null) {
-        operation += " " + info.getTable();
+      if (info.getMainIdentifier() != null) {
+        operation += " " + info.getMainIdentifier();
       }
     }
     return operation;

Diff for: instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/src/test/groovy/ProcedureCallTest.groovy

@@ -104,7 +104,7 @@ class ProcedureCallTest extends AgentInstrumentationSpecification {
           }
         }
         span(2) {
-          name "test"
+          name "CALL test.TEST_PROC"
           kind CLIENT
           childOf span(1)
           attributes {
@@ -113,6 +113,7 @@ class ProcedureCallTest extends AgentInstrumentationSpecification {
             "$SemanticAttributes.DB_USER" "sa"
             "$SemanticAttributes.DB_STATEMENT" "{call TEST_PROC()}"
             "$SemanticAttributes.DB_CONNECTION_STRING" "hsqldb:mem:"
+            "$SemanticAttributes.DB_OPERATION" "CALL"
           }
         }
         span(3) {

Diff for: instrumentation/spring/spring-data/spring-data-common/testing/src/main/groovy/AbstractSpringJpaTest.groovy

@@ -111,7 +111,7 @@ abstract class AbstractSpringJpaTest<ENTITY, REPOSITORY extends JpaRepository<EN
         if (!isHibernate4) {
           offset = 1
           span(1) {
-            name "test"
+            name "CALL test"
             kind CLIENT
             childOf span(0)
             attributes {
@@ -120,6 +120,7 @@ abstract class AbstractSpringJpaTest<ENTITY, REPOSITORY extends JpaRepository<EN
               "$SemanticAttributes.DB_USER" "sa"
               "$SemanticAttributes.DB_CONNECTION_STRING" "hsqldb:mem:"
               "$SemanticAttributes.DB_STATEMENT" ~/^call next value for /
+              "$SemanticAttributes.DB_OPERATION" "CALL"
             }
           }
         }