Add a 0.30 compatibility package/layer

[carlosalberto]

This is an updated PR for the compatibility layer for the 0.30 API.

The changes are mostly:

• Put all the 0.30 packages under a new opentracing-v030 package (so not mess with the existing code). For keeping things uniform, the code from 0.30 has been mostly copied over.

• Have a io.opentracing.v_030.shim.TracerShim class to wrap io.opentracing.Tracer instances and expose them under the 0.30 API. This required the addition of AutoFinishScopeManager, which is a ScopeManager supporting reference count. I decided to put it under util as it may be used in other components later on, and not only the Shim (like in Scala asynchronous frameworks).

Any feedback is appreciated ;) Observe the idea is to let instrumentation authors to incrementally support the new API while keeping backwards compatibility against the old one.

[rbtcollins]

Haven't reviewed in detail, but thank you!

[coveralls]

Coverage decreased (-6.3%) to 72.27% when pulling eb4ae34 on carlosalberto:v_030_compat_layer into b9feb48 on opentracing:v0.31.0.

[malafeev]

I cannot set tag on ActiveSpan now:

import io.opentracing.v_030.ActiveSpan;
import io.opentracing.v_030.tag.Tags;

ActiveSpan activeSpan = ...;
Tags.COMPONENT.set(activeSpan, "component");

[coveralls]

Coverage decreased (-4.0%) to 74.601% when pulling c1a568e on carlosalberto:v_030_compat_layer into b9feb48 on opentracing:v0.31.0.

[carlosalberto]

@malafeev Hey, thanks for the catch. Now it's fixed (the tags subpackage was referencing Span instead of BaseSpan).

[malafeev]

thanks @carlosalberto , verified

[pavolloffay]

Have a io.opentracing.v_030.shim.TracerShim

does it mean that all imports (ActiveSpan, BaseSpan) have to be changed to this?

[carlosalberto]

@pavolloffay Yes - for people using this Shim + the old API, that is (as io.opentracing. will contain the new API).

There were some ideas in how to help people with this. Maybe some Gradle/Maven plugin/script to refactor their 'old' code. Maybe this can be played with down the road?

As a note, the idea is to let people using the 0.31 API to incrementally update their instrumentation code using 0.30, so this would be a small price to pay.

[malafeev]

I would be more strict: don't add compatibility package.
Who wants to migrate - will do.
Using this compatibility also takes time. Better migrate to new api.

[tylerbenson]

I agree with @malafeev. I would rather not add all this additional complexity. My main concern then is does this require trace implementers to then implement both apis?

[rbtcollins]

Updating an import: 30 seconds. Changing programming model across a framework? Hours or days.

The refcount tracking code should be entirely generic and not need any implementation by backend providers AIUI.

[malafeev]

If we have production issue then 30 seconds vs hours/days makes sense.
But in case of not stable yet api supporting previous api just to free couple of hours/(days) looks not right.
I prefer force people to upgrade until we have 1.0 and start thinking how to make breaking changes.

[rbtcollins]

So you'll place users in a choice of having to do a more complex thing, or be at risk of old dependencies which may have security flaws in it. As soon e.g. spring upgrades their bindings to 0.31, everyone with custom code written in spring also has to upgrade, also in lockstep: this is a transitive problem.

Forcing a lock-step upgrade when none is strictly required brings no benefit to the project, but can make your users life more stressful. I really don't understand why you'd ever do that.

Given that code is already written, please don't force other people to coordinate changes to your schedule.

[malafeev]

is there any example of using it?
I'm not sure that it will work as you described for spring app.
You cannot mix instrumented libs with v030 and v031, it will not work even with this package.

[rbtcollins]

But you can update with super-little work with the shim, no?

[malafeev]

ok, if it works for you. for me it's a headache to make this super-little work.

[pavolloffay]

I didn't find any tests for Sope-ActiveSpan move (and vice versa).

[pavolloffay]

Isn't it better to use maven plugin to move all classes into the new package?

[pavolloffay]

this should not throw exception

[pavolloffay]

the same for other child/references methods

[carlosalberto]

Yeah, I was thinking about it earlier. I'd like to report bad arguments, but then again, these ones should stick to the standard. Will adjust them as expected.

[carlosalberto]

Closing this in favor of the new opentracing-java-v030 repository effort.