Apache HttpClient instrumentation

[pavolloffay]

Hello,

I have working POC of Apache HTTP client 1 instrumentation. I would like to move it to contrib org. It is implemented with standard Http[Request|Response]Interceptor 2. It works also for httpasyncclient.

Repository name could be:

• java-apache-httpcomponents - This could group all instrumentations related to Apache HC. Currently I can think only about opentracing-apache-http-interceptors artifact.
• java-apache-httpclient

cc @bhs @yurishkuro @wu-sheng @objectiser

[yurishkuro]

@pavolloffay fyi we have Apache HTTP instrumentation in jaeger-client-java. It uses our internal context propagation mechanism, but may be of interest.

[wu-sheng]

@pavolloffay 👍👏 httpclient may be a better name. More popular I think.

[pavolloffay]

@yurishkuro thanks, I completely missed it.

Here is dependency tree of jaeger-apachehttpclient.

[INFO] --- maven-dependency-plugin:2.8:tree (default-cli) @ ot-sandbox ---
[INFO] sk.loffay.test:ot-sandbox:jar:1.0-SNAPSHOT
[INFO] \- com.uber.jaeger:jaeger-apachehttpclient:jar:0.17.0:compile
[INFO]    +- org.apache.httpcomponents:httpcore:jar:4.1.2:compile
[INFO]    +- org.apache.httpcomponents:httpasyncclient:jar:4.1.2:compile
[INFO]    |  +- org.apache.httpcomponents:httpcore-nio:jar:4.4.5:compile
[INFO]    |  \- commons-logging:commons-logging:jar:1.2:compile
[INFO]    +- org.apache.httpcomponents:httpclient:jar:4.1.2:compile
[INFO]    |  \- commons-codec:commons-codec:jar:1.4:compile
[INFO]    +- com.uber.jaeger:jaeger-core:jar:0.17.0:compile
[INFO]    |  +- com.uber.jaeger:jaeger-thrift:jar:0.17.0:compile
[INFO]    |  |  \- org.apache.thrift:libthrift:jar:0.9.2:compile
[INFO]    |  +- io.opentracing:opentracing-api:jar:0.15.0:compile
[INFO]    |  +- com.fasterxml.jackson.core:jackson-databind:jar:2.7.4:compile
[INFO]    |  |  +- com.fasterxml.jackson.core:jackson-annotations:jar:2.7.0:compile
[INFO]    |  |  \- com.fasterxml.jackson.core:jackson-core:jar:2.7.4:compile
[INFO]    |  \- org.slf4j:slf4j-api:jar:1.7.16:compile
[INFO]    \- com.uber.jaeger:jaeger-context:jar:0.17.0:compile

It's not optimal.

Other thing if there is redirect it would produce two spans as children of context get by TraceContext parentContext = TracingUtils.getTraceContext();. If there is no parent context spans would be in different traces.

example output:

inject(io.opentracing.mock.MockSpan$MockContext@6442b0a6, io.opentracing.propagation.Format$Builtin@60f82f98, com.uber.jaeger.httpclient.ClientRequestCarrier@35f983a6)
MockSpan{traceId=1, spanId=3, parentId=2, operationName='GET', startMicros=1489660386665000, finishMicros=1489660386776000, tags={http.status_code=302, span.kind=client, http.url=/search?q=httpClient, peer.hostname=www.google.com, peer.port=-1}, logs=[]}
inject(io.opentracing.mock.MockSpan$MockContext@46d56d67, io.opentracing.propagation.Format$Builtin@60f82f98, com.uber.jaeger.httpclient.ClientRequestCarrier@d8355a8)
MockSpan{traceId=1, spanId=4, parentId=2, operationName='GET', startMicros=1489660386804000, finishMicros=1489660386942000, tags={http.status_code=200, span.kind=client, http.url=/search?q=httpClient&gws_rd=cr&ei=4WnKWLmALsjoaqCkv6gI, peer.hostname=www.google.cz, peer.port=-1}, logs=[]}
MockSpan{traceId=1, spanId=2, parentId=0, operationName='parent', startMicros=1489660386359000, finishMicros=1489660386945000, tags={}, logs=[]}

No parent:

inject(io.opentracing.mock.MockSpan$MockContext@6442b0a6, io.opentracing.propagation.Format$Builtin@60f82f98, com.uber.jaeger.httpclient.ClientRequestCarrier@35f983a6)
MockSpan{traceId=3, spanId=4, parentId=0, operationName='GET', startMicros=1489660604869000, finishMicros=1489660604979000, tags={http.status_code=302, span.kind=client, http.url=/search?q=httpClient, peer.hostname=www.google.com, peer.port=-1}, logs=[]}
inject(io.opentracing.mock.MockSpan$MockContext@46d56d67, io.opentracing.propagation.Format$Builtin@60f82f98, com.uber.jaeger.httpclient.ClientRequestCarrier@d8355a8)
MockSpan{traceId=5, spanId=6, parentId=0, operationName='GET', startMicros=1489660605002000, finishMicros=1489660605118000, tags={http.status_code=200, span.kind=client, http.url=/search?q=httpClient&gws_rd=cr&ei=u2rKWOSjOoT5apf7juAO, peer.hostname=www.google.cz, peer.port=-1}, logs=[]}

I don't like to create instrumentations which already exist see https://github.com/opentracing-contrib/meta/issues/15. I can see some benefits in having it here.

[pavolloffay]

Here https://github.com/pavolloffay/java-apache-httpcomponents is my impl.

[yurishkuro]

Object spanAttribute = context.getAttribute(ACTIVE_SPAN);

so it isn't using span manager?

[pavolloffay]

no I will add it in the moment. At the same time I'm not super happy with it because it will cause breaking changes once in-process propagation is "solved" at standard level.

[bhs]

@pavolloffay just catching up on this – sounds like we shouldn't actually create a new repo yet, but let me know when you think the time is right for that. Thanks.

[pavolloffay]

I would like to move this forward. I have spent more time on it and current instrumentation looks even better. Now it instruments all requests including errors like UnknownHostException.

thanks goes to @adriancole

Repository can be java-apache-httpclient

[bhs]

@pavolloffay https://github.com/opentracing-contrib/java-apache-httpclient/invitations

Thank you!

[pavolloffay]

Closing, it has been merged and released.