Simplify db getter and extractor hierarchy

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

@@ -12,6 +12,9 @@
 import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
 import io.opentelemetry.instrumentation.api.internal.SemconvStability;
+import io.opentelemetry.instrumentation.api.internal.SpanKey;
+import io.opentelemetry.instrumentation.api.internal.SpanKeyProvider;
+import javax.annotation.Nullable;
 
 /**
  * Extractor of <a
@@ -22,38 +25,66 @@
  * attribute extraction from request/response objects.
  */
 public final class DbClientAttributesExtractor<REQUEST, RESPONSE>
-    extends DbClientCommonAttributesExtractor<
-        REQUEST, RESPONSE, DbClientAttributesGetter<REQUEST>> {
+    implements AttributesExtractor<REQUEST, RESPONSE>, SpanKeyProvider {
 
   // copied from DbIncubatingAttributes
+  private static final AttributeKey<String> DB_NAME = AttributeKey.stringKey("db.name");
+  private static final AttributeKey<String> DB_NAMESPACE = AttributeKey.stringKey("db.namespace");
+  private static final AttributeKey<String> DB_SYSTEM = AttributeKey.stringKey("db.system");
+  private static final AttributeKey<String> DB_USER = AttributeKey.stringKey("db.user");
+  private static final AttributeKey<String> DB_CONNECTION_STRING =
+      AttributeKey.stringKey("db.connection_string");
   private static final AttributeKey<String> DB_STATEMENT = AttributeKey.stringKey("db.statement");
   private static final AttributeKey<String> DB_QUERY_TEXT = AttributeKey.stringKey("db.query.text");
 
   private static final AttributeKey<String> DB_OPERATION = AttributeKey.stringKey("db.operation");
   private static final AttributeKey<String> DB_OPERATION_NAME =
       AttributeKey.stringKey("db.operation.name");
 
+  private final DbClientAttributesGetter<REQUEST> getter;
+
   /** Creates the database client attributes extractor with default configuration. */
   public static <REQUEST, RESPONSE> AttributesExtractor<REQUEST, RESPONSE> create(
       DbClientAttributesGetter<REQUEST> getter) {
     return new DbClientAttributesExtractor<>(getter);
   }
 
   DbClientAttributesExtractor(DbClientAttributesGetter<REQUEST> getter) {
-    super(getter);
+    this.getter = getter;
   }
 
   @Override
+  @SuppressWarnings("deprecation") // using deprecated semconv
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
-    super.onStart(attributes, parentContext, request);
-
+    internalSet(attributes, DB_SYSTEM, getter.getDbSystem(request));
     if (SemconvStability.emitStableDatabaseSemconv()) {
+      internalSet(attributes, DB_NAMESPACE, getter.getDbNamespace(request));
       internalSet(attributes, DB_QUERY_TEXT, getter.getDbQueryText(request));
       internalSet(attributes, DB_OPERATION_NAME, getter.getDbOperationName(request));
     }
     if (SemconvStability.emitOldDatabaseSemconv()) {
+      internalSet(attributes, DB_USER, getter.getUser(request));
+      internalSet(attributes, DB_NAME, getter.getDbNamespace(request));
+      internalSet(attributes, DB_CONNECTION_STRING, getter.getConnectionString(request));
       internalSet(attributes, DB_STATEMENT, getter.getDbQueryText(request));
       internalSet(attributes, DB_OPERATION, getter.getDbOperationName(request));
     }
   }
+
+  @Override
+  public void onEnd(
+      AttributesBuilder attributes,
+      Context context,
+      REQUEST request,
+      @Nullable RESPONSE response,
+      @Nullable Throwable error) {}
+
+  /**
+   * This method is internal and is hence not for public use. Its API is unstable and can change at
+   * any time.
+   */
+  @Override
+  public SpanKey internalGetSpanKey() {
+    return SpanKey.DB_CLIENT;
+  }
 }

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

@@ -18,6 +18,7 @@
  * from the attribute methods, but implement as many as possible for best compliance with the
  * OpenTelemetry specification.
  */
