Deprecate instrumentations in favor of opentracing-contrib

[yurishkuro]

Many of the frameworks we instrumented in jaeger-client-java are also instrumented in opentracing-contrib. We should deprecate Jaeger-specific instrumentation.

Criteria:

• Need to implement Update to opentracing-java 0.30.0 #179 to support active span
• Deprecate jaeger-context (make it a wrapper around active span, if possible)
• Need to investigate compatibility of opentracing-contrib libs with how Jaeger instrumentation was done. Ideally we want to avoid forcing changes to existing code by keeping Jaeger classes as thin wrappers around opentracing-contrib
• If complete backwards compatibility is not possible, document exact migration steps

[pavolloffay]

I'm assigning it to myself.

Later on, do we want to deprecate jaeger wrappers too?

[vprithvi]

Later on, do we want to deprecate jaeger wrappers too?

If by deprecate, you mean adding @Deprecated to the wrappers, I feel that we should do it when the wrappers are introduced.

I think we should remove jaeger wrappers with the first major release.

[pavolloffay]

I'm more in favor of deprecating and let users upgrade to the contrib versions.

There are some potential problems:

• jax-rs adds PEER_SERVICE tag when com.uber.jaeger.Constants.X_UBER_SOURCE(x-uber-source) is present. Users might provide custom span decorator to do this but I think it's inconvenient.
• dropwizzard: sampler can be specified when in config. OT contrib integration constructrs tracer in Application class.

[yurishkuro]

Re x-uber-source, at Uber applications usually do not import Jaeger directly, but via central service frameworks, so a custom decorator might work.

[pavolloffay]

But who should provide this decorator? Can we expect that they will correctly implement it? Or should be provided as jaeger-jaxrs2 module?

[yurishkuro]

the decorator that actually contains the x-uber-source string will be provided by our internal infra libs.

[pavolloffay]

I have to do some work on dropwizard contrib lib:

The configuration will accept:

tracing:
   serviceName: // string
   serverAddress: // string could be `http://localhost:9411/api/v1/spans` or `localhost:5984`
   enabled: // to quickly disable tracing by using NoopTracer

Then in the Application class users will provide tracer instance. Is this acceptable for Uber users?

Other problem is that jaeger metrics won't work https://github.com/uber/jaeger-client-java/tree/master/jaeger-dropwizard#metrics, Could be https://github.com/uber/jaeger-client-java/blob/master/jaeger-dropwizard/src/main/java/com/uber/jaeger/dropwizard/StatsFactory.java and other referenced classes also provided by internal infra libs?

[yurishkuro]

Why do we need to change configuration from its current form?

StatsFactory delegates to com.codahale.metrics.MetricRegistry. I don't think we have any other internal impl for it (if we do, it's backed but the internal metrics API).

[pavolloffay]

@yurishkuro we don't have to change jaeger configuration. I'm talking about dropwizzard configuration which could be used with any OT tracer.

[yurishkuro]

Oh... That sounds like an anti pattern. There's never going to be a generic configuration that will satisfy ALL tracers, so why even bother? Why can't each implementation have its own config? Is this somehow forced by dropwizard?

[pavolloffay]

It's not forced by dropwizard. It's just an idea how to simplify integration for most tracer. In the Application users would still have to instantiate tracer manually and pass it to integration code which would initialize jax-rs instrumentation

[objectiser]

@yurishkuro For 1.0 milestone, would you be ok with the framework instrumentations being moved into a separate repo, remaining under the com.uber package - while the other modules would be moved under io.jaegertracing as part of #287 ?

[objectiser]

Current plan, as discussed on the last bi-weekly call, is to retain the com.uber maven artifacts for both the tracer and framework instrumentations - but in a separate repo. The tracer, configuration classes, etc would then become wrappers around the io.jaegertracing versions.

[pavolloffay]

@yurishkuro we would like to move instrumentations and context to a separate repository. Could you please create one? Possibly under uber org if not then here in jaegertracing.

This new repo will depend on com.uber jaeger client. (if it has to depend on the client)

[yurishkuro]

I don't want to move it back to uber org because of approvals, CLAs, etc.

How about jaegertracing/legacy-client-java ?

[pavolloffay]

sounds good to me

unfortunately we cannot fork the repo in the same org, so I will create a repo and push sources there (with full history).

[pavolloffay]

Done, instrumentations are now in legacy-client-java repo.

[yurishkuro]

Not really the same, so I rebooked it in the legacy repo.