Adds support for "current span" formerly supported with thread binders

[codefromthecrypt]

This implements a "current span" concept which represents the in-flight
operation. Tracer.currentSpan() can be used to add custom tags to a
span and Tracer.nextSpan() can be used to create a child of whatever
is in-flight.

The backend of this is CurrentTraceContext which can be extended to
support SLF4J or Finagle synchronization. It also exposes concurrent
wrappers for types like ExecutorService.

Particularly, this includes the ability to get the "current span" and assign is to a scope. In most cases the implementation will be backed by thread locals, whether that's directly (as brave's former thread state was) or indirectly via a GRPC or Finagle Context.

The (power) user apis are located in the Tracer api:

• Span currentSpan - retrieves any span in scope or null
• Span nextSpan - creates a new child or trace if there's no current span
• SpanInScope withSpanInScope(span) - makes a span current and available to calls within the scope

The SPI for this is called CurrentTraceContext, and it can be implemented to support MDC integration or alternate span storage

• TraceContext get() - returns the current trace context or null
• Scope newScope(context) - creates a scope where calls to get will return the input

By default, CurrentTraceContext is implemented by a thread local, though there are tests to show it can be implemented by Finagle (or gRPC or otherwise).

Fixes #166

[devinsba]

I'm not sold on the name SpanScoper any more than I like Scheduler from the OT PR that is like this one. Possibly SpanManager or SpanExecutionManager or SpanExecutionContext

Really though, naming things is hard so I hope others have some input

[codefromthecrypt]

I'm not sold on the name SpanScoper any more than I like Scheduler from the OT PR that is like this one. Possibly SpanManager or SpanExecutionManager or SpanExecutionContext

food for thought.. One thing I noticed is almost all identical or similar apis have the word "scope" in either naming of functions type or in code docs. I'm open to something besides that, but it seems important (and very different than scheduling). Here's some things I considered, which might jog ideas for better names * in instrumentation-java a support class called ContextUtils (implicitly uses grpc context) where withSpan is described as "Enters the scope of code where the given Span is in the current context, and returns an object that represents that scope." * the name SpanScoper was derived from Guice's RequestScoper which is described as an "Object that can be used to apply a request scope to a block of code." * zipkin-js has a function Tracer.scoped which is roughly same as Tracer.withSpan. This is supported by a Context impl which has two functions letContext and scoped * finagle has a function Trace.letId which is roughtly the same as Tracer.withSpan. This is described as "run computation `f` with the given traceId." * opentracing has SpanManager.activate which is documented as "Makes span the current span within the running process." The span manager type has more features including one to clear all scopes. does anything above spark naming ideas? https://github.com/google/instrumentation-java/blob/master/core/src/main/java/com/google/instrumentation/trace/ContextUtils.java https://github.com/google/guice/blob/master/extensions/servlet/src/com/google/inject/servlet/RequestScoper.java https://github.com/openzipkin/zipkin-js/blob/master/packages/zipkin/src/tracer/index.js#L34 https://github.com/twitter/finagle/blob/develop/finagle-core/src/main/scala/com/twitter/finagle/tracing/Trace.scala#L161 https://github.com/opentracing-contrib/java-spanmanager/blob/master/src/main/java/io/opentracing/contrib/spanmanager/SpanManager.java

[codefromthecrypt]

@eirslett @kristofa what are your thoughts on this.. I think you have used different names in the past. what are your thoughts today? (also if you have any thoughts about the impl please shout)

[llinder]

I like the direction so far. SpanScoper does seem a bit unusual. If better names pop into my head I will share them :)

[codefromthecrypt]

PS I updated my text above to include 5 examples of similar code, hopefully to help hone in on best vocab

[codefromthecrypt]

the words: "scope context current" seem the more popular words in almost all the existing types that do stuff like this. The subject in the Tracer methods is usually a what we call here either a span or a trace context.

The design in this PR and the thing currently called SpanScoper actually acts on a TraceContext, which is more specific than Span. How about this?

interface CurrentTraceContext {
  @Nullable TraceContext get();
  TraceContextScope newScope(TraceContext traceContext);
}

[codefromthecrypt]

added a trace context interop example between this api and finagle cc @eirslett @klingerf @kristofa

[codefromthecrypt]

Added a commit that reuses code from https://github.com/DistributedTracing/continuation-local-storage-jvm/pulls to make a better base impl of thread storage. thanks to @eirslett cc @schlosna

[codefromthecrypt]

progress so far:

• started with a name which seems unpopular
• proved this works with log4j2 (or slf4j)
• proved this can do finagle interop

next steps:

• get feedback on esp on naming .. I personally like CurrentTraceContext
• integrate with real instrumentation

[llinder]

+1 for CurrentTraceContext

[codefromthecrypt]

+1 for CurrentTraceContext

cool.. I'll reshuffle commits over that until we find a better name

[tramchamploo]

+1 for CurrentTraceContext

[codefromthecrypt]

actually I'm going to revert the continuation-local based default impl as it is over-engineering for what we are using it for. For example, we don't yet search or take action on multiple contexts (such as clearing them). In that case, the static array is a bit distracting as it doesn't do anything. We can add it later or re-use a full implementation in a different PR.

[codefromthecrypt]

updated the code and PR description with the following.. please let me know if this is not good :)

---

The (power) user apis are located in the Tracer api:

• Span currentSpan - retrieves any span in scope or null
• Span nextSpan - creates a new child or trace if there's no current span
• SpanInScope withSpanInScope(span) - makes a span current and available to calls within the scope

The SPI for this is called CurrentTraceContext, and it can be implemented to support MDC integration or alternate span storage

• TraceContext get() - returns the current trace context or null
• Scope newScope(context) - creates a scope where calls to get will return the input

By default, CurrentTraceContext is implemented by a thread local, though there are tests to show it can be implemented by Finagle (or gRPC or otherwise).

[codefromthecrypt]

Added Tracer.nextSpan based on feedback from @reta #356 (comment)

[codefromthecrypt]

starting to revisit the PR to actually use this code. writing a test for currentSpan access via user code is interesting as it smoked out a bug in our approach of instrumenting apache hc https://github.com/openzipkin/brave/pull/350/files#r107617533

[codefromthecrypt]

in the latest work on #350, I had to place a span in scope in one callback and close it in another.

TL;DR; this is one of those scenarios where it would be nice to use try-with-resources, but the
actual library has no closure over user code. You have to set it in one callback and close it in another.

details..

Apache HttpClient includes a way to customize its HttpClientBuilder, which allows for some decoration. If you set a span in scope in the decorateProtocolExec callback, any interceptor
can use the "current span" without any special knowledge. For example, it can grab the trace ID
and add it as a different header (we have a test for this). The decorateMainExec callback gets
the span in scope attribute and closes it when finished tracing (as main exec is the real http call).

in simplified pseudocode

Result protocolExec(Request request) {
  Span span = tracer.nextSpan();
  // it is as if we started the try-with-resources here
  context.setAttribute(SpanInScope.class, tracer.withSpanInScope(next));
  return delegate.protocolExec(request);
}

Result mainExec(Request request) {
  Span span = tracer.currentSpan(); // works because it is set in scope!
  try {
    // do tracing work
    return delegate.protocolExec(request);
  } finally {
    // it is as if we ended the try-with-resources here
    context.getAttribute(SpanInScope.class).close();
  }
}

[kmaci3k]

I'm wondering how brave is related to opentracing. In opentracing website there is information that brave is an experimental bridge. How (and if) should it be used with opentracing ?

Nevermind, I found brave-opentracing project...

Is there any plan when those changes will be available in master ?

[codefromthecrypt]

OpenTracing aims to be like slf4j-api, where you plugin something like logback (or in this case brave) as its impl. Brave is a complete library as opposed to just an interface. This means we can do things like integration tests including reading back from zipkin. On the other hand, Brave is only written for Zipkin and does not include things zipkin doesn't support such as structured logging. One very important detail is that Brave is the de-facto java library for Zipkin. That's why you'll notice we support it and make sure it works. In general, you can either instrument with brave directly or via the experimental opentracing bridge (aka adapter). OpenTracing traditionally had no propagation api (way to get the current span). This made the original brave bridge awkward as brave had an in-process propagation api (thread binder) that was always used. That bridge has been redone recently, but is still a work in progress. OpenTracing also has work in progress in making propagation pluggable. This pull request exposes an api which would allow you to read and access the current span flexibly, whether that's stored in brave, finagle, grpc, or some other similar trace implementation.

[bogdandrutu]

Question:

Why not withSpan? Golang defined this model withX for their context API and I think having consistency between languages is a win for developers.

[codefromthecrypt]

Why not withSpan? Golang defined this model withX for their context API and I think having consistency between languages is a win for developers.

I don't think most java developers know the implementation choices of golang's context api. Since tracer isn't a context api, withSpan doesn't explain what's going on. Ex. withX is also a builder syntax. withSpanInScope says what's going to occur and also helps prepare users for the domain of problems it might imply (scoping) :)

[codefromthecrypt]

@kmaci3k I think you asked when this will land.. probably this weekend. I wanted to go back and implement all of the instrumentation on top before cementing in the api. I'm almost done with that.

So far, at least we'll need something to support executors.. maybe that's the only change to this PR.

[codefromthecrypt]

note implementations can completely replace any of these features below as they are non-final

[codefromthecrypt]

doing like this allows extenders to not rely on any brave internal classes if they have their own executorservice

[codefromthecrypt]

Added tests. will update README and do polishing. will merge and release in the next 24hrs

[kmaci3k]

If I understand correctly a developer who wants to trace asynchronous call should implement on their own a wrapper (similar to this one: https://github.com/spring-cloud/spring-cloud-sleuth/blob/master/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceCallable.java) using Brave API ?

[codefromthecrypt]

If I understand correctly a developer who wants to trace asynchronous call should implement on their own a wrapper (similar to this one: https://github.com/spring-cloud/spring-cloud-sleuth/ blob/master/spring-cloud-sleuth-core/src/main/java/org/ springframework/cloud/sleuth/TraceCallable.java) using Brave API ?

If your asynchronous framework uses java.util.concurrent stuff you can use CurrentTraceContext.wrap(callable) or .executorService helpers https://github.com/openzipkin/brave/tree/master/brave#setting-a-span-in-scope-via-custom-executors