Fix HBase stable DB semconv attributes (#18990)

Author: trask

instrumentation/hbase-client-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hbase/client/v2_0/HbaseAttributesGetter.java

@@ -9,6 +9,7 @@
 import io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DbSystemNameIncubatingValues;
 import java.net.InetSocketAddress;
 import javax.annotation.Nullable;
+import org.apache.hadoop.hbase.TableName;
 
 final class HbaseAttributesGetter implements DbClientAttributesGetter<HbaseRequest, Void> {
 
@@ -21,12 +22,29 @@ public String getDbSystemName(HbaseRequest hbaseRequest) {
   @Nullable
   @Override
   public String getDbNamespace(HbaseRequest hbaseRequest) {
-    return hbaseRequest.getTable();
+    TableName tableName = hbaseRequest.getTableName();
+    return tableName == null ? null : tableName.getNamespaceAsString();
   }
 
   @Nullable
   @Override
-  // Old database semconv still use db.user, so we must implement the deprecated hook.
+  public String getDbCollectionName(HbaseRequest hbaseRequest) {
+    TableName tableName = hbaseRequest.getTableName();
+    return tableName == null ? null : tableName.getNameAsString();
+  }
+
+  @Nullable
+  @Override
+  // Old database semconv still uses db.name, so we must implement the deprecated hook.
+  @SuppressWarnings("deprecation")
+  public String getDbName(HbaseRequest hbaseRequest) {
+    TableName tableName = hbaseRequest.getTableName();
+    return tableName == null ? null : tableName.getNameAsString();
+  }
+
+  @Nullable
+  @Override
+  // Old database semconv still uses db.user, so we must implement the deprecated hook.
   @SuppressWarnings("deprecation")
   public String getUser(HbaseRequest hbaseRequest) {
     return hbaseRequest.getUser();

instrumentation/hbase-client-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hbase/client/v2_0/HbaseRequest.java

@@ -7,24 +7,25 @@
 
 import com.google.auto.value.AutoValue;
 import javax.annotation.Nullable;
+import org.apache.hadoop.hbase.TableName;
 
 @AutoValue
 public abstract class HbaseRequest {
 
   public static HbaseRequest create(
       @Nullable String operation,
-      @Nullable String table,
+      @Nullable TableName tableName,
       @Nullable String user,
       @Nullable String host,
       @Nullable Integer port) {
-    return new AutoValue_HbaseRequest(operation, table, user, host, port);
+    return new AutoValue_HbaseRequest(operation, tableName, user, host, port);
   }
 
   @Nullable
   public abstract String getOperation();
 
   @Nullable
-  public abstract String getTable();
+  public abstract TableName getTableName();
 
   @Nullable
   public abstract String getUser();

instrumentation/hbase-client-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hbase/client/v2_0/HbaseSingletons.java

@@ -12,21 +12,22 @@
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor;
 import javax.annotation.Nullable;
+import org.apache.hadoop.hbase.TableName;
 
 public class HbaseSingletons {
 
   private static final String INSTRUMENTATION_NAME = "io.opentelemetry.hbase-client-2.0";
-  private static final ThreadLocal<String> tableNameThreadLocal = new ThreadLocal<>();
+  private static final ThreadLocal<TableName> tableNameThreadLocal = new ThreadLocal<>();
   private static final ThreadLocal<RequestAndContext> requestAndContextThreadLocal =
       new ThreadLocal<>();
   private static final Instrumenter<HbaseRequest, Void> instrumenter = createInstrumenter();
 
-  public static void setTableName(String tableName) {
+  public static void setTableName(TableName tableName) {
     tableNameThreadLocal.set(tableName);
   }
 
   @Nullable
-  public static String getTableName() {
+  public static TableName getTableName() {
     return tableNameThreadLocal.get();
   }
 

instrumentation/hbase-client-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hbase/client/v2_0/RegionServerCallableInstrumentation.java

@@ -34,9 +34,9 @@ public void transform(TypeTransformer transformer) {
   @SuppressWarnings("unused")
   public static class RpcCallAdvice {
     @Advice.OnMethodEnter(suppress = Throwable.class)
-    public static void onEnter(@Advice.FieldValue(value = "tableName") TableName table) {
-      if (table != null) {
-        setTableName(table.getNameAsString());
+    public static void onEnter(@Advice.FieldValue(value = "tableName") TableName tableName) {
+      if (tableName != null) {
+        setTableName(tableName);
       }
     }
 

instrumentation/hbase-client-2.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/hbase/testing/AbstractHbaseTest.java

@@ -11,6 +11,7 @@
 import static io.opentelemetry.instrumentation.testing.junit.db.SemconvStabilityUtil.maybeStableDbSystemName;
 import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
 import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies;
+import static io.opentelemetry.semconv.DbAttributes.DB_COLLECTION_NAME;
 import static io.opentelemetry.semconv.DbAttributes.DB_SYSTEM_NAME;
 import static io.opentelemetry.semconv.ErrorAttributes.ERROR_TYPE;
 import static io.opentelemetry.semconv.ExceptionAttributes.EXCEPTION_MESSAGE;
@@ -273,7 +274,16 @@ void testGetTimeout() throws IOException {
                                     maybeStable(DB_SYSTEM),
                                     maybeStableDbSystemName(DB_SYSTEM_VALUE)),
                                 equalTo(maybeStable(DB_OPERATION), GET),
-                                equalTo(maybeStable(DB_NAME), TABLE_NAME.getNameAsString()),
+                                equalTo(
+                                    maybeStable(DB_NAME),
+                                    emitStableDatabaseSemconv()
+                                        ? TABLE_NAME.getNamespaceAsString()
+                                        : TABLE_NAME.getNameAsString()),
+                                equalTo(
+                                    DB_COLLECTION_NAME,
+                                    emitStableDatabaseSemconv()
+                                        ? TABLE_NAME.getNameAsString()
+                                        : null),
                                 equalTo(SERVER_ADDRESS, hostname),
                                 equalTo(SERVER_PORT, REGION_SERVER_PORT),
                                 equalTo(
@@ -509,6 +519,7 @@ void hasDurationMetric() throws IOException {
         DB_SYSTEM_NAME,
         maybeStable(DB_OPERATION),
         maybeStable(DB_NAME),
+        DB_COLLECTION_NAME,
         SERVER_ADDRESS,
         SERVER_PORT);
   }
@@ -531,7 +542,8 @@ protected Consumer<TraceAssert> traceAssertConsumer(
                     .hasAttributesSatisfyingExactly(
                         equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(DB_SYSTEM_VALUE)),
                         equalTo(maybeStable(DB_OPERATION), operation),
-                        equalTo(maybeStable(DB_NAME), hasTable ? table.getNameAsString() : null),
+                        equalTo(maybeStable(DB_NAME), dbNamespace(table, hasTable)),
+                        equalTo(DB_COLLECTION_NAME, dbCollectionName(table, hasTable)),
                         equalTo(SERVER_ADDRESS, hostname),
                         equalTo(SERVER_PORT, port),
                         satisfies(
@@ -540,4 +552,21 @@ protected Consumer<TraceAssert> traceAssertConsumer(
                                 ? AbstractAssert::isNull
                                 : AbstractAssert::isNotNull)));
   }
+
+  private static String dbNamespace(TableName table, boolean hasTable) {
+    if (!hasTable) {
+      return null;
+    }
+    if (emitStableDatabaseSemconv()) {
+      return table.getNamespaceAsString();
+    }
+    return table.getNameAsString();
+  }
+
+  private static String dbCollectionName(TableName table, boolean hasTable) {
+    if (hasTable && emitStableDatabaseSemconv()) {
+      return table.getNameAsString();
+    }
+    return null;
+  }
 }