Test captured HTTP headers - HTTP server tests, part 1 (#4320)

* Test captured HTTP headers - HTTP server tests, part 1 * Upgrade undertow in resteasy tests (Undertow 1.0 had a bug where it thrown NPE on getHeaders())
open-telemetry · Oct 7, 2021 · 581a5e3 · 581a5e3
1 parent fd872b0
commit 581a5e3
Show file tree

Hide file tree

Showing 9 changed files with 126 additions and 6 deletions.
diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-3.0/javaagent/build.gradle.kts b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-3.0/javaagent/build.gradle.kts
@@ -40,7 +40,7 @@ dependencies {
   testLibrary("org.jboss.resteasy:resteasy-undertow:3.0.4.Final") {
     exclude("org.jboss.resteasy", "resteasy-client")
   }
-  testLibrary("io.undertow:undertow-servlet:1.0.0.Final")
+  testLibrary("io.undertow:undertow-servlet:1.4.28.Final")
   testLibrary("org.jboss.resteasy:resteasy-servlet-initializer:3.0.4.Final")
 
   latestDepTestLibrary("org.jboss.resteasy:resteasy-jaxrs:3.+")

diff --git a/...in/java/io/opentelemetry/instrumentation/restlet/v1_0/RestletHttpAttributesExtractor.java b/...in/java/io/opentelemetry/instrumentation/restlet/v1_0/RestletHttpAttributesExtractor.java
@@ -14,6 +14,7 @@
 import java.util.Collections;
 import java.util.List;
 import org.checkerframework.checker.nullness.qual.Nullable;
+import org.restlet.data.Form;
 import org.restlet.data.Parameter;
 import org.restlet.data.Reference;
 import org.restlet.data.Request;
@@ -51,7 +52,11 @@ protected String method(Request request) {
 
   @Override
   protected List<String> requestHeader(Request request, String name) {
-    return parametersToList(getHeaders(request).subList(name, /* ignoreCase = */ true));
+    Form headers = getHeaders(request);
+    if (headers == null) {
+      return Collections.emptyList();
+    }
+    return parametersToList(headers.subList(name, /* ignoreCase = */ true));
   }
 
   @Override
@@ -103,7 +108,11 @@ protected Integer statusCode(Request request, Response response) {
 
   @Override
   protected List<String> responseHeader(Request request, Response response, String name) {
-    return parametersToList(getHeaders(response).subList(name, /* ignoreCase = */ true));
+    Form headers = getHeaders(response);
+    if (headers == null) {
+      return Collections.emptyList();
+    }
+    return parametersToList(headers.subList(name, /* ignoreCase = */ true));
   }
 
   // minimize memory overhead by not using streams

diff --git a/...src/test/groovy/io/opententelemetry/instrumentation/restlet/v1_0/RestletServerTest.groovy b/...src/test/groovy/io/opententelemetry/instrumentation/restlet/v1_0/RestletServerTest.groovy
@@ -15,8 +15,9 @@ class RestletServerTest extends AbstractRestletServerTest implements LibraryTest
 
   @Override
   Restlet wrapRestlet(Restlet restlet, String path){
-
-    RestletTracing tracing = RestletTracing.create(openTelemetry)
+    RestletTracing tracing = RestletTracing.newBuilder(openTelemetry)
+      .captureHttpHeaders(capturedHttpHeadersForTesting())
+      .build()
 
     def tracingFilter = tracing.newFilter(path)
     def statusFilter = new StatusFilter(component.getContext(), false, null, null)

diff --git a/...ain/groovy/io/opentelemetry/instrumentation/restlet/v1_0/AbstractRestletServerTest.groovy b/...ain/groovy/io/opentelemetry/instrumentation/restlet/v1_0/AbstractRestletServerTest.groovy
@@ -15,13 +15,15 @@ import org.restlet.Restlet
 import org.restlet.Router
 import org.restlet.Server
 import org.restlet.VirtualHost
+import org.restlet.data.Form
 import org.restlet.data.MediaType
 import org.restlet.data.Protocol
 import org.restlet.data.Request
 import org.restlet.data.Response
 import org.restlet.data.Status
 import org.restlet.util.Template
 
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.CAPTURE_HEADERS
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.INDEXED_CHILD
@@ -121,6 +123,20 @@ abstract class AbstractRestletServerTest extends HttpServerTest<Server> {
       }
     })
 
+    attachAndWrap("/captureHeaders", new Restlet() {
+      @Override
+      void handle(Request request, Response response) {
+        controller(CAPTURE_HEADERS) {
+          Form requestHeaders = request.getAttributes().get("org.restlet.http.headers")
+          Form responseHeaders = response.getAttributes().computeIfAbsent("org.restlet.http.headers", { new Form() })
+          responseHeaders.add("X-Test-Response", requestHeaders.getValues("X-Test-Request"))
+
+          response.setEntity(CAPTURE_HEADERS.getBody(), MediaType.TEXT_PLAIN)
+          response.setStatus(Status.valueOf(CAPTURE_HEADERS.getStatus()), CAPTURE_HEADERS.getBody())
+        }
+      }
+    })
+
     attachAndWrap(INDEXED_CHILD.path, new Restlet() {
       @Override
       void handle(Request request, Response response) {
@@ -150,6 +166,11 @@ abstract class AbstractRestletServerTest extends HttpServerTest<Server> {
     true
   }
 
+  @Override
+  boolean testCapturedHttpHeaders() {
+    true
+  }
+
   @Override
   String expectedServerSpanName(ServerEndpoint endpoint) {
     switch (endpoint) {

diff --git a/...n/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/boot/SpringBootBasedTest.groovy b/...n/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/boot/SpringBootBasedTest.groovy
@@ -71,6 +71,11 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
     true
   }
 
+  @Override
+  boolean testCapturedHttpHeaders() {
+    true
+  }
+
   @Override
   boolean hasErrorPageSpans(ServerEndpoint endpoint) {
     endpoint == NOT_FOUND

diff --git a/...tation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/boot/TestController.groovy b/...tation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/boot/TestController.groovy
@@ -11,11 +11,13 @@ import org.springframework.http.ResponseEntity
 import org.springframework.stereotype.Controller
 import org.springframework.web.bind.annotation.ExceptionHandler
 import org.springframework.web.bind.annotation.PathVariable
+import org.springframework.web.bind.annotation.RequestHeader
 import org.springframework.web.bind.annotation.RequestMapping
 import org.springframework.web.bind.annotation.RequestParam
 import org.springframework.web.bind.annotation.ResponseBody
 import org.springframework.web.servlet.view.RedirectView
 
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.CAPTURE_HEADERS
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
@@ -72,6 +74,15 @@ class TestController {
     }
   }
 
+  @RequestMapping("/captureHeaders")
+  ResponseEntity capture_headers(@RequestHeader("X-Test-Request") String testRequestHeader) {
+    HttpServerTest.controller(CAPTURE_HEADERS) {
+      ResponseEntity.ok()
+        .header("X-Test-Response", testRequestHeader)
+        .body(CAPTURE_HEADERS.body)
+    }
+  }
+
   @RequestMapping("/path/{id}/param")
   @ResponseBody
   String path_param(@PathVariable("id") int id) {

diff --git a/testing-common/build.gradle.kts b/testing-common/build.gradle.kts
@@ -25,6 +25,7 @@ dependencies {
   api("io.opentelemetry:opentelemetry-sdk-testing")
   api("io.opentelemetry:opentelemetry-sdk-metrics")
   api("io.opentelemetry:opentelemetry-sdk-metrics-testing")
+  api(project(":instrumentation-api"))
 
   api("org.assertj:assertj-core")
 
@@ -48,7 +49,6 @@ dependencies {
   implementation("org.slf4j:jul-to-slf4j")
   implementation("io.opentelemetry:opentelemetry-extension-annotations")
   implementation("io.opentelemetry:opentelemetry-exporter-logging")
-  implementation(project(":instrumentation-api"))
 
   annotationProcessor("com.google.auto.service:auto-service")
   compileOnly("com.google.auto.service:auto-service")

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
@@ -11,6 +11,7 @@ import io.opentelemetry.api.trace.Span
 import io.opentelemetry.api.trace.SpanKind
 import io.opentelemetry.api.trace.StatusCode
 import io.opentelemetry.context.Context
+import io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHttpHeaders
 import io.opentelemetry.instrumentation.test.InstrumentationSpecification
 import io.opentelemetry.instrumentation.test.asserts.TraceAssert
 import io.opentelemetry.sdk.trace.data.SpanData
@@ -25,6 +26,7 @@ import spock.lang.Unroll
 import java.util.concurrent.Callable
 import java.util.concurrent.CountDownLatch
 
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.CAPTURE_HEADERS
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.INDEXED_CHILD
@@ -35,11 +37,21 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTrace
 import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP
+import static java.util.Collections.singletonList
 import static org.junit.Assume.assumeTrue
 
 @Unroll
 abstract class HttpServerTest<SERVER> extends InstrumentationSpecification implements HttpServerTestTrait<SERVER> {
 
+  static final String TEST_REQUEST_HEADER = "X-Test-Request"
+  static final String TEST_RESPONSE_HEADER = "X-Test-Response"
+
+  static CapturedHttpHeaders capturedHttpHeadersForTesting() {
+    CapturedHttpHeaders.create(
+      singletonList(TEST_REQUEST_HEADER),
+      singletonList(TEST_RESPONSE_HEADER))
+  }
+
   String expectedServerSpanName(ServerEndpoint endpoint) {
     switch (endpoint) {
       case PATH_PARAM:
@@ -91,6 +103,11 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
     false
   }
 
+  // TODO: enable it everywhere
+  boolean testCapturedHttpHeaders() {
+    false
+  }
+
   boolean testErrorBody() {
     true
   }
@@ -125,6 +142,7 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
     ERROR("error-status", 500, "controller error"), // "error" is a special path for some frameworks
     EXCEPTION("exception", 500, "controller exception"),
     NOT_FOUND("notFound", 404, "not found"),
+    CAPTURE_HEADERS("captureHeaders", 200, "headers captured"),
 
     // TODO: add tests for the following cases:
     QUERY_PARAM("query?some=query", 200, "some=query"),
@@ -369,6 +387,24 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
     method = "GET"
   }
 
+  def "test captured HTTP headers"() {
+    setup:
+    assumeTrue(testCapturedHttpHeaders())
+
+    def request = AggregatedHttpRequest.of(request(CAPTURE_HEADERS, "GET").headers()
+      .toBuilder()
+      .add(TEST_REQUEST_HEADER, "test")
+      .build())
+    def response = client.execute(request).aggregate().join()
+
+    expect:
+    response.status().code() == CAPTURE_HEADERS.status
+    response.contentUtf8() == CAPTURE_HEADERS.body
+
+    and:
+    assertTheTraces(1, null, null, "GET", CAPTURE_HEADERS, null, response)
+  }
+
   /*
   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.
@@ -604,6 +640,10 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
         if (extraAttributes.contains(SemanticAttributes.NET_TRANSPORT)) {
           "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
         }
+        if (endpoint == ServerEndpoint.CAPTURE_HEADERS) {
+          "http.request.header.x_test_request" { it == ["test"] }
+          "http.response.header.x_test_response" { it == ["test"] }
+        }
       }
     }
   }

diff --git a/...ain/java/io/opentelemetry/javaagent/testing/http/CapturedHttpHeadersTestConfigSource.java b/...ain/java/io/opentelemetry/javaagent/testing/http/CapturedHttpHeadersTestConfigSource.java
@@ -0,0 +1,33 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.testing.http;
+
+import com.google.auto.service.AutoService;
+import io.opentelemetry.javaagent.extension.config.ConfigPropertySource;
+import java.util.HashMap;
+import java.util.Map;
+
+@AutoService(ConfigPropertySource.class)
+public class CapturedHttpHeadersTestConfigSource implements ConfigPropertySource {
+
+  @Override
+  public Map<String, String> getProperties() {
+    Map<String, String> testConfig = new HashMap<>();
+    testConfig.put(
+        "otel.instrumentation.common.experimental.capture-http-headers.client.request",
+        "X-Test-Request");
+    testConfig.put(
+        "otel.instrumentation.common.experimental.capture-http-headers.client.response",
+        "X-Test-Response");
+    testConfig.put(
+        "otel.instrumentation.common.experimental.capture-http-headers.server.request",
+        "X-Test-Request");
+    testConfig.put(
+        "otel.instrumentation.common.experimental.capture-http-headers.server.response",
+        "X-Test-Response");
+    return testConfig;
+  }
+}