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

Allow multivalue multiline header extraction #8112

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ abstract class AkkaHttpClientInstrumentationTest extends HttpClientTest {
abstract CompletionStage<HttpResponse> doRequest(HttpRequest request)

@Override
int doRequest(String method, URI uri, Map<String, String> headers, String body, Closure callback) {
int doRequest(String method, URI uri, List<List<String>> headers, String body, Closure callback) {
def request = HttpRequest.create(uri.toString())
.withMethod(HttpMethods.lookup(method).get())
.addHeaders(headers.collect { RawHeader.create(it.key, it.value) })
.addHeaders(headers.collect { RawHeader.create(it[0], it[1]) })

def response
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ abstract class AkkaHttpClientInstrumentationTest extends HttpClientTest {
abstract CompletionStage<HttpResponse> doRequest(HttpRequest request)

@Override
int doRequest(String method, URI uri, Map<String, String> headers, String body, Closure callback) {
int doRequest(String method, URI uri, List<List<String>> headers, String body, Closure callback) {
def request = HttpRequest.create(uri.toString())
.withMethod(HttpMethods.lookup(method).get())
.addHeaders(headers.collect { RawHeader.create(it.key, it.value) })
.addHeaders(headers.collect { RawHeader.create(it[0], it[1]) })

def response
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,16 @@ protected int status(final HttpContext context) {

@Override
protected String getRequestHeader(HttpUriRequest request, String headerName) {
Header header = request.getFirstHeader(headerName);
if (header != null) {
return header.getValue();
Header[] headers = request.getHeaders(headerName);
if (headers.length > 0) {
StringBuilder result = new StringBuilder();
for (int i = 0; i < headers.length; i++) {
result.append(headers[i].getValue());
if (i + 1 < headers.length) {
result.append(",");
}
}
return result.toString();
}
return null;
}
Expand All @@ -71,9 +78,16 @@ protected String getRequestHeader(HttpUriRequest request, String headerName) {
protected String getResponseHeader(HttpContext context, String headerName) {
final Object responseObject = context.getAttribute(HttpCoreContext.HTTP_RESPONSE);
if (responseObject instanceof HttpResponse) {
Header header = ((HttpResponse) responseObject).getFirstHeader(headerName);
if (header != null) {
return header.getValue();
Header[] headers = ((HttpResponse) responseObject).getHeaders(headerName);
if (headers.length > 0) {
StringBuilder result = new StringBuilder();
for (int i = 0; i < headers.length; i++) {
result.append(headers[i].getValue());
if (i + 1 < headers.length) {
result.append(",");
}
}
return result.toString();
}
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ public Header getFirstHeader(String name) {
return actualRequest.getFirstHeader(name);
}

@Override
public Header[] getHeaders(String name) {
return actualRequest.getHeaders(name);
}

public HttpRequest getActualRequest() {
return actualRequest;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import datadog.trace.agent.test.base.HttpClientTest
import datadog.trace.agent.test.base.HttpClientTest2
import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions
import datadog.trace.instrumentation.apachehttpasyncclient.ApacheHttpAsyncClientDecorator
import org.apache.http.HttpResponse
Expand All @@ -14,7 +14,7 @@ import java.util.concurrent.CompletableFuture
import java.util.concurrent.TimeUnit

@Timeout(5)
class ApacheHttpAsyncClientCallbackTest extends HttpClientTest implements TestingGenericHttpNamingConventions.ClientV0 {
class ApacheHttpAsyncClientCallbackTest extends HttpClientTest2 implements TestingGenericHttpNamingConventions.ClientV0 {

@Shared
RequestConfig requestConfig = RequestConfig.custom()
Expand All @@ -31,11 +31,9 @@ class ApacheHttpAsyncClientCallbackTest extends HttpClientTest implements Testin
}

@Override
int doRequest(String method, URI uri, Map<String, String> headers, String body, Closure callback) {
int doRequest(String method, URI uri, List<List<String>> headers, String body, Closure callback) {
def request = new HttpUriRequest(method, uri)
headers.entrySet().each {
request.addHeader(new BasicHeader(it.key, it.value))
}
headers.each { request.addHeader(new BasicHeader(it[0], it[1])) }

def responseFuture = new CompletableFuture<>()

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import datadog.trace.agent.test.base.HttpClientTest
import datadog.trace.agent.test.base.HttpClientTest2
import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions
import datadog.trace.instrumentation.apachehttpasyncclient.ApacheHttpAsyncClientDecorator
import org.apache.http.client.config.RequestConfig
Expand All @@ -11,7 +11,7 @@ import spock.lang.Timeout
import java.util.concurrent.Future

@Timeout(5)
class ApacheHttpAsyncClientNullCallbackTest extends HttpClientTest implements TestingGenericHttpNamingConventions.ClientV0{
class ApacheHttpAsyncClientNullCallbackTest extends HttpClientTest2 implements TestingGenericHttpNamingConventions.ClientV0{

@Shared
RequestConfig requestConfig = RequestConfig.custom()
Expand All @@ -28,11 +28,9 @@ class ApacheHttpAsyncClientNullCallbackTest extends HttpClientTest implements Te
}

@Override
int doRequest(String method, URI uri, Map<String, String> headers, String body, Closure callback) {
int doRequest(String method, URI uri, List<List<String>> headers, String body, Closure callback) {
def request = new HttpUriRequest(method, uri)
headers.entrySet().each {
request.addHeader(new BasicHeader(it.key, it.value))
}
headers.each { request.addHeader(new BasicHeader(it[0], it[1])) }

// The point here is to test case when callback is null - fire-and-forget style
// So to make sure request is done we start request, wait for future to finish
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import datadog.trace.agent.test.asserts.TraceAssert
import datadog.trace.agent.test.base.HttpClientTest
import datadog.trace.agent.test.base.HttpClientTest2
import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions
import datadog.trace.instrumentation.apachehttpasyncclient.ApacheHttpAsyncClientDecorator
import org.apache.http.HttpHost
Expand All @@ -16,7 +16,7 @@ import spock.lang.Timeout
import java.util.concurrent.CountDownLatch

@Timeout(5)
abstract class ApacheHttpAsyncClientTest extends HttpClientTest {
abstract class ApacheHttpAsyncClientTest extends HttpClientTest2 {

@Shared
RequestConfig requestConfig = RequestConfig.custom()
Expand All @@ -41,11 +41,9 @@ abstract class ApacheHttpAsyncClientTest extends HttpClientTest {
}

@Override
int doRequest(String method, URI uri, Map<String, String> headers, String body, Closure callback) {
int doRequest(String method, URI uri, List<List<String>> headers, String body, Closure callback) {
def request = createRequest(method, uri)
headers.entrySet().each {
request.addHeader(new BasicHeader(it.key, it.value))
}
headers.each { request.addHeader(new BasicHeader(it[0], it[1])) }

def latch = new CountDownLatch(callback == null ? 0 : 1)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,32 @@ protected int status(final HttpResponse httpResponse) {

@Override
protected String getRequestHeader(HttpUriRequest request, String headerName) {
Header header = request.getFirstHeader(headerName);
if (null != header) {
return header.getValue();
Header[] headers = request.getHeaders(headerName);
if (headers.length > 0) {
StringBuilder result = new StringBuilder();
for (int i = 0; i < headers.length; i++) {
result.append(headers[i].getValue());
if (i + 1 < headers.length) {
result.append(",");
}
}
return result.toString();
}
return null;
}

@Override
protected String getResponseHeader(HttpResponse response, String headerName) {
Header header = response.getFirstHeader(headerName);
if (null != header) {
return header.getValue();
Header[] headers = response.getHeaders(headerName);
if (headers.length > 0) {
StringBuilder result = new StringBuilder();
for (int i = 0; i < headers.length; i++) {
result.append(headers[i].getValue());
if (i + 1 < headers.length) {
result.append(",");
}
}
return result.toString();
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ public Header getFirstHeader(String name) {
return actualRequest.getFirstHeader(name);
}

@Override
public Header[] getHeaders(String name) {
return actualRequest.getHeaders(name);
}

public HttpRequest getActualRequest() {
return actualRequest;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import datadog.trace.agent.test.base.HttpClientTest
import datadog.trace.agent.test.base.HttpClientTest2
import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions
import datadog.trace.instrumentation.apachehttpclient.ApacheHttpClientDecorator
import org.apache.http.HttpResponse
Expand All @@ -11,7 +11,7 @@ import spock.lang.Shared
import spock.lang.Timeout

@Timeout(5)
class ApacheHttpClientResponseHandlerTest extends HttpClientTest implements TestingGenericHttpNamingConventions.ClientV0 {
class ApacheHttpClientResponseHandlerTest extends HttpClientTest2 implements TestingGenericHttpNamingConventions.ClientV0 {

@Shared
def client = new DefaultHttpClient()
Expand All @@ -31,11 +31,9 @@ class ApacheHttpClientResponseHandlerTest extends HttpClientTest implements Test
}

@Override
int doRequest(String method, URI uri, Map<String, String> headers, String body, Closure callback) {
int doRequest(String method, URI uri, List<List<String>> headers, String body, Closure callback) {
def request = new HttpUriRequest(method, uri)
headers.entrySet().each {
request.addHeader(new BasicHeader(it.key, it.value))
}
headers.each { request.addHeader(new BasicHeader(it[0], it[1])) }

def status = client.execute(request, handler)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import datadog.trace.agent.test.base.HttpClientTest
import datadog.trace.agent.test.base.HttpClientTest2
import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions
import datadog.trace.instrumentation.apachehttpclient.ApacheHttpClientDecorator
import org.apache.http.HttpHost
Expand All @@ -13,7 +13,7 @@ import org.apache.http.protocol.BasicHttpContext
import spock.lang.Shared
import spock.lang.Timeout

abstract class ApacheHttpClientTest<T extends HttpRequest> extends HttpClientTest implements TestingGenericHttpNamingConventions.ClientV0 {
abstract class ApacheHttpClientTest<T extends HttpRequest> extends HttpClientTest2 implements TestingGenericHttpNamingConventions.ClientV0 {
@Shared
def client = new DefaultHttpClient()

Expand All @@ -29,11 +29,9 @@ abstract class ApacheHttpClientTest<T extends HttpRequest> extends HttpClientTes
}

@Override
int doRequest(String method, URI uri, Map<String, String> headers, String body, Closure callback) {
int doRequest(String method, URI uri, List<List<String>> headers, String body, Closure callback) {
def request = createRequest(method, uri)
headers.entrySet().each {
request.addHeader(new BasicHeader(it.key, it.value))
}
headers.each { request.addHeader(new BasicHeader(it[0], it[1])) }

def response = executeRequest(request, uri)
callback?.call()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,32 @@ protected int status(final HttpResponse httpResponse) {

@Override
protected String getRequestHeader(HttpRequest request, String headerName) {
Header header = request.getFirstHeader(headerName);
if (null != header) {
return header.getValue();
Header[] headers = request.getHeaders(headerName);
if (headers.length > 0) {
StringBuilder result = new StringBuilder();
for (int i = 0; i < headers.length; i++) {
result.append(headers[i].getValue());
if (i + 1 < headers.length) {
result.append(",");
}
}
return result.toString();
}
return null;
}

@Override
protected String getResponseHeader(HttpResponse response, String headerName) {
Header header = response.getFirstHeader(headerName);
if (null != header) {
return header.getValue();
Header[] headers = response.getHeaders(headerName);
if (headers.length > 0) {
StringBuilder result = new StringBuilder();
for (int i = 0; i < headers.length; i++) {
result.append(headers[i].getValue());
if (i + 1 < headers.length) {
result.append(",");
}
}
return result.toString();
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,9 @@ public void setHeader(String name, Object value) {
public Header getFirstHeader(String name) {
return actualRequest.getFirstHeader(name);
}

@Override
public Header[] getHeaders(String name) {
return actualRequest.getHeaders(name);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import datadog.trace.agent.test.base.HttpClientTest
import datadog.trace.agent.test.base.HttpClientTest2
import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions
import datadog.trace.instrumentation.apachehttpclient5.ApacheHttpClientDecorator
import org.apache.hc.client5.http.async.methods.SimpleHttpRequests
Expand All @@ -12,7 +12,7 @@ import java.util.concurrent.TimeUnit

import static org.apache.hc.core5.reactor.IOReactorConfig.custom

abstract class ApacheHttpAsyncClient5Test<T extends HttpRequest> extends HttpClientTest {
abstract class ApacheHttpAsyncClient5Test<T extends HttpRequest> extends HttpClientTest2 {

@Shared
def ioReactorConfig = custom()
Expand Down Expand Up @@ -45,10 +45,10 @@ abstract class ApacheHttpAsyncClient5Test<T extends HttpRequest> extends HttpCli
}

@Override
int doRequest(String method, URI uri, Map<String, String> headers, String body, Closure callback) {
int doRequest(String method, URI uri, List<List<String>> headers, String body, Closure callback) {
def request = SimpleHttpRequests.create(method, uri)
request.setConfig(RequestConfig.custom().setConnectTimeout(CONNECT_TIMEOUT_MS, TimeUnit.MILLISECONDS).build())
headers.each { request.addHeader(it.key, it.value) }
headers.each { request.addHeader(it[0], it[1]) }

def future = client.execute(request, null)
def response = future.get(READ_TIMEOUT_MS, TimeUnit.MILLISECONDS)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import datadog.trace.agent.test.base.HttpClientTest
import datadog.trace.agent.test.base.HttpClientTest2
import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions
import datadog.trace.instrumentation.apachehttpclient5.ApacheHttpClientDecorator
import org.apache.hc.client5.http.config.RequestConfig
Expand All @@ -15,7 +15,7 @@ import spock.lang.Timeout
import java.util.concurrent.TimeUnit

@Timeout(5)
class ApacheHttpClientResponseHandlerTest extends HttpClientTest implements TestingGenericHttpNamingConventions.ClientV0 {
class ApacheHttpClientResponseHandlerTest extends HttpClientTest2 implements TestingGenericHttpNamingConventions.ClientV0 {

@Shared
def client = HttpClients.custom()
Expand All @@ -38,11 +38,9 @@ class ApacheHttpClientResponseHandlerTest extends HttpClientTest implements Test
}

@Override
int doRequest(String method, URI uri, Map<String, String> headers, String body, Closure callback) {
int doRequest(String method, URI uri, List<List<String>> headers, String body, Closure callback) {
def request = new BasicClassicHttpRequest(method, uri)
headers.entrySet().each {
request.addHeader(new BasicHeader(it.key, it.value))
}
headers.each { request.addHeader(new BasicHeader(it[0], it[1])) }

CloseableHttpResponse response = null
def status = client.execute(request, handler)
Expand Down
Loading
Loading