Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undertow 1.0 had a bug where it thrown NPE on getHeaders()

😱

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    public Collection<String> getHeaders(String name) {
        return new ArrayList(this.exchange.getResponseHeaders().get(name));
    }

Guess what happens when this.exchange.getResponseHeaders() returns null because there are no headers in the response yet 🙈

testLibrary("org.jboss.resteasy:resteasy-servlet-initializer:3.0.4.Final")

latestDepTestLibrary("org.jboss.resteasy:resteasy-jaxrs:3.+")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -150,6 +166,11 @@ abstract class AbstractRestletServerTest extends HttpServerTest<Server> {
true
}

@Override
boolean testCapturedHttpHeaders() {
true
}

@Override
String expectedServerSpanName(ServerEndpoint endpoint) {
switch (endpoint) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
true
}

@Override
boolean testCapturedHttpHeaders() {
true
}

@Override
boolean hasErrorPageSpans(ServerEndpoint endpoint) {
endpoint == NOT_FOUND
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion testing-common/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -91,6 +103,11 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
false
}

// TODO: enable it everywhere
boolean testCapturedHttpHeaders() {
false
}

boolean testErrorBody() {
true
}
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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"] }
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}