Fix http.server.active_requests metric with async requests

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

@@ -11,6 +11,7 @@
 import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.api.trace.Tracer;
 import io.opentelemetry.context.Context;
+import io.opentelemetry.context.ContextKey;
 import io.opentelemetry.instrumentation.api.internal.HttpRouteState;
 import io.opentelemetry.instrumentation.api.internal.InstrumenterAccess;
 import io.opentelemetry.instrumentation.api.internal.InstrumenterUtil;
@@ -41,6 +42,9 @@
  */
 public class Instrumenter<REQUEST, RESPONSE> {
 
+  private static final ContextKey<OperationListener[]> START_OPERATION_LISTENERS =
+      ContextKey.named("instrumenter-start-operation-listeners");
+
   /**
    * Returns a new {@link InstrumenterBuilder}.
    *
@@ -198,6 +202,10 @@ private Context doStart(Context parentContext, REQUEST request, @Nullable Instan
         context = operationListeners[i].onStart(context, attributes, startNanos);
       }
     }
+    // when start and end are not called on the same instrumenter we need to use the operation
+    // listeners that were used during start and end to correctly handle metrics like
+    // http.server.active_requests that is recorded both in start and end
+    context = context.with(START_OPERATION_LISTENERS, operationListeners);
 
     if (localRoot) {
       context = LocalRootSpan.store(context, span);
@@ -228,10 +236,11 @@ private void doEnd(
     }
     span.setAllAttributes(attributes);
 
-    if (operationListeners.length != 0) {
+    OperationListener[] startOperationListeners = context.get(START_OPERATION_LISTENERS);
+    if (startOperationListeners != null && startOperationListeners.length != 0) {
       long endNanos = getNanos(endTime);
-      for (int i = operationListeners.length - 1; i >= 0; i--) {
-        operationListeners[i].onEnd(context, attributes, endNanos);
+      for (int i = startOperationListeners.length - 1; i >= 0; i--) {
+        startOperationListeners[i].onEnd(context, attributes, endNanos);
       }
     }
 

instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServlet3Test.groovy

@@ -149,13 +149,6 @@ class JettyServlet3TestAsync extends JettyServlet3Test {
   boolean isAsyncTest() {
     true
   }
-
-  @Override
-  String getMetricsInstrumentationName() {
-    // with async requests the span is started in one instrumentation (server instrumentation)
-    // but ended from another (servlet instrumentation)
-    "io.opentelemetry.servlet-3.0"
-  }
 }
 
 class JettyServlet3TestFakeAsync extends JettyServlet3Test {
@@ -164,13 +157,6 @@ class JettyServlet3TestFakeAsync extends JettyServlet3Test {
   Class<Servlet> servlet() {
     TestServlet3.FakeAsync
   }
-
-  @Override
-  String getMetricsInstrumentationName() {
-    // with async requests the span is started in one instrumentation (server instrumentation)
-    // but ended from another (servlet instrumentation)
-    "io.opentelemetry.servlet-3.0"
-  }
 }
 
 class JettyServlet3TestForward extends JettyDispatchTest {
@@ -247,13 +233,6 @@ class JettyServlet3TestDispatchImmediate extends JettyDispatchTest {
     true
   }
 
-  @Override
-  String getMetricsInstrumentationName() {
-    // with async requests the span is started in one instrumentation (server instrumentation)
-    // but ended from another (servlet instrumentation)
-    "io.opentelemetry.servlet-3.0"
-  }
-
   @Override
   protected void setupServlets(ServletContextHandler context) {
     super.setupServlets(context)
@@ -283,13 +262,6 @@ class JettyServlet3TestDispatchAsync extends JettyDispatchTest {
     true
   }
 
-  @Override
-  String getMetricsInstrumentationName() {
-    // with async requests the span is started in one instrumentation (server instrumentation)
-    // but ended from another (servlet instrumentation)
-    "io.opentelemetry.servlet-3.0"
-  }
-
   @Override
   protected void setupServlets(ServletContextHandler context) {
     super.setupServlets(context)

instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/JettyServlet5Test.groovy

@@ -123,13 +123,6 @@ class JettyServlet5TestAsync extends JettyServlet5Test {
   boolean errorEndpointUsesSendError() {
     false
   }
-
-  @Override
-  String getMetricsInstrumentationName() {
-    // with async requests the span is started in one instrumentation (server instrumentation)
-    // but ended from another (servlet instrumentation)
-    "io.opentelemetry.servlet-5.0"
-  }
 }
 
 @IgnoreIf({ !jvm.java11Compatible })
@@ -139,13 +132,6 @@ class JettyServlet5TestFakeAsync extends JettyServlet5Test {
   Class<Servlet> servlet() {
     TestServlet5.FakeAsync
   }
-
-  @Override
-  String getMetricsInstrumentationName() {
-    // with async requests the span is started in one instrumentation (server instrumentation)
-    // but ended from another (servlet instrumentation)
-    "io.opentelemetry.servlet-5.0"
-  }
 }
 
 @IgnoreIf({ !jvm.java11Compatible })
@@ -237,13 +223,6 @@ class JettyServlet5TestDispatchImmediate extends JettyDispatchTest {
     addServlet(context, "/dispatch" + INDEXED_CHILD.path, TestServlet5.DispatchImmediate)
     addServlet(context, "/dispatch/recursive", TestServlet5.DispatchRecursive)
   }
-
-  @Override
-  String getMetricsInstrumentationName() {
-    // with async requests the span is started in one instrumentation (server instrumentation)
-    // but ended from another (servlet instrumentation)
-    "io.opentelemetry.servlet-5.0"
-  }
 }
 
 @IgnoreIf({ !jvm.java11Compatible })
@@ -275,13 +254,6 @@ class JettyServlet5TestDispatchAsync extends JettyDispatchTest {
   boolean errorEndpointUsesSendError() {
     false
   }
-
-  @Override
-  String getMetricsInstrumentationName() {
-    // with async requests the span is started in one instrumentation (server instrumentation)
-    // but ended from another (servlet instrumentation)
-    "io.opentelemetry.servlet-5.0"
-  }
 }
 
 abstract class JettyDispatchTest extends JettyServlet5Test {

instrumentation/tomcat/tomcat-10.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/TomcatAsyncTest.java

@@ -113,10 +113,6 @@ protected void configure(HttpServerTestOptions options) {
     options.setExpectedException(new ServletException(EXCEPTION.getBody()));
 
     options.setHasResponseSpan(endpoint -> endpoint == NOT_FOUND || endpoint == REDIRECT);
-
-    // with async requests the span is started in one instrumentation (server instrumentation)
-    // but ended from another (servlet instrumentation)
-    options.setMetricsInstrumentationName(() -> "io.opentelemetry.servlet-5.0");
   }
 
   @Override

instrumentation/tomcat/tomcat-7.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/TomcatAsyncTest.java

@@ -108,10 +108,6 @@ protected void configure(HttpServerTestOptions options) {
     options.setExpectedException(new ServletException(EXCEPTION.getBody()));
 
     options.setHasResponseSpan(endpoint -> endpoint == NOT_FOUND || endpoint == REDIRECT);
-
-    // with async requests the span is started in one instrumentation (server instrumentation)
-    // but ended from another (servlet instrumentation)
-    options.setMetricsInstrumentationName(() -> "io.opentelemetry.servlet-3.0");
   }
 
   @Override

testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy

@@ -173,10 +173,6 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
     return true
   }
 
-  String getMetricsInstrumentationName() {
-    null
-  }
-
   /** A list of additional HTTP server span attributes extracted by the instrumentation per URI. */
   Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
     [
@@ -237,9 +233,6 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
       options.sockPeerAddr = { endpoint ->
         HttpServerTest.this.sockPeerAddr(endpoint)
       }
-      options.metricsInstrumentationName = {
-        HttpServerTest.this.getMetricsInstrumentationName()
-      }
       options.responseCodeOnNonStandardHttpMethod = getResponseCodeOnNonStandardHttpMethod()
 
       options.testRedirect = testRedirect()

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java

@@ -353,12 +353,8 @@ void httpServerMetrics() {
               spanData -> assertServerSpan(assertThat(spanData), method, SUCCESS, SUCCESS.status));
         });
 
