Enable span suppression by SpanKey by default (open-telemetry#5779)

* Enable span suppression by SpanKey by default

* fix HTTP tests (probably)

* add exception for camel

* remove suppression tests from @WithSpan instrumentations

* remove suppression tests from @WithSpan instrumentation; spring boot autoconfigure

* fix twilio tests

* fix netty-based HTTP clients, remove AWS SDK 1.11 unit test

* fix elasticsearch tests

* codenarc

* spotless

* fix AWS SDK 1.11 tests

* remove a TODO

* code review comments

* fix merge conflict

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java

@@ -13,6 +13,8 @@
 
 import io.opentelemetry.api.common.AttributesBuilder;
 import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.internal.SpanKey;
+import io.opentelemetry.instrumentation.api.internal.SpanKeyProvider;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import java.util.List;
 import java.util.function.Function;
@@ -29,7 +31,8 @@
  */
 public final class HttpServerAttributesExtractor<REQUEST, RESPONSE>
     extends HttpCommonAttributesExtractor<
-        REQUEST, RESPONSE, HttpServerAttributesGetter<REQUEST, RESPONSE>> {
+        REQUEST, RESPONSE, HttpServerAttributesGetter<REQUEST, RESPONSE>>
+    implements SpanKeyProvider {
 
   /** Creates the HTTP server attributes extractor with default configuration. */
   public static <REQUEST, RESPONSE> HttpServerAttributesExtractor<REQUEST, RESPONSE> create(
@@ -136,4 +139,13 @@ private String clientIp(REQUEST request) {
 
     return null;
   }
+
+  /**
+   * 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.HTTP_SERVER;
+  }
 }

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/rpc/RpcServerAttributesExtractor.java

@@ -5,6 +5,9 @@
 
 package io.opentelemetry.instrumentation.api.instrumenter.rpc;
 
+import io.opentelemetry.instrumentation.api.internal.SpanKey;
+import io.opentelemetry.instrumentation.api.internal.SpanKeyProvider;
+
 /**
  * Extractor of <a
  * href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/rpc.md">RPC
@@ -14,7 +17,7 @@
  * extraction from request/response objects.
  */
 public final class RpcServerAttributesExtractor<REQUEST, RESPONSE>
-    extends RpcCommonAttributesExtractor<REQUEST, RESPONSE> {
+    extends RpcCommonAttributesExtractor<REQUEST, RESPONSE> implements SpanKeyProvider {
 
   /** Creates the RPC server attributes extractor. */
   public static <REQUEST, RESPONSE> RpcServerAttributesExtractor<REQUEST, RESPONSE> create(
@@ -25,4 +28,13 @@ public static <REQUEST, RESPONSE> RpcServerAttributesExtractor<REQUEST, RESPONSE
   private RpcServerAttributesExtractor(RpcAttributesGetter<REQUEST> getter) {
     super(getter);
   }
+
+  /**
+   * 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.RPC_SERVER;
+  }
 }

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/server/ServerSpan.java

@@ -23,7 +23,13 @@ public final class ServerSpan {
    */
   @Nullable
   public static Span fromContextOrNull(Context context) {
-    return SpanKey.SERVER.fromContextOrNull(context);
+    // try the HTTP semconv span first
+    Span span = SpanKey.HTTP_SERVER.fromContextOrNull(context);
+    // if it's absent then try the SERVER one - perhaps suppression by span kind is enabled
+    if (span == null) {
+      span = SpanKey.KIND_SERVER.fromContextOrNull(context);
+    }
+    return span;
   }
 
   private ServerSpan() {}

instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolderTest.java

@@ -31,7 +31,7 @@ void shouldSetRouteEvenIfSpanIsNotSampled() {
 
     Context context = Context.root();
     context = context.with(span);
-    context = SpanKey.SERVER.storeInContext(context, span);
+    context = SpanKey.HTTP_SERVER.storeInContext(context, span);
     context = HttpRouteHolder.get().start(context, null, Attributes.empty());
 
     assertNull(HttpRouteHolder.getRoute(context));

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/config/ConfigBuilder.java

@@ -9,6 +9,7 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Properties;
+import javax.annotation.Nullable;
 
 /** A builder of a {@link Config}. */
 public final class ConfigBuilder {
@@ -19,8 +20,10 @@ public final class ConfigBuilder {
   ConfigBuilder() {}
 
   /** Adds a single property named to the config. */
-  public ConfigBuilder addProperty(String name, String value) {
-    allProperties.put(NamingConvention.DOT.normalize(name), value);
+  public ConfigBuilder addProperty(String name, @Nullable String value) {
+    if (value != null) {
+      allProperties.put(NamingConvention.DOT.normalize(name), value);
+    }
     return this;
   }
 

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java

@@ -76,7 +76,7 @@ public static <REQUEST, RESPONSE> InstrumenterBuilder<REQUEST, RESPONSE> builder
   private final ErrorCauseExtractor errorCauseExtractor;
   @Nullable private final TimeExtractor<REQUEST, RESPONSE> timeExtractor;
   private final boolean enabled;
-  private final SpanSuppressionStrategy spanSuppressionStrategy;
+  private final SpanSuppressor spanSuppressor;
 
   Instrumenter(InstrumenterBuilder<REQUEST, RESPONSE> builder) {
     this.instrumentationName = builder.instrumentationName;
@@ -91,7 +91,7 @@ public static <REQUEST, RESPONSE> InstrumenterBuilder<REQUEST, RESPONSE> builder
     this.errorCauseExtractor = builder.errorCauseExtractor;
     this.timeExtractor = builder.timeExtractor;
     this.enabled = builder.enabled;
-    this.spanSuppressionStrategy = builder.buildSpanSuppressionStrategy();
+    this.spanSuppressor = builder.buildSpanSuppressor();
   }
 
   /**
@@ -105,7 +105,7 @@ public boolean shouldStart(Context parentContext, REQUEST request) {
       return false;
     }
     SpanKind spanKind = spanKindExtractor.extract(request);
-    boolean suppressed = spanSuppressionStrategy.shouldSuppress(parentContext, spanKind);
+    boolean suppressed = spanSuppressor.shouldSuppress(parentContext, spanKind);
 
     if (suppressed) {
       supportability.recordSuppressedSpan(spanKind, instrumentationName);
@@ -162,7 +162,7 @@ public Context start(Context parentContext, REQUEST request) {
       }
     }
 
-    return spanSuppressionStrategy.storeInContext(context, spanKind, span);
+    return spanSuppressor.storeInContext(context, spanKind, span);
   }
 
   /**

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java

@@ -10,7 +10,6 @@
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.metrics.Meter;
 import io.opentelemetry.api.metrics.MeterBuilder;
-import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.api.trace.StatusCode;
 import io.opentelemetry.api.trace.Tracer;
 import io.opentelemetry.api.trace.TracerBuilder;
@@ -36,10 +35,8 @@
  */
 public final class InstrumenterBuilder<REQUEST, RESPONSE> {
 
-  /** Instrumentation type suppression configuration property key. */
-  private static final boolean ENABLE_SPAN_SUPPRESSION_BY_TYPE =
-      Config.get()
-          .getBoolean("otel.instrumentation.experimental.outgoing-span-suppression-by-type", false);
+  private static final SpanSuppressionStrategy spanSuppressionStrategy =
+      SpanSuppressionStrategy.fromConfig(Config.get());
 
   final OpenTelemetry openTelemetry;
   final String instrumentationName;
@@ -61,8 +58,6 @@ public final class InstrumenterBuilder<REQUEST, RESPONSE> {
   @Nullable TimeExtractor<REQUEST, RESPONSE> timeExtractor = null;
   boolean enabled = true;
 
-  private boolean enableSpanSuppressionByType = ENABLE_SPAN_SUPPRESSION_BY_TYPE;
-
   InstrumenterBuilder(
       OpenTelemetry openTelemetry,
       String instrumentationName,
@@ -192,42 +187,6 @@ public InstrumenterBuilder<REQUEST, RESPONSE> setEnabled(boolean enabled) {
     return this;
   }
 
-  // visible for tests
-  /**
-   * Enables CLIENT nested span suppression based on the instrumentation type.
-   *
-   * <p><strong>When enabled:</strong>.
-   *
-   * <ul>
-   *   <li>CLIENT nested spans are suppressed depending on their type: {@code
-   *       HttpClientAttributesExtractor HTTP}, {@code RpcClientAttributesExtractor RPC} or {@code
-   *       DbClientAttributesExtractor database} clients. If a span with the same type is present in
-   *       the parent context object, new span of the same type will not be started.
-   * </ul>
-   *
-   * <p><strong>When disabled:</strong>
-   *
-   * <ul>
-   *   <li>CLIENT nested spans are always suppressed.
-   * </ul>
-   *
-   * <p>For all other {@linkplain SpanKind span kinds} the suppression rules are as follows:
-   *
-   * <ul>
-   *   <li>SERVER nested spans are always suppressed. If a SERVER span is present in the parent
-   *       context object, new SERVER span will not be started.
-   *   <li>Messaging (PRODUCER and CONSUMER) nested spans are suppressed depending on their {@code
-   *       MessageOperation operation}. If a span with the same operation is present in the parent
-   *       context object, new span with the same operation will not be started.
-   *   <li>INTERNAL spans are never suppressed.
-   * </ul>
-   */
-  InstrumenterBuilder<REQUEST, RESPONSE> enableInstrumentationTypeSuppression(
-      boolean enableInstrumentationType) {
-    this.enableSpanSuppressionByType = enableInstrumentationType;
-    return this;
-  }
-
   /**
    * Returns a new {@link Instrumenter} which will create client spans and inject context into
    * requests.
@@ -327,13 +286,8 @@ List<RequestListener> buildRequestListeners() {
     return listeners;
   }
 
-  SpanSuppressionStrategy buildSpanSuppressionStrategy() {
-    Set<SpanKey> spanKeys = getSpanKeysFromAttributesExtractors();
-    if (enableSpanSuppressionByType) {
-      return SpanSuppressionStrategy.from(spanKeys);
-    }
-    // if not enabled, preserve current behavior, not distinguishing CLIENT instrumentation types
-    return SpanSuppressionStrategy.suppressNestedClients(spanKeys);
+  SpanSuppressor buildSpanSuppressor() {
+    return spanSuppressionStrategy.create(getSpanKeysFromAttributesExtractors());
   }
 
   private Set<SpanKey> getSpanKeysFromAttributesExtractors() {