Remove duplicate Okhttp span and stop separate service creation of okhttp. Instead okhttp will now appear as part of the service that is using okhttp to make the call.
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
@@ -33,14 +33,9 @@ public class OkHttp3Instrumentation extends Instrumenter.Default {
"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 + ".OkHttpClientDecorator",
packageName + ".TracingInterceptor",
- packageName + ".TracingCallFactory",
- packageName + ".TracingCallFactory$NetworkInterceptor",
- packageName + ".TracingCallFactory$1",
};
}
@@ -62,7 +57,6 @@ public class OkHttp3Instrumentation extends Instrumenter.Default {
}
final TracingInterceptor interceptor = new TracingInterceptor();
builder.addInterceptor(interceptor);
- builder.addNetworkInterceptor(interceptor);
}
}
}

MODIFY
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/OkHttpClientDecorator.java
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/OkHttpClientDecorator.java
@@ -7,13 +7,6 @@ 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() {
@@ -22,7 +15,7 @@ public class OkHttpClientDecorator extends HttpClientDecorator<Request, Response
@Override
protected String service() {
- return "okhttp";
+ return null;
}
@Override

UNKNOWN
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/RequestBuilderInjectAdapter.java
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/RequestBuilderInjectAdapter.java
DELETE
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/TagWrapper.java
-package datadog.trace.instrumentation.okhttp3;
-
-import datadog.trace.instrumentation.api.AgentSpan;
-
-/**
- * Tag wrapper to store parent span context and user defined tags.
- *
- * @author Pavol Loffay
- */
-public class TagWrapper {
- private AgentSpan span;
-
- private Object tag;
-
- /** @param tag user tag */
- public TagWrapper(final Object tag) {
- this.tag = tag;
- }
-
- /**
- * @param wrapper previous wrapper
- * @param span span
- */
- TagWrapper(final TagWrapper wrapper, final AgentSpan span) {
- this.span = span;
- tag = wrapper.tag;
- }
-
- public void setTag(final Object tag) {
- this.tag = tag;
- }
-
- public Object getTag() {
- return tag;
- }
-
- AgentSpan getSpan() {
- return span;
- }
-}

DELETE
dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/TracingCallFactory.java
-package datadog.trace.instrumentation.okhttp3;
-
-import static datadog.trace.instrumentation.api.AgentTracer.activateSpan;
-import static datadog.trace.instrumentation.api.AgentTracer.propagate;
-import static datadog.trace.instrumentation.api.AgentTracer.startSpan;
-import static datadog.trace.instrumentation.okhttp3.OkHttpClientDecorator.DECORATE;
-import static datadog.trace.instrumentation.okhttp3.OkHttpClientDecorator.NETWORK_DECORATE;
-import static datadog.trace.instrumentation.okhttp3.RequestBuilderInjectAdapter.SETTER;
-
-import datadog.trace.instrumentation.api.AgentScope;
-import datadog.trace.instrumentation.api.AgentSpan;
-import datadog.trace.instrumentation.api.AgentSpan.Context;
-import java.io.IOException;
-import okhttp3.Call;
-import okhttp3.Interceptor;
-import okhttp3.OkHttpClient;
-import okhttp3.Request;
-import okhttp3.Response;
-
-public class TracingCallFactory implements Call.Factory {
-
- private final OkHttpClient okHttpClient;
-
- public TracingCallFactory(final OkHttpClient okHttpClient) {
- this.okHttpClient = okHttpClient;
- }
-
- @Override
- public Call newCall(final Request request) {
- final AgentSpan span = startSpan("okhttp.http");
- try (final AgentScope scope = activateSpan(span, false)) {
- DECORATE.afterStart(span);
- DECORATE.onRequest(span, request);
-
- /** In case of exception network interceptor is not called */
- final OkHttpClient.Builder okBuilder = okHttpClient.newBuilder();
- okBuilder.networkInterceptors().add(0, new NetworkInterceptor(span.context()));
-
- okBuilder
- .interceptors()
- .add(
- 0,
- new Interceptor() {
- @Override
- public Response intercept(final Chain chain) throws IOException {
- try (final AgentScope interceptorScope = activateSpan(span, false)) {
- return chain.proceed(chain.request());
- } catch (final Exception ex) {
- DECORATE.onError(scope, ex);
- throw ex;
- } finally {
- DECORATE.beforeFinish(span);
- span.finish();
- }
- }
- });
- return okBuilder.build().newCall(request);
- } catch (final Throwable ex) {
- // Not sure what would cause it to get here.
- DECORATE.onError(span, ex);
- throw new RuntimeException(ex);
- }
- }
-
- static class NetworkInterceptor implements Interceptor {
- public Context parentContext;
-
- NetworkInterceptor(final Context spanContext) {
- parentContext = spanContext;
- }
-
- @Override
- public Response intercept(final Chain chain) throws IOException {
- final AgentSpan networkSpan = startSpan("okhttp.http", parentContext);
- try (final AgentScope networkScope = activateSpan(networkSpan, true)) {
- NETWORK_DECORATE.afterStart(networkSpan);
- NETWORK_DECORATE.onRequest(networkSpan, chain.request());
- NETWORK_DECORATE.onPeerConnection(
- networkSpan, chain.connection().socket().getInetAddress());
-
- final Request.Builder requestBuilder = chain.request().newBuilder();
- propagate().inject(networkSpan, requestBuilder, SETTER);
-
- final Response response;
- try {
- response = chain.proceed(requestBuilder.build());
- } catch (final Exception e) {
- NETWORK_DECORATE.onError(networkScope, e);
- throw e;
- }
-
- NETWORK_DECORATE.onResponse(networkSpan, 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.api.AgentTracer.activateSpan;
+import static datadog.trace.instrumentation.api.AgentTracer.propagate;
import static datadog.trace.instrumentation.api.AgentTracer.startSpan;
import static datadog.trace.instrumentation.okhttp3.OkHttpClientDecorator.DECORATE;
+import static datadog.trace.instrumentation.okhttp3.RequestBuilderInjectAdapter.SETTER;
import datadog.trace.instrumentation.api.AgentScope;
import datadog.trace.instrumentation.api.AgentSpan;
@@ -14,49 +16,33 @@ import okhttp3.Response;
@Slf4j
public class TracingInterceptor implements Interceptor {
-
@Override
public Response intercept(final Chain chain) throws IOException {
if (chain.request().header("Datadog-Meta-Lang") != null) {
+ log.error("there's a chain.request.header(Datadog-Meta-Lang");
return chain.proceed(chain.request());
}
- Response response = null;
+ final AgentSpan span = startSpan("okhttp.request");
- // application interceptor?
- if (chain.connection() == null) {
- final AgentSpan span = startSpan("okhttp.http");
+ try (final AgentScope scope = activateSpan(span, true)) {
DECORATE.afterStart(span);
+ DECORATE.onRequest(span, chain.request());
final Request.Builder requestBuilder = chain.request().newBuilder();
+ propagate().inject(span, requestBuilder, SETTER);
- final Object tag = chain.request().tag();
- final TagWrapper tagWrapper =
- tag instanceof TagWrapper ? (TagWrapper) tag : new TagWrapper(tag);
- requestBuilder.tag(new TagWrapper(tagWrapper, span));
-
- final AgentScope scope = activateSpan(span, true);
+ final Response response;
try {
response = chain.proceed(requestBuilder.build());
- } catch (final Throwable ex) {
- DECORATE.onError(scope, ex);
- throw ex;
- } finally {
- DECORATE.beforeFinish(scope);
- scope.close();
- }
- } else {
- final Object tag = chain.request().tag();
- if (tag instanceof TagWrapper) {
- final TagWrapper tagWrapper = (TagWrapper) tag;
- response =
- new TracingCallFactory.NetworkInterceptor(tagWrapper.getSpan().context())
- .intercept(chain);
- } else {
- log.error("tag is null or not an instance of TagWrapper, skipping decorator onResponse()");
- }
+ } catch (final Exception e) {
+ DECORATE.onError(span, e);
+ throw e;
}
+ DECORATE.onResponse(span, response);
+ DECORATE.beforeFinish(span);
return response;
}
}
+}

UNKNOWN
dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3AsyncTest.groovy
dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3AsyncTest.groovy
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.opentracing.DDSpan
-import datadog.trace.agent.test.asserts.TraceAssert
import datadog.trace.agent.test.base.HttpClientTest
-import datadog.trace.api.DDSpanTypes
-import datadog.trace.api.DDTags
-import datadog.trace.instrumentation.api.Tags
import datadog.trace.instrumentation.okhttp3.OkHttpClientDecorator
import okhttp3.Headers
import okhttp3.MediaType
@@ -12,8 +7,6 @@ import okhttp3.Request
import okhttp3.RequestBody
import okhttp3.internal.http.HttpMethod
-import static datadog.trace.instrumentation.okhttp3.OkHttpClientDecorator.NETWORK_DECORATE
-
class OkHttp3Test extends HttpClientTest<OkHttpClientDecorator> {
def client = new OkHttpClient()
@@ -30,71 +23,17 @@ class OkHttp3Test extends HttpClientTest<OkHttpClientDecorator> {
return response.code()
}
+
@Override
OkHttpClientDecorator decorator() {
return OkHttpClientDecorator.DECORATE
}
@Override
- // parent span must be cast otherwise it breaks debugging classloading (junit loads it early)
- void clientSpan(TraceAssert trace, int index, Object parentSpan, String method = "GET", boolean renameService = false, boolean tagQueryString = false, URI uri = server.address.resolve("/success"), Integer status = 200, Throwable exception = null) {
- trace.span(index) {
- if (parentSpan == null) {
- parent()
- } else {
- childOf((DDSpan) parentSpan)
- }
- serviceName decorator().service()
- operationName "okhttp.http"
- resourceName "okhttp.http"
-// resourceName "GET $uri.path"
- spanType DDSpanTypes.HTTP_CLIENT
- errored exception != null
- tags {
- defaultTags()
- if (exception) {
- errorTags(exception.class, exception.message)
- }
- "$Tags.COMPONENT" clientDecorator.component()
- "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
- }
- }
- if (!exception) {
- trace.span(index + 1) {
- serviceName renameService ? "localhost" : decorator().service()
- operationName "okhttp.http"
- resourceName "$method $uri.path"
- childOf trace.span(index)
- spanType DDSpanTypes.HTTP_CLIENT
- errored exception != null
- tags {
- defaultTags()
- if (exception) {
- errorTags(exception.class, exception.message)
- }
- "$Tags.COMPONENT" NETWORK_DECORATE.component()
- if (status) {
- "$Tags.HTTP_STATUS" status
- }
- "$Tags.HTTP_URL" "${uri.resolve(uri.path)}"
- if (tagQueryString) {
- "$DDTags.HTTP_QUERY" uri.query
- "$DDTags.HTTP_FRAGMENT" uri.fragment
- }
- "$Tags.PEER_HOSTNAME" "localhost"
- "$Tags.PEER_PORT" server.address.port
- "$Tags.PEER_HOST_IPV4" "127.0.0.1"
- "$Tags.HTTP_METHOD" method
- "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
- }
- }
- }
+ String expectedOperationName() {
+ return "okhttp.request"
}
- @Override
- int size(int size) {
- return size + 1
- }
boolean testRedirects() {
false

MODIFY
dd-trace-ot/src/main/java/datadog/opentracing/decorators/OperationDecorator.java
dd-trace-ot/src/main/java/datadog/opentracing/decorators/OperationDecorator.java
@@ -20,7 +20,6 @@ public class OperationDecorator extends AbstractDecorator {
mappings.put("java-aws-sdk", "aws.http");
// FIXME: JMS ops card is low (jms-send or jms-receive), may be this mapping is useless
mappings.put("java-jms", "jms");
- mappings.put("okhttp", "okhttp.http");
// Cassandra, Mongo, JDBC are set via DBTypeDecorator
MAPPINGS = Collections.unmodifiableMap(mappings);
}

Remove duplicate Okhttp span and stop separate service creation of okhttp. Instead okhttp will now appear as part of the service that is using okhttp to make the call.