Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify the location of tracing's Context interaction. #1187

Conversation

carlosalberto
Copy link
Contributor

Clarifies the location of the tracing interaction with Context, i.e. we want to say that this logic can exist anywhere in the API, as long as it exists in a single place.

The mention of a TracingContextUtils class seemed like a too-specific suggestion, hence the removal (in Java we had this class, but ended up putting the Context interaction as static/instance Span directly).

@carlosalberto carlosalberto requested review from a team November 3, 2020 14:51
@carlosalberto carlosalberto added area:api Cross language API specification issue release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:context Related to the specification/context directory spec:trace Related to the specification/trace directory priority:p3 Lowest priority level labels Nov 3, 2020
specification/trace/api.md Outdated Show resolved Hide resolved
Co-authored-by: Christian Neumüller <christian+github@neumueller.me>
@carlosalberto
Copy link
Contributor Author

Actually, we may need to also remove the mention of static, as IIRC Ruby poked with the idea of keeping these in the Tracer as instance methods. @open-telemetry/ruby-approvers ? ;)

@Oberon00
Copy link
Member

Oberon00 commented Nov 3, 2020

I am against removing "static". I mean I'm open to improving the wording to make it more language neutral, but a Tracer instance MUST NOT be required to interact with the current span. This often leads to misunderstandings, such as that each named tracer manages it's own active span, or complaints such as "why do I need to provide an instrumentation library name to access the current span" (#586).

EDIT: I would be OK with allowing nonstatic methods on the TracerProvider.

specification/trace/api.md Outdated Show resolved Hide resolved
Bogdan Drutu and others added 2 commits November 4, 2020 12:31
Co-authored-by: Nikita Salnikov-Tarnovski <gnikem@gmail.com>
@bogdandrutu
Copy link
Member

@carlosalberto we can do a separate PR for the "static" if you feel strongly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Cross language API specification issue priority:p3 Lowest priority level release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:context Related to the specification/context directory spec:trace Related to the specification/trace directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants