Set the scope_manager RFC to Test.

opentracing / specification

A place to document (and discuss) the OpenTracing specification. 🛑 This project is DEPRECATED! https://github.com/opentracing/specification/issues/163

http://opentracing.io/spec

Apache License 2.0

1.17k stars 182 forks source link

Set the scope_manager RFC to Test. #122

Closed carlosalberto closed 1 year ago

carlosalberto commented 6 years ago

Initial take on the ScopeManager RFC.

Notes:

Used 'thread or execution unit' in order to abstract things such as coroutines.
Mentioned Scopes are not thread-safe.
Mentioned the null the value (it hadn't been mentioned before in the Specification).
Light take on the recent start_active_span() vs start_active_scope() debate.
Used ScopeManager.active() (as opposed to ScopeManager.activeSpan()). Prone to change based on further discussion.

Please provide feedback ;)

@tedsuo @palazzem @pavolloffay @pglombardo @yurishkuro @jpkrohling @indrekj @mwear @felixbarny @cwe1ss @adriancole

indrekj commented 5 years ago

Maybe not directly related to this PR but anyway:

The doc explains the behavior of Scope / ScopeManager, but I feel like it's missing the why: Why is scope / scope manager needed? Why wouldn't Tracer#activate(span) :: span work instead? I've been experimenting with opentracing spec for elixir and the scope manager part is confusing to me atm. There's likely an answer in some old PR but it's hard to find it.

I've implemented the scope manager part in ruby zipkin OT and we've used it in production for some time now. So far I haven't encountered a use case where a simple activate(span) :: span & deactivate() without Scope container wouldn't work as well.

carlosalberto commented 5 years ago

Hey @indrekj @pavolloffay @palazzem @tylerbenson @tedsuo

I've updated the document with minor corrections and updating it based on the latest OT Java iteration (making some operations optional). Let me know.

carlosalberto commented 5 years ago

Hey @indrekj

Why is scope / scope manager needed?

We essentially the concept has been to separate the active Span handling from the Tracer, API-wise. Now, as we have iterated, the idea has been to provide shortcuts for the most common operations - but stuff is still expected to be in the ScopeManager instance ;)

carlosalberto commented 5 years ago

Hey all!

I think I've updated all the requested bits of this PR, and I think it's ready to go. If you have some time over the next days, let me know - else I will be merging it soon (remember it's still an ongoing effort and can be changed/updated later).