Direct import of global tracer

[felixfbecker]

The documentation in global_tracer says:

// Allows direct importing/requiring of the global tracer:
//
// let globalTracer = require('opentracing/global');
//      OR
// import globalTracer from 'opentracing/global';

but opentracing/global doesn't actually exist. Is there any reason we have to call a function to get the global tracer, instead of just importing it? And do we actually need a Singleton tracer, can't we just pass Spans around?

[bcronin]

Is there any reason we have to call a function to get the global tracer, instead of just importing it?

An import happens by value so it's unclear to me how you could set the global tracer in one file and then import it in another file.

You'd have to return a object with a reference field to the global tracer, which is certainly possible but slightly unconventional. That said, I also see having to call a function also feels a bit unconventional (for some definition of that word).

And do we actually need a Singleton tracer, can't we just pass Spans around?

Sometimes you may have to "disconnected" parts of the code (disconnected in the sense that they don't pass spans between them) where you might still want to create spans via the same tracer object. That's one reason. If the code base were passing spans everywhere, I agree, there'd be no need for the Singleton (just call span.tracer() instead if you need to).

[tedsuo]

Hi @felixfbecker, we need a singleton because there is decoupleing between tracer initialization and consumption. Increasingly we are moving to a singleton approach in general – since some libraries must rely on it, applications would do well to always initialize it. Also, as Ben says, it reduces the number of API's you have to mutate. You shouldn't need to manually pass a tracer around.

In the future, we'd like to stop having to manually pass spans around as well, and add a concept of an Active Span that the tracer manages for each context, so that you can start a child span without having a reference to the parent. That further decreases the amount of API mutation, and allows more libraries to integrate with open tracing. However, it's not clear what an efficient implementation of this would look like in JS. See this Java discussion, or the original spec issue (Warning: Odyssey-length discussion):
opentracing/opentracing-java#115
opentracing/specification#23

[felixfbecker]

Isn't that decoupling a bit worrying? What about spans created between consumption and initialisation? Explicit passing circumvents this.

It would work with continuation-local-storage, but that is super slow. I think passing stuff around explicitly is the only way to go for having acceptable performance for production. You only really need to pass a Tracer instance to libraries, only spans, because you can do span.tracer(). A Tracer instance would only be needed at the point where you create the root span or where the only parent comes from an inbound RPC call.

An import happens by value so it's unclear to me how you could set the global tracer in one file and then import it in another file.

I don't know if it works in vanilla ES6 (there is no runtime with ES6 modules yet after all) but it works if you compile TS to CommonJS, because the exports object is an object.
It's definitely a bit smelly, but you already use it for noop: https://sourcegraph.com/github.com/opentracing/opentracing-javascript@master/-/blob/src/noop.js

[bcronin]

I don't know if it works in vanilla ES6

I don't think it does work in vanilla ES6 according to the spec. You're right though, if we're compiling down to CommonJS anyway and there are no fully compliant runtimes being used anyway, that approach will work for the near future - possibly for a very long time. However, it makes me a wary to construct things that we suspect might not work with future runtimes that are compliant. I would hate to think that, say a year from now, due to upgrades in various runtimes and tooling that a consumer of the library might need to change the existing instrumentation code. I'd like to do everything we can to avoid consumers of the library from having to change their code.

As for the link to the existing code, I didn't realize the code was "violating" that rule already (even though I wrote that code, oops)! One key different here though (unless I misread the code), is that's "an implementation detail" that's not exposed to the consumer of the library (i.e. that could be changed without external API impact). Changing how the external consumer of the library accesses the global tracer actually has API impact, whereas this does not. Or at least that's my understanding after a cursory look.