Fix InfluxDB stable DB semconv attributes

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbClientAttributesExtractor.java

@@ -7,6 +7,7 @@
 
 import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitOldDatabaseSemconv;
 import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableDatabaseSemconv;
+import static io.opentelemetry.semconv.DbAttributes.DB_COLLECTION_NAME;
 import static io.opentelemetry.semconv.DbAttributes.DB_NAMESPACE;
 import static io.opentelemetry.semconv.DbAttributes.DB_OPERATION_BATCH_SIZE;
 import static io.opentelemetry.semconv.DbAttributes.DB_OPERATION_NAME;
@@ -99,6 +100,7 @@ static <REQUEST, RESPONSE> void onStartCommon(
       attributes.put(
           DB_SYSTEM_NAME, SemconvStability.stableDbSystemName(getter.getDbSystemName(request)));
       attributes.put(DB_NAMESPACE, getter.getDbNamespace(request));
+      attributes.put(DB_COLLECTION_NAME, getter.getDbCollectionName(request));
       attributes.put(DB_QUERY_TEXT, getter.getDbQueryText(request));
       attributes.put(DB_OPERATION_NAME, getter.getDbOperationName(request));
       attributes.put(DB_QUERY_SUMMARY, getter.getDbQuerySummary(request));
@@ -112,7 +114,7 @@ static <REQUEST, RESPONSE> void onStartCommon(
       attributes.put(DB_NAME, getter.getDbName(request));
       attributes.put(DB_CONNECTION_STRING, getter.getConnectionString(request));
       attributes.put(DB_STATEMENT, getter.getDbQueryText(request));
-      attributes.put(DB_OPERATION, getter.getDbOperationName(request));
+      attributes.put(DB_OPERATION, getter.getDbOperation(request));
     }
 
     // Query parameters are an incubating feature and work with both old and new semconv

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbClientAttributesGetter.java

@@ -38,6 +38,17 @@ default String getDbQuerySummary(REQUEST request) {
   @Nullable
   String getDbOperationName(REQUEST request);
 
+  /**
+   * Returns the old db.operation value. This is only used for old semantic conventions.
+   *
+   * @deprecated Use {@link #getDbOperationName(Object)} instead.
+   */
+  @Deprecated // to be removed in 3.0
+  @Nullable
+  default String getDbOperation(REQUEST request) {
+    return getDbOperationName(request);
+  }
+
   // TODO: make this required to implement
   String getDbSystemName(REQUEST request);
 
@@ -59,6 +70,12 @@ default String getDbSystem(REQUEST request) {
   @Nullable
   String getDbNamespace(REQUEST request);
 
+  // TODO: make this required to implement?
+  @Nullable
+  default String getDbCollectionName(REQUEST request) {
+    return null;
+  }
+
   /**
    * Returns the old db.name value. This is only used for old semantic conventions.
    *

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbClientSpanNameExtractor.java

@@ -106,10 +106,9 @@ private static <REQUEST> String computeSpanNameStable(
       DbClientAttributesGetter<REQUEST, ?> getter,
       REQUEST request,
       @Nullable String operation,
-      @Nullable String collectionName,
       @Nullable String storedProcedureName) {
 
-    String target = collectionName;
+    String target = getter.getDbCollectionName(request);
     if (target == null) {
       target = storedProcedureName;
     }
@@ -164,10 +163,10 @@ public String extract(REQUEST request) {
           return querySummary;
         }
         String operationName = getter.getDbOperationName(request);
-        return computeSpanNameStable(getter, request, operationName, null, null);
+        return computeSpanNameStable(getter, request, operationName, null);
       }
       String dbName = getter.getDbName(request);
-      String operationName = getter.getDbOperationName(request);
+      String operationName = getter.getDbOperation(request);
       return computeSpanName(dbName, operationName, null, null);
     }
   }
@@ -193,11 +192,10 @@ public String extract(REQUEST request) {
           if (querySummary != null) {
             return querySummary;
           }
-          String operationName = getter.getDbOperationName(request);
-          return computeSpanNameStable(getter, request, operationName, null, null);
+          return computeSpanNameStable(getter, request, null, null);
         }
         String dbName = getter.getDbName(request);
-        String operationName = getter.getDbOperationName(request);
+        String operationName = getter.getDbOperation(request);
         return computeSpanName(dbName, operationName, null, null);
       }
 
@@ -224,16 +222,15 @@ public String extract(REQUEST request) {
           return batch ? "BATCH " + querySummary : querySummary;
         }
         return computeSpanNameStable(
-            getter, request, batch ? "BATCH" : null, null, analyzedQuery.getStoredProcedureName());
+            getter, request, batch ? "BATCH" : null, analyzedQuery.getStoredProcedureName());
       }
 
       MultiQuery multiQuery = MultiQuery.analyzeWithSummary(rawQueryTexts, dialect);
       String querySummary = multiQuery.getQuerySummary();
       if (querySummary != null) {
         return querySummary;
       }
-      return computeSpanNameStable(
-          getter, request, null, null, multiQuery.getStoredProcedureName());
+      return computeSpanNameStable(getter, request, null, multiQuery.getStoredProcedureName());
     }
 
     private boolean isBatch(REQUEST request) {
@@ -277,7 +274,7 @@ public String extract(REQUEST request) {
         operationName = analyzedQuery.getOperationName();
       }
       if (operationName == null) {
-        operationName = getter.getDbOperationName(request);
+        operationName = getter.getDbOperation(request);
       }
       return computeSpanName(dbName, operationName, null, null);
     }

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractor.java

@@ -141,10 +141,12 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
       }
     }
 
-    // calling this last so explicit getDbOperationName(), getDbCollectionName(),
-    // getDbQueryText(), and getDbQuerySummary() implementations can override
-    // the parsed values from above
-    DbClientAttributesExtractor.onStartCommon(attributes, getter, request, captureQueryParameters);
+    if (emitOldDatabaseSemconv()) {
+      // calling this last so explicit getDbOperationName(), getDbCollectionName(),
+      // getDbQueryText(), and getDbQuerySummary() implementations can override
+      // the parsed values from above
+      DbClientAttributesExtractor.onStartCommon(attributes, getter, request, captureQueryParameters);
+    }
     serverAttributesExtractor.onStart(attributes, parentContext, request);
   }
 

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesGetter.java

@@ -23,8 +23,11 @@ public interface SqlClientAttributesGetter<REQUEST, RESPONSE>
     extends DbClientAttributesGetter<REQUEST, RESPONSE> {
 
   /**
-   * SqlClientAttributesExtractor will try to populate db.operation.name based on {@link
-   * #getRawQueryTexts(REQUEST)}, but this can be used to explicitly provide the operation name.
+   * DO NOT USE OR OVERRIDE THIS METHOD.
+   *
+   * <p>{@link SqlClientAttributesExtractor} will try to populate {@code db.operation.name} based on
+   * {@link #getRawQueryTexts(REQUEST)} when {@link
+   * SqlClientAttributesExtractorBuilder#setSingleOperationAndCollection(boolean)} is enabled.
    */
   @Override
   @Nullable
@@ -52,6 +55,19 @@ default String getDbQuerySummary(REQUEST request) {
     return null;
   }
 
+  /**
+   * DO NOT USE OR OVERRIDE THIS METHOD.
+   *
+   * <p>{@link SqlClientAttributesExtractor} will try to populate {@code db.collection.name} based
+   * on {@link #getRawQueryTexts(REQUEST)} when {@link
+   * SqlClientAttributesExtractorBuilder#setSingleOperationAndCollection(boolean)} is enabled.
+   */
+  @Override
+  @Nullable
+  default String getDbCollectionName(REQUEST request) {
+    return null;
+  }
+
   /** Returns the SQL dialect used by the database. */
   SqlDialect getSqlDialect(REQUEST request);
 

instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbClientAttributesExtractorTest.java

@@ -8,6 +8,7 @@
 import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitOldDatabaseSemconv;
 import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableDatabaseSemconv;
 import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
+import static io.opentelemetry.semconv.DbAttributes.DB_COLLECTION_NAME;
 import static io.opentelemetry.semconv.DbAttributes.DB_NAMESPACE;
 import static io.opentelemetry.semconv.DbAttributes.DB_OPERATION_NAME;
 import static io.opentelemetry.semconv.DbAttributes.DB_QUERY_SUMMARY;
@@ -50,6 +51,11 @@ public String getDbNamespace(Map<String, String> map) {
       return map.get("db.namespace");
     }
 
+    @Override
+    public String getDbCollectionName(Map<String, String> map) {
+      return map.get("db.collection.name");
+    }
+
     @Deprecated
     @Override
     public String getConnectionString(Map<String, String> map) {
@@ -71,6 +77,12 @@ public String getDbQuerySummary(Map<String, String> map) {
     public String getDbOperationName(Map<String, String> map) {
       return map.get("db.operation.name");
     }
+
+    @Deprecated
+    @Override
+    public String getDbOperation(Map<String, String> map) {
+      return map.get("db.operation");
+    }
   }
 
   @SuppressWarnings("deprecation") // TODO DB_CONNECTION_STRING deprecation
@@ -81,9 +93,11 @@ void shouldExtractAllAvailableAttributes() {
     request.put("db.system", "myDb");
     request.put("db.user", "username");
     request.put("db.namespace", "potatoes");
+    request.put("db.collection.name", "potato");
     request.put("db.connection_string", "mydb:///potatoes");
     request.put("db.query.text", "SELECT * FROM potato");
     request.put("db.query_summary", "SELECT potato");
+    request.put("db.operation", "old SELECT");
     request.put("db.operation.name", "SELECT");
 
     Context context = Context.root();
@@ -108,7 +122,8 @@ void shouldExtractAllAvailableAttributes() {
               entry(DB_NAME, "potatoes"),
               entry(DB_CONNECTION_STRING, "mydb:///potatoes"),
               entry(DB_STATEMENT, "SELECT * FROM potato"),
-              entry(DB_OPERATION, "SELECT"),
+              entry(DB_OPERATION, "old SELECT"),
+              entry(DB_COLLECTION_NAME, "potato"),
               entry(DB_NAMESPACE, "potatoes"),
               entry(DB_QUERY_TEXT, "SELECT * FROM potato"),
               entry(DB_QUERY_SUMMARY, "SELECT potato"),
@@ -121,11 +136,12 @@ void shouldExtractAllAvailableAttributes() {
               entry(DB_NAME, "potatoes"),
               entry(DB_CONNECTION_STRING, "mydb:///potatoes"),
               entry(DB_STATEMENT, "SELECT * FROM potato"),
-              entry(DB_OPERATION, "SELECT"));
+              entry(DB_OPERATION, "old SELECT"));
     } else if (emitStableDatabaseSemconv()) {
       assertThat(startAttributes.build())
           .containsOnly(
               entry(DB_SYSTEM_NAME, "myDb"),
+              entry(DB_COLLECTION_NAME, "potato"),
               entry(DB_NAMESPACE, "potatoes"),
               entry(DB_QUERY_TEXT, "SELECT * FROM potato"),
               entry(DB_QUERY_SUMMARY, "SELECT potato"),

instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbClientSpanNameExtractorTest.java

@@ -100,11 +100,12 @@ void shouldExtractOperationAndName() {
     // given
     DbRequest dbRequest = new DbRequest();
 
-    when(dbAttributesGetter.getDbOperationName(dbRequest)).thenReturn("SELECT");
     if (emitStableDatabaseSemconv()) {
+      when(dbAttributesGetter.getDbOperationName(dbRequest)).thenReturn("SELECT");
       when(dbAttributesGetter.getDbNamespace(dbRequest)).thenReturn("database");
     }
     if (emitOldDatabaseSemconv() && !emitStableDatabaseSemconv()) {
+      when(dbAttributesGetter.getDbOperation(dbRequest)).thenReturn("SELECT");
       when(dbAttributesGetter.getDbName(dbRequest)).thenReturn("database");
     }
 
@@ -117,12 +118,42 @@ void shouldExtractOperationAndName() {
     assertThat(spanName).isEqualTo("SELECT database");
   }
 
+  @Test
+  void shouldPreferCollectionNameOverNamespace() {
+    // given
+    DbRequest dbRequest = new DbRequest();
+
+    if (emitStableDatabaseSemconv()) {
+      when(dbAttributesGetter.getDbOperationName(dbRequest)).thenReturn("SELECT");
+      lenient().when(dbAttributesGetter.getDbNamespace(dbRequest)).thenReturn("database");
+      when(dbAttributesGetter.getDbCollectionName(dbRequest)).thenReturn("users");
+    }
+    if (emitOldDatabaseSemconv() && !emitStableDatabaseSemconv()) {
+      when(dbAttributesGetter.getDbOperation(dbRequest)).thenReturn("SELECT");
+      when(dbAttributesGetter.getDbName(dbRequest)).thenReturn("database");
+    }
+
+    SpanNameExtractor<DbRequest> underTest = DbClientSpanNameExtractor.create(dbAttributesGetter);
+
+    // when
+    String spanName = underTest.extract(dbRequest);
+
+    // then
+    assertThat(spanName)
+        .isEqualTo(emitStableDatabaseSemconv() ? "SELECT users" : "SELECT database");
+  }
+
   @Test
   void shouldExtractOperation() {
     // given
     DbRequest dbRequest = new DbRequest();
 
-    when(dbAttributesGetter.getDbOperationName(dbRequest)).thenReturn("SELECT");
+    if (emitStableDatabaseSemconv()) {
+      when(dbAttributesGetter.getDbOperationName(dbRequest)).thenReturn("SELECT");
+    }
+    if (emitOldDatabaseSemconv() && !emitStableDatabaseSemconv()) {
+      when(dbAttributesGetter.getDbOperation(dbRequest)).thenReturn("SELECT");
+    }
 
     SpanNameExtractor<DbRequest> underTest = DbClientSpanNameExtractor.create(dbAttributesGetter);
 
@@ -177,7 +208,7 @@ void shouldUseQuerySummaryWhenAvailable() {
       when(dbAttributesGetter.getDbQuerySummary(dbRequest)).thenReturn("SELECT users");
     }
     if (emitOldDatabaseSemconv() && !emitStableDatabaseSemconv()) {
-      when(dbAttributesGetter.getDbOperationName(dbRequest)).thenReturn("SELECT");
+      when(dbAttributesGetter.getDbOperation(dbRequest)).thenReturn("SELECT");
       when(dbAttributesGetter.getDbName(dbRequest)).thenReturn("database");
     }
 
@@ -236,12 +267,11 @@ void shouldExtractFullSpanNameForSingleQueryBatch() {
   }
 
   @Test
-  void shouldFallBackToExplicitOperationNameForEmptySqlQuery() {
+  void shouldFallBackToNamespaceForEmptySqlQuery() {
     // given
     DbRequest dbRequest = new DbRequest();
 
     when(sqlAttributesGetter.getRawQueryTexts(dbRequest)).thenReturn(emptyList());
-    when(sqlAttributesGetter.getDbOperationName(dbRequest)).thenReturn("WRITE");
     if (emitStableDatabaseSemconv()) {
       when(sqlAttributesGetter.getDbNamespace(dbRequest)).thenReturn("mydb");
     }
@@ -255,7 +285,7 @@ void shouldFallBackToExplicitOperationNameForEmptySqlQuery() {
     String spanName = underTest.extract(dbRequest);
 
     // then
-    assertThat(spanName).isEqualTo("WRITE mydb");
+    assertThat(spanName).isEqualTo("mydb");
   }
 
   @Test
@@ -283,12 +313,11 @@ void shouldPreserveOldSemconvSpanNameForMigration() {
 
   @Test
   @SuppressWarnings("deprecation") // testing deprecated method
-  void shouldFallBackToExplicitOperationForEmptySqlQueryInMigration() {
+  void shouldFallBackToNamespaceForEmptySqlQueryInMigration() {
     // given
     DbRequest dbRequest = new DbRequest();
 
     when(sqlAttributesGetter.getRawQueryTexts(dbRequest)).thenReturn(emptyList());
-    when(sqlAttributesGetter.getDbOperationName(dbRequest)).thenReturn("WRITE");
     if (emitStableDatabaseSemconv()) {
       when(sqlAttributesGetter.getDbNamespace(dbRequest)).thenReturn("mydb");
     }
@@ -303,7 +332,7 @@ void shouldFallBackToExplicitOperationForEmptySqlQueryInMigration() {
     String spanName = underTest.extract(dbRequest);
 
     // then
-    assertThat(spanName).isEqualTo("WRITE mydb");
+    assertThat(spanName).isEqualTo("mydb");
   }
 
   static class DbRequest {}