+@SuppressWarnings("deprecation") // using deprecated semconv
 public interface DbClientAttributesGetter<REQUEST> extends DbClientCommonAttributesGetter<REQUEST> {
 
   /**
@@ -26,27 +27,50 @@ public interface DbClientAttributesGetter<REQUEST> extends DbClientCommonAttribu
   @Deprecated
   @Nullable
   default String getStatement(REQUEST request) {
-    return null;
+    return getDbQueryText(request);
   }
 
-  // TODO: make this required to implement
+  // TODO (trask) add default implementation that returns null
   @Nullable
-  default String getDbQueryText(REQUEST request) {
-    return getStatement(request);
-  }
+  String getDbQueryText(REQUEST request);
 
   /**
    * @deprecated Use {@link #getDbOperationName(REQUEST)} instead.
    */
   @Deprecated
   @Nullable
   default String getOperation(REQUEST request) {
-    return null;
+    return getDbOperationName(request);
+  }
+
+  // TODO (trask) add default implementation that returns null
+  //  after https://github.com/open-telemetry/semantic-conventions/pull/1566
+  @Nullable
+  String getDbOperationName(REQUEST request);
+
+  @Nullable
+  String getDbSystem(REQUEST request);
+
+  @Nullable
+  String getDbNamespace(REQUEST request);
+
+  /**
+   * @deprecated Use {@link #getDbSystem(Object)} instead.
+   */
+  @Deprecated
+  @Override
+  @Nullable
+  default String getSystem(REQUEST request) {
+    return getDbSystem(request);
   }
 
-  // TODO: make this required to implement
+  /**
+   * @deprecated Use {@link #getDbNamespace(Object)} instead.
+   */
+  @Deprecated
+  @Override
   @Nullable
-  default String getDbOperationName(REQUEST request) {
-    return getOperation(request);
+  default String getName(REQUEST request) {
+    return getDbNamespace(request);
   }
 }

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

@@ -8,40 +8,18 @@
 import javax.annotation.Nullable;
 
 /** An interface for getting attributes common to database clients. */
+@Deprecated // to be removed in 3.0.0
 public interface DbClientCommonAttributesGetter<REQUEST> {
 
-  @Deprecated
   @Nullable
-  default String getSystem(REQUEST request) {
-    return null;
-  }
+  String getSystem(REQUEST request);
 
-  // TODO: make this required to implement
-  @Nullable
-  default String getDbSystem(REQUEST request) {
-    return getSystem(request);
-  }
-
-  @Deprecated
   @Nullable
   String getUser(REQUEST request);
 
-  /**
-   * @deprecated Use {@link #getDbNamespace(Object)} instead.
-   */
-  @Deprecated
-  @Nullable
-  default String getName(REQUEST request) {
-    return null;
-  }
-
-  // TODO: make this required to implement
   @Nullable
-  default String getDbNamespace(REQUEST request) {
-    return getName(request);
-  }
+  String getName(REQUEST request);
 
-  @Deprecated
   @Nullable
   String getConnectionString(REQUEST request);
 }

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

@@ -12,6 +12,9 @@
 import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
 import io.opentelemetry.instrumentation.api.internal.SemconvStability;
+import io.opentelemetry.instrumentation.api.internal.SpanKey;
+import io.opentelemetry.instrumentation.api.internal.SpanKeyProvider;
+import javax.annotation.Nullable;
 
 /**
  * Extractor of <a
@@ -24,8 +27,7 @@
  * parameters are removed.
  */
 public final class SqlClientAttributesExtractor<REQUEST, RESPONSE>
-    extends DbClientCommonAttributesExtractor<
-        REQUEST, RESPONSE, SqlClientAttributesGetter<REQUEST>> {
+    implements AttributesExtractor<REQUEST, RESPONSE>, SpanKeyProvider {
 
   // copied from DbIncubatingAttributes
   private static final AttributeKey<String> DB_OPERATION = AttributeKey.stringKey("db.operation");
@@ -58,18 +60,22 @@ public static <REQUEST, RESPONSE> SqlClientAttributesExtractorBuilder<REQUEST, R
   private final AttributeKey<String> oldSemconvTableAttribute;
   private final boolean statementSanitizationEnabled;
 
+  private final AttributesExtractor<REQUEST, RESPONSE> delegate;
+  private final SqlClientAttributesGetter<REQUEST> getter;
+
   SqlClientAttributesExtractor(
       SqlClientAttributesGetter<REQUEST> getter,
       AttributeKey<String> oldSemconvTableAttribute,
       boolean statementSanitizationEnabled) {
-    super(getter);
+    this.delegate = DbClientAttributesExtractor.create(getter);
     this.oldSemconvTableAttribute = oldSemconvTableAttribute;
     this.statementSanitizationEnabled = statementSanitizationEnabled;
+    this.getter = getter;
   }
 
   @Override
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
-    super.onStart(attributes, parentContext, request);
+    delegate.onStart(attributes, parentContext, request);
 
     String rawQueryText = getter.getRawQueryText(request);
     SqlStatementInfo sanitizedStatement = sanitizer.sanitize(rawQueryText);
@@ -97,4 +103,23 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
       }
     }
   }
+
+  @Override
+  public void onEnd(
+      AttributesBuilder attributes,
+      Context context,
+      REQUEST request,
+      @Nullable RESPONSE response,
+      @Nullable Throwable error) {
+    delegate.onEnd(attributes, context, request, response, error);
+  }
+
+  /**
+   * This method is internal and is hence not for public use. Its API is unstable and can change at
+   * any time.
+   */
+  @Override
+  public SpanKey internalGetSpanKey() {
+    return SpanKey.DB_CLIENT;
+  }
 }

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

@@ -18,8 +18,7 @@
  * from the attribute methods, but implement as many as possible for best compliance with the
  * OpenTelemetry specification.
  */
-public interface SqlClientAttributesGetter<REQUEST>
-    extends DbClientCommonAttributesGetter<REQUEST> {
+public interface SqlClientAttributesGetter<REQUEST> extends DbClientAttributesGetter<REQUEST> {
 
   /**
    * Get the raw SQL statement. The value returned by this method is later sanitized by the {@link