Test http metrics (#9353)

open-telemetry · Sep 1, 2023 · b0a8bd4 · b0a8bd4
1 parent 71db9ff
commit b0a8bd4
Show file tree

Hide file tree

Showing 10 changed files with 201 additions and 37 deletions.
diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServlet3Test.groovy b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServlet3Test.groovy
@@ -149,6 +149,13 @@ 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 {
@@ -157,6 +164,13 @@ 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 {
@@ -233,6 +247,13 @@ 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)
@@ -262,6 +283,13 @@ 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)

diff --git a/...est/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/TomcatAsyncTest.groovy b/...est/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/TomcatAsyncTest.groovy
@@ -76,6 +76,13 @@ class TomcatAsyncTest extends HttpServerTest<Tomcat> implements AgentTestTrait {
     return "/tomcat-context"
   }
 
+  @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"
+  }
+
   protected void setupServlets(Context context) {
     def servlet = servlet()
 

diff --git a/...test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/TomcatAsyncTest.groovy b/...test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/TomcatAsyncTest.groovy
@@ -76,6 +76,13 @@ class TomcatAsyncTest extends HttpServerTest<Tomcat> implements AgentTestTrait {
     return "/tomcat-context"
   }
 
+  @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"
+  }
+
   protected void setupServlets(Context context) {
     def servlet = servlet()
 

diff --git a/...g-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpClientTest.groovy b/...g-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpClientTest.groovy
@@ -332,6 +332,11 @@ abstract class HttpClientTest<REQUEST> extends InstrumentationSpecification {
     junitTest.httpsRequest()
   }
 
+  def "http client metrics"() {
+    expect:
+    junitTest.httpClientMetrics()
+  }
+
   /**
    * This test fires a large number of concurrent requests.
    * Each request first hits a HTTP server and then makes another client request.

diff --git a/...g-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy b/...g-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy
@@ -152,6 +152,10 @@ 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) {
     [
@@ -212,6 +216,9 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
       options.sockPeerAddr = { endpoint ->
         HttpServerTest.this.sockPeerAddr(endpoint)
       }
+      options.metricsInstrumentationName = {
+        HttpServerTest.this.getMetricsInstrumentationName()
+      }
 
       options.testRedirect = testRedirect()
       options.testError = testError()
@@ -314,6 +321,11 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
     junitTest.captureRequestParameters()
   }
 
+  def "http server metrics"() {
+    expect:
+    junitTest.httpServerMetrics()
+  }
+
   def "high concurrency test"() {
     expect:
     junitTest.highConcurrency()

diff --git a/...mon/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java b/...mon/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java
@@ -5,6 +5,7 @@
 
 package io.opentelemetry.instrumentation.testing;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.awaitility.Awaitility.await;
 
 import io.opentelemetry.api.OpenTelemetry;
@@ -13,17 +14,22 @@
 import io.opentelemetry.instrumentation.testing.util.ThrowingSupplier;
 import io.opentelemetry.sdk.logs.data.LogRecordData;
 import io.opentelemetry.sdk.metrics.data.MetricData;
+import io.opentelemetry.sdk.testing.assertj.MetricAssert;
+import io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions;
 import io.opentelemetry.sdk.testing.assertj.TraceAssert;
 import io.opentelemetry.sdk.testing.assertj.TracesAssert;
 import io.opentelemetry.sdk.trace.data.SpanData;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Comparator;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.function.Consumer;
+import java.util.stream.Collectors;
 import javax.annotation.Nullable;
+import org.assertj.core.api.ListAssert;
 import org.awaitility.core.ConditionTimeoutException;
 
 /**
@@ -139,6 +145,47 @@ private <T extends Consumer<TraceAssert>> void doAssertTraces(
     TracesAssert.assertThat(traces).hasTracesSatisfyingExactly(assertionsList);
   }
 
+  /**
+   * Waits for the assertion applied to all metrics of the given instrumentation and metric name to
+   * pass.
+   */
+  public final void waitAndAssertMetrics(
+      String instrumentationName, String metricName, Consumer<ListAssert<MetricData>> assertion) {
+    await()
+        .untilAsserted(
+            () ->
+                assertion.accept(
+                    assertThat(getExportedMetrics())
+                        .filteredOn(
+                            data ->
+                                data.getInstrumentationScopeInfo()
+                                        .getName()
+                                        .equals(instrumentationName)
+                                    && data.getName().equals(metricName))));
+  }
+
+  @SafeVarargs
+  public final void waitAndAssertMetrics(
+      String instrumentationName, Consumer<MetricAssert>... assertions) {
+    await()
+        .untilAsserted(
+            () -> {
+              Collection<MetricData> metrics = instrumentationMetrics(instrumentationName);
+              assertThat(metrics).isNotEmpty();
+              for (Consumer<MetricAssert> assertion : assertions) {
+                assertThat(metrics)
+                    .anySatisfy(
+                        metric -> assertion.accept(OpenTelemetryAssertions.assertThat(metric)));
+              }
+            });
+  }
+
+  private List<MetricData> instrumentationMetrics(String instrumentationName) {
+    return getExportedMetrics().stream()
+        .filter(m -> m.getInstrumentationScopeInfo().getName().equals(instrumentationName))
+        .collect(Collectors.toList());
+  }
+
   /**
    * Runs the provided {@code callback} inside the scope of an INTERNAL span with name {@code
    * spanName}.

diff --git a/...rc/main/java/io/opentelemetry/instrumentation/testing/junit/InstrumentationExtension.java b/...rc/main/java/io/opentelemetry/instrumentation/testing/junit/InstrumentationExtension.java
@@ -19,15 +19,12 @@
 import io.opentelemetry.sdk.logs.data.LogRecordData;
 import io.opentelemetry.sdk.metrics.data.MetricData;
 import io.opentelemetry.sdk.testing.assertj.MetricAssert;
-import io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions;
 import io.opentelemetry.sdk.testing.assertj.TraceAssert;
 import io.opentelemetry.sdk.trace.data.SpanData;
 import java.time.Duration;
-import java.util.Collection;
 import java.util.Comparator;
 import java.util.List;
 import java.util.function.Consumer;
-import java.util.stream.Collectors;
 import org.assertj.core.api.ListAssert;
 import org.junit.jupiter.api.extension.AfterAllCallback;
 import org.junit.jupiter.api.extension.AfterEachCallback;
@@ -80,12 +77,6 @@ public List<MetricData> metrics() {
     return testRunner.getExportedMetrics();
   }
 
-  private List<MetricData> instrumentationMetrics(String instrumentationName) {
-    return metrics().stream()
-        .filter(m -> m.getInstrumentationScopeInfo().getName().equals(instrumentationName))
-        .collect(Collectors.toList());
-  }
-
   /** Return a list of all captured logs. */
   public List<LogRecordData> logRecords() {
     return testRunner.getExportedLogRecords();
@@ -95,35 +86,15 @@ public List<LogRecordData> logRecords() {
    * Waits for the assertion applied to all metrics of the given instrumentation and metric name to
    * pass.
    */
-  public void waitAndAssertMetrics(
+  public final void waitAndAssertMetrics(
       String instrumentationName, String metricName, Consumer<ListAssert<MetricData>> assertion) {
-    await()
-        .untilAsserted(
-            () ->
-                assertion.accept(
-                    assertThat(metrics())
-                        .filteredOn(
-                            data ->
-                                data.getInstrumentationScopeInfo()
-                                        .getName()
-                                        .equals(instrumentationName)
-                                    && data.getName().equals(metricName))));
+    testRunner.waitAndAssertMetrics(instrumentationName, metricName, assertion);
   }
 
   @SafeVarargs
   public final void waitAndAssertMetrics(
       String instrumentationName, Consumer<MetricAssert>... assertions) {
-    await()
-        .untilAsserted(
-            () -> {
-              Collection<MetricData> metrics = instrumentationMetrics(instrumentationName);
-              assertThat(metrics).isNotEmpty();
-              for (Consumer<MetricAssert> assertion : assertions) {
-                assertThat(metrics)
-                    .anySatisfy(
-                        metric -> assertion.accept(OpenTelemetryAssertions.assertThat(metric)));
-              }
-            });
+    testRunner.waitAndAssertMetrics(instrumentationName, assertions);
   }
 
   /**

diff --git a/...main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java b/...main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java
@@ -42,6 +42,7 @@
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
@@ -114,11 +115,10 @@ void successfulGetRequest(String path) throws Exception {
     assertThat(responseCode).isEqualTo(200);
 
     testing.waitAndAssertTraces(
-        trace -> {
-          trace.hasSpansSatisfyingExactly(
-              span -> assertClientSpan(span, uri, method, responseCode, null).hasNoParent(),
-              span -> assertServerSpan(span).hasParent(trace.getSpan(0)));
-        });
+        trace ->
+            trace.hasSpansSatisfyingExactly(
+                span -> assertClientSpan(span, uri, method, responseCode, null).hasNoParent(),
+                span -> assertServerSpan(span).hasParent(trace.getSpan(0))));
   }
 
   @Test
@@ -712,6 +712,43 @@ void httpsRequest() throws Exception {
         });
   }
 
+  @Test
+  void httpClientMetrics() throws Exception {
+    URI uri = resolveAddress("/success");
+    String method = "GET";
+    int responseCode = doRequest(method, uri);
+
+    assertThat(responseCode).isEqualTo(200);
+
+    AtomicReference<String> instrumentationName = new AtomicReference<>();
+    testing.waitAndAssertTraces(
+        trace -> {
+          instrumentationName.set(trace.getSpan(0).getInstrumentationScopeInfo().getName());
+          trace.hasSpansSatisfyingExactly(
+              span -> assertClientSpan(span, uri, method, responseCode, null).hasNoParent(),
+              span -> assertServerSpan(span).hasParent(trace.getSpan(0)));
+        });
+
+    String durationInstrumentName =
+        SemconvStability.emitStableHttpSemconv()
+            ? "http.client.request.duration"
+            : "http.client.duration";
+
+    testing.waitAndAssertMetrics(
+        instrumentationName.get(),
+        durationInstrumentName,
+        metrics ->
+            metrics.anySatisfy(
+                metric ->
+                    assertThat(metric)
+                        .hasDescription("The duration of the outbound HTTP request")
+                        .hasUnit(SemconvStability.emitStableHttpSemconv() ? "s" : "ms")
+                        .hasHistogramSatisfying(
+                            histogram ->
+                                histogram.hasPointsSatisfying(
+                                    point -> point.hasSumGreaterThan(0.0)))));
+  }
+
   /**
    * This test fires a large number of concurrent requests. Each request first hits a HTTP server
    * and then makes another client request. The goal of this test is to verify that in highly

diff --git a/...main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java b/...main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java
@@ -69,6 +69,7 @@
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 import java.util.stream.Stream;
@@ -325,6 +326,46 @@ void captureRequestParameters() {
     assertTheTraces(1, null, null, spanId, "POST", CAPTURE_PARAMETERS, response);
   }
 
+  @Test
+  void httpServerMetrics() {
+    String method = "GET";
+    AggregatedHttpRequest request = request(SUCCESS, method);
+    AggregatedHttpResponse response = client.execute(request).aggregate().join();
+
+    assertThat(response.status().code()).isEqualTo(SUCCESS.getStatus());
+    assertThat(response.contentUtf8()).isEqualTo(SUCCESS.getBody());
+
+    AtomicReference<String> instrumentationName = new AtomicReference<>();
+    testing.waitAndAssertTraces(
+        trace -> {
+          instrumentationName.set(trace.getSpan(0).getInstrumentationScopeInfo().getName());
+          trace.anySatisfy(spanData -> assertServerSpan(assertThat(spanData), method, SUCCESS));
+        });
+
+    String durationInstrumentName =
+        SemconvStability.emitStableHttpSemconv()
+            ? "http.server.request.duration"
+            : "http.server.duration";
+
+    String metricsInstrumentationName = options.metricsInstrumentationName.get();
+    if (metricsInstrumentationName == null) {
+      metricsInstrumentationName = instrumentationName.get();
+    }
+    testing.waitAndAssertMetrics(
+        metricsInstrumentationName,
+        durationInstrumentName,
+        metrics ->
+            metrics.anySatisfy(
+                metric ->
+                    assertThat(metric)
+                        .hasDescription("The duration of the inbound HTTP request")
+                        .hasUnit(SemconvStability.emitStableHttpSemconv() ? "s" : "ms")
+                        .hasHistogramSatisfying(
+                            histogram ->
+                                histogram.hasPointsSatisfying(
+                                    point -> point.hasSumGreaterThan(0.0)))));
+  }
+
   /**
    * This test fires a bunch of parallel request to the fixed backend endpoint. That endpoint is
    * supposed to create a new child span in the context of the SERVER span. That child span is