Browse Source

Enable span suppression by SpanKey by default (#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>
Mateusz Rzeszutek 2 years ago
parent
commit
4e3f19d469
50 changed files with 867 additions and 939 deletions
  1. 13 1
      instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java
  2. 13 1
      instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/rpc/RpcServerAttributesExtractor.java
  3. 7 1
      instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/server/ServerSpan.java
  4. 1 1
      instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolderTest.java
  5. 5 2
      instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/config/ConfigBuilder.java
  6. 0 62
      instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/CompositeSuppressionStrategy.java
  7. 4 4
      instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java
  8. 4 50
      instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java
  9. 0 25
      instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/NoopSuppressionStrategy.java
  10. 75 39
      instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressionStrategy.java
  11. 105 0
      instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressor.java
  12. 0 39
      instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SuppressIfSameSpanKeyStrategy.java
  13. 31 21
      instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SpanKey.java
  14. 27 224
      instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java
  15. 116 102
      instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressionStrategyTest.java
  16. 3 0
      instrumentation/apache-camel-2.20/javaagent/build.gradle.kts
  17. 1 1
      instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientRequest.java
  18. 0 11
      instrumentation/aws-sdk/aws-sdk-1.11/javaagent-unit-tests/build.gradle.kts
  19. 0 74
      instrumentation/aws-sdk/aws-sdk-1.11/javaagent-unit-tests/src/test/java/TracingRequestHandlerTest.java
  20. 0 5
      instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/testSqs/groovy/io/opentelemetry/javaagent/instrumentation/awssdk/v1_11/SqsTracingTest.groovy
  21. 2 15
      instrumentation/aws-sdk/aws-sdk-1.11/testing/src/main/groovy/io/opentelemetry/instrumentation/awssdk/v1_11/AbstractSqsTracingTest.groovy
  22. 13 3
      instrumentation/azure-core/azure-core-1.19/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/azurecore/v1_19/SuppressNestedClientMono.java
  23. 30 2
      instrumentation/elasticsearch/elasticsearch-rest-5.0/javaagent/src/test/groovy/ElasticsearchRest5Test.groovy
  24. 30 2
      instrumentation/elasticsearch/elasticsearch-rest-6.4/javaagent/src/test/groovy/ElasticsearchRest6Test.groovy
  25. 30 2
      instrumentation/elasticsearch/elasticsearch-rest-7.0/javaagent/src/test/groovy/ElasticsearchRest7Test.groovy
  26. 3 3
      instrumentation/jaxrs/jaxrs-1.0/javaagent/src/test/groovy/JaxRsAnnotations1InstrumentationTest.groovy
  27. 2 2
      instrumentation/jaxrs/jaxrs-1.0/javaagent/src/test/groovy/JerseyTest.groovy
  28. 3 3
      instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-annotations/javaagent/src/test/groovy/JaxrsAnnotationsInstrumentationTest.groovy
  29. 1 1
      instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/testing/src/main/groovy/JaxRsFilterTest.groovy
  30. 44 0
      instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/client/HttpClientSpanKeyAttributesExtractor.java
  31. 20 6
      instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/client/NettyClientInstrumenterFactory.java
  32. 0 37
      instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/WithSpanInstrumentationTest.groovy
  33. 0 20
      instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/java/io/opentelemetry/test/annotation/TracedWithSpan.java
  34. 8 2
      instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/AgentContextStorage.java
  35. 8 2
      instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/test/groovy/ContextBridgeTest.groovy
  36. 2 2
      instrumentation/opentelemetry-api/opentelemetry-api-1.0/testing/src/main/java/AgentSpanTestingInstrumentation.java
  37. 49 5
      instrumentation/opentelemetry-api/opentelemetry-api-1.0/testing/src/main/java/AgentSpanTestingInstrumenter.java
  38. 0 39
      instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java
  39. 1 1
      instrumentation/spring/spring-integration-4.1/javaagent/src/test/groovy/SpringIntegrationAndRabbitTest.groovy
  40. 1 1
      instrumentation/spring/spring-web-3.1/library/src/test/java/io/opentelemetry/instrumentation/spring/web/SpringWebTelemetryTest.java
  41. 1 0
      instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioAsyncInstrumentation.java
  42. 29 0
      instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioAsyncMarker.java
  43. 2 1
      instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioSyncInstrumentation.java
  44. 104 84
      instrumentation/twilio-6.6/javaagent/src/test/groovy/test/TwilioClientTest.groovy
  45. 0 1
      settings.gradle.kts
  46. 6 6
      testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/InstrumentationSpecification.groovy
  47. 12 12
      testing-common/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java
  48. 48 11
      testing-common/src/main/java/io/opentelemetry/instrumentation/testing/TestInstrumenters.java
  49. 12 12
      testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/InstrumentationExtension.java
  50. 1 1
      testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java

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

@@ -13,6 +13,8 @@ import static io.opentelemetry.instrumentation.api.internal.AttributesExtractorU
 
 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 @@ import javax.annotation.Nullable;
  */
 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 @@ public final class HttpServerAttributesExtractor<REQUEST, RESPONSE>
 
     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;
+  }
 }

+ 13 - 1
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 @@ package io.opentelemetry.instrumentation.api.instrumenter.rpc;
  * 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 final class 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;
+  }
 }

+ 7 - 1
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() {}

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

@@ -31,7 +31,7 @@ class HttpRouteHolderTest {
 
     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));

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

@@ -9,6 +9,7 @@ import java.util.Collections;
 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;
   }
 

+ 0 - 62
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/CompositeSuppressionStrategy.java

@@ -1,62 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.instrumentation.api.instrumenter;
-
-import io.opentelemetry.api.trace.Span;
-import io.opentelemetry.api.trace.SpanKind;
-import io.opentelemetry.context.Context;
-
-final class CompositeSuppressionStrategy extends SpanSuppressionStrategy {
-  private final SpanSuppressionStrategy clientStrategy;
-  private final SpanSuppressionStrategy producerStrategy;
-  private final SpanSuppressionStrategy serverStrategy;
-  private final SpanSuppressionStrategy consumerStrategy;
-
-  CompositeSuppressionStrategy(
-      SpanSuppressionStrategy client,
-      SpanSuppressionStrategy producer,
-      SpanSuppressionStrategy server,
-      SpanSuppressionStrategy consumer) {
-    this.clientStrategy = client;
-    this.producerStrategy = producer;
-    this.serverStrategy = server;
-    this.consumerStrategy = consumer;
-  }
-
-  @Override
-  Context storeInContext(Context context, SpanKind spanKind, Span span) {
-    switch (spanKind) {
-      case CLIENT:
-        return clientStrategy.storeInContext(context, spanKind, span);
-      case PRODUCER:
-        return producerStrategy.storeInContext(context, spanKind, span);
-      case SERVER:
-        return serverStrategy.storeInContext(context, spanKind, span);
-      case CONSUMER:
-        return consumerStrategy.storeInContext(context, spanKind, span);
-      case INTERNAL:
-        return context;
-    }
-    return context;
-  }
-
-  @Override
-  boolean shouldSuppress(Context parentContext, SpanKind spanKind) {
-    switch (spanKind) {
-      case CLIENT:
-        return clientStrategy.shouldSuppress(parentContext, spanKind);
-      case PRODUCER:
-        return producerStrategy.shouldSuppress(parentContext, spanKind);
-      case SERVER:
-        return serverStrategy.shouldSuppress(parentContext, spanKind);
-      case CONSUMER:
-        return consumerStrategy.shouldSuppress(parentContext, spanKind);
-      case INTERNAL:
-        return false;
-    }
-    return false;
-  }
-}

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

@@ -76,7 +76,7 @@ public class Instrumenter<REQUEST, RESPONSE> {
   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 class Instrumenter<REQUEST, RESPONSE> {
     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 class Instrumenter<REQUEST, RESPONSE> {
       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 class Instrumenter<REQUEST, RESPONSE> {
       }
     }
 
-    return spanSuppressionStrategy.storeInContext(context, spanKind, span);
+    return spanSuppressor.storeInContext(context, spanKind, span);
   }
 
   /**

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

@@ -10,7 +10,6 @@ import static java.util.Objects.requireNonNull;
 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 @@ import javax.annotation.Nullable;
  */
 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 final class InstrumenterBuilder<REQUEST, RESPONSE> {
     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 @@ public final class InstrumenterBuilder<REQUEST, RESPONSE> {
     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() {

+ 0 - 25
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/NoopSuppressionStrategy.java

@@ -1,25 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.instrumentation.api.instrumenter;
-
-import io.opentelemetry.api.trace.Span;
-import io.opentelemetry.api.trace.SpanKind;
-import io.opentelemetry.context.Context;
-
-final class NoopSuppressionStrategy extends SpanSuppressionStrategy {
-
-  static final SpanSuppressionStrategy INSTANCE = new NoopSuppressionStrategy();
-
-  @Override
-  Context storeInContext(Context context, SpanKind spanKind, Span span) {
-    return context;
-  }
-
-  @Override
-  boolean shouldSuppress(Context parentContext, SpanKind spanKind) {
-    return false;
-  }
-}

+ 75 - 39
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressionStrategy.java

@@ -7,53 +7,89 @@ package io.opentelemetry.instrumentation.api.instrumenter;
 
 import static java.util.Collections.singleton;
 
-import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.SpanKind;
-import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.config.Config;
+import io.opentelemetry.instrumentation.api.instrumenter.SpanSuppressor.BySpanKey;
+import io.opentelemetry.instrumentation.api.instrumenter.SpanSuppressor.DelegateBySpanKind;
+import io.opentelemetry.instrumentation.api.instrumenter.SpanSuppressor.JustStoreServer;
+import io.opentelemetry.instrumentation.api.instrumenter.SpanSuppressor.Noop;
 import io.opentelemetry.instrumentation.api.internal.SpanKey;
+import io.opentelemetry.instrumentation.api.internal.SpanKeyProvider;
+import java.util.EnumMap;
+import java.util.Locale;
+import java.util.Map;
 import java.util.Set;
 
-abstract class SpanSuppressionStrategy {
-  private static final SpanSuppressionStrategy SERVER_STRATEGY =
-      new SuppressIfSameSpanKeyStrategy(singleton(SpanKey.SERVER));
-  private static final SpanSuppressionStrategy ALL_CLIENTS_STRATEGY =
-      new SuppressIfSameSpanKeyStrategy(singleton(SpanKey.ALL_CLIENTS));
-
-  private static final SpanSuppressionStrategy SUPPRESS_GENERIC_CLIENTS_AND_SERVERS =
-      new CompositeSuppressionStrategy(
-          ALL_CLIENTS_STRATEGY,
-          NoopSuppressionStrategy.INSTANCE,
-          SERVER_STRATEGY,
-          NoopSuppressionStrategy.INSTANCE);
-
-  private static final SpanSuppressionStrategy SUPPRESS_ONLY_SERVERS =
-      new CompositeSuppressionStrategy(
-          NoopSuppressionStrategy.INSTANCE,
-          NoopSuppressionStrategy.INSTANCE,
-          SERVER_STRATEGY,
-          NoopSuppressionStrategy.INSTANCE);
-
-  static SpanSuppressionStrategy suppressNestedClients(Set<SpanKey> spanKeys) {
-    if (spanKeys.isEmpty()) {
-      return SUPPRESS_GENERIC_CLIENTS_AND_SERVERS;
+enum SpanSuppressionStrategy {
+  /**
+   * Do not suppress spans at all.
+   *
+   * <p>This strategy will mark spans of {@link SpanKind#SERVER SERVER} kind in the context, so that
+   * they can be accessed using {@code ServerSpan}, but will not suppress server spans.
+   */
+  NONE {
+    @Override
+    SpanSuppressor create(Set<SpanKey> spanKeys) {
+      return JustStoreServer.INSTANCE;
     }
+  },
+  /**
+   * Suppress spans by {@link SpanKind}. This is equivalent to the "legacy" suppression strategy
+   * used in the agent.
+   *
+   * <p>Child spans of the same kind will be suppressed; e.g. if there already is a {@link
+   * SpanKind#CLIENT CLIENT} span in the context, a second {@code CLIENT} span won't be started.
+   */
+  SPAN_KIND {
+    @SuppressWarnings("ImmutableEnumChecker") // this field actually is immutable
+    private final SpanSuppressor strategy;
 
-    SpanSuppressionStrategy spanKeyStrategy = new SuppressIfSameSpanKeyStrategy(spanKeys);
-    return new CompositeSuppressionStrategy(
-        ALL_CLIENTS_STRATEGY, spanKeyStrategy, SERVER_STRATEGY, spanKeyStrategy);
-  }
-
-  static SpanSuppressionStrategy from(Set<SpanKey> spanKeys) {
-    if (spanKeys.isEmpty()) {
-      return SUPPRESS_ONLY_SERVERS;
+    {
+      Map<SpanKind, SpanSuppressor> delegates = new EnumMap<>(SpanKind.class);
+      delegates.put(SpanKind.SERVER, new BySpanKey(singleton(SpanKey.KIND_SERVER)));
+      delegates.put(SpanKind.CLIENT, new BySpanKey(singleton(SpanKey.KIND_CLIENT)));
+      delegates.put(SpanKind.CONSUMER, new BySpanKey(singleton(SpanKey.KIND_CONSUMER)));
+      delegates.put(SpanKind.PRODUCER, new BySpanKey(singleton(SpanKey.KIND_PRODUCER)));
+      strategy = new DelegateBySpanKind(delegates);
     }
 
-    SpanSuppressionStrategy spanKeyStrategy = new SuppressIfSameSpanKeyStrategy(spanKeys);
-    return new CompositeSuppressionStrategy(
-        spanKeyStrategy, spanKeyStrategy, SERVER_STRATEGY, spanKeyStrategy);
-  }
+    @Override
+    SpanSuppressor create(Set<SpanKey> spanKeys) {
+      return strategy;
+    }
+  },
+  /**
+   * Suppress spans by the semantic convention they're supposed to represent. This strategy uses
+   * {@linkplain SpanKey span keys} returned by the {@link SpanKeyProvider#internalGetSpanKey()}
+   * method to determine if the span can be created or not. An {@link AttributesExtractor} can
+   * implement that method to associate itself (and the {@link Instrumenter} it is a part of) with a
+   * particular convention.
+   *
+   * <p>For example, nested HTTP client spans will be suppressed; but an RPC client span will not
+   * suppress an HTTP client span, if the instrumented RPC client uses HTTP as transport.
+   */
+  SEMCONV {
+    @Override
+    SpanSuppressor create(Set<SpanKey> spanKeys) {
+      if (spanKeys.isEmpty()) {
+        return Noop.INSTANCE;
+      }
+      return new BySpanKey(spanKeys);
+    }
+  };
 
-  abstract Context storeInContext(Context context, SpanKind spanKind, Span span);
+  abstract SpanSuppressor create(Set<SpanKey> spanKeys);
 
-  abstract boolean shouldSuppress(Context parentContext, SpanKind spanKind);
+  static SpanSuppressionStrategy fromConfig(Config config) {
+    String value =
+        config.getString("otel.instrumentation.experimental.span-suppression-strategy", "semconv");
+    switch (value.toLowerCase(Locale.ROOT)) {
+      case "none":
+        return NONE;
+      case "span-kind":
+        return SPAN_KIND;
+      default:
+        return SEMCONV;
+    }
+  }
 }

+ 105 - 0
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressor.java

@@ -0,0 +1,105 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.instrumenter;
+
+import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.api.trace.SpanKind;
+import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.internal.SpanKey;
+import java.util.Map;
+import java.util.Set;
+
+interface SpanSuppressor {
+
+  Context storeInContext(Context context, SpanKind spanKind, Span span);
+
+  boolean shouldSuppress(Context parentContext, SpanKind spanKind);
+
+  enum Noop implements SpanSuppressor {
+    INSTANCE;
+
+    @Override
+    public Context storeInContext(Context context, SpanKind spanKind, Span span) {
+      return context;
+    }
+
+    @Override
+    public boolean shouldSuppress(Context parentContext, SpanKind spanKind) {
+      return false;
+    }
+  }
+
+  enum JustStoreServer implements SpanSuppressor {
+    INSTANCE;
+
+    @Override
+    public Context storeInContext(Context context, SpanKind spanKind, Span span) {
+      if (spanKind == SpanKind.SERVER) {
+        return SpanKey.KIND_SERVER.storeInContext(context, span);
+      }
+      return context;
+    }
+
+    @Override
+    public boolean shouldSuppress(Context parentContext, SpanKind spanKind) {
+      return false;
+    }
+  }
+
+  final class DelegateBySpanKind implements SpanSuppressor {
+
+    private final Map<SpanKind, SpanSuppressor> delegates;
+
+    DelegateBySpanKind(Map<SpanKind, SpanSuppressor> delegates) {
+      this.delegates = delegates;
+    }
+
+    @Override
+    public Context storeInContext(Context context, SpanKind spanKind, Span span) {
+      SpanSuppressor delegate = delegates.get(spanKind);
+      if (delegate == null) {
+        return context;
+      }
+      return delegate.storeInContext(context, spanKind, span);
+    }
+
+    @Override
+    public boolean shouldSuppress(Context parentContext, SpanKind spanKind) {
+      SpanSuppressor delegate = delegates.get(spanKind);
+      if (delegate == null) {
+        return false;
+      }
+      return delegate.shouldSuppress(parentContext, spanKind);
+    }
+  }
+
+  final class BySpanKey implements SpanSuppressor {
+
+    private final Set<SpanKey> spanKeys;
+
+    BySpanKey(Set<SpanKey> spanKeys) {
+      this.spanKeys = spanKeys;
+    }
+
+    @Override
+    public Context storeInContext(Context context, SpanKind spanKind, Span span) {
+      for (SpanKey spanKey : spanKeys) {
+        context = spanKey.storeInContext(context, span);
+      }
+      return context;
+    }
+
+    @Override
+    public boolean shouldSuppress(Context parentContext, SpanKind spanKind) {
+      for (SpanKey spanKey : spanKeys) {
+        if (spanKey.fromContextOrNull(parentContext) == null) {
+          return false;
+        }
+      }
+      return true;
+    }
+  }
+}

+ 0 - 39
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SuppressIfSameSpanKeyStrategy.java

@@ -1,39 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.instrumentation.api.instrumenter;
-
-import io.opentelemetry.api.trace.Span;
-import io.opentelemetry.api.trace.SpanKind;
-import io.opentelemetry.context.Context;
-import io.opentelemetry.instrumentation.api.internal.SpanKey;
-import java.util.Set;
-
-final class SuppressIfSameSpanKeyStrategy extends SpanSuppressionStrategy {
-
-  private final Set<SpanKey> outgoingSpanKeys;
-
-  SuppressIfSameSpanKeyStrategy(Set<SpanKey> outgoingSpanKeys) {
-    this.outgoingSpanKeys = outgoingSpanKeys;
-  }
-
-  @Override
-  Context storeInContext(Context context, SpanKind spanKind, Span span) {
-    for (SpanKey outgoingSpanKey : outgoingSpanKeys) {
-      context = outgoingSpanKey.storeInContext(context, span);
-    }
-    return context;
-  }
-
-  @Override
-  boolean shouldSuppress(Context parentContext, SpanKind spanKind) {
-    for (SpanKey outgoingSpanKey : outgoingSpanKeys) {
-      if (outgoingSpanKey.fromContextOrNull(parentContext) == null) {
-        return false;
-      }
-    }
-    return true;
-  }
-}

+ 31 - 21
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SpanKey.java

@@ -18,26 +18,30 @@ import javax.annotation.Nullable;
  */
 public final class SpanKey {
 
-  // server span key
-
-  private static final ContextKey<Span> SERVER_KEY =
-      ContextKey.named("opentelemetry-traces-span-key-server");
-
-  // client span keys
+  /* Context keys */
+
+  // span kind keys
+  private static final ContextKey<Span> KIND_SERVER_KEY =
+      ContextKey.named("opentelemetry-traces-span-key-kind-server");
+  private static final ContextKey<Span> KIND_CLIENT_KEY =
+      ContextKey.named("opentelemetry-traces-span-key-kind-client");
+  private static final ContextKey<Span> KIND_CONSUMER_KEY =
+      ContextKey.named("opentelemetry-traces-span-key-kind-consumer");
+  private static final ContextKey<Span> KIND_PRODUCER_KEY =
+      ContextKey.named("opentelemetry-traces-span-key-kind-producer");
+
+  // semantic convention keys
+  private static final ContextKey<Span> HTTP_SERVER_KEY =
+      ContextKey.named("opentelemetry-traces-span-key-http-server");
+  private static final ContextKey<Span> RPC_SERVER_KEY =
+      ContextKey.named("opentelemetry-traces-span-key-rpc-server");
 
   private static final ContextKey<Span> HTTP_CLIENT_KEY =
-      ContextKey.named("opentelemetry-traces-span-key-http");
+      ContextKey.named("opentelemetry-traces-span-key-http-client");
   private static final ContextKey<Span> RPC_CLIENT_KEY =
-      ContextKey.named("opentelemetry-traces-span-key-rpc");
+      ContextKey.named("opentelemetry-traces-span-key-rpc-client");
   private static final ContextKey<Span> DB_CLIENT_KEY =
-      ContextKey.named("opentelemetry-traces-span-key-db");
-
-  // this is used instead of above, depending on the configuration value for
-  // otel.instrumentation.experimental.outgoing-span-suppression-by-type
-  private static final ContextKey<Span> CLIENT_KEY =
-      ContextKey.named("opentelemetry-traces-span-key-client");
-
-  // producer & consumer (messaging) span keys
+      ContextKey.named("opentelemetry-traces-span-key-db-client");
 
   private static final ContextKey<Span> PRODUCER_KEY =
       ContextKey.named("opentelemetry-traces-span-key-producer");
@@ -46,16 +50,22 @@ public final class SpanKey {
   private static final ContextKey<Span> CONSUMER_PROCESS_KEY =
       ContextKey.named("opentelemetry-traces-span-key-consumer-process");
 
-  public static final SpanKey SERVER = new SpanKey(SERVER_KEY);
+  /* Span keys */
+
+  // span kind keys
+  public static final SpanKey KIND_SERVER = new SpanKey(KIND_SERVER_KEY);
+  public static final SpanKey KIND_CLIENT = new SpanKey(KIND_CLIENT_KEY);
+  public static final SpanKey KIND_CONSUMER = new SpanKey(KIND_CONSUMER_KEY);
+  public static final SpanKey KIND_PRODUCER = new SpanKey(KIND_PRODUCER_KEY);
+
+  // semantic convention keys
+  public static final SpanKey HTTP_SERVER = new SpanKey(HTTP_SERVER_KEY);
+  public static final SpanKey RPC_SERVER = new SpanKey(RPC_SERVER_KEY);
 
   public static final SpanKey HTTP_CLIENT = new SpanKey(HTTP_CLIENT_KEY);
   public static final SpanKey RPC_CLIENT = new SpanKey(RPC_CLIENT_KEY);
   public static final SpanKey DB_CLIENT = new SpanKey(DB_CLIENT_KEY);
 
-  // this is used instead of above, depending on the configuration value for
-  // otel.instrumentation.experimental.outgoing-span-suppression-by-type
-  public static final SpanKey ALL_CLIENTS = new SpanKey(CLIENT_KEY);
-
   public static final SpanKey PRODUCER = new SpanKey(PRODUCER_KEY);
   public static final SpanKey CONSUMER_RECEIVE = new SpanKey(CONSUMER_RECEIVE_KEY);
   public static final SpanKey CONSUMER_PROCESS = new SpanKey(CONSUMER_PROCESS_KEY);

+ 27 - 224
instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java

@@ -162,12 +162,6 @@ class InstrumenterTest {
   @Mock(extraInterfaces = SpanKeyProvider.class)
   AttributesExtractor<Map<String, String>, Map<String, String>> mockDbClientAttributes;
 
-  @Mock(extraInterfaces = SpanKeyProvider.class)
-  AttributesExtractor<Map<String, String>, Map<String, String>> mockRpcClientAttributes;
-
-  @Mock(extraInterfaces = SpanKeyProvider.class)
-  AttributesExtractor<Map<String, String>, Map<String, String>> mockMessagingProducerAttributes;
-
   @Mock AttributesExtractor<Map<String, String>, Map<String, String>> mockNetClientAttributes;
 
   @Test
@@ -181,9 +175,7 @@ class InstrumenterTest {
 
     Context context = instrumenter.start(Context.root(), REQUEST);
     SpanContext spanContext = Span.fromContext(context).getSpanContext();
-
     assertThat(spanContext.isValid()).isTrue();
-    assertThat(SpanKey.SERVER.fromContextOrNull(context).getSpanContext()).isEqualTo(spanContext);
 
     instrumenter.end(context, REQUEST, RESPONSE, null);
 
@@ -222,10 +214,7 @@ class InstrumenterTest {
             .newServerInstrumenter(new MapGetter());
 
     Context context = instrumenter.start(Context.root(), REQUEST);
-    SpanContext spanContext = Span.fromContext(context).getSpanContext();
-
-    assertThat(spanContext.isValid()).isTrue();
-    assertThat(SpanKey.SERVER.fromContextOrNull(context).getSpanContext()).isEqualTo(spanContext);
+    assertThat(Span.fromContext(context).getSpanContext().isValid()).isTrue();
 
     instrumenter.end(context, REQUEST, RESPONSE, new IllegalStateException("test"));
 
@@ -258,11 +247,9 @@ class InstrumenterTest {
                             TraceState.getDefault()))),
             request,
             Map::put);
-    Context context = instrumenter.start(Context.root(), request);
-    SpanContext spanContext = Span.fromContext(context).getSpanContext();
 
-    assertThat(spanContext.isValid()).isTrue();
-    assertThat(SpanKey.SERVER.fromContextOrNull(context).getSpanContext()).isEqualTo(spanContext);
+    Context context = instrumenter.start(Context.root(), request);
+    assertThat(Span.fromContext(context).getSpanContext().isValid()).isTrue();
 
     instrumenter.end(context, request, RESPONSE, null);
 
@@ -274,7 +261,7 @@ class InstrumenterTest {
                     span ->
                         span.hasName("span")
                             .hasTraceId("ff01020304050600ff0a0b0c0d0e0f00")
-                            .hasSpanId(spanContext.getSpanId())
+                            .hasSpanId(Span.fromContext(context).getSpanContext().getSpanId())
                             .hasParentSpanId("090a0b0c0d0e0f00")));
   }
 
@@ -527,198 +514,6 @@ class InstrumenterTest {
     assertThat(instrumenter.shouldStart(Context.root(), "request")).isFalse();
   }
 
-  @Test
-  void clientNestedSpansSuppressed_whenInstrumentationTypeDisabled() {
-    // this test depends on default config option for InstrumentationType
-
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenterOuter =
-        getInstrumenterWithType(false);
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenterInner =
-        getInstrumenterWithType(false);
-
-    Map<String, String> request = new HashMap<>(REQUEST);
-
-    Context context = instrumenterOuter.start(Context.root(), request);
-    assertThat(instrumenterInner.shouldStart(context, request)).isFalse();
-  }
-
-  @Test
-  void clientNestedSpansSuppressed_whenInstrumentationTypeDisabled2() {
-    when(((SpanKeyProvider) mockHttpClientAttributes).internalGetSpanKey())
-        .thenReturn(SpanKey.HTTP_CLIENT);
-    when(((SpanKeyProvider) mockDbClientAttributes).internalGetSpanKey())
-        .thenReturn(SpanKey.DB_CLIENT);
-
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenterOuter =
-        getInstrumenterWithType(false, mockDbClientAttributes);
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenterInner =
-        getInstrumenterWithType(false, mockHttpClientAttributes);
-
-    Map<String, String> request = new HashMap<>(REQUEST);
-
-    Context context = instrumenterOuter.start(Context.root(), request);
-    assertThat(instrumenterInner.shouldStart(context, request)).isFalse();
-  }
-
-  @Test
-  void clientNestedSuppressed_whenSameInstrumentationType() {
-    when(((SpanKeyProvider) mockDbClientAttributes).internalGetSpanKey())
-        .thenReturn(SpanKey.DB_CLIENT);
-
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenterOuter =
-        getInstrumenterWithType(true, mockDbClientAttributes);
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenterInner =
-        getInstrumenterWithType(true, mockDbClientAttributes);
-
-    Map<String, String> request = new HashMap<>(REQUEST);
-
-    assertThat(instrumenterOuter.shouldStart(Context.root(), request)).isTrue();
-    assertThat(instrumenterInner.shouldStart(Context.root(), request)).isTrue();
-
-    Context context = instrumenterOuter.start(Context.root(), request);
-    assertThat(instrumenterInner.shouldStart(context, request)).isFalse();
-  }
-
-  @Test
-  void clientNestedNotSuppressed_wehnDifferentInstrumentationCategories() {
-    when(((SpanKeyProvider) mockHttpClientAttributes).internalGetSpanKey())
-        .thenReturn(SpanKey.HTTP_CLIENT);
-    when(((SpanKeyProvider) mockDbClientAttributes).internalGetSpanKey())
-        .thenReturn(SpanKey.DB_CLIENT);
-
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenterOuter =
-        getInstrumenterWithType(true, mockDbClientAttributes);
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenterInner =
-        getInstrumenterWithType(true, mockHttpClientAttributes);
-
-    Map<String, String> request = new HashMap<>(REQUEST);
-
-    Context context = instrumenterOuter.start(Context.root(), request);
-    assertThat(instrumenterInner.shouldStart(context, request)).isTrue();
-  }
-
-  @Test
-  void clientNestedGenericNotSuppressed() {
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenterOuter =
-        getInstrumenterWithType(true, new AttributesExtractor1());
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenterInner =
-        getInstrumenterWithType(true, new AttributesExtractor1());
-
-    Map<String, String> request = new HashMap<>(REQUEST);
-    Context context = instrumenterOuter.start(Context.root(), request);
-    assertThat(instrumenterInner.shouldStart(context, request)).isTrue();
-  }
-
-  @Test
-  void clientNestedGenericSpansNotSuppressed_whenNoExtractors() {
-    // this test depends on default config option for InstrumentationType
-
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenterOuter =
-        getInstrumenterWithType(true);
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenterInner =
-        getInstrumenterWithType(true, null, null);
-
-    Map<String, String> request = new HashMap<>(REQUEST);
-
-    Context context = instrumenterOuter.start(Context.root(), request);
-    assertThat(instrumenterInner.shouldStart(context, request)).isTrue();
-  }
-
-  @Test
-  void instrumentationTypeDetected_http() {
-    when(((SpanKeyProvider) mockHttpClientAttributes).internalGetSpanKey())
-        .thenReturn(SpanKey.HTTP_CLIENT);
-
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenter =
-        getInstrumenterWithType(true, mockHttpClientAttributes, new AttributesExtractor1());
-
-    Map<String, String> request = new HashMap<>(REQUEST);
-
-    Context context = instrumenter.start(Context.root(), request);
-    validateInstrumentationTypeSpanPresent(SpanKey.HTTP_CLIENT, context);
-  }
-
-  @Test
-  void instrumentationTypeDetected_db() {
-    when(((SpanKeyProvider) mockDbClientAttributes).internalGetSpanKey())
-        .thenReturn(SpanKey.DB_CLIENT);
-
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenter =
-        getInstrumenterWithType(true, mockDbClientAttributes, new AttributesExtractor2());
-
-    Map<String, String> request = new HashMap<>(REQUEST);
-
-    Context context = instrumenter.start(Context.root(), request);
-    validateInstrumentationTypeSpanPresent(SpanKey.DB_CLIENT, context);
-  }
-
-  @Test
-  void instrumentationTypeDetected_rpc() {
-    when(((SpanKeyProvider) mockRpcClientAttributes).internalGetSpanKey())
-        .thenReturn(SpanKey.RPC_CLIENT);
-
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenter =
-        getInstrumenterWithType(true, mockRpcClientAttributes);
-
-    Map<String, String> request = new HashMap<>(REQUEST);
-
-    Context context = instrumenter.start(Context.root(), request);
-    validateInstrumentationTypeSpanPresent(SpanKey.RPC_CLIENT, context);
-  }
-
-  @Test
-  void instrumentationTypeDetected_producer() {
-    when(((SpanKeyProvider) mockMessagingProducerAttributes).internalGetSpanKey())
-        .thenReturn(SpanKey.PRODUCER);
-
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenter =
-        getInstrumenterWithType(true, mockMessagingProducerAttributes);
-
-    Map<String, String> request = new HashMap<>(REQUEST);
-
-    Context context = instrumenter.start(Context.root(), request);
-    validateInstrumentationTypeSpanPresent(SpanKey.PRODUCER, context);
-  }
-
-  @Test
-  void instrumentationTypeDetected_mix() {
-    when(((SpanKeyProvider) mockDbClientAttributes).internalGetSpanKey())
-        .thenReturn(SpanKey.DB_CLIENT);
-    when(((SpanKeyProvider) mockMessagingProducerAttributes).internalGetSpanKey())
-        .thenReturn(SpanKey.PRODUCER);
-
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenter =
-        getInstrumenterWithType(
-            true,
-            new AttributesExtractor2(),
-            mockMessagingProducerAttributes,
-            mockNetClientAttributes,
-            mockDbClientAttributes);
-
-    Map<String, String> request = new HashMap<>(REQUEST);
-
-    Context context = instrumenter.start(Context.root(), request);
-    validateInstrumentationTypeSpanPresent(SpanKey.PRODUCER, context);
-  }
-
-  @Test
-  void instrumentationTypeDetected_generic() {
-    Instrumenter<Map<String, String>, Map<String, String>> instrumenter =
-        getInstrumenterWithType(true, new AttributesExtractor2(), mockNetClientAttributes);
-
-    Map<String, String> request = new HashMap<>(REQUEST);
-
-    Context context = instrumenter.start(Context.root(), request);
-    Span span = Span.fromContext(context);
-
-    assertThat(span).isNotNull();
-
-    assertThat(SpanKey.HTTP_CLIENT.fromContextOrNull(context)).isNull();
-    assertThat(SpanKey.DB_CLIENT.fromContextOrNull(context)).isNull();
-    assertThat(SpanKey.RPC_CLIENT.fromContextOrNull(context)).isNull();
-    assertThat(SpanKey.PRODUCER.fromContextOrNull(context)).isNull();
-  }
-
   @Test
   void instrumentationVersion_default() {
     InstrumenterBuilder<Map<String, String>, Map<String, String>> builder =
@@ -792,25 +587,33 @@ class InstrumenterTest {
                     span -> span.hasName("span").hasInstrumentationScopeInfo(expectedLibraryInfo)));
   }
 
-  private static void validateInstrumentationTypeSpanPresent(SpanKey spanKey, Context context) {
-    Span span = Span.fromContext(context);
-
-    assertThat(span).isNotNull();
-    assertThat(spanKey.fromContextOrNull(context)).isSameAs(span);
-  }
+  @Test
+  void shouldRetrieveSpanKeysFromAttributesExtractors() {
+    when(((SpanKeyProvider) mockDbClientAttributes).internalGetSpanKey())
+        .thenReturn(SpanKey.DB_CLIENT);
+    when(((SpanKeyProvider) mockHttpClientAttributes).internalGetSpanKey())
+        .thenReturn(SpanKey.HTTP_CLIENT);
 
-  @SafeVarargs
-  @SuppressWarnings("varargs")
-  private static Instrumenter<Map<String, String>, Map<String, String>> getInstrumenterWithType(
-      boolean enableInstrumentation,
-      AttributesExtractor<Map<String, String>, Map<String, String>>... attributeExtractors) {
-    InstrumenterBuilder<Map<String, String>, Map<String, String>> builder =
+    Instrumenter<Map<String, String>, Map<String, String>> instrumenter =
         Instrumenter.<Map<String, String>, Map<String, String>>builder(
                 otelTesting.getOpenTelemetry(), "test", unused -> "span")
-            .addAttributesExtractors(attributeExtractors)
-            .enableInstrumentationTypeSuppression(enableInstrumentation);
+            .addAttributesExtractors(
+                new AttributesExtractor2(),
+                mockHttpClientAttributes,
+                mockNetClientAttributes,
+                mockDbClientAttributes)
+            .newInstrumenter();
 
-    return builder.newClientInstrumenter(Map::put);
+    Context context = instrumenter.start(Context.root(), REQUEST);
+
+    assertThatSpanKeyWasStored(SpanKey.DB_CLIENT, context);
+    assertThatSpanKeyWasStored(SpanKey.HTTP_CLIENT, context);
+  }
+
+  private static void assertThatSpanKeyWasStored(SpanKey spanKey, Context context) {
+    Span span = Span.fromContext(context);
+    assertThat(span).isNotNull();
+    assertThat(spanKey.fromContextOrNull(context)).isSameAs(span);
   }
 
   private static LinkData expectedSpanLink() {

+ 116 - 102
instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressionStrategyTest.java

@@ -5,158 +5,172 @@
 
 package io.opentelemetry.instrumentation.api.instrumenter;
 
+import static java.util.Arrays.asList;
 import static java.util.Collections.emptySet;
-import static java.util.Collections.singleton;
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.config.Config;
 import io.opentelemetry.instrumentation.api.internal.SpanKey;
-import java.util.stream.Collectors;
+import java.util.HashSet;
+import java.util.Set;
 import java.util.stream.Stream;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.ArgumentsProvider;
+import org.junit.jupiter.params.provider.ArgumentsSource;
+import org.junit.jupiter.params.provider.EnumSource;
 
-public class SpanSuppressionStrategyTest {
+class SpanSuppressionStrategyTest {
 
-  private static final Span SPAN = Span.getInvalid();
+  static final Span span = Span.getInvalid();
 
-  @Test
-  public void serverSpan() {
-    // SpanKey.SERVER will never be passed to SpanSuppressionStrategy.from(), it cannot be
-    // automatically determined by te builder - thus it does not make any sense to test it (for now)
-    SpanSuppressionStrategy strategy = SpanSuppressionStrategy.from(emptySet());
-
-    Context context = strategy.storeInContext(Context.root(), SpanKind.SERVER, SPAN);
-
-    assertThat(strategy.shouldSuppress(context, SpanKind.SERVER)).isTrue();
-    Stream.of(SpanKind.CLIENT, SpanKind.CONSUMER, SpanKind.PRODUCER)
-        .forEach(spanKind -> assertThat(strategy.shouldSuppress(context, spanKind)).isFalse());
+  @ParameterizedTest
+  @ArgumentsSource(ConfigArgs.class)
+  void shouldParseConfig(String value, SpanSuppressionStrategy expectedStrategy) {
+    Config config =
+        Config.builder()
+            .addProperty("otel.instrumentation.experimental.span-suppression-strategy", value)
+            .build();
+    assertEquals(expectedStrategy, SpanSuppressionStrategy.fromConfig(config));
+  }
 
-    verifySpanKey(SpanKey.SERVER, context);
+  static final class ConfigArgs implements ArgumentsProvider {
+
+    @Override
+    public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
+      return Stream.of(
+          Arguments.of("none", SpanSuppressionStrategy.NONE),
+          Arguments.of("NONE", SpanSuppressionStrategy.NONE),
+          Arguments.of("span-kind", SpanSuppressionStrategy.SPAN_KIND),
+          Arguments.of("Span-Kind", SpanSuppressionStrategy.SPAN_KIND),
+          Arguments.of("semconv", SpanSuppressionStrategy.SEMCONV),
+          Arguments.of("SemConv", SpanSuppressionStrategy.SEMCONV),
+          Arguments.of("asdfasdfasdf", SpanSuppressionStrategy.SEMCONV),
+          Arguments.of(null, SpanSuppressionStrategy.SEMCONV));
+    }
   }
 
   @ParameterizedTest
-  @MethodSource("consumerSpanKeys")
-  public void consumerSpan(SpanKey spanKey) {
-    SpanSuppressionStrategy strategy = SpanSuppressionStrategy.from(singleton(spanKey));
+  @ArgumentsSource(SpanKindsAndKeys.class)
+  void none_shouldNotSuppressAnything(SpanKind spanKind, SpanKey spanKey) {
+    SpanSuppressor suppressor = SpanSuppressionStrategy.NONE.create(emptySet());
+
+    Context context = spanKey.storeInContext(Context.root(), span);
 
-    verifyNoSuppression(strategy, Context.root());
+    assertFalse(suppressor.shouldSuppress(context, spanKind));
+  }
 
-    Context context = strategy.storeInContext(Context.root(), SpanKind.CONSUMER, SPAN);
+  @Test
+  void none_shouldStoreServerSpanInContext() {
+    SpanSuppressor suppressor = SpanSuppressionStrategy.NONE.create(emptySet());
+    Context context = Context.root();
 
-    assertThat(strategy.shouldSuppress(context, SpanKind.SERVER)).isFalse();
-    Stream.of(SpanKind.CLIENT, SpanKind.CONSUMER, SpanKind.PRODUCER)
-        .forEach(spanKind -> assertThat(strategy.shouldSuppress(context, spanKind)).isTrue());
+    Context newContext = suppressor.storeInContext(context, SpanKind.SERVER, span);
 
-    verifySpanKey(spanKey, context);
+    assertNotSame(newContext, context);
+    assertSame(span, SpanKey.KIND_SERVER.fromContextOrNull(newContext));
   }
 
   @ParameterizedTest
-  @MethodSource("clientSpanKeys")
-  public void clientSpan(SpanKey spanKey) {
-    SpanSuppressionStrategy strategy = SpanSuppressionStrategy.from(singleton(spanKey));
-
-    verifyNoSuppression(strategy, Context.root());
+  @EnumSource(value = SpanKind.class, mode = EnumSource.Mode.EXCLUDE, names = "SERVER")
+  void none_shouldNotStoreNonServerSpansInContext(SpanKind spanKind) {
+    SpanSuppressor suppressor = SpanSuppressionStrategy.NONE.create(emptySet());
+    Context context = Context.root();
 
-    Context context = strategy.storeInContext(Context.root(), SpanKind.CLIENT, SPAN);
+    Context newContext = suppressor.storeInContext(context, spanKind, span);
 
-    assertThat(strategy.shouldSuppress(context, SpanKind.SERVER)).isFalse();
-    Stream.of(SpanKind.CLIENT, SpanKind.CONSUMER, SpanKind.PRODUCER)
-        .forEach(spanKind -> assertThat(strategy.shouldSuppress(context, spanKind)).isTrue());
-
-    verifySpanKey(spanKey, context);
+    assertSame(newContext, context);
   }
 
-  @Test
-  public void producerSpan() {
-    SpanSuppressionStrategy strategy = SpanSuppressionStrategy.from(singleton(SpanKey.PRODUCER));
-
-    verifyNoSuppression(strategy, Context.root());
-
-    Context context = strategy.storeInContext(Context.root(), SpanKind.PRODUCER, SPAN);
+  @ParameterizedTest
+  @ArgumentsSource(SpanKindsAndKeys.class)
+  void spanKind_shouldStoreInContext(SpanKind spanKind, SpanKey spanKey) {
+    SpanSuppressor suppressor = SpanSuppressionStrategy.SPAN_KIND.create(emptySet());
+    Context context = Context.root();
 
-    assertThat(strategy.shouldSuppress(context, SpanKind.SERVER)).isFalse();
-    Stream.of(SpanKind.CLIENT, SpanKind.CONSUMER, SpanKind.PRODUCER)
-        .forEach(spanKind -> assertThat(strategy.shouldSuppress(context, spanKind)).isTrue());
+    Context newContext = suppressor.storeInContext(context, spanKind, span);
 
-    verifySpanKey(SpanKey.PRODUCER, context);
+    assertNotSame(newContext, context);
+    assertSame(span, spanKey.fromContextOrNull(newContext));
   }
 
-  @Test
-  public void multipleClientKeys() {
-    SpanSuppressionStrategy strategy =
-        SpanSuppressionStrategy.from(clientSpanKeys().collect(Collectors.toSet()));
-
-    Context context = strategy.storeInContext(Context.root(), SpanKind.CLIENT, SPAN);
+  @ParameterizedTest
+  @ArgumentsSource(SpanKindsAndKeys.class)
+  void spanKind_shouldSuppressSameKind(SpanKind spanKind, SpanKey spanKey) {
+    SpanSuppressor suppressor = SpanSuppressionStrategy.SPAN_KIND.create(emptySet());
+    Context context = Context.root();
 
-    assertThat(strategy.shouldSuppress(context, SpanKind.CLIENT)).isTrue();
-    assertThat(strategy.shouldSuppress(context, SpanKind.PRODUCER)).isTrue();
-    assertThat(strategy.shouldSuppress(context, SpanKind.SERVER)).isFalse();
-    assertThat(strategy.shouldSuppress(context, SpanKind.CONSUMER)).isTrue();
+    Context newContext = suppressor.storeInContext(context, spanKind, span);
 
-    clientSpanKeys().forEach(key -> assertThat(key.fromContextOrNull(context)).isSameAs(SPAN));
+    assertNotSame(newContext, context);
+    assertSame(span, spanKey.fromContextOrNull(newContext));
   }
 
-  @ParameterizedTest
-  @MethodSource("nonServerSpanKinds")
-  public void noKeys_nonServerSpanKindsAreNotSuppressed(SpanKind spanKind) {
-    SpanSuppressionStrategy strategy = SpanSuppressionStrategy.from(emptySet());
+  static final class SpanKindsAndKeys implements ArgumentsProvider {
 
-    Context context = strategy.storeInContext(Context.root(), spanKind, SPAN);
+    @Override
+    public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
+      return Stream.of(
+          Arguments.of(SpanKind.SERVER, SpanKey.KIND_SERVER),
+          Arguments.of(SpanKind.CLIENT, SpanKey.KIND_CLIENT),
+          Arguments.of(SpanKind.CONSUMER, SpanKey.KIND_CONSUMER),
+          Arguments.of(SpanKind.PRODUCER, SpanKey.KIND_PRODUCER));
+    }
+  }
+
+  @Test
+  void semconv_shouldNotSuppressAnythingWhenThereAreNoSpanKeys() {
+    SpanSuppressor suppressor = SpanSuppressionStrategy.SEMCONV.create(emptySet());
+    Context context = Context.root();
 
-    assertThat(context).isSameAs(Context.root());
-    verifyNoSuppression(strategy, context);
+    assertFalse(suppressor.shouldSuppress(context, SpanKind.SERVER));
 
-    allSpanKeys().forEach(key -> assertThat(key.fromContextOrNull(context)).isNull());
+    Context newContext = suppressor.storeInContext(context, SpanKind.SERVER, span);
+    assertSame(newContext, context);
   }
 
   @Test
-  public void nestedClientsDisabled_useAllClientsSpanKey() {
-    SpanSuppressionStrategy strategy =
-        SpanSuppressionStrategy.suppressNestedClients(allSpanKeys().collect(Collectors.toSet()));
+  void semconv_shouldStoreProvidedSpanKeysInContext() {
+    Set<SpanKey> spanKeys = new HashSet<>(asList(SpanKey.DB_CLIENT, SpanKey.RPC_CLIENT));
+    SpanSuppressor suppressor = SpanSuppressionStrategy.SEMCONV.create(spanKeys);
+    Context context = Context.root();
 
-    Context context = strategy.storeInContext(Context.root(), SpanKind.CLIENT, SPAN);
+    Context newContext = suppressor.storeInContext(context, SpanKind.SERVER, span);
+    assertNotSame(newContext, context);
 
-    assertThat(strategy.shouldSuppress(context, SpanKind.CLIENT)).isTrue();
-
-    assertThat(SpanKey.ALL_CLIENTS.fromContextOrNull(context)).isSameAs(SPAN);
-    allSpanKeys()
-        .filter(key -> key != SpanKey.ALL_CLIENTS)
-        .forEach(key -> assertThat(key.fromContextOrNull(context)).isNull());
+    spanKeys.forEach(key -> assertSame(span, key.fromContextOrNull(newContext)));
   }
 
-  @SuppressWarnings("unused")
-  private static Stream<SpanKind> nonServerSpanKinds() {
-    return Stream.of(SpanKind.CONSUMER, SpanKind.CLIENT, SpanKind.PRODUCER);
-  }
+  @Test
+  void semconv_shouldSuppressContextWhenAllSpanKeysArePresent() {
+    Set<SpanKey> spanKeys = new HashSet<>(asList(SpanKey.DB_CLIENT, SpanKey.RPC_CLIENT));
+    SpanSuppressor suppressor = SpanSuppressionStrategy.SEMCONV.create(spanKeys);
 
-  private static void verifyNoSuppression(SpanSuppressionStrategy strategy, Context context) {
-    Stream.of(SpanKind.values())
-        .forEach(spanKind -> assertThat(strategy.shouldSuppress(context, spanKind)).isFalse());
-  }
+    Context context =
+        SpanKey.RPC_CLIENT.storeInContext(
+            SpanKey.DB_CLIENT.storeInContext(Context.root(), span), span);
 
-  private static void verifySpanKey(SpanKey spanKey, Context context) {
-    assertThat(spanKey.fromContextOrNull(context)).isSameAs(SPAN);
-    allSpanKeys()
-        .filter(key -> key != spanKey)
-        .forEach(key -> assertThat(key.fromContextOrNull(context)).isNull());
+    assertTrue(suppressor.shouldSuppress(context, SpanKind.SERVER));
   }
 
-  private static Stream<SpanKey> allSpanKeys() {
-    return Stream.concat(
-        Stream.of(SpanKey.PRODUCER, SpanKey.SERVER),
-        Stream.concat(consumerSpanKeys(), clientSpanKeys()));
-  }
+  @Test
+  void semconv_shouldNotSuppressContextWithPartiallyDifferentSpanKeys() {
+    Set<SpanKey> spanKeys = new HashSet<>(asList(SpanKey.DB_CLIENT, SpanKey.RPC_CLIENT));
+    SpanSuppressor suppressor = SpanSuppressionStrategy.SEMCONV.create(spanKeys);
 
-  private static Stream<SpanKey> consumerSpanKeys() {
-    return Stream.of(SpanKey.CONSUMER_RECEIVE, SpanKey.CONSUMER_PROCESS);
-  }
+    Context context =
+        SpanKey.HTTP_CLIENT.storeInContext(
+            SpanKey.DB_CLIENT.storeInContext(Context.root(), span), span);
 
-  private static Stream<SpanKey> clientSpanKeys() {
-    return Stream.of(
-        SpanKey.ALL_CLIENTS, SpanKey.HTTP_CLIENT, SpanKey.DB_CLIENT, SpanKey.RPC_CLIENT);
+    assertFalse(suppressor.shouldSuppress(context, SpanKind.SERVER));
   }
 }

+ 3 - 0
instrumentation/apache-camel-2.20/javaagent/build.gradle.kts

@@ -64,5 +64,8 @@ tasks {
     // TODO run tests both with and without experimental span attributes
     jvmArgs("-Dotel.instrumentation.apache-camel.experimental-span-attributes=true")
     jvmArgs("-Dotel.instrumentation.aws-sdk.experimental-span-attributes=true")
+
+    // TODO: fix camel instrumentation so that it uses semantic attributes extractors
+    jvmArgs("-Dotel.instrumentation.experimental.span-suppression-strategy=span-kind")
   }
 }

+ 1 - 1
instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientRequest.java

@@ -50,7 +50,7 @@ public final class ApacheHttpClientRequest {
 
   // minimize memory overhead by not using streams
   static List<String> headersToList(Header[] headers) {
-    if (headers.length == 0) {
+    if (headers == null || headers.length == 0) {
       return Collections.emptyList();
     }
     List<String> headersList = new ArrayList<>(headers.length);

+ 0 - 11
instrumentation/aws-sdk/aws-sdk-1.11/javaagent-unit-tests/build.gradle.kts

@@ -1,11 +0,0 @@
-plugins {
-  id("otel.java-conventions")
-}
-
-dependencies {
-  api(project(":testing-common"))
-  testImplementation(project(":instrumentation:aws-sdk:aws-sdk-1.11:javaagent"))
-
-  testImplementation("com.amazonaws:aws-java-sdk-core:1.11.0")
-  testImplementation("com.amazonaws:aws-java-sdk-sqs:1.11.106")
-}

+ 0 - 74
instrumentation/aws-sdk/aws-sdk-1.11/javaagent-unit-tests/src/test/java/TracingRequestHandlerTest.java

@@ -1,74 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-import static org.assertj.core.api.Assertions.assertThat;
-
-import com.amazonaws.DefaultRequest;
-import com.amazonaws.Request;
-import com.amazonaws.Response;
-import com.amazonaws.http.HttpResponse;
-import com.amazonaws.services.sqs.model.SendMessageRequest;
-import com.amazonaws.services.sqs.model.SendMessageResult;
-import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension;
-import io.opentelemetry.javaagent.instrumentation.awssdk.v1_11.TracingRequestHandler;
-import java.net.URI;
-import org.apache.http.client.methods.HttpGet;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
-
-class TracingRequestHandlerTest {
-
-  private static Response<SendMessageResult> response(Request<?> request) {
-    return new Response<>(new SendMessageResult(), new HttpResponse(request, new HttpGet()));
-  }
-
-  private static Request<SendMessageRequest> request() {
-    // Using a subclass of SendMessageRequest because for SendMessageRequest instrumentation
-    // creates PRODUCER span, for others CLIENT span. We need to use CLIENT spans for
-    // runWithClientSpan in shouldNotSetScopeAndNotFailIfClientSpanAlreadyPresent to work.
-    class CustomSendMessageRequest extends SendMessageRequest {}
-
-    Request<SendMessageRequest> request =
-        new DefaultRequest<>(new CustomSendMessageRequest(), "test");
-    request.setEndpoint(URI.create("http://test.uri"));
-    return request;
-  }
-
-  @RegisterExtension
-  static final LibraryInstrumentationExtension testing = LibraryInstrumentationExtension.create();
-
-  @Test
-  void shouldNotSetScopeAndNotFailIfClientSpanAlreadyPresent() {
-    // given
-    TracingRequestHandler underTest = new TracingRequestHandler();
-    Request<SendMessageRequest> request = request();
-
-    testing.runWithClientSpan(
-        "test",
-        () -> {
-          // when
-          underTest.beforeRequest(request);
-          // then - no exception and scope not set
-          assertThat(request.getHandlerContext(TracingRequestHandler.SCOPE)).isNull();
-          underTest.afterResponse(request, response(request));
-        });
-  }
-
-  @Test
-  void shouldSetScopeIfClientSpanNotPresent() {
-    // given
-    TracingRequestHandler underTest = new TracingRequestHandler();
-    Request<SendMessageRequest> request = request();
-
-    // when
-    underTest.beforeRequest(request);
-    // then - no exception and scope not set
-    assertThat(request.getHandlerContext(TracingRequestHandler.SCOPE)).isNotNull();
-    underTest.afterResponse(request, response(request));
-
-    // cleanup
-    underTest.afterError(request, null, null);
-  }
-}

+ 0 - 5
instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/testSqs/groovy/io/opentelemetry/javaagent/instrumentation/awssdk/v1_11/SqsTracingTest.groovy

@@ -14,9 +14,4 @@ class SqsTracingTest extends AbstractSqsTracingTest implements AgentTestTrait {
   AmazonSQSAsyncClientBuilder configureClient(AmazonSQSAsyncClientBuilder client) {
     return client
   }
-
-  @Override
-  boolean hasHttpClientSpan() {
-    true
-  }
 }

+ 2 - 15
instrumentation/aws-sdk/aws-sdk-1.11/testing/src/main/groovy/io/opentelemetry/instrumentation/awssdk/v1_11/AbstractSqsTracingTest.groovy

@@ -50,10 +50,6 @@ abstract class AbstractSqsTracingTest extends InstrumentationSpecification {
     }
   }
 
-  boolean hasHttpClientSpan() {
-    false
-  }
-
   def "simple sqs producer-consumer services"() {
     setup:
     client.createQueue("testSdkSqs")
@@ -88,7 +84,7 @@ abstract class AbstractSqsTracingTest extends InstrumentationSpecification {
           }
         }
       }
-      trace(1, hasHttpClientSpan() ? 3 : 2) {
+      trace(1, 2) {
         span(0) {
           name "SQS.SendMessage"
           kind PRODUCER
@@ -109,16 +105,7 @@ abstract class AbstractSqsTracingTest extends InstrumentationSpecification {
             "net.transport" IP_TCP
           }
         }
-        def offset = 0
-        if (hasHttpClientSpan()) {
-          offset = 1
-          span(1) {
-            name "HTTP POST"
-            kind CLIENT
-            childOf span(0)
-          }
-        }
-        span(1 + offset) {
+        span(1) {
           name "SQS.ReceiveMessage"
           kind CONSUMER
           childOf span(0)

+ 13 - 3
instrumentation/azure-core/azure-core-1.19/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/azurecore/v1_19/SuppressNestedClientMono.java

@@ -25,13 +25,23 @@ public class SuppressNestedClientMono<T> extends Mono<T> {
   @Override
   public void subscribe(CoreSubscriber<? super T> actual) {
     Context parentContext = currentContext();
-    if (SpanKey.ALL_CLIENTS.fromContextOrNull(parentContext) == null) {
-      try (Scope ignored =
-          SpanKey.ALL_CLIENTS.storeInContext(parentContext, Span.getInvalid()).makeCurrent()) {
+    if (doesNotHaveClientSpan(parentContext)) {
+      try (Scope ignored = disallowNestedClientSpan(parentContext).makeCurrent()) {
         delegate.subscribe(actual);
       }
     } else {
       delegate.subscribe(actual);
     }
   }
+
+  private static boolean doesNotHaveClientSpan(Context parentContext) {
+    return SpanKey.KIND_CLIENT.fromContextOrNull(parentContext) == null
+        && SpanKey.HTTP_CLIENT.fromContextOrNull(parentContext) == null;
+  }
+
+  private static Context disallowNestedClientSpan(Context parentContext) {
+    Span span = Span.getInvalid();
+    return SpanKey.HTTP_CLIENT.storeInContext(
+        SpanKey.KIND_CLIENT.storeInContext(parentContext, span), span);
+  }
 }

+ 30 - 2
instrumentation/elasticsearch/elasticsearch-rest-5.0/javaagent/src/test/groovy/ElasticsearchRest5Test.groovy

@@ -69,7 +69,7 @@ class ElasticsearchRest5Test extends AgentInstrumentationSpecification {
     result.status == "green"
 
     assertTraces(1) {
-      trace(0, 1) {
+      trace(0, 2) {
         span(0) {
           name "GET"
           kind CLIENT
@@ -83,6 +83,20 @@ class ElasticsearchRest5Test extends AgentInstrumentationSpecification {
             "$SemanticAttributes.NET_PEER_PORT" httpHost.port
           }
         }
+        span(1) {
+          name "HTTP GET"
+          kind CLIENT
+          childOf span(0)
+          attributes {
+            "$SemanticAttributes.NET_TRANSPORT" SemanticAttributes.NetTransportValues.IP_TCP
+            "$SemanticAttributes.NET_PEER_NAME" httpHost.hostName
+            "$SemanticAttributes.NET_PEER_PORT" httpHost.port
+            "$SemanticAttributes.HTTP_METHOD" "GET"
+            "$SemanticAttributes.HTTP_FLAVOR" SemanticAttributes.HttpFlavorValues.HTTP_1_1
+            "$SemanticAttributes.HTTP_URL" "${httpHost.toURI()}/_cluster/health"
+            "$SemanticAttributes.HTTP_STATUS_CODE" 200
+          }
+        }
       }
     }
   }
@@ -124,7 +138,7 @@ class ElasticsearchRest5Test extends AgentInstrumentationSpecification {
     result.status == "green" || result.status == "yellow"
 
     assertTraces(1) {
-      trace(0, 3) {
+      trace(0, 4) {
         span(0) {
           name "parent"
           kind INTERNAL
@@ -144,6 +158,20 @@ class ElasticsearchRest5Test extends AgentInstrumentationSpecification {
           }
         }
         span(2) {
+          name "HTTP GET"
+          kind CLIENT
+          childOf span(1)
+          attributes {
+            "$SemanticAttributes.NET_TRANSPORT" SemanticAttributes.NetTransportValues.IP_TCP
+            "$SemanticAttributes.NET_PEER_NAME" httpHost.hostName
+            "$SemanticAttributes.NET_PEER_PORT" httpHost.port
+            "$SemanticAttributes.HTTP_METHOD" "GET"
+            "$SemanticAttributes.HTTP_FLAVOR" SemanticAttributes.HttpFlavorValues.HTTP_1_1
+            "$SemanticAttributes.HTTP_URL" "${httpHost.toURI()}/_cluster/health"
+            "$SemanticAttributes.HTTP_STATUS_CODE" 200
+          }
+        }
+        span(3) {
           name "callback"
           kind INTERNAL
           childOf(span(0))

+ 30 - 2
instrumentation/elasticsearch/elasticsearch-rest-6.4/javaagent/src/test/groovy/ElasticsearchRest6Test.groovy

@@ -64,7 +64,7 @@ class ElasticsearchRest6Test extends AgentInstrumentationSpecification {
     result.status == "green"
 
     assertTraces(1) {
-      trace(0, 1) {
+      trace(0, 2) {
         span(0) {
           name "GET"
           kind CLIENT
@@ -78,6 +78,20 @@ class ElasticsearchRest6Test extends AgentInstrumentationSpecification {
             "$SemanticAttributes.NET_PEER_PORT" httpHost.port
           }
         }
+        span(1) {
+          name "HTTP GET"
+          kind CLIENT
+          childOf span(0)
+          attributes {
+            "$SemanticAttributes.NET_TRANSPORT" SemanticAttributes.NetTransportValues.IP_TCP
+            "$SemanticAttributes.NET_PEER_NAME" httpHost.hostName
+            "$SemanticAttributes.NET_PEER_PORT" httpHost.port
+            "$SemanticAttributes.HTTP_METHOD" "GET"
+            "$SemanticAttributes.HTTP_FLAVOR" SemanticAttributes.HttpFlavorValues.HTTP_1_1
+            "$SemanticAttributes.HTTP_URL" "${httpHost.toURI()}/_cluster/health"
+            "$SemanticAttributes.HTTP_STATUS_CODE" 200
+          }
+        }
       }
     }
   }
@@ -118,7 +132,7 @@ class ElasticsearchRest6Test extends AgentInstrumentationSpecification {
     result.status == "green"
 
     assertTraces(1) {
-      trace(0, 3) {
+      trace(0, 4) {
         span(0) {
           name "parent"
           kind INTERNAL
@@ -138,6 +152,20 @@ class ElasticsearchRest6Test extends AgentInstrumentationSpecification {
           }
         }
         span(2) {
+          name "HTTP GET"
+          kind CLIENT
+          childOf span(1)
+          attributes {
+            "$SemanticAttributes.NET_TRANSPORT" SemanticAttributes.NetTransportValues.IP_TCP
+            "$SemanticAttributes.NET_PEER_NAME" httpHost.hostName
+            "$SemanticAttributes.NET_PEER_PORT" httpHost.port
+            "$SemanticAttributes.HTTP_METHOD" "GET"
+            "$SemanticAttributes.HTTP_FLAVOR" SemanticAttributes.HttpFlavorValues.HTTP_1_1
+            "$SemanticAttributes.HTTP_URL" "${httpHost.toURI()}/_cluster/health"
+            "$SemanticAttributes.HTTP_STATUS_CODE" 200
+          }
+        }
+        span(3) {
           name "callback"
           kind INTERNAL
           childOf(span(0))

+ 30 - 2
instrumentation/elasticsearch/elasticsearch-rest-7.0/javaagent/src/test/groovy/ElasticsearchRest7Test.groovy

@@ -63,7 +63,7 @@ class ElasticsearchRest7Test extends AgentInstrumentationSpecification {
     result.status == "green"
 
     assertTraces(1) {
-      trace(0, 1) {
+      trace(0, 2) {
         span(0) {
           name "GET"
           kind CLIENT
@@ -77,6 +77,20 @@ class ElasticsearchRest7Test extends AgentInstrumentationSpecification {
             "$SemanticAttributes.NET_PEER_PORT" httpHost.port
           }
         }
+        span(1) {
+          name "HTTP GET"
+          kind CLIENT
+          childOf span(0)
+          attributes {
+            "$SemanticAttributes.NET_TRANSPORT" SemanticAttributes.NetTransportValues.IP_TCP
+            "$SemanticAttributes.NET_PEER_NAME" httpHost.hostName
+            "$SemanticAttributes.NET_PEER_PORT" httpHost.port
+            "$SemanticAttributes.HTTP_METHOD" "GET"
+            "$SemanticAttributes.HTTP_FLAVOR" SemanticAttributes.HttpFlavorValues.HTTP_1_1
+            "$SemanticAttributes.HTTP_URL" "${httpHost.toURI()}/_cluster/health"
+            "$SemanticAttributes.HTTP_STATUS_CODE" 200
+          }
+        }
       }
     }
   }
@@ -117,7 +131,7 @@ class ElasticsearchRest7Test extends AgentInstrumentationSpecification {
     result.status == "green"
 
     assertTraces(1) {
-      trace(0, 3) {
+      trace(0, 4) {
         span(0) {
           name "parent"
           kind INTERNAL
@@ -137,6 +151,20 @@ class ElasticsearchRest7Test extends AgentInstrumentationSpecification {
           }
         }
         span(2) {
+          name "HTTP GET"
+          kind CLIENT
+          childOf span(1)
+          attributes {
+            "$SemanticAttributes.NET_TRANSPORT" SemanticAttributes.NetTransportValues.IP_TCP
+            "$SemanticAttributes.NET_PEER_NAME" httpHost.hostName
+            "$SemanticAttributes.NET_PEER_PORT" httpHost.port
+            "$SemanticAttributes.HTTP_METHOD" "GET"
+            "$SemanticAttributes.HTTP_FLAVOR" SemanticAttributes.HttpFlavorValues.HTTP_1_1
+            "$SemanticAttributes.HTTP_URL" "${httpHost.toURI()}/_cluster/health"
+            "$SemanticAttributes.HTTP_STATUS_CODE" 200
+          }
+        }
+        span(3) {
           name "callback"
           kind INTERNAL
           childOf(span(0))

+ 3 - 3
instrumentation/jaxrs/jaxrs-1.0/javaagent/src/test/groovy/JaxRsAnnotations1InstrumentationTest.groovy

@@ -23,7 +23,7 @@ class JaxRsAnnotations1InstrumentationTest extends AgentInstrumentationSpecifica
   @Unroll
   def "span named '#paramName' from annotations on class '#className' when is not root span"() {
     setup:
-    runWithServerSpan("test") {
+    runWithHttpServerSpan("test") {
       obj.call()
     }
 
@@ -50,7 +50,7 @@ class JaxRsAnnotations1InstrumentationTest extends AgentInstrumentationSpecifica
     }
 
     when: "multiple calls to the same method"
-    runWithServerSpan("test") {
+    runWithHttpServerSpan("test") {
       (1..10).each {
         obj.call()
       }
@@ -112,7 +112,7 @@ class JaxRsAnnotations1InstrumentationTest extends AgentInstrumentationSpecifica
 
   def "no annotations has no effect"() {
     setup:
-    runWithServerSpan("test") {
+    runWithHttpServerSpan("test") {
       obj.call()
     }
 

+ 2 - 2
instrumentation/jaxrs/jaxrs-1.0/javaagent/src/test/groovy/JerseyTest.groovy

@@ -27,7 +27,7 @@ class JerseyTest extends AgentInstrumentationSpecification {
   def "test #resource"() {
     when:
     // start a trace because the test doesn't go through any servlet or other instrumentation.
-    def response = runWithServerSpan("test.span") {
+    def response = runWithHttpServerSpan("test.span") {
       resources.client().resource(resource).post(String)
     }
 
@@ -66,7 +66,7 @@ class JerseyTest extends AgentInstrumentationSpecification {
 
     when:
     // start a trace because the test doesn't go through any servlet or other instrumentation.
-    def response = runWithServerSpan("test.span") {
+    def response = runWithHttpServerSpan("test.span") {
       resources.client().resource(resource).post(String)
     }
 

+ 3 - 3
instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-annotations/javaagent/src/test/groovy/JaxrsAnnotationsInstrumentationTest.groovy

@@ -23,7 +23,7 @@ class JaxrsAnnotationsInstrumentationTest extends AgentInstrumentationSpecificat
   @Unroll
   def "span named '#paramName' from annotations on class '#className' when is not root span"() {
     setup:
-    runWithServerSpan("test") {
+    runWithHttpServerSpan("test") {
       obj.call()
     }
 
@@ -50,7 +50,7 @@ class JaxrsAnnotationsInstrumentationTest extends AgentInstrumentationSpecificat
     }
 
     when: "multiple calls to the same method"
-    runWithServerSpan("test") {
+    runWithHttpServerSpan("test") {
       (1..10).each {
         obj.call()
       }
@@ -112,7 +112,7 @@ class JaxrsAnnotationsInstrumentationTest extends AgentInstrumentationSpecificat
 
   def "no annotations has no effect"() {
     setup:
-    runWithServerSpan("test") {
+    runWithHttpServerSpan("test") {
       obj.call()
     }
 

+ 1 - 1
instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/testing/src/main/groovy/JaxRsFilterTest.groovy

@@ -36,7 +36,7 @@ abstract class JaxRsFilterTest extends AgentInstrumentationSpecification {
       return makeRequest(resource)
     }
     // start a trace because the test doesn't go through any servlet or other instrumentation.
-    return runWithServerSpan("test.span") {
+    return runWithHttpServerSpan("test.span") {
       makeRequest(resource)
     }
   }

+ 44 - 0
instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/client/HttpClientSpanKeyAttributesExtractor.java

@@ -0,0 +1,44 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.netty.common.client;
+
+import io.netty.channel.Channel;
+import io.opentelemetry.api.common.AttributesBuilder;
+import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesExtractor;
+import io.opentelemetry.instrumentation.api.internal.SpanKey;
+import io.opentelemetry.instrumentation.api.internal.SpanKeyProvider;
+import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest;
+import javax.annotation.Nullable;
+
+/**
+ * Attributes extractor that pretends it's a {@link HttpClientAttributesExtractor} so that error
+ * only CONNECT spans can be suppressed by higher level HTTP clients based on netty.
+ */
+enum HttpClientSpanKeyAttributesExtractor
+    implements AttributesExtractor<NettyConnectionRequest, Channel>, SpanKeyProvider {
+  INSTANCE;
+
+  @Override
+  public void onStart(
+      AttributesBuilder attributes,
+      Context parentContext,
+      NettyConnectionRequest nettyConnectionRequest) {}
+
+  @Override
+  public void onEnd(
+      AttributesBuilder attributes,
+      Context context,
+      NettyConnectionRequest nettyConnectionRequest,
+      @Nullable Channel channel,
+      @Nullable Throwable error) {}
+
+  @Override
+  public SpanKey internalGetSpanKey() {
+    return SpanKey.HTTP_CLIENT;
+  }
+}

+ 20 - 6
instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/client/NettyClientInstrumenterFactory.java

@@ -9,6 +9,7 @@ import io.netty.channel.Channel;
 import io.netty.handler.codec.http.HttpResponse;
 import io.opentelemetry.api.GlobalOpenTelemetry;
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
+import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder;
 import io.opentelemetry.instrumentation.api.instrumenter.PeerServiceAttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesExtractor;
@@ -51,16 +52,29 @@ public final class NettyClientInstrumenterFactory {
 
   public NettyConnectionInstrumenter createConnectionInstrumenter() {
     NettyConnectNetAttributesGetter netAttributesGetter = new NettyConnectNetAttributesGetter();
-    Instrumenter<NettyConnectionRequest, Channel> instrumenter =
+
+    InstrumenterBuilder<NettyConnectionRequest, Channel> instrumenterBuilder =
         Instrumenter.<NettyConnectionRequest, Channel>builder(
                 GlobalOpenTelemetry.get(), instrumentationName, NettyConnectionRequest::spanName)
             .addAttributesExtractor(NetClientAttributesExtractor.create(netAttributesGetter))
             .addAttributesExtractor(PeerServiceAttributesExtractor.create(netAttributesGetter))
-            .setTimeExtractor(new NettyConnectionTimeExtractor())
-            .newInstrumenter(
-                connectionTelemetryEnabled
-                    ? SpanKindExtractor.alwaysInternal()
-                    : SpanKindExtractor.alwaysClient());
+            .setTimeExtractor(new NettyConnectionTimeExtractor());
+    if (!connectionTelemetryEnabled) {
+      // when the connection telemetry is not enabled, netty creates CONNECT spans whenever a
+      // connection error occurs - because there is no HTTP span in that scenario, if raw netty
+      // connection occurs before an HTTP message is even formed
+      // we don't want that span when a higher-level HTTP library (like reactor-netty or async http
+      // client) is used, the connection phase is a part of the HTTP span for these
+      // for that to happen, the CONNECT span will "pretend" to be a full HTTP span when connection
+      // telemetry is off
+      instrumenterBuilder.addAttributesExtractor(HttpClientSpanKeyAttributesExtractor.INSTANCE);
+    }
+
+    Instrumenter<NettyConnectionRequest, Channel> instrumenter =
+        instrumenterBuilder.newInstrumenter(
+            connectionTelemetryEnabled
+                ? SpanKindExtractor.alwaysInternal()
+                : SpanKindExtractor.alwaysClient());
 
     return connectionTelemetryEnabled
         ? new NettyConnectionInstrumenterImpl(instrumenter)

+ 0 - 37
instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/groovy/WithSpanInstrumentationTest.groovy

@@ -18,7 +18,6 @@ import net.bytebuddy.matcher.ElementMatchers
 import java.lang.reflect.Modifier
 import java.util.concurrent.CompletableFuture
 
-import static io.opentelemetry.api.trace.SpanKind.CLIENT
 import static io.opentelemetry.api.trace.SpanKind.INTERNAL
 import static io.opentelemetry.api.trace.SpanKind.PRODUCER
 import static io.opentelemetry.api.trace.SpanKind.SERVER
@@ -106,42 +105,6 @@ class WithSpanInstrumentationTest extends AgentInstrumentationSpecification {
     }
   }
 
-  def "should not capture multiple server spans"() {
-    setup:
-    new TracedWithSpan().nestedServers()
-
-    expect:
-    assertTraces(1) {
-      trace(0, 1) {
-        span(0) {
-          name "TracedWithSpan.nestedServers"
-          kind SERVER
-          hasNoParent()
-          attributes {
-          }
-        }
-      }
-    }
-  }
-
-  def "should not capture multiple client spans"() {
-    setup:
-    new TracedWithSpan().nestedClients()
-
-    expect:
-    assertTraces(1) {
-      trace(0, 1) {
-        span(0) {
-          name "TracedWithSpan.nestedClients"
-          kind CLIENT
-          hasNoParent()
-          attributes {
-          }
-        }
-      }
-    }
-  }
-
   def "should ignore method excluded by trace.annotated.methods.exclude configuration"() {
     setup:
     new TracedWithSpan().ignored()

+ 0 - 20
instrumentation/opentelemetry-annotations-1.0/javaagent/src/test/java/io/opentelemetry/test/annotation/TracedWithSpan.java

@@ -38,26 +38,6 @@ public class TracedWithSpan {
     return otel();
   }
 
-  @WithSpan(kind = SpanKind.SERVER)
-  public String nestedServers() {
-    return innerServer();
-  }
-
-  @WithSpan(kind = SpanKind.SERVER)
-  public String innerServer() {
-    return "hello!";
-  }
-
-  @WithSpan(kind = SpanKind.CLIENT)
-  public String nestedClients() {
-    return innerClient();
-  }
-
-  @WithSpan(kind = SpanKind.CLIENT)
-  public String innerClient() {
-    return "hello!";
-  }
-
   @WithSpan
   public String withSpanAttributes(
       @SpanAttribute String implicitName,

+ 8 - 2
instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/AgentContextStorage.java

@@ -157,11 +157,17 @@ public class AgentContextStorage implements ContextStorage, AutoCloseable {
             "io.opentelemetry.api.baggage.BaggageContextKey",
             BaggageBridging::toApplication,
             BaggageBridging::toAgent),
-        bridgeSpanKey("SERVER_KEY"),
+        // span kind keys
+        bridgeSpanKey("KIND_SERVER_KEY"),
+        bridgeSpanKey("KIND_CLIENT_KEY"),
+        bridgeSpanKey("KIND_CONSUMER_KEY"),
+        bridgeSpanKey("KIND_PRODUCER_KEY"),
+        // semantic convention keys
+        bridgeSpanKey("HTTP_SERVER_KEY"),
+        bridgeSpanKey("RPC_SERVER_KEY"),
         bridgeSpanKey("HTTP_CLIENT_KEY"),
         bridgeSpanKey("RPC_CLIENT_KEY"),
         bridgeSpanKey("DB_CLIENT_KEY"),
-        bridgeSpanKey("CLIENT_KEY"),
         bridgeSpanKey("PRODUCER_KEY"),
         bridgeSpanKey("CONSUMER_RECEIVE_KEY"),
         bridgeSpanKey("CONSUMER_PROCESS_KEY"),

+ 8 - 2
instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/test/groovy/ContextBridgeTest.groovy

@@ -164,11 +164,17 @@ class ContextBridgeTest extends AgentInstrumentationSpecification {
     AgentSpanTesting.runWithAllSpanKeys("parent") {
       assert Span.current() != null
       def spanKeys = [
-        SpanKey.SERVER,
+        // span kind keys
+        SpanKey.KIND_SERVER,
+        SpanKey.KIND_CLIENT,
+        SpanKey.KIND_CONSUMER,
+        SpanKey.KIND_PRODUCER,
+        // semantic convention keys
+        SpanKey.HTTP_SERVER,
+        SpanKey.RPC_SERVER,
         SpanKey.HTTP_CLIENT,
         SpanKey.RPC_CLIENT,
         SpanKey.DB_CLIENT,
-        SpanKey.ALL_CLIENTS,
         SpanKey.PRODUCER,
         SpanKey.CONSUMER_RECEIVE,
         SpanKey.CONSUMER_PROCESS,

+ 2 - 2
instrumentation/opentelemetry-api/opentelemetry-api-1.0/testing/src/main/java/AgentSpanTestingInstrumentation.java

@@ -38,7 +38,7 @@ public class AgentSpanTestingInstrumentation implements TypeInstrumentation {
         @Advice.Argument(0) String spanName,
         @Advice.Local("otelContext") Context context,
         @Advice.Local("otelScope") Scope scope) {
-      context = AgentSpanTestingInstrumenter.startServerSpan(spanName);
+      context = AgentSpanTestingInstrumenter.startHttpServerSpan(spanName);
       scope = context.makeCurrent();
     }
 
@@ -48,7 +48,7 @@ public class AgentSpanTestingInstrumentation implements TypeInstrumentation {
         @Advice.Local("otelContext") Context context,
         @Advice.Local("otelScope") Scope scope) {
       scope.close();
-      AgentSpanTestingInstrumenter.end(context, throwable);
+      AgentSpanTestingInstrumenter.endHttpServer(context, throwable);
     }
   }
 

+ 49 - 5
instrumentation/opentelemetry-api/opentelemetry-api-1.0/testing/src/main/java/AgentSpanTestingInstrumenter.java

@@ -4,12 +4,16 @@
  */
 
 import io.opentelemetry.api.GlobalOpenTelemetry;
+import io.opentelemetry.api.common.AttributesBuilder;
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.ContextKey;
+import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 import io.opentelemetry.instrumentation.api.internal.SpanKey;
+import io.opentelemetry.instrumentation.api.internal.SpanKeyProvider;
+import javax.annotation.Nullable;
 
 public final class AgentSpanTestingInstrumenter {
 
@@ -23,8 +27,16 @@ public final class AgentSpanTestingInstrumenter {
               (context, request, startAttributes) -> context.with(REQUEST_CONTEXT_KEY, request))
           .newInstrumenter(request -> request.kind);
 
-  public static Context startServerSpan(String name) {
-    return start(name, SpanKind.SERVER);
+  private static final Instrumenter<Request, Void> INSTRUMENTER_HTTP_SERVER =
+      Instrumenter.<Request, Void>builder(
+              GlobalOpenTelemetry.get(), "test", request -> request.name)
+          .addAttributesExtractor(HttpServerSpanKeyAttributesExtractor.INSTANCE)
+          .addContextCustomizer(
+              (context, request, startAttributes) -> context.with(REQUEST_CONTEXT_KEY, request))
+          .newInstrumenter(request -> request.kind);
+
+  public static Context startHttpServerSpan(String name) {
+    return INSTRUMENTER_HTTP_SERVER.start(Context.current(), new Request(name, SpanKind.SERVER));
   }
 
   public static Context startClientSpan(String name) {
@@ -48,6 +60,10 @@ public final class AgentSpanTestingInstrumenter {
     INSTRUMENTER.end(context, context.get(REQUEST_CONTEXT_KEY), null, error);
   }
 
+  public static void endHttpServer(Context context, Throwable error) {
+    INSTRUMENTER_HTTP_SERVER.end(context, context.get(REQUEST_CONTEXT_KEY), null, error);
+  }
+
   private static final class Request {
     private final String name;
     private final SpanKind kind;
@@ -60,16 +76,44 @@ public final class AgentSpanTestingInstrumenter {
 
   private static SpanKey[] getSpanKeys() {
     return new SpanKey[] {
-      SpanKey.SERVER,
+      // span kind keys
+      SpanKey.KIND_SERVER,
+      SpanKey.KIND_CLIENT,
+      SpanKey.KIND_CONSUMER,
+      SpanKey.KIND_PRODUCER,
+      // semantic convention keys
+      SpanKey.HTTP_SERVER,
+      SpanKey.RPC_SERVER,
       SpanKey.HTTP_CLIENT,
       SpanKey.RPC_CLIENT,
       SpanKey.DB_CLIENT,
-      SpanKey.ALL_CLIENTS,
       SpanKey.PRODUCER,
       SpanKey.CONSUMER_RECEIVE,
-      SpanKey.CONSUMER_PROCESS
+      SpanKey.CONSUMER_PROCESS,
     };
   }
 
+  // simulate a real HTTP server implementation
+  private enum HttpServerSpanKeyAttributesExtractor
+      implements AttributesExtractor<Request, Void>, SpanKeyProvider {
+    INSTANCE;
+
+    @Override
+    public void onStart(AttributesBuilder attributes, Context parentContext, Request request) {}
+
+    @Override
+    public void onEnd(
+        AttributesBuilder attributes,
+        Context context,
+        Request request,
+        @Nullable Void unused,
+        @Nullable Throwable error) {}
+
+    @Override
+    public SpanKey internalGetSpanKey() {
+      return SpanKey.HTTP_SERVER;
+    }
+  }
+
   private AgentSpanTestingInstrumenter() {}
 }

+ 0 - 39
instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspectTest.java

@@ -7,13 +7,11 @@ package io.opentelemetry.instrumentation.spring.autoconfigure.aspects;
 
 import static io.opentelemetry.api.trace.SpanKind.CLIENT;
 import static io.opentelemetry.api.trace.SpanKind.INTERNAL;
-import static io.opentelemetry.api.trace.SpanKind.SERVER;
 import static io.opentelemetry.sdk.testing.assertj.TracesAssert.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
-import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.extension.annotations.SpanAttribute;
 import io.opentelemetry.extension.annotations.WithSpan;
 import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension;
@@ -58,11 +56,6 @@ public class WithSpanAspectTest {
       return "Span with name testWithClientSpan and SpanKind.CLIENT was created";
     }
 
-    @WithSpan(kind = SpanKind.SERVER)
-    public String testWithServerSpan() {
-      return "Span with name testWithServerSpan and SpanKind.SERVER was created";
-    }
-
     @WithSpan
     public CompletionStage<String> testAsyncCompletionStage(CompletionStage<String> stage) {
       return stage;
@@ -188,38 +181,6 @@ public class WithSpanAspectTest {
                             .hasParentSpanId(traces.get(0).get(0).getSpanId())));
   }
 
-  @Test
-  @DisplayName(
-      "when method is annotated with @WithSpan(kind=CLIENT) and context already contains a CLIENT span should suppress span")
-  void suppressClientSpan() throws Throwable {
-    // when
-    testing.runWithClientSpan("parent", withSpanTester::testWithClientSpan);
-
-    // then
-    List<List<SpanData>> traces = testing.waitForTraces(1);
-    assertThat(traces)
-        .hasTracesSatisfyingExactly(
-            trace ->
-                trace.hasSpansSatisfyingExactly(
-                    parentSpan -> parentSpan.hasName("parent").hasKind(CLIENT)));
-  }
-
-  @Test
-  @DisplayName(
-      "when method is annotated with @WithSpan(kind=SERVER) and context already contains a SERVER span should suppress span")
-  void suppressServerSpan() throws Throwable {
-    // when
-    testing.runWithServerSpan("parent", withSpanTester::testWithServerSpan);
-
-    // then
-    List<List<SpanData>> traces = testing.waitForTraces(1);
-    assertThat(traces)
-        .hasTracesSatisfyingExactly(
-            trace ->
-                trace.hasSpansSatisfyingExactly(
-                    parentSpan -> parentSpan.hasName("parent").hasKind(SERVER)));
-  }
-
   @Test
   @DisplayName("")
   void withSpanAttributes() throws Throwable {

+ 1 - 1
instrumentation/spring/spring-integration-4.1/javaagent/src/test/groovy/SpringIntegrationAndRabbitTest.groovy

@@ -24,7 +24,7 @@ class SpringIntegrationAndRabbitTest extends AgentInstrumentationSpecification i
   def "should cooperate with existing RabbitMQ instrumentation"() {
     when:
     // simulate the workflow being triggered by HTTP request
-    runWithServerSpan("HTTP GET") {
+    runWithHttpServerSpan("HTTP GET") {
       producerContext.getBean("producer", Runnable).run()
     }
 

+ 1 - 1
instrumentation/spring/spring-web-3.1/library/src/test/java/io/opentelemetry/instrumentation/spring/web/SpringWebTelemetryTest.java

@@ -35,7 +35,7 @@ class SpringWebTelemetryTest {
         SpringWebTelemetry.create(testing.getOpenTelemetry()).newInterceptor();
 
     // when
-    testing.runWithClientSpan(
+    testing.runWithHttpClientSpan(
         "parent",
         () -> {
           interceptor.intercept(httpRequestMock, requestBody, requestExecutionMock);

+ 1 - 0
instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioAsyncInstrumentation.java

@@ -86,6 +86,7 @@ public class TwilioAsyncInstrumentation implements TypeInstrumentation {
       }
 
       context = instrumenter().start(parentContext, spanName);
+      context = TwilioAsyncMarker.markAsync(context);
       scope = context.makeCurrent();
     }
 

+ 29 - 0
instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioAsyncMarker.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.twilio;
+
+import io.opentelemetry.context.Context;
+import io.opentelemetry.context.ContextKey;
+
+/**
+ * Twilio async operations just simply call sync ones internally. To suppress duplicate sync
+ * telemetry we add a marker entry to the context.
+ */
+public final class TwilioAsyncMarker {
+
+  private static final ContextKey<Boolean> MARKER_KEY =
+      ContextKey.named("opentelemetry-instrumentation-twilio-async-marker");
+
+  public static Context markAsync(Context context) {
+    return context.with(MARKER_KEY, Boolean.TRUE);
+  }
+
+  public static boolean isMarkedAsync(Context context) {
+    return context.get(MARKER_KEY) != null;
+  }
+
+  private TwilioAsyncMarker() {}
+}

+ 2 - 1
instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioSyncInstrumentation.java

@@ -73,7 +73,8 @@ public class TwilioSyncInstrumentation implements TypeInstrumentation {
         @Advice.Local("otelSpanName") String spanName) {
       Context parentContext = currentContext();
       spanName = TwilioSingletons.spanName(that, methodName);
-      if (!instrumenter().shouldStart(parentContext, spanName)) {
+      if (!instrumenter().shouldStart(parentContext, spanName)
+          || TwilioAsyncMarker.isMarkedAsync(parentContext)) {
         return;
       }
 

+ 104 - 84
instrumentation/twilio-6.6/javaagent/src/test/groovy/test/TwilioClientTest.groovy

@@ -16,8 +16,8 @@ import com.twilio.rest.api.v2010.account.Call
 import com.twilio.rest.api.v2010.account.Message
 import com.twilio.type.PhoneNumber
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
+import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
 import org.apache.http.HttpEntity
-import org.apache.http.HttpStatus
 import org.apache.http.StatusLine
 import org.apache.http.client.methods.CloseableHttpResponse
 import org.apache.http.impl.client.CloseableHttpClient
@@ -196,28 +196,14 @@ class TwilioClientTest extends AgentInstrumentationSpecification {
     }
   }
 
-
   def "http client"() {
     setup:
-    HttpClientBuilder clientBuilder = Mock()
     CloseableHttpClient httpClient = Mock()
-    CloseableHttpResponse httpResponse = Mock()
-    HttpEntity httpEntity = Mock()
-    StatusLine statusLine = Mock()
+    httpClient.execute(_) >> mockResponse(MESSAGE_RESPONSE_BODY, 200)
 
+    HttpClientBuilder clientBuilder = Mock()
     clientBuilder.build() >> httpClient
 
-    httpClient.execute(_) >> httpResponse
-
-    httpResponse.getEntity() >> httpEntity
-    httpResponse.getStatusLine() >> statusLine
-
-    httpEntity.getContent() >> { new ByteArrayInputStream(MESSAGE_RESPONSE_BODY.getBytes()) }
-    httpEntity.isRepeatable() >> true
-    httpEntity.getContentLength() >> MESSAGE_RESPONSE_BODY.length()
-
-    statusLine.getStatusCode() >> HttpStatus.SC_OK
-
     NetworkHttpClient networkHttpClient = new NetworkHttpClient(clientBuilder)
 
     TwilioRestClient realTwilioRestClient =
@@ -239,7 +225,7 @@ class TwilioClientTest extends AgentInstrumentationSpecification {
     message.body == "Hello, World!"
 
     assertTraces(1) {
-      trace(0, 2) {
+      trace(0, 3) {
         span(0) {
           name "test"
           hasNoParent()
@@ -257,48 +243,35 @@ class TwilioClientTest extends AgentInstrumentationSpecification {
             "twilio.status" "sent"
           }
         }
+        span(2) {
+          name "HTTP POST"
+          kind CLIENT
+          childOf span(1)
+          attributes {
+            "$SemanticAttributes.NET_TRANSPORT.key" SemanticAttributes.NetTransportValues.IP_TCP
+            "$SemanticAttributes.NET_PEER_NAME.key" "api.twilio.com"
+            "$SemanticAttributes.NET_PEER_PORT.key" 443
+            "$SemanticAttributes.HTTP_FLAVOR.key" SemanticAttributes.HttpFlavorValues.HTTP_1_1
+            "$SemanticAttributes.HTTP_METHOD.key" "POST"
+            "$SemanticAttributes.HTTP_URL.key" "https://api.twilio.com/2010-04-01/Accounts/abc/Messages.json"
+            "$SemanticAttributes.HTTP_STATUS_CODE.key" 200
+          }
+        }
       }
     }
   }
 
   def "http client retry"() {
     setup:
-    HttpClientBuilder clientBuilder = Mock()
     CloseableHttpClient httpClient = Mock()
-    CloseableHttpResponse httpResponse1 = Mock()
-    CloseableHttpResponse httpResponse2 = Mock()
-    HttpEntity httpEntity1 = Mock()
-    HttpEntity httpEntity2 = Mock()
-    StatusLine statusLine1 = Mock()
-    StatusLine statusLine2 = Mock()
+    httpClient.execute(_) >>> [
+      mockResponse(ERROR_RESPONSE_BODY, 500),
+      mockResponse(MESSAGE_RESPONSE_BODY, 200)
+    ]
 
+    HttpClientBuilder clientBuilder = Mock()
     clientBuilder.build() >> httpClient
 
-    httpClient.execute(_) >>> [httpResponse1, httpResponse2]
-
-    // First response is an HTTP/500 error, which should drive a retry
-    httpResponse1.getEntity() >> httpEntity1
-    httpResponse1.getStatusLine() >> statusLine1
-
-    httpEntity1.getContent() >> { new ByteArrayInputStream(ERROR_RESPONSE_BODY.getBytes()) }
-
-    httpEntity1.isRepeatable() >> true
-    httpEntity1.getContentLength() >> ERROR_RESPONSE_BODY.length()
-
-    statusLine1.getStatusCode() >> HttpStatus.SC_INTERNAL_SERVER_ERROR
-
-    // Second response is HTTP/200 success
-    httpResponse2.getEntity() >> httpEntity2
-    httpResponse2.getStatusLine() >> statusLine2
-
-    httpEntity2.getContent() >> {
-      new ByteArrayInputStream(MESSAGE_RESPONSE_BODY.getBytes())
-    }
-    httpEntity2.isRepeatable() >> true
-    httpEntity2.getContentLength() >> MESSAGE_RESPONSE_BODY.length()
-
-    statusLine2.getStatusCode() >> HttpStatus.SC_OK
-
     NetworkHttpClient networkHttpClient = new NetworkHttpClient(clientBuilder)
 
     TwilioRestClient realTwilioRestClient =
@@ -319,7 +292,7 @@ class TwilioClientTest extends AgentInstrumentationSpecification {
     message.body == "Hello, World!"
 
     assertTraces(1) {
-      trace(0, 2) {
+      trace(0, 4) {
         span(0) {
           name "test"
           hasNoParent()
@@ -337,48 +310,50 @@ class TwilioClientTest extends AgentInstrumentationSpecification {
             "twilio.status" "sent"
           }
         }
+        span(2) {
+          name "HTTP POST"
+          kind CLIENT
+          childOf span(1)
+          status ERROR
+          attributes {
+            "$SemanticAttributes.NET_TRANSPORT.key" SemanticAttributes.NetTransportValues.IP_TCP
+            "$SemanticAttributes.NET_PEER_NAME.key" "api.twilio.com"
+            "$SemanticAttributes.NET_PEER_PORT.key" 443
+            "$SemanticAttributes.HTTP_FLAVOR.key" SemanticAttributes.HttpFlavorValues.HTTP_1_1
+            "$SemanticAttributes.HTTP_METHOD.key" "POST"
+            "$SemanticAttributes.HTTP_URL.key" "https://api.twilio.com/2010-04-01/Accounts/abc/Messages.json"
+            "$SemanticAttributes.HTTP_STATUS_CODE.key" 500
+          }
+        }
+        span(3) {
+          name "HTTP POST"
+          kind CLIENT
+          childOf span(1)
+          attributes {
+            "$SemanticAttributes.NET_TRANSPORT.key" SemanticAttributes.NetTransportValues.IP_TCP
+            "$SemanticAttributes.NET_PEER_NAME.key" "api.twilio.com"
+            "$SemanticAttributes.NET_PEER_PORT.key" 443
+            "$SemanticAttributes.HTTP_FLAVOR.key" SemanticAttributes.HttpFlavorValues.HTTP_1_1
+            "$SemanticAttributes.HTTP_METHOD.key" "POST"
+            "$SemanticAttributes.HTTP_URL.key" "https://api.twilio.com/2010-04-01/Accounts/abc/Messages.json"
+            "$SemanticAttributes.HTTP_STATUS_CODE.key" 200
+          }
+        }
       }
     }
   }
 
   def "http client retry async"() {
     setup:
-    HttpClientBuilder clientBuilder = Mock()
     CloseableHttpClient httpClient = Mock()
-    CloseableHttpResponse httpResponse1 = Mock()
-    CloseableHttpResponse httpResponse2 = Mock()
-    HttpEntity httpEntity1 = Mock()
-    HttpEntity httpEntity2 = Mock()
-    StatusLine statusLine1 = Mock()
-    StatusLine statusLine2 = Mock()
+    httpClient.execute(_) >>> [
+      mockResponse(ERROR_RESPONSE_BODY, 500),
+      mockResponse(MESSAGE_RESPONSE_BODY, 200)
+    ]
 
+    HttpClientBuilder clientBuilder = Mock()
     clientBuilder.build() >> httpClient
 
-    httpClient.execute(_) >>> [httpResponse1, httpResponse2]
-
-    // First response is an HTTP/500 error, which should drive a retry
-    httpResponse1.getEntity() >> httpEntity1
-    httpResponse1.getStatusLine() >> statusLine1
-
-    httpEntity1.getContent() >> { new ByteArrayInputStream(ERROR_RESPONSE_BODY.getBytes()) }
-
-    httpEntity1.isRepeatable() >> true
-    httpEntity1.getContentLength() >> ERROR_RESPONSE_BODY.length()
-
-    statusLine1.getStatusCode() >> HttpStatus.SC_INTERNAL_SERVER_ERROR
-
-    // Second response is HTTP/200 success
-    httpResponse2.getEntity() >> httpEntity2
-    httpResponse2.getStatusLine() >> statusLine2
-
-    httpEntity2.getContent() >> {
-      new ByteArrayInputStream(MESSAGE_RESPONSE_BODY.getBytes())
-    }
-    httpEntity2.isRepeatable() >> true
-    httpEntity2.getContentLength() >> MESSAGE_RESPONSE_BODY.length()
-
-    statusLine2.getStatusCode() >> HttpStatus.SC_OK
-
     NetworkHttpClient networkHttpClient = new NetworkHttpClient(clientBuilder)
 
     TwilioRestClient realTwilioRestClient =
@@ -406,7 +381,7 @@ class TwilioClientTest extends AgentInstrumentationSpecification {
     message.body == "Hello, World!"
 
     assertTraces(1) {
-      trace(0, 2) {
+      trace(0, 4) {
         span(0) {
           name "test"
           hasNoParent()
@@ -424,6 +399,35 @@ class TwilioClientTest extends AgentInstrumentationSpecification {
             "twilio.status" "sent"
           }
         }
+        span(2) {
+          name "HTTP POST"
+          kind CLIENT
+          childOf span(1)
+          status ERROR
+          attributes {
+            "$SemanticAttributes.NET_TRANSPORT.key" SemanticAttributes.NetTransportValues.IP_TCP
+            "$SemanticAttributes.NET_PEER_NAME.key" "api.twilio.com"
+            "$SemanticAttributes.NET_PEER_PORT.key" 443
+            "$SemanticAttributes.HTTP_FLAVOR.key" SemanticAttributes.HttpFlavorValues.HTTP_1_1
+            "$SemanticAttributes.HTTP_METHOD.key" "POST"
+            "$SemanticAttributes.HTTP_URL.key" "https://api.twilio.com/2010-04-01/Accounts/abc/Messages.json"
+            "$SemanticAttributes.HTTP_STATUS_CODE.key" 500
+          }
+        }
+        span(3) {
+          name "HTTP POST"
+          kind CLIENT
+          childOf span(1)
+          attributes {
+            "$SemanticAttributes.NET_TRANSPORT.key" SemanticAttributes.NetTransportValues.IP_TCP
+            "$SemanticAttributes.NET_PEER_NAME.key" "api.twilio.com"
+            "$SemanticAttributes.NET_PEER_PORT.key" 443
+            "$SemanticAttributes.HTTP_FLAVOR.key" SemanticAttributes.HttpFlavorValues.HTTP_1_1
+            "$SemanticAttributes.HTTP_METHOD.key" "POST"
+            "$SemanticAttributes.HTTP_URL.key" "https://api.twilio.com/2010-04-01/Accounts/abc/Messages.json"
+            "$SemanticAttributes.HTTP_STATUS_CODE.key" 200
+          }
+        }
       }
     }
 
@@ -608,4 +612,20 @@ class TwilioClientTest extends AgentInstrumentationSpecification {
       }
     }
   }
+
+  private CloseableHttpResponse mockResponse(String body, int statusCode) {
+    HttpEntity httpEntity = Mock()
+    httpEntity.getContent() >> { new ByteArrayInputStream(body.getBytes()) }
+    httpEntity.isRepeatable() >> true
+    httpEntity.getContentLength() >> body.length()
+
+    StatusLine statusLine = Mock()
+    statusLine.getStatusCode() >> statusCode
+
+    CloseableHttpResponse httpResponse = Mock()
+    httpResponse.getEntity() >> httpEntity
+    httpResponse.getStatusLine() >> statusLine
+
+    return httpResponse
+  }
 }

+ 0 - 1
settings.gradle.kts

@@ -152,7 +152,6 @@ include(":instrumentation:aws-lambda:aws-lambda-events-2.2:javaagent")
 include(":instrumentation:aws-lambda:aws-lambda-events-2.2:library")
 include(":instrumentation:aws-lambda:aws-lambda-events-2.2:testing")
 include(":instrumentation:aws-sdk:aws-sdk-1.11:javaagent")
-include(":instrumentation:aws-sdk:aws-sdk-1.11:javaagent-unit-tests")
 include(":instrumentation:aws-sdk:aws-sdk-1.11:library")
 include(":instrumentation:aws-sdk:aws-sdk-1.11:library-autoconfigure")
 include(":instrumentation:aws-sdk:aws-sdk-1.11:testing")

+ 6 - 6
testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/InstrumentationSpecification.groovy

@@ -121,18 +121,18 @@ abstract class InstrumentationSpecification extends Specification {
   }
 
   /**
-   * Runs the provided {@code callback} inside the scope of an CLIENT span with name {@code
+   * Runs the provided {@code callback} inside the scope of an HTTP CLIENT span with name {@code
    * spanName}.
    */
-  def <T> T runWithClientSpan(String spanName, Closure callback) {
-    return (T) testRunner().runWithClientSpan(spanName, (ThrowingSupplier) callback)
+  def <T> T runWithHttpClientSpan(String spanName, Closure callback) {
+    return (T) testRunner().runWithHttpClientSpan(spanName, (ThrowingSupplier) callback)
   }
 
   /**
-   * Runs the provided {@code callback} inside the scope of an CLIENT span with name {@code
+   * Runs the provided {@code callback} inside the scope of an HTTP CLIENT span with name {@code
    * spanName}.
    */
-  def <T> T runWithServerSpan(String spanName, Closure callback) {
-    return (T) testRunner().runWithServerSpan(spanName, (ThrowingSupplier) callback)
+  def <T> T runWithHttpServerSpan(String spanName, Closure callback) {
+    return (T) testRunner().runWithHttpServerSpan(spanName, (ThrowingSupplier) callback)
   }
 }

+ 12 - 12
testing-common/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java

@@ -120,12 +120,12 @@ public abstract class InstrumentationTestRunner {
   }
 
   /**
-   * Runs the provided {@code callback} inside the scope of an CLIENT span with name {@code
+   * Runs the provided {@code callback} inside the scope of an HTTP CLIENT span with name {@code
    * spanName}.
    */
-  public final <E extends Throwable> void runWithClientSpan(
+  public final <E extends Throwable> void runWithHttpClientSpan(
       String spanName, ThrowingRunnable<E> callback) throws E {
-    runWithClientSpan(
+    runWithHttpClientSpan(
         spanName,
         () -> {
           callback.run();
@@ -134,21 +134,21 @@ public abstract class InstrumentationTestRunner {
   }
 
   /**
-   * Runs the provided {@code callback} inside the scope of an CLIENT span with name {@code
+   * Runs the provided {@code callback} inside the scope of an HTTP CLIENT span with name {@code
    * spanName}.
    */
-  public final <T, E extends Throwable> T runWithClientSpan(
+  public final <T, E extends Throwable> T runWithHttpClientSpan(
       String spanName, ThrowingSupplier<T, E> callback) throws E {
-    return testInstrumenters.runWithClientSpan(spanName, callback);
+    return testInstrumenters.runWithHttpClientSpan(spanName, callback);
   }
 
   /**
-   * Runs the provided {@code callback} inside the scope of an CLIENT span with name {@code
+   * Runs the provided {@code callback} inside the scope of an HTTP SERVER span with name {@code
    * spanName}.
    */
-  public final <E extends Throwable> void runWithServerSpan(
+  public final <E extends Throwable> void runWithHttpServerSpan(
       String spanName, ThrowingRunnable<E> callback) throws E {
-    runWithServerSpan(
+    runWithHttpServerSpan(
         spanName,
         () -> {
           callback.run();
@@ -157,12 +157,12 @@ public abstract class InstrumentationTestRunner {
   }
 
   /**
-   * Runs the provided {@code callback} inside the scope of an CLIENT span with name {@code
+   * Runs the provided {@code callback} inside the scope of an HTTP SERVER span with name {@code
    * spanName}.
    */
-  public final <T, E extends Throwable> T runWithServerSpan(
+  public final <T, E extends Throwable> T runWithHttpServerSpan(
       String spanName, ThrowingSupplier<T, E> callback) throws E {
-    return testInstrumenters.runWithServerSpan(spanName, callback);
+    return testInstrumenters.runWithHttpServerSpan(spanName, callback);
   }
 
   /** Runs the provided {@code callback} inside the scope of a non-recording span. */

+ 48 - 11
testing-common/src/main/java/io/opentelemetry/instrumentation/testing/TestInstrumenters.java

@@ -6,32 +6,43 @@
 package io.opentelemetry.instrumentation.testing;
 
 import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.api.common.AttributesBuilder;
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.SpanContext;
 import io.opentelemetry.api.trace.TraceFlags;
 import io.opentelemetry.api.trace.TraceState;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
+import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor;
+import io.opentelemetry.instrumentation.api.internal.SpanKey;
+import io.opentelemetry.instrumentation.api.internal.SpanKeyProvider;
 import io.opentelemetry.instrumentation.testing.util.ThrowingSupplier;
+import javax.annotation.Nullable;
 
 /** {@link Instrumenter}s to use when executing test code. */
 final class TestInstrumenters {
 
-  private final Instrumenter<String, Void> testInstrumenter;
-  private final Instrumenter<String, Void> testClientInstrumenter;
-  private final Instrumenter<String, Void> testServerInstrumenter;
+  private final Instrumenter<String, Void> instrumenter;
+  private final Instrumenter<String, Void> httpClientInstrumenter;
+  private final Instrumenter<String, Void> httpServerInstrumenter;
 
   TestInstrumenters(OpenTelemetry openTelemetry) {
-    testInstrumenter =
+    instrumenter =
         Instrumenter.<String, Void>builder(openTelemetry, "test", name -> name)
             .newInstrumenter(SpanKindExtractor.alwaysInternal());
-    testClientInstrumenter =
+    httpClientInstrumenter =
         Instrumenter.<String, Void>builder(openTelemetry, "test", name -> name)
+            // cover both semconv and span-kind strategies
+            .addAttributesExtractor(new SpanKeyAttributesExtractor(SpanKey.HTTP_CLIENT))
+            .addAttributesExtractor(new SpanKeyAttributesExtractor(SpanKey.KIND_CLIENT))
             .newInstrumenter(SpanKindExtractor.alwaysClient());
-    testServerInstrumenter =
+    httpServerInstrumenter =
         Instrumenter.<String, Void>builder(openTelemetry, "test", name -> name)
+            // cover both semconv and span-kind strategies
+            .addAttributesExtractor(new SpanKeyAttributesExtractor(SpanKey.HTTP_SERVER))
+            .addAttributesExtractor(new SpanKeyAttributesExtractor(SpanKey.KIND_SERVER))
             .newInstrumenter(SpanKindExtractor.alwaysServer());
   }
 
@@ -41,25 +52,25 @@ final class TestInstrumenters {
    */
   <T, E extends Throwable> T runWithSpan(String spanName, ThrowingSupplier<T, E> callback)
       throws E {
-    return runWithInstrumenter(spanName, testInstrumenter, callback);
+    return runWithInstrumenter(spanName, instrumenter, callback);
   }
 
   /**
    * Runs the provided {@code callback} inside the scope of an CLIENT span with name {@code
    * spanName}.
    */
-  <T, E extends Throwable> T runWithClientSpan(String spanName, ThrowingSupplier<T, E> callback)
+  <T, E extends Throwable> T runWithHttpClientSpan(String spanName, ThrowingSupplier<T, E> callback)
       throws E {
-    return runWithInstrumenter(spanName, testClientInstrumenter, callback);
+    return runWithInstrumenter(spanName, httpClientInstrumenter, callback);
   }
 
   /**
    * Runs the provided {@code callback} inside the scope of an CLIENT span with name {@code
    * spanName}.
    */
-  <T, E extends Throwable> T runWithServerSpan(String spanName, ThrowingSupplier<T, E> callback)
+  <T, E extends Throwable> T runWithHttpServerSpan(String spanName, ThrowingSupplier<T, E> callback)
       throws E {
-    return runWithInstrumenter(spanName, testServerInstrumenter, callback);
+    return runWithInstrumenter(spanName, httpServerInstrumenter, callback);
   }
 
   /** Runs the provided {@code callback} inside the scope of a non-recording span. */
@@ -89,4 +100,30 @@ final class TestInstrumenters {
       instrumenter.end(context, spanName, null, err);
     }
   }
+
+  private static final class SpanKeyAttributesExtractor
+      implements AttributesExtractor<String, Void>, SpanKeyProvider {
+
+    private final SpanKey spanKey;
+
+    private SpanKeyAttributesExtractor(SpanKey spanKey) {
+      this.spanKey = spanKey;
+    }
+
+    @Override
+    public void onStart(AttributesBuilder attributes, Context parentContext, String s) {}
+
+    @Override
+    public void onEnd(
+        AttributesBuilder attributes,
+        Context context,
+        String s,
+        @Nullable Void unused,
+        @Nullable Throwable error) {}
+
+    @Override
+    public SpanKey internalGetSpanKey() {
+      return spanKey;
+    }
+  }
 }

+ 12 - 12
testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/InstrumentationExtension.java

@@ -138,39 +138,39 @@ public abstract class InstrumentationExtension
   }
 
   /**
-   * Runs the provided {@code callback} inside the scope of an CLIENT span with name {@code
+   * Runs the provided {@code callback} inside the scope of an HTTP CLIENT span with name {@code
    * spanName}.
    */
-  public <E extends Throwable> void runWithClientSpan(String spanName, ThrowingRunnable<E> callback)
-      throws E {
-    testRunner.runWithClientSpan(spanName, callback);
+  public <E extends Throwable> void runWithHttpClientSpan(
+      String spanName, ThrowingRunnable<E> callback) throws E {
+    testRunner.runWithHttpClientSpan(spanName, callback);
   }
 
   /**
-   * Runs the provided {@code callback} inside the scope of an CLIENT span with name {@code
+   * Runs the provided {@code callback} inside the scope of an HTTP CLIENT span with name {@code
    * spanName}.
    */
-  public <T, E extends Throwable> T runWithClientSpan(
+  public <T, E extends Throwable> T runWithHttpClientSpan(
       String spanName, ThrowingSupplier<T, E> callback) throws E {
-    return testRunner.runWithClientSpan(spanName, callback);
+    return testRunner.runWithHttpClientSpan(spanName, callback);
   }
 
   /**
    * Runs the provided {@code callback} inside the scope of an CLIENT span with name {@code
    * spanName}.
    */
-  public <E extends Throwable> void runWithServerSpan(String spanName, ThrowingRunnable<E> callback)
-      throws E {
-    testRunner.runWithServerSpan(spanName, callback);
+  public <E extends Throwable> void runWithHttpServerSpan(
+      String spanName, ThrowingRunnable<E> callback) throws E {
+    testRunner.runWithHttpServerSpan(spanName, callback);
   }
 
   /**
    * Runs the provided {@code callback} inside the scope of an CLIENT span with name {@code
    * spanName}.
    */
-  public <T, E extends Throwable> T runWithServerSpan(
+  public <T, E extends Throwable> T runWithHttpServerSpan(
       String spanName, ThrowingSupplier<T, E> callback) throws E {
-    return testRunner.runWithServerSpan(spanName, callback);
+    return testRunner.runWithHttpServerSpan(spanName, callback);
   }
 
   /** Returns whether forceFlush was called. */

+ 1 - 1
testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java

@@ -262,7 +262,7 @@ public abstract class AbstractHttpClientTest<REQUEST> {
 
     URI uri = resolveAddress("/success");
     int responseCode =
-        testing.runWithClientSpan("parent-client-span", () -> doRequest(method, uri));
+        testing.runWithHttpClientSpan("parent-client-span", () -> doRequest(method, uri));
 
     assertThat(responseCode).isEqualTo(200);