Migrate OkHttp Instrumentation to Decorators
MODIFY
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/BaseDecorator.java
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/BaseDecorator.java
@@ -96,7 +96,14 @@ public abstract class BaseDecorator {
span.setTag(Tags.PEER_HOSTNAME.getKey(), remoteConnection.getHostName());
span.setTag(Tags.PEER_PORT.getKey(), remoteConnection.getPort());
- final InetAddress remoteAddress = remoteConnection.getAddress();
+ onPeerConnection(span, remoteConnection.getAddress());
+ }
+ return span;
+ }
+
+ public Span onPeerConnection(final Span span, final InetAddress remoteAddress) {
+ assert span != null;
+ if (remoteAddress != null) {
if (remoteAddress instanceof Inet4Address) {
Tags.PEER_HOST_IPV4.set(span, remoteAddress.getHostAddress());
} else if (remoteAddress instanceof Inet6Address) {

MODIFY
dd-java-agent/dd-java-agent.gradle
dd-java-agent/dd-java-agent.gradle
@@ -104,9 +104,6 @@ dependencies {
testCompile group: 'org.mongodb', name: 'mongodb-driver-async', version: '3.4.2'
// run embeded mongodb for integration testing
testCompile group: 'de.flapdoodle.embed', name: 'de.flapdoodle.embed.mongo', version: '1.50.5'
-
- testCompile group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.3'
- testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0'
}
tasks.withType(Test).configureEach {

MODIFY
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/OkHttp3Instrumentation.java
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/OkHttp3Instrumentation.java
package datadog.trace.instrumentation.okhttp3;
-import static datadog.trace.instrumentation.okhttp3.OkHttpClientSpanDecorator.STANDARD_TAGS;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -8,8 +7,6 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.Instrumenter;
-import io.opentracing.util.GlobalTracer;
-import java.util.Collections;
import java.util.Map;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.method.MethodDescription;
@@ -33,18 +30,17 @@ public class OkHttp3Instrumentation extends Instrumenter.Default {
@Override
public String[] helperClassNames() {
return new String[] {
- "datadog.trace.instrumentation.okhttp3.OkHttpClientSpanDecorator",
- "datadog.trace.instrumentation.okhttp3.OkHttpClientSpanDecorator$1",
- "datadog.trace.instrumentation.okhttp3.RequestBuilderInjectAdapter",
- "datadog.trace.instrumentation.okhttp3.TagWrapper",
- "datadog.trace.instrumentation.okhttp3.TracedCallable",
- "datadog.trace.instrumentation.okhttp3.TracedExecutor",
- "datadog.trace.instrumentation.okhttp3.TracedExecutorService",
- "datadog.trace.instrumentation.okhttp3.TracedRunnable",
- "datadog.trace.instrumentation.okhttp3.TracingInterceptor",
- "datadog.trace.instrumentation.okhttp3.TracingCallFactory",
- "datadog.trace.instrumentation.okhttp3.TracingCallFactory$NetworkInterceptor",
- "datadog.trace.instrumentation.okhttp3.TracingCallFactory$1"
+ "datadog.trace.agent.decorator.BaseDecorator",
+ "datadog.trace.agent.decorator.ClientDecorator",
+ "datadog.trace.agent.decorator.HttpClientDecorator",
+ packageName + ".OkHttpClientDecorator",
+ packageName + ".OkHttpClientDecorator$1",
+ packageName + ".RequestBuilderInjectAdapter",
+ packageName + ".TagWrapper",
+ packageName + ".TracingInterceptor",
+ packageName + ".TracingCallFactory",
+ packageName + ".TracingCallFactory$NetworkInterceptor",
+ packageName + ".TracingCallFactory$1",
};
}
@@ -64,8 +60,7 @@ public class OkHttp3Instrumentation extends Instrumenter.Default {
return;
}
}
- final TracingInterceptor interceptor =
- new TracingInterceptor(GlobalTracer.get(), Collections.singletonList(STANDARD_TAGS));
+ final TracingInterceptor interceptor = new TracingInterceptor();
builder.addInterceptor(interceptor);
builder.addNetworkInterceptor(interceptor);
}

ADD
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/OkHttpClientDecorator.java
+package datadog.trace.instrumentation.okhttp3;
+
+import datadog.trace.agent.decorator.HttpClientDecorator;
+import okhttp3.Request;
+import okhttp3.Response;
+
+public class OkHttpClientDecorator extends HttpClientDecorator<Request, Response> {
+ public static final OkHttpClientDecorator DECORATE = new OkHttpClientDecorator();
+ public static final OkHttpClientDecorator NETWORK_DECORATE =
+ new OkHttpClientDecorator() {
+ @Override
+ protected String component() {
+ return "okhttp-network";
+ }
+ };
+
+ @Override
+ protected String[] instrumentationNames() {
+ return new String[] {"okhttp", "okhttp-3"};
+ }
+
+ @Override
+ protected String service() {
+ return "okhttp";
+ }
+
+ @Override
+ protected String component() {
+ return "okhttp";
+ }
+
+ @Override
+ protected String method(final Request httpRequest) {
+ return httpRequest.method();
+ }
+
+ @Override
+ protected String url(final Request httpRequest) {
+ return httpRequest.url().toString();
+ }
+
+ @Override
+ protected String hostname(final Request httpRequest) {
+ return httpRequest.url().host();
+ }
+
+ @Override
+ protected Integer port(final Request httpRequest) {
+ return httpRequest.url().port();
+ }
+
+ @Override
+ protected Integer status(final Response httpResponse) {
+ return httpResponse.code();
+ }
+}

DELETE
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/OkHttpClientSpanDecorator.java
-package datadog.trace.instrumentation.okhttp3;
-
-import static io.opentracing.log.Fields.ERROR_OBJECT;
-
-import datadog.trace.api.Config;
-import datadog.trace.api.DDTags;
-import io.opentracing.Span;
-import io.opentracing.tag.Tags;
-import java.net.Inet6Address;
-import java.net.InetAddress;
-import java.util.Collections;
-import okhttp3.Connection;
-import okhttp3.Request;
-import okhttp3.Response;
-
-/**
- * Span decorator to add tags, logs and operation name.
- *
- * @author Pavol Loffay
- */
-public interface OkHttpClientSpanDecorator {
-
- /**
- * Decorate span before a request is made.
- *
- * @param request request
- * @param span span
- */
- void onRequest(Request request, Span span);
-
- /**
- * Decorate span on an error e.g. {@link java.net.UnknownHostException} or any exception in
- * interceptor.
- *
- * @param throwable exception
- * @param span span
- */
- void onError(Throwable throwable, Span span);
-
- /**
- * This is invoked after {@link okhttp3.Interceptor.Chain#proceed(Request)} in network
- * interceptor. In this method it is possible to capture server address, log redirects etc.
- *
- * @param connection connection
- * @param response response
- * @param span span
- */
- void onResponse(Connection connection, Response response, Span span);
-
- /**
- * Decorator which adds standard HTTP and peer tags to the span.
- *
- * <p>
- *
- * <p>On error it adds {@link Tags#ERROR} with log representing exception and on redirects adds
- * log entries with peer tags.
- */
- OkHttpClientSpanDecorator STANDARD_TAGS =
- new OkHttpClientSpanDecorator() {
- @Override
- public void onRequest(final Request request, final Span span) {
- Tags.COMPONENT.set(span, TracingCallFactory.COMPONENT_NAME);
- Tags.HTTP_METHOD.set(span, request.method());
- Tags.HTTP_URL.set(span, request.url().toString());
- if (Config.get().isHttpClientSplitByDomain()) {
- span.setTag(DDTags.SERVICE_NAME, request.url().host());
- }
- }
-
- @Override
- public void onError(final Throwable throwable, final Span span) {
- Tags.ERROR.set(span, Boolean.TRUE);
- span.log(Collections.singletonMap(ERROR_OBJECT, throwable));
- }
-
- @Override
- public void onResponse(
- final Connection connection, final Response response, final Span span) {
- final InetAddress inetAddress = connection.socket().getInetAddress();
-
- Tags.HTTP_STATUS.set(span, response.code());
- Tags.PEER_HOSTNAME.set(span, inetAddress.getHostName());
- Tags.PEER_PORT.set(span, connection.socket().getPort());
-
- String ipvKey = Tags.PEER_HOST_IPV4.getKey();
- if (inetAddress instanceof Inet6Address) {
- ipvKey = Tags.PEER_HOST_IPV6.getKey();
- }
- span.setTag(ipvKey, inetAddress.getHostAddress());
- }
- };
-}

DELETE
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/TracedCallable.java
-package datadog.trace.instrumentation.okhttp3;
-
-import io.opentracing.Scope;
-import io.opentracing.Span;
-import io.opentracing.Tracer;
-import java.util.concurrent.Callable;
-
-/** @author Pavol Loffay */
-public class TracedCallable<V> implements Callable<V> {
-
- private final Callable<V> delegate;
- private final Span span;
- private final Tracer tracer;
-
- public TracedCallable(final Callable<V> delegate, final Tracer tracer) {
- this.delegate = delegate;
- this.tracer = tracer;
- this.span = tracer.activeSpan();
- }
-
- @Override
- public V call() throws Exception {
- final Scope scope = span == null ? null : tracer.scopeManager().activate(span, false);
- try {
- return delegate.call();
- } finally {
- if (scope != null) {
- scope.close();
- }
- }
- }
-}

DELETE
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/TracedExecutor.java
-package datadog.trace.instrumentation.okhttp3;
-
-import io.opentracing.Tracer;
-import java.util.concurrent.Executor;
-
-/**
- * Executor which propagates span from parent thread to submitted {@link Runnable}.
- *
- * @author Pavol Loffay
- */
-public class TracedExecutor implements Executor {
-
- protected final Tracer tracer;
- private final Executor delegate;
-
- public TracedExecutor(final Executor executor, final Tracer tracer) {
- this.delegate = executor;
- this.tracer = tracer;
- }
-
- @Override
- public void execute(final Runnable runnable) {
- delegate.execute(tracer.activeSpan() == null ? runnable : new TracedRunnable(runnable, tracer));
- }
-}

DELETE
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/TracedExecutorService.java
-package datadog.trace.instrumentation.okhttp3;
-
-import io.opentracing.Tracer;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.List;
-import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Future;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
-
-/** @author Pavol Loffay */
-public class TracedExecutorService extends TracedExecutor implements ExecutorService {
-
- private final ExecutorService delegate;
-
- public TracedExecutorService(final ExecutorService delegate, final Tracer tracer) {
- super(delegate, tracer);
- this.delegate = delegate;
- }
-
- @Override
- public void shutdown() {
- delegate.shutdown();
- }
-
- @Override
- public List<Runnable> shutdownNow() {
- return delegate.shutdownNow();
- }
-
- @Override
- public boolean isShutdown() {
- return delegate.isShutdown();
- }
-
- @Override
- public boolean isTerminated() {
- return delegate.isTerminated();
- }
-
- @Override
- public boolean awaitTermination(final long l, final TimeUnit timeUnit)
- throws InterruptedException {
- return delegate.awaitTermination(l, timeUnit);
- }
-
- @Override
- public <T> Future<T> submit(final Callable<T> callable) {
- return delegate.submit(
- tracer.activeSpan() == null ? callable : new TracedCallable<T>(callable, tracer));
- }
-
- @Override
- public <T> Future<T> submit(final Runnable runnable, final T t) {
- return delegate.submit(
- tracer.activeSpan() == null ? runnable : new TracedRunnable(runnable, tracer), t);
- }
-
- @Override
- public Future<?> submit(final Runnable runnable) {
- return delegate.submit(
- tracer.activeSpan() == null ? runnable : new TracedRunnable(runnable, tracer));
- }
-
- @Override
- public <T> List<Future<T>> invokeAll(final Collection<? extends Callable<T>> collection)
- throws InterruptedException {
- return delegate.invokeAll(toTraced(collection));
- }
-
- @Override
- public <T> List<Future<T>> invokeAll(
- final Collection<? extends Callable<T>> collection, final long l, final TimeUnit timeUnit)
- throws InterruptedException {
- return delegate.invokeAll(toTraced(collection), l, timeUnit);
- }
-
- @Override
- public <T> T invokeAny(final Collection<? extends Callable<T>> collection)
- throws InterruptedException, ExecutionException {
- return delegate.invokeAny(toTraced(collection));
- }
-
- @Override
- public <T> T invokeAny(
- final Collection<? extends Callable<T>> collection, final long l, final TimeUnit timeUnit)
- throws InterruptedException, ExecutionException, TimeoutException {
- return delegate.invokeAny(toTraced(collection), l, timeUnit);
- }
-
- private <T> Collection<? extends Callable<T>> toTraced(
- final Collection<? extends Callable<T>> delegate) {
- final List<Callable<T>> tracedCallables = new ArrayList<>(delegate.size());
-
- for (final Callable<T> callable : delegate) {
- tracedCallables.add(
- tracer.activeSpan() == null ? callable : new TracedCallable<T>(callable, tracer));
- }
-
- return tracedCallables;
- }
-}

DELETE
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/TracedRunnable.java
-package datadog.trace.instrumentation.okhttp3;
-
-import io.opentracing.Scope;
-import io.opentracing.Span;
-import io.opentracing.Tracer;
-
-/** @author Pavol Loffay */
-public class TracedRunnable implements Runnable {
-
- private final Runnable delegate;
- private final Span span;
- private final Tracer tracer;
-
- public TracedRunnable(final Runnable delegate, final Tracer tracer) {
- this.delegate = delegate;
- this.tracer = tracer;
- this.span = tracer.activeSpan();
- }
-
- @Override
- public void run() {
- final Scope scope = span == null ? null : tracer.scopeManager().activate(span, false);
- try {
- delegate.run();
- } finally {
- if (scope != null) {
- scope.close();
- }
- }
- }
-}

MODIFY
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/TracingCallFactory.java
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/TracingCallFactory.java
package datadog.trace.instrumentation.okhttp3;
-import datadog.trace.api.Config;
-import datadog.trace.api.DDSpanTypes;
-import datadog.trace.api.DDTags;
+import static datadog.trace.instrumentation.okhttp3.OkHttpClientDecorator.DECORATE;
+import static datadog.trace.instrumentation.okhttp3.OkHttpClientDecorator.NETWORK_DECORATE;
+
import io.opentracing.Scope;
+import io.opentracing.Span;
import io.opentracing.SpanContext;
-import io.opentracing.Tracer;
import io.opentracing.propagation.Format;
-import io.opentracing.tag.Tags;
+import io.opentracing.util.GlobalTracer;
import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
import okhttp3.Call;
import okhttp3.Interceptor;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;
-/**
- * OkHttp client instrumentation.
- *
- * @author Pavol Loffay
- */
public class TracingCallFactory implements Call.Factory {
- static final String COMPONENT_NAME = "okhttp";
private final OkHttpClient okHttpClient;
- private final Tracer tracer;
- private final List<OkHttpClientSpanDecorator> decorators;
-
- public TracingCallFactory(final OkHttpClient okHttpClient, final Tracer tracer) {
- this(okHttpClient, tracer, Collections.singletonList(OkHttpClientSpanDecorator.STANDARD_TAGS));
- }
-
- public TracingCallFactory(
- final OkHttpClient okHttpClient,
- final Tracer tracer,
- final List<OkHttpClientSpanDecorator> decorators) {
+ public TracingCallFactory(final OkHttpClient okHttpClient) {
this.okHttpClient = okHttpClient;
- this.tracer = tracer;
- this.decorators = new ArrayList<>(decorators);
}
@Override
public Call newCall(final Request request) {
- Scope scope = null;
- try {
- scope =
- tracer
- .buildSpan("http.request")
- .withTag(Tags.COMPONENT.getKey(), COMPONENT_NAME)
- .withTag(DDTags.SERVICE_NAME, COMPONENT_NAME)
- .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT)
- .startActive(false);
-
- if (Config.get().isHttpClientSplitByDomain()) {
- scope.span().setTag(DDTags.SERVICE_NAME, request.url().host());
- }
+ final Span span = GlobalTracer.get().buildSpan("okhttp.http").start();
+ try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) {
+ DECORATE.afterStart(scope);
+ DECORATE.onRequest(scope.span(), request);
/** In case of exception network interceptor is not called */
final OkHttpClient.Builder okBuilder = okHttpClient.newBuilder();
- okBuilder
- .networkInterceptors()
- .add(0, new NetworkInterceptor(tracer, scope.span().context(), decorators));
+ okBuilder.networkInterceptors().add(0, new NetworkInterceptor(scope.span().context()));
- final Scope finalScope = scope;
okBuilder
.interceptors()
.add(
@@ -74,71 +41,59 @@ public class TracingCallFactory implements Call.Factory {
new Interceptor() {
@Override
public Response intercept(final Chain chain) throws IOException {
- final Scope activeInterceptorSpan =
- tracer.scopeManager().activate(finalScope.span(), true);
- try {
+ try (final Scope interceptorScope =
+ GlobalTracer.get().scopeManager().activate(span, false)) {
return chain.proceed(chain.request());
} catch (final Exception ex) {
- for (final OkHttpClientSpanDecorator spanDecorator : decorators) {
- spanDecorator.onError(ex, activeInterceptorSpan.span());
- }
+ DECORATE.onError(scope, ex);
throw ex;
} finally {
- activeInterceptorSpan.close();
+ DECORATE.beforeFinish(span);
+ span.finish();
}
}
});
return okBuilder.build().newCall(request);
} catch (final Throwable ex) {
- for (final OkHttpClientSpanDecorator spanDecorator : decorators) {
- spanDecorator.onError(ex, scope.span());
- }
+ // Not sure what would cause it to get here.
+ DECORATE.onError(span, ex);
throw new RuntimeException(ex);
- } finally {
- scope.close();
}
}
static class NetworkInterceptor implements Interceptor {
public SpanContext parentContext;
- public Tracer tracer;
- public List<OkHttpClientSpanDecorator> decorators;
- NetworkInterceptor(
- final Tracer tracer,
- final SpanContext spanContext,
- final List<OkHttpClientSpanDecorator> decorators) {
+ NetworkInterceptor(final SpanContext spanContext) {
parentContext = spanContext;
- this.tracer = tracer;
- this.decorators = decorators;
}
@Override
public Response intercept(final Chain chain) throws IOException {
try (final Scope networkScope =
- tracer
- .buildSpan("http.request")
- .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT)
- .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT)
- .withTag(Tags.COMPONENT.getKey(), COMPONENT_NAME + "-network")
- .asChildOf(parentContext)
- .startActive(true)) {
-
- for (final OkHttpClientSpanDecorator spanDecorator : decorators) {
- spanDecorator.onRequest(chain.request(), networkScope.span());
- }
+ GlobalTracer.get().buildSpan("okhttp.http").asChildOf(parentContext).startActive(true)) {
+ NETWORK_DECORATE.afterStart(networkScope);
+ NETWORK_DECORATE.onRequest(networkScope.span(), chain.request());
+ NETWORK_DECORATE.onPeerConnection(
+ networkScope.span(), chain.connection().socket().getInetAddress());
final Request.Builder requestBuilder = chain.request().newBuilder();
- tracer.inject(
+ GlobalTracer.get()
+ .inject(
networkScope.span().context(),
Format.Builtin.HTTP_HEADERS,
new RequestBuilderInjectAdapter(requestBuilder));
- final Response response = chain.proceed(requestBuilder.build());
- for (final OkHttpClientSpanDecorator spanDecorator : decorators) {
- spanDecorator.onResponse(chain.connection(), response, networkScope.span());
+ final Response response;
+ try {
+ response = chain.proceed(requestBuilder.build());
+ } catch (final Exception e) {
+ NETWORK_DECORATE.onError(networkScope, e);
+ throw e;
}
+ NETWORK_DECORATE.onResponse(networkScope.span(), response);
+ NETWORK_DECORATE.beforeFinish(networkScope);
return response;
}
}

MODIFY
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/TracingInterceptor.java
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/TracingInterceptor.java
package datadog.trace.instrumentation.okhttp3;
-import static datadog.trace.instrumentation.okhttp3.TracingCallFactory.COMPONENT_NAME;
+import static datadog.trace.instrumentation.okhttp3.OkHttpClientDecorator.DECORATE;
-import datadog.trace.api.DDSpanTypes;
-import datadog.trace.api.DDTags;
import io.opentracing.Scope;
-import io.opentracing.Tracer;
-import io.opentracing.tag.Tags;
+import io.opentracing.util.GlobalTracer;
import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
-import java.util.concurrent.Executors;
import lombok.extern.slf4j.Slf4j;
-import okhttp3.Dispatcher;
import okhttp3.Interceptor;
-import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;
-/**
- * OkHttp interceptor to trace client requests. Interceptor adds span context into outgoing
- * requests. Please only use this instrumentation when {@link TracingCallFactory} is not possible to
- * use. This instrumentation fails to properly infer parent span when doing simultaneously
- * asynchronous calls.
- *
- * <p>Initialization via {@link TracingInterceptor#addTracing(OkHttpClient.Builder, Tracer, List)}
- *
- * <p>or instantiate the interceptor and add it to {@link
- * OkHttpClient.Builder#addInterceptor(Interceptor)} and {@link
- * OkHttpClient.Builder#addNetworkInterceptor(Interceptor)}.
- *
- * @author Pavol Loffay
- */
@Slf4j
public class TracingInterceptor implements Interceptor {
- private final Tracer tracer;
- private final List<OkHttpClientSpanDecorator> decorators;
-
- /**
- * Create tracing interceptor. Interceptor has to be added to {@link
- * OkHttpClient.Builder#addInterceptor(Interceptor)} and {@link
- * OkHttpClient.Builder#addNetworkInterceptor(Interceptor)}.
- *
- * @param tracer tracer
- */
- public TracingInterceptor(final Tracer tracer) {
- this(tracer, Collections.singletonList(OkHttpClientSpanDecorator.STANDARD_TAGS));
- }
-
- /**
- * Create tracing interceptor. Interceptor has to be added to {@link
- * OkHttpClient.Builder#addInterceptor(Interceptor)} and {@link
- * OkHttpClient.Builder#addNetworkInterceptor(Interceptor)}.
- *
- * @param tracer tracer
- * @param decorators decorators
- */
- public TracingInterceptor(final Tracer tracer, final List<OkHttpClientSpanDecorator> decorators) {
- this.tracer = tracer;
- this.decorators = new ArrayList<>(decorators);
- }
-
- public static OkHttpClient addTracing(final OkHttpClient.Builder builder, final Tracer tracer) {
- return TracingInterceptor.addTracing(
- builder, tracer, Collections.singletonList(OkHttpClientSpanDecorator.STANDARD_TAGS));
- }
-
- public static OkHttpClient addTracing(
- final OkHttpClient.Builder builder,
- final Tracer tracer,
- final List<OkHttpClientSpanDecorator> decorators) {
- final TracingInterceptor tracingInterceptor = new TracingInterceptor(tracer, decorators);
- builder.interceptors().add(0, tracingInterceptor);
- builder.networkInterceptors().add(0, tracingInterceptor);
- builder.dispatcher(
- new Dispatcher(new TracedExecutorService(Executors.newFixedThreadPool(10), tracer)));
- return builder.build();
- }
-
@Override
public Response intercept(final Chain chain) throws IOException {
Response response = null;
// application interceptor?
if (chain.connection() == null) {
- final Scope scope =
- tracer
- .buildSpan("http.request")
- .withTag(Tags.COMPONENT.getKey(), COMPONENT_NAME)
- .withTag(DDTags.SERVICE_NAME, COMPONENT_NAME)
- .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT)
- .startActive(true);
+ final Scope scope = GlobalTracer.get().buildSpan("okhttp.http").startActive(true);
+ DECORATE.afterStart(scope);
final Request.Builder requestBuilder = chain.request().newBuilder();
@@ -104,11 +32,10 @@ public class TracingInterceptor implements Interceptor {
try {
response = chain.proceed(requestBuilder.build());
} catch (final Throwable ex) {
- for (final OkHttpClientSpanDecorator spanDecorator : decorators) {
- spanDecorator.onError(ex, scope.span());
- }
+ DECORATE.onError(scope, ex);
throw ex;
} finally {
+ DECORATE.beforeFinish(scope);
scope.close();
}
} else {
@@ -116,8 +43,7 @@ public class TracingInterceptor implements Interceptor {
if (tag instanceof TagWrapper) {
final TagWrapper tagWrapper = (TagWrapper) tag;
response =
- new TracingCallFactory.NetworkInterceptor(
- tracer, tagWrapper.getSpan().context(), decorators)
+ new TracingCallFactory.NetworkInterceptor(tagWrapper.getSpan().context())
.intercept(chain);
} else {
log.error("tag is null or not an instance of TagWrapper, skipping decorator onResponse()");

MODIFY
dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3Test.groovy
dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3Test.groovy
import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.agent.test.utils.PortUtils
import datadog.trace.api.Config
import datadog.trace.api.DDSpanTypes
import io.opentracing.tag.Tags
+import okhttp3.Call
+import okhttp3.Callback
import okhttp3.OkHttpClient
import okhttp3.Request
+import okhttp3.Response
+import spock.lang.AutoCleanup
+import spock.lang.Shared
+
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.atomic.AtomicReference
import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride
+import static java.util.concurrent.TimeUnit.SECONDS
class OkHttp3Test extends AgentTestRunner {
-
- def "sending a request creates spans and sends headers"() {
- setup:
+ @AutoCleanup
+ @Shared
def server = httpServer {
handlers {
all {
@@ -19,13 +28,35 @@ class OkHttp3Test extends AgentTestRunner {
}
}
}
+
def client = new OkHttpClient()
+
+ def "sending a request creates spans and sends headers"() {
+ setup:
def request = new Request.Builder()
.url("http://localhost:$server.address.port/ping")
.build()
- def response = withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
- client.newCall(request).execute()
+ Response response = withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
+ if (!async) {
+ return client.newCall(request).execute()
+ }
+
+ AtomicReference<Response> responseRef = new AtomicReference()
+ def latch = new CountDownLatch(1)
+
+ client.newCall(request).enqueue(new Callback() {
+ void onResponse(Call call, Response response) {
+ responseRef.set(response)
+ latch.countDown()
+ }
+
+ void onFailure(Call call, IOException e) {
+ latch.countDown()
+ }
+ })
+ latch.await(10, SECONDS)
+ return responseRef.get()
}
expect:
@@ -40,7 +71,8 @@ class OkHttp3Test extends AgentTestRunner {
errored false
parent()
tags {
- "component" "okhttp"
+ "$Tags.COMPONENT.key" "okhttp"
+ "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
defaultTags()
}
}
@@ -53,7 +85,7 @@ class OkHttp3Test extends AgentTestRunner {
childOf(span(0))
tags {
defaultTags()
- "component" "okhttp"
+ "$Tags.COMPONENT.key" "okhttp-network"
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
"$Tags.HTTP_METHOD.key" "GET"
"$Tags.HTTP_STATUS.key" 200
@@ -69,8 +101,47 @@ class OkHttp3Test extends AgentTestRunner {
server.lastRequest.headers.get("x-datadog-trace-id") == TEST_WRITER[0][1].traceId
server.lastRequest.headers.get("x-datadog-parent-id") == TEST_WRITER[0][1].spanId
- cleanup:
- server.close()
+ where:
+ renameService | async
+ false | false
+ true | false
+ false | true
+ true | true
+ }
+
+ def "sending an invalid request creates an error span"() {
+ setup:
+ def unusablePort = PortUtils.UNUSABLE_PORT
+ def request = new Request.Builder()
+ .url("http://localhost:$unusablePort/ping")
+ .build()
+
+ when:
+ withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
+ client.newCall(request).execute()
+ }
+
+ then:
+ thrown(ConnectException)
+
+ assertTraces(1) {
+ trace(0, 1) {
+ span(0) {
+ operationName "okhttp.http"
+ serviceName "okhttp"
+ resourceName "okhttp.http"
+ spanType DDSpanTypes.HTTP_CLIENT
+ errored true
+ parent()
+ tags {
+ "$Tags.COMPONENT.key" "okhttp"
+ "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+ errorTags(ConnectException, ~/Failed to connect to localhost\/[\d:\.]+:61/)
+ defaultTags()
+ }
+ }
+ }
+ }
where:
renameService << [false, true]

DELETE
dd-java-agent/src/test/java/datadog/trace/agent/integration/OkHTTPInstrumentationTest.java
-package datadog.trace.agent.integration;
-
-import static org.assertj.core.api.Assertions.assertThat;
-
-import okhttp3.OkHttpClient;
-import org.junit.Test;
-
-public class OkHTTPInstrumentationTest {
-
- @Test
- public void test() {
- final OkHttpClient client = new OkHttpClient().newBuilder().build();
-
- assertThat(client.interceptors().size()).isEqualTo(1);
- assertThat(client.interceptors().get(0).getClass().getSimpleName())
- .isEqualTo("TracingInterceptor");
- }
-}

Migrate OkHttp Instrumentation to Decorators