Simplify Apache HttpAsyncClient instrumentation (#1894)

* Simplify Apache HttpAsyncClient instrumentation

* Bump baseline version to 4.1

* Remove all the Intellij auto-formatting of README
This commit is contained in:
Trask Stalnaker 2020-12-13 22:16:06 -08:00 committed by GitHub
parent c741eaa501
commit 8bcd5f1229
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 25 additions and 106 deletions

View File

@ -214,7 +214,7 @@ Because the automatic instrumentation runs in a different classpath than the ins
| Library/Framework | Versions |
|---------------------------------------------------------------------------------------------------------------------------------------|--------------------------------|
| [Akka HTTP](https://doc.akka.io/docs/akka-http/current/index.html) | 10.0+ |
| [Apache HttpAsyncClient](https://hc.apache.org/index.html) | 4.0+ |
| [Apache HttpAsyncClient](https://hc.apache.org/index.html) | 4.1+ |
| [Apache HttpClient](https://hc.apache.org/index.html) | 2.0+ |
| [Armeria](https://armeria.dev) | 0.99.8+ |
| [AsyncHttpClient](https://github.com/AsyncHttpClient/async-http-client) | 1.9+ (not including 2.x yet) |

View File

@ -1,14 +0,0 @@
apply from: "$rootDir/gradle/instrumentation.gradle"
muzzle {
pass {
group = "org.apache.httpcomponents"
module = "httpasyncclient"
versions = "[4.0,)"
assertInverse = true
}
}
dependencies {
library group: 'org.apache.httpcomponents', name: 'httpasyncclient', version: '4.0'
}

View File

@ -1,82 +0,0 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.apachehttpasyncclient;
import static io.opentelemetry.javaagent.tooling.ClassLoaderMatcher.hasClassesNamed;
import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.implementsInterface;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
import java.util.Map;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.implementation.bytecode.assign.Assigner;
import net.bytebuddy.matcher.ElementMatcher;
import org.apache.http.Header;
import org.apache.http.HttpRequest;
/**
* Early versions don't copy headers over on redirect. This instrumentation copies our headers over
* manually. Inspired by
* https://github.com/elastic/apm-agent-java/blob/master/apm-agent-plugins/apm-apache-httpclient-plugin/src/main/java/co/elastic/apm/agent/httpclient/ApacheHttpAsyncClientRedirectInstrumentation.java
*/
public class ApacheHttpClientRedirectInstrumentation implements TypeInstrumentation {
@Override
public ElementMatcher<ClassLoader> classLoaderOptimization() {
return hasClassesNamed("org.apache.http.client.RedirectStrategy");
}
@Override
public ElementMatcher<TypeDescription> typeMatcher() {
return implementsInterface(named("org.apache.http.client.RedirectStrategy"));
}
@Override
public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
return singletonMap(
isMethod()
.and(named("getRedirect"))
.and(takesArgument(0, named("org.apache.http.HttpRequest"))),
ApacheHttpClientRedirectInstrumentation.class.getName() + "$ClientRedirectAdvice");
}
public static class ClientRedirectAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
private static void onAfterExecute(
@Advice.Argument(0) HttpRequest original,
@Advice.Return(typing = Assigner.Typing.DYNAMIC) HttpRequest redirect) {
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 (Header header : original.getAllHeaders()) {
String name = header.getName().toLowerCase();
if (name.equals("traceparent") || name.equals("tracestate")) {
if (!redirect.containsHeader(header.getName())) {
redirect.setHeader(header.getName(), header.getValue());
}
}
}
}
}
}
}

View File

@ -0,0 +1,16 @@
apply from: "$rootDir/gradle/instrumentation.gradle"
muzzle {
pass {
group = "org.apache.httpcomponents"
module = "httpasyncclient"
// 4.0 and 4.0.1 don't copy over the traceparent (etc) http headers on redirect
versions = "[4.1,)"
// TODO implement a muzzle check so that 4.0.x (at least 4.0 and 4.0.1) do not get applied
// and then bring back assertInverse
}
}
dependencies {
library group: 'org.apache.httpcomponents', name: 'httpasyncclient', version: '4.1'
}

View File

@ -15,12 +15,11 @@ import java.util.List;
@AutoService(InstrumentationModule.class)
public class ApacheHttpAsyncClientInstrumentationModule extends InstrumentationModule {
public ApacheHttpAsyncClientInstrumentationModule() {
super("apache-httpasyncclient", "apache-httpasyncclient-4.0");
super("apache-httpasyncclient", "apache-httpasyncclient-4.1");
}
@Override
public List<TypeInstrumentation> typeInstrumentations() {
return asList(
new ApacheHttpAsyncClientInstrumentation(), new ApacheHttpClientRedirectInstrumentation());
return asList(new ApacheHttpAsyncClientInstrumentation());
}
}

View File

@ -26,7 +26,7 @@ dependencies {
implementation project(':instrumentation:elasticsearch:elasticsearch-rest-common:javaagent')
testImplementation project(':instrumentation:apache-httpclient:apache-httpclient-4.0:javaagent')
testImplementation project(':instrumentation:apache-httpasyncclient-4.0:javaagent')
testImplementation project(':instrumentation:apache-httpasyncclient-4.1:javaagent')
testImplementation group: 'org.apache.logging.log4j', name: 'log4j-core', version: '2.11.0'
testImplementation group: 'org.apache.logging.log4j', name: 'log4j-api', version: '2.11.0'

View File

@ -26,7 +26,7 @@ dependencies {
implementation project(':instrumentation:elasticsearch:elasticsearch-rest-common:javaagent')
testImplementation project(':instrumentation:apache-httpclient:apache-httpclient-4.0:javaagent')
testImplementation project(':instrumentation:apache-httpasyncclient-4.0:javaagent')
testImplementation project(':instrumentation:apache-httpasyncclient-4.1:javaagent')
//TODO: review the following claim, we are not using embedded ES anymore
// Netty is used, but it adds complexity to the tests since we're using embedded ES.
//testImplementation project(':instrumentation:netty:netty-4.1:javaagent')

View File

@ -22,7 +22,7 @@ dependencies {
// Ensure no cross interference
testImplementation project(':instrumentation:elasticsearch:elasticsearch-rest-5.0:javaagent')
testImplementation project(':instrumentation:apache-httpasyncclient-4.0:javaagent')
testImplementation project(':instrumentation:apache-httpasyncclient-4.1:javaagent')
testImplementation project(':instrumentation:netty:netty-4.1:javaagent')
testImplementation group: 'org.apache.logging.log4j', name: 'log4j-core', version: '2.11.0'

View File

@ -20,7 +20,7 @@ dependencies {
implementation project(':instrumentation:elasticsearch:elasticsearch-transport-common:javaagent')
testImplementation project(':instrumentation:apache-httpasyncclient-4.0:javaagent')
testImplementation project(':instrumentation:apache-httpasyncclient-4.1:javaagent')
testImplementation project(':instrumentation:netty:netty-4.1:javaagent')
testImplementation project(':instrumentation:spring:spring-data-1.8:javaagent')

View File

@ -30,7 +30,7 @@ dependencies {
// Ensure no cross interference
testImplementation project(':instrumentation:elasticsearch:elasticsearch-rest-5.0:javaagent')
testImplementation project(':instrumentation:apache-httpasyncclient-4.0:javaagent')
testImplementation project(':instrumentation:apache-httpasyncclient-4.1:javaagent')
testImplementation project(':instrumentation:netty:netty-4.1:javaagent')
testLibrary group: 'org.elasticsearch.plugin', name: 'transport-netty4-client', version: '6.0.0'

View File

@ -56,7 +56,7 @@ include ':smoke-tests'
include ':instrumentation:akka-actor-2.5:javaagent'
include ':instrumentation:akka-http-10.0:javaagent'
include ':instrumentation:apache-camel-2.20:javaagent'
include ':instrumentation:apache-httpasyncclient-4.0:javaagent'
include ':instrumentation:apache-httpasyncclient-4.1:javaagent'
include ':instrumentation:apache-httpclient:apache-httpclient-2.0:javaagent'
include ':instrumentation:apache-httpclient:apache-httpclient-4.0:javaagent'
include ':instrumentation:armeria-1.0:javaagent'