-
Notifications
You must be signed in to change notification settings - Fork 888
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
Enable static functions to work with active span #924
Comments
One question. What are the scenarios in which you typically call |
The common scenario is adding new attributes or events to the active span. It could also be arguable that a new span should be created, at which point you would have a handle and no longer need to use get_current_span. But I have seen a strong preference for some individuals to add more attributes to an existing span, with reasoning being reducing the number of spans visible in a trace (less visual clutter). Another use case is those using an instrumentation library. You won't have access to the span directly, so you call get_current_span in a place where the current instrumentation has already set the span as active. |
One original motivation for supplying an instrumentation library name was to stop one by name from emitting telemetry. If the current span is used to do that, it would be worth considering if using the tracer for that by default has merits. |
This enters the subjective realm (although this whole issue is subjective), but I would find such methods a bit less intuitive, primarily since retrieving and object and modifying it is a common object-oriented pattern. I'm not sure if the intuition follows to retrieve a tracer, and modify the spans via indirect methods. That said, the approach would be about the same number of operations:
vs
|
My 2c: I think one should not need to provide instrumentation library name to get current existing span. It does not make any sense to require one. |
I think that opinion is valid, and I don't necessarily disagree, except for the "any sense" part: Consider an instrumentation that adds lots of attributes or a large attribute to a span. For example, there might be an instrumentation that adds the exception semantic conventions (including stacktrace) for every thrown exception to the currently active span. Or an instrumentation that adds an event to a span for every time an asyncronous tasks gets suspended/resumed, etc. It would then make sense to have that instrumentations use a separate tracer. |
I would strongly vote for the simplified experience. The named tracer approach is straightforward for instrumentation plugins, but can be awkward for application code. One approach might be to add a constant |
I would +1 a default tracer, although I think that's not strictly necessary for this experience. If we went that route, I would worry about the ramifications regarding instrumentations authored outside of core opentelemetry. It would be easy for app developer to:
Of course it can be rectified later and many would look at other instrumentation and notice the named tracer pattern. Thus why I'm a +1 |
Yeah, I agree that someone could forget, but it's an easy fix for all the reasons you mention. Just to clarify, that would solve the issue, correct? |
It should be enough if new spans are created through named tracers. Having all active-span manipulations traced through named tracers would be nice-to-have, but I think the approach in #923 is fine. Feels far better to me than having a default tracer in any case 😃 Although I don't think we have to decide here (#586 is orthogonal to this IMHO) |
What are you trying to achieve?
In languages such as Python, there exist static functions that can live at the package / module level. It would improve the user experience if consumers did not have to go through the work of creating an object to retrieve the current span:
Work to simplify this has already been implemented in Python as an experiment, and has worked quite well in terms of reducing boilerplate and confusion around the behavior of context.
Before:
After:
Under the hood, this simplification is possible because:
What did you expect to see?
The changes to the spec needed to enable a function at the module level are:
Additional context.
What about languages that don't allow static methods at the package level?
The optional nature of these changes allows languages where it is convenient to add span manipulation to the tracer directly to continue to work the same way.
However:
Languages such as Java may choose to make the active span behavior completely static and create static methods instead, avoiding creating an instance.
Related issues
The text was updated successfully, but these errors were encountered: