ApacheHttpClientRedirectInstrumentation copy headers from original request to redirect if original redirect headers were empty (DataDog/dd-trace-java#1539)
MODIFY
instrumentation/apache-httpasyncclient-4.0/src/main/java/io/opentelemetry/auto/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentation.java
instrumentation/apache-httpasyncclient-4.0/src/main/java/io/opentelemetry/auto/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentation.java
@@ -73,10 +73,21 @@ public class ApacheHttpClientRedirectInstrumentation extends Instrumenter.Defaul
if (redirect == null) {
return;
}
-
+ // TODO this only handles W3C headers
+ // Apache HttpClient 4.0.1+ copies headers from original to redirect only
+ // if redirect headers are empty. Because we add headers
+ // "traceparent" and "tracestate" to redirect: it means redirect headers never
+ // will be empty. So in case if not-instrumented redirect had no headers,
+ // we just copy all not set headers from original to redirect (doing same
+ // thing as apache httpclient does).
+ if (!redirect.headerIterator().hasNext()) {
+ // redirect didn't have other headers besides tracing, so we need to do copy
+ // (same work as Apache HttpClient 4.0.1+ does w/o instrumentation)
+ redirect.setHeaders(original.getAllHeaders());
+ } else {
for (final Header header : original.getAllHeaders()) {
final String name = header.getName().toLowerCase();
- if (name.equals("traceparent")) {
+ if (name.equals("traceparent") || name.equals("tracestate")) {
if (!redirect.containsHeader(header.getName())) {
redirect.setHeader(header.getName(), header.getValue());
}
@@ -85,3 +96,4 @@ public class ApacheHttpClientRedirectInstrumentation extends Instrumenter.Defaul
}
}
}
+}

MODIFY
instrumentation/google-http-client-1.19/src/test/groovy/AbstractGoogleHttpClientTest.groovy
instrumentation/google-http-client-1.19/src/test/groovy/AbstractGoogleHttpClientTest.groovy
@@ -40,7 +40,13 @@ abstract class AbstractGoogleHttpClientTest extends HttpClientTest {
HttpRequest request = requestFactory.buildRequest(method, genericUrl, null)
request.connectTimeout = CONNECT_TIMEOUT_MS
request.readTimeout = READ_TIMEOUT_MS
- request.getHeaders().putAll(headers)
+
+ // GenericData::putAll method converts all known http headers to List<String>
+ // and lowercase all other headers
+ def ci = request.getHeaders().getClassInfo()
+ request.getHeaders().putAll(headers.collectEntries { name, value
+ -> [(name) : (ci.getFieldInfo(name) != null ? [value] : value.toLowerCase())]})
+
request.setThrowExceptionOnExecuteError(throwExceptionOnError)
HttpResponse response = executeRequest(request)

MODIFY
testing/src/main/groovy/io/opentelemetry/auto/test/base/HttpClientTest.groovy
testing/src/main/groovy/io/opentelemetry/auto/test/base/HttpClientTest.groovy
@@ -44,6 +44,8 @@ abstract class HttpClientTest extends AgentTestRunner {
protected static final BODY_METHODS = ["POST", "PUT"]
protected static final CONNECT_TIMEOUT_MS = 1000
protected static final READ_TIMEOUT_MS = 2000
+ protected static final BASIC_AUTH_KEY = "custom authorization header"
+ protected static final BASIC_AUTH_VAL = "plain text auth token"
@AutoCleanup
@Shared
@@ -71,6 +73,18 @@ abstract class HttpClientTest extends AgentTestRunner {
handleDistributedRequest()
redirect(server.address.resolve("/circular-redirect").toURL().toString())
}
+ prefix("secured") {
+ handleDistributedRequest()
+ if (request.headers.get(BASIC_AUTH_KEY) == BASIC_AUTH_VAL) {
+ response.status(200).send("secured string under basic auth")
+ } else {
+ response.status(401).send("Unauthorized")
+ }
+ }
+ prefix("to-secured") {
+ handleDistributedRequest()
+ redirect(server.address.resolve("/secured").toURL().toString())
+ }
}
}
@@ -281,6 +295,28 @@ abstract class HttpClientTest extends AgentTestRunner {
method = "GET"
}
+ def "redirect #method to secured endpoint copies auth header"() {
+ given:
+ assumeTrue(testRedirects())
+ def uri = server.address.resolve("/to-secured")
+
+ when:
+ def status = doRequest(method, uri, [(BASIC_AUTH_KEY): BASIC_AUTH_VAL])
+
+ then:
+ status == 200
+ assertTraces(1) {
+ trace(0, 3 + extraClientSpans()) {
+ clientSpan(it, 0, null, method, false, uri)
+ serverSpan(it, 1 + extraClientSpans(), span(extraClientSpans()))
+ serverSpan(it, 2 + extraClientSpans(), span(extraClientSpans()))
+ }
+ }
+
+ where:
+ method = "GET"
+ }
+
def "connection error (unopened port)"() {
given:
assumeTrue(testConnectionFailure())

ApacheHttpClientRedirectInstrumentation copy headers from original request to redirect if original redirect headers were empty (DataDog/dd-trace-java#1539)