-    String metricsInstrumentationName = options.metricsInstrumentationName.get();
-    if (metricsInstrumentationName == null) {
-      metricsInstrumentationName = instrumentationName.get();
-    }
     testing.waitAndAssertMetrics(
-        metricsInstrumentationName,
+        instrumentationName.get(),
         "http.server.request.duration",
         metrics ->
             metrics.anySatisfy(

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpServerTestOptions.java

@@ -19,7 +19,6 @@
 import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.function.Predicate;
-import java.util.function.Supplier;
 import javax.annotation.Nullable;
 
 public final class HttpServerTestOptions {
@@ -42,7 +41,6 @@ public final class HttpServerTestOptions {
   Function<ServerEndpoint, String> sockPeerAddr = unused -> "127.0.0.1";
   String contextPath = "";
   Throwable expectedException = new Exception(EXCEPTION.body);
-  Supplier<String> metricsInstrumentationName = () -> null;
   // we're calling /success in the test, and most servers respond with 200 anyway
   int responseCodeOnNonStandardHttpMethod = ServerEndpoint.SUCCESS.status;
 
@@ -108,13 +106,6 @@ public HttpServerTestOptions setExpectedException(Throwable expectedException) {
     return this;
   }
 
-  @CanIgnoreReturnValue
-  public HttpServerTestOptions setMetricsInstrumentationName(
-      Supplier<String> metricsInstrumentationName) {
-    this.metricsInstrumentationName = metricsInstrumentationName;
-    return this;
-  }
-
   @CanIgnoreReturnValue
   public HttpServerTestOptions setResponseCodeOnNonStandardHttpMethod(
       int responseCodeOnNonStandardHttpMethod) {