opentracing-cpp icon indicating copy to clipboard operation
opentracing-cpp copied to clipboard

ScopeManager with default thread_local implementation

Open eyjohn opened this issue 6 years ago • 0 comments

PR for https://github.com/opentracing/opentracing-cpp/issues/97 to provide span propagation using the ScopeManager approach.

I've based this on the Java version https://github.com/opentracing/opentracing-java/blob/master/opentracing-api/src/main/java/io/opentracing/ScopeManager.java including making it part of the Tracer (originally I had planned to keep it a stand-alone utility).

Most of the design decisions are in-line with the Java interface, the following are probably worth highlighting:

1. ScopeManagers use shared_ptr rather than unique_ptr/reference

The rationale here is that the nature of sharing a Span over a scope, means that multiple downstream consumers may access it, OR even take ownership of it. Therefore a conversion to shared_ptr is required before a ScopeManager is used

2. Integration with Tracer

In-line with the Java design, the ScopeManager is accessible via the Tracer::ScopeManager() method, which can be overriden by tracers.

For backwards compatibility, I've had to add some logic to the Tracer interface and unfortunately it is no longer a pure interface.

3. Scope

I've added a "guard-like" Scope object to track the ScopeManager::Activate scope but kept the constraints quite open (very difficult to enforce at interface level), e.g. avoiding moving or storing it outside of the stack.

The specific implementations of ScopeManagers may have different constraints so I've left it to the ScopeManager documentation to provide the exact behaviour (such as for the thread local one, all construction/destruction must be in the same thread).


I hope you don't mind that I've delivered a complete PR prior to major discussion, I was hoping given that this is very similar to the Java one that there would be less design concerns.

On a final note, can you please help with what changes are needed to pass the PR validation?

Thanks

eyjohn avatar Dec 25 '19 23:12 eyjohn