0.31.0 Release Candidate

[tedsuo]

This issue tracks the final push for releasing the new 0.31 API for OpenTracing-Java.

After six months of research and exploration, the next version of the Java edition of OpenTracing is now available. This represents a significant effort on the part of a large group of people. The OpenTracing community has grown substantially in the last year, and that diversity is reflected in the quality of this work.

RC Branch: https://github.com/opentracing/opentracing-java/tree/v0.31.0

Current Status

• Scopes API approved.
• Documentation must be cleaned up and reviewed.
• Open call for Backwards compatibility and migration strategies.
• Open call for final API changes related to improving how we instrument code.
• Additions to the API (SpanContext extenstions, Observers, etc) should wait till the next version.

Finalized Changes

Scopes

The core motivating issue for this release was the simplification of context-propagation mechanism introduced in 0.30, and removal of reference-counting and lifecycle-management for spans.

• Scopes proposal: https://gist.github.com/tedsuo/16977473e6808773736f784b56e93d73
• Motivating issue: understand and address concerns with ActiveSpan refcounting specification#80

Backwards Compatibility

The current proposal is to retain the v0.30 interfaces and related objects by moving them to the io.opentracing.v_030 namespace, and create a "shim" to allow v0.31 tracers to be wrapped in the v0.30 interface.

Repo: https://github.com/opentracing/opentracing-java-v030

Proposal: https://docs.google.com/document/d/1JIEBn0K0vQgMvyNhcJr57utizeWZ5Vf5cwmUr2GzwYc
Initial compatibility PR (no shim yet): #215

Compatibility Goals

• Allow tracer implementors to support v0.30 without having to maintain both a v0.30 and a v0.31 API.
• Allow users to continue to use the v0.30 API while still consuming tracer implementation updates.
• Ideally, also allow users to progressively migrate their codebase to v0.31.

Release Process

• The new API is being developed on the 0.31.0 branch.
• Revisions should be requested as Pull Requests against this branch.
• master will continue to point at the latest patch version of the 0.30 branch, until 0.31 is officially released.
• Changes to the release candidate will be released on maven as 0.31.0.RC1, 0.31.0.RC2, etc.

Current RC snapshots

• http://search.maven.org/#artifactdetails%7Cio.opentracing%7Copentracing-api%7C0.31.0-RC1%7Cjar

Delayed until 0.32

Improved Binary format

The current binary format requires predetermined memory allocation, which requires a size calculation of baggage. A variable sized binary format would be preferable.

• Pull Request: Change BINARY to be resizable and stream-oriented. #223
• Motivating issue: Add a new Binary Format to allow for variable SpanContext size #95

Previous API proposals for 0.31

• Continuations: RFC / WIP: make "auto-finishing" optional and non-Tracer-specific #166
• Scopes with optional Continuations: Prototype for continuable scopes #172
• Scopes: Implement a simpler Span propagation without lifetime handling. #182
• TimeUnits: Tracer API uses non-idiomatic means to pass/specify timestamps #180

[yurishkuro]

@tedsuo can we enumerate breaking API changes and migration steps people need to go through (if any)?

For example, the time stamp format change sounds breaking but iirc it's backwards compatible via deprecation. The scope change probably requires migration.

[tedsuo]

@yurishkuro good suggestion; we need to flush out release notes and migration steps as part of this process.

• I agree that timestamp changes should be backwards compatible.
• For Continuation -> Scope changes, is it possible to build a shim that would allow 0.30 instrumentation to continue to work with a 0.31 tracer?
• For Continuation -> Scope changes, is it possible to to write a set of refactoring tools that assist in migrating 0.30 instrumentation over?

[pavolloffay]

For the record, there is no tag for 0.31.0-RC1 or release-0.31.RC1, however, the release was successful http://search.maven.org/#artifactdetails%7Cio.opentracing%7Copentracing-api%7C0.31.0-RC1%7Cjar.

Release script publish.sh worked well. Travis canceled the build https://travis-ci.org/opentracing/opentracing-java/requests b1fb889. That was for Branch "0.31.0-RC1" excluded see travis config (excluded branches), but then fb86feel synced with bintray (branch v.0.31.0) which is what we want.

@carlosalberto thought that the build failed so he removed tags.

[pavolloffay]

I just noticed that release commits were also removed, which is not good. I think we cannot release 0.31.0-RC1 again but we could add commits without deploying to bintray (although tag would be still missing).

Or we can release RC2. Or just leave it as it is for now.

[carlosalberto]

@pavolloffay What would you suggest at this point? For the record: I know we will be trying to get RC2 out in 1 week or so, with some cleanups, async test samples and potentially the finish() changes.

[objectiser]

Two issues currently:

• Update to use the OT Java 0.31.0-RC1 API opentracing-contrib/java-web-servlet-filter#27 (comment) - the servlet filter does not know if in sync/async mode when the active span is created - so currently the scope cannot be closed if in async mode, as this would prematurely close the associated span. One workaround might be to startActive(false) and then if in sync mode manually finish the associated span. But wondering if there is any other options?

• Update for OpenTracing Java v0.31.0-RC1 jaegertracing/jaeger-client-java#246 (comment) - the log methods should just be deprecated

[carlosalberto]

@objectiser For the first item: correct, that's what usually would need to be done for async scenarios: startActive(false) and then let the callback finish it. The problem, as you say, is that you would need to detect if the instrumentation code is running in sync or async mode. Interesting feedback, nevertheless. We had been checking for instrumentation libraries truly needing a Continuation concept, so this piece of information can be helpful.

[pavolloffay]

jaegertracing/jaeger-client-java#246 (comment) - the log methods should just be deprecated

the "log object" methods have been deprecated for quite some time. e.g. https://github.com/opentracing/opentracing-java/blob/release-0.22.0/opentracing-api/src/main/java/io/opentracing/Span.java#L171

[mabn]

If #203 is merged then ScopeManager.active() should also be marked as @Nullable

[pavolloffay]

Default auto-finishing on Scope.close

The current release candidate 0.31.0-RC1 uses default auto-finishing on Scope.close(). It can be overridden when creating the Scope like:

2. Scope foo = tracer.buildSpan("foo").startActive(false);
2. Scope bar = tracer.scopeManager().activate(foo, false);

The later is more interesting, we usually don't want to finish a span, just prapagate to different thread.

Do we want auto-finishing as default behavior?

So far we have updated two instrumentations, both have to override the default behavior.
Servlet instrumentation:
https://github.com/opentracing-contrib/java-web-servlet-filter/pull/27/files#diff-7dcd9d8f74292f884899407726385349R157
Spring Boot:
server: https://github.com/opentracing-contrib/java-spring-web/pull/42/files#diff-29f727382a1c6e730b5080a7ce449520R85
AsyncRestTemplate: https://github.com/opentracing-contrib/java-spring-web/pull/42/files#diff-cfcd93a5ac9c2c102ab8bc7ee8043962R53

Auto-finishing makes sense for simple end-user use cases or simple synchronous calls. The question is: are we creating API which will be used by end app developers or API which leads to a proper instrumentation?

References from other tracing systems:

• https://github.com/Nike-Inc/wingtips/pull/43/files#diff-04c6e90faac2675aa89e2176d2eec7d8R142
• Auto-Closable Span may be problematic census-instrumentation/opencensus-java#646
• https://github.com/openzipkin/brave/blob/master/brave/src/main/java/brave/Tracer.java#L262

[malafeev]

for me here default value is confusing. I try to avoid it and explicit specify true/false. Who knows what default value will be in next release.

[pavolloffay]

Who knows what default value will be in next release.

It definitely shouldn't be changed once 1.0 is out, it would break existing code

[carlosalberto]

The initial approach of Ben, when creating this new API, was to have the default to not-finish-Span. So it could as well be changed back to that.

For me, one of the interesting things is how often this not-finish-Span scenario happens outside asynchronous frameworks: if it happens enough outside them, then we may as well do it as @pavolloffay suggests.

[carlosalberto]

@malafeev @pavolloffay Agreed on that - once this is released, it shouldn't change as it would break stuff around.

[malafeev]

The initial approach of Ben, when creating this new API, was to have the default to not-finish-Span

that's why it started confusing me: my testing instrumentation became broken. And I'm not sure what startActive() return :)

[yurishkuro]

If we don't know which way is the right way, it's better not to force that decision via the API and let the user decide. We can always add a method with default behavior later; we can't take one away or change its behavior.

[wu-sheng]

@tedsuo @yurishkuro @bhs I am absent about 0.31.0 for a long time, because too many things for releasing new versions of skywalking. And I jump back to OT-java 0.31.0, so forgive me if asking the questions if you discussed before.

Feedbacks at high level:

1. For Continuation -> Scope, why using the new concept? IMHO, understanding an new API is a hard thing, costs much for user and implementations to process. If I remember right, for 0.30, some people(including me) just recommend to remove the auto-finish counter of Continuation. No one said about removing the Continuation. Is there any news?
2. These is no readme documents(https://github.com/opentracing/opentracing-java/blob/v0.31.0/README.md) for 0.31 to describe about how to use the scope API. From this point, can't tell the RC is good enough or not. No one can make sure they use the API right if formal documents are missing.

From my quick codes review:
In 0.30, Continuation is from the ActiveSpan, which is easy for understanding. Span or ActiveSpan is also the status of current trace context; In 0.31, Scope/ScopeManager seems more independency from span/context. From my understanding, these concepts are for cross-thread propagation, should be part of common usage of the API. What is the benefit for adding a new one? Cross-thread is not useful in every/most scenario, right?

Just an option: if you truly want to remove the ActiveSpan, and support the cross-thread propagation, we can consider to add Capture/Activate in Tracer interface, like inject/extract did.

@bhs Also should consider the concepts in Spec Level about how to propagate cross thread, since Java is not the only language has thread models, right? I always felt that inject/extract did a great example.

IMHO, we should keep the API consistency if possible since we are so near for 1.0. We should keep the Java-API like the OT spec said. Tracer->Span(Builder)->Tag/Log is only the major road for every user, any other concepts are for propagation.

CC @adriancole

[rbtcollins]

Being most of the way through writing an integration - please don't make 0.31 incompatible with 0.30 without really clear docs:
old code -> new code examples for:

• async wrapping
• threaded handoffs

ideally a thunk layer that implements the 0.30 API would be useful too. I realise that as a pre < 1.0 adopter, we're taking some risk, but given the goal of being the lingua franca of tracing, you're going to depend on folk keeping up with your changes to be successful: changes that add upgrade pain and no new capabilities are just overhead IMO.

[hypnoce]

Hi all,

I find the Scope semantics makes more sense then ActiveSpan (regardless of the Continuation).
I just ported the p6spy instrumenter to the 0.31 API : https://github.com/opentracing-contrib/java-p6spy/tree/v0.31.0

I just had a comment on the actual Java API :
in ScopeManager#L33 maybe make the activate method a default method :

public interface ScopeManager {
    default Scope activate(Span span) {
        return activate(span, true);
    }
    Scope activate(Span span, boolean finishSpanOnClose);
    Scope active();
}

Even if the documentation says so, it makes really clear that default span activation will finish the span on scope closing.

What do you think ?

Thanks all

[yurishkuro]

it won't be compatible with Java 1.6

[hypnoce]

@yurishkuro indeed I was just thinking of that. Also won't work in java 7.

[mabn]

About default auto-finishing - IMHO SpanBuilder.startActive() should finish-on-close by default (like it does now), but SpanManager.activate() shouldn't.
Isn't the main use of activate to "activate" existing span in another thread? If that's the case then we'd always want to finish the span in the original thread (and any other approach will be very specific).

It would be better to remove SpanManager.activate() than to leave the current default.

Additionally I'm not a fan of boolean parameters, it's hard to read:

SpanManager.activate(span, true)  <- what does it do?

This would be better:

SpanManager.activate(span) // does not finish on close
SpanManager.activateWithFinish(span)

[carlosalberto]

I personally like removing the SpanManager.activate() overload with the default value (while keeping the default value for buildSpan().startActive()). Also, since Observers will be brought back on the next 0.31.0 iteration, we will probably see calls like ScopeManager.activate(span, Scope.FINISH), so no more boolean values.

[objectiser]

Can we elaborate what are the steps/decisions that are required before this issue can be closed? We ideally don't want to maintain the separate release candidate branches in tracers/instrumentation for too long.

[tedsuo]

Hi @objectiser, we're going to discuss next steps on the OTSC call this friday. I believe we can approve the release candidate at this point. But we should discuss what other steps should be taken before we merge it in.

[rbtcollins]

Where / when is this OTSC call?

The RC docs still refer to the refcount based activeSpan model - https://github.com/opentracing/opentracing-java/tree/v0.31.0 - I'd expect at least the documentation updated as a minimum before having an RC :)

I went looking there to get an overview of the new API, so that I can reason about how it will be integrated into our framework. Is there a good source of updated docs for it?

[yurishkuro]

@rbtcollins the call is this Friday 11:30 EST. There is a document linked from https://github.com/opentracing/specification/blob/master/project_organization.md with minutes, agenda, call details, etc.

[carlosalberto]

@rbtcollins Sorry about that. Now that we have a more solid/tested approach we will definitely update the documentation (definitely important for the second RC). Part of the lack of it was in part due to the fact we didn't know how much the API could it still change at (that's why the referenced #182 includes notes about it, i.e. https://gist.github.com/tedsuo/16977473e6808773736f784b56e93d73)

[hypnoce]

I'm late to the party :( Anyway I was thinking about code like :
async <-> sync
buildAsyncSpan().startActive() <-> buildSyncSpan().startActive()
buildSpan().async().startActive() <-> buildSpan().sync().startActive()
buildSpan().startAsyncActive() <-> buildSpan().startSyncActive()

If we want the default to be not auto finishing :
buildSpan().startActive() <-> buildSpan().startActive().sync()

I'm not sure but it seems a bit more idiomatic since not-auto-finishing spans are mostly used in async code.

Thanks !

[tedsuo]

Thanks @hypnoce. I believe that would be too high-level an API. At any rate, the current candidate has been thoroughly vetted, I don't think there's appetite for a radical change at this point.

But! Once the API settles, it should be considered a "low level" API. Personally, I'm interested to see what kinds of "higher order" APIs people can build on top of OT to provide convenience, without having to worry about getting all of the edge cases correct.

[CodingFabian]

We at https://github.com/instana are ready for 0.31: instana/instana-java-opentracing#4

[tedsuo]

Thanks @CodingFabian!

There is one last minute change: we have dropped the new BINARY format, in favor of a longer discussion and a separate release.

[tedsuo]

Huzzah, v0.31 is complete!