Clarify context interaction for trace module #1063
Conversation
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
bogdandrutu
added
area:api
Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com>
Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com>
Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
|
@arminru More important than discouraging overridability would be to specify that this functionality must be implemented in the API. |
|
Ping @bogdandrutu ;) |
Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>
| All the above functionalities operate solely on the context API, and they MAY be | ||
| exposed as static methods on the trace module, as static methods on a class | ||
| inside the trace module (it MAY be named `TracingContextUtilities`), or on the | ||
| [`Tracer`](#tracer) class. This functionality SHOULD be fully implemented in the |
There was a problem hiding this comment.
Breaking news - there seems to be a lot of support for removing the mention of Tracer here. What do you think?
There was a problem hiding this comment.
The spec here does not say that this should be on the tracer, and if it is, it says it should be static.
There was a problem hiding this comment.
Ah static is written twice, but not a third time. So with this wording, static does not apply to Tracer I think and I didn't read it that way. If it's also static it makes sense to me.
There was a problem hiding this comment.
What one language decided to do is not breaking news :) In the Ruby implementation they want to work directly with Tracer (cc @mwear). The options are given above, and the language is free to implement them either way they want.
@bogdandrutu to just clarify, we still allow this to exist as instance methods in Tracer, correct?
There was a problem hiding this comment.
I think we should discourage an instance method on tracer, as this has been known to cause confusion and has caused people to call for allowing an unnamed tracer just to access the current span.
There was a problem hiding this comment.
I think we should discourage an instance method on tracer
I agree.
There was a problem hiding this comment.
I think the current text does exactly that, and in edge cases where that cannot be avoided, it allows to be an instance method.
There was a problem hiding this comment.
open-telemetry/opentelemetry-ruby#425 (comment)
TL;DR: I agree, including for Ruby.
|
reopened to reset EasyCLA |
Signed-off-by: Bogdan Drutu bogdandrutu@gmail.com
Fixes #1019