Skip to content

Commit

Permalink
All named tracers now share the same context (#424)
Browse files Browse the repository at this point in the history
The conversation in open-telemetry/opentelemetry-specification#455 is
specifying that the tracer is no longer responsible for handling the setting
and getting of active spans. As such, named tracers would only be responsible
for creating new spans, but not for setting the active one.

This implies that there tracers do not have their own active spans.

In addition, there is a benefit of having a single active span, as it vastly
simplifies the process to modify the current span (no need to explicitly
retrieve the tracer responsible for getting the span).
  • Loading branch information
toumorokoshi authored Feb 20, 2020
1 parent 18edbbe commit f41f83d
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,4 @@
from opentelemetry.trace import INVALID_SPAN_CONTEXT, Span, SpanContext

_SPAN_CONTEXT_KEY = "extracted-span-context"
_SPAN_KEY = "current-span"


def get_span_key(tracer_source_id: Optional[str] = None) -> str:
key = _SPAN_KEY
if tracer_source_id is not None:
key = "{}-{}".format(key, tracer_source_id)
return key
SPAN_KEY = "current-span"
14 changes: 5 additions & 9 deletions opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from opentelemetry.sdk import util
from opentelemetry.sdk.util import BoundedDict, BoundedList
from opentelemetry.trace import SpanContext, sampling
from opentelemetry.trace.propagation import get_span_key
from opentelemetry.trace.propagation import SPAN_KEY
from opentelemetry.trace.status import Status, StatusCanonicalCode
from opentelemetry.util import time_ns, types

Expand Down Expand Up @@ -542,9 +542,7 @@ def use_span(
"""See `opentelemetry.trace.Tracer.use_span`."""
try:
context_snapshot = context_api.get_current()
context_api.set_current(
context_api.set_value(self.source.key, span)
)
context_api.set_current(context_api.set_value(SPAN_KEY, span))
try:
yield span
finally:
Expand Down Expand Up @@ -577,9 +575,6 @@ def __init__(
sampler: sampling.Sampler = trace_api.sampling.ALWAYS_ON,
shutdown_on_exit: bool = True,
):
# TODO: How should multiple TracerSources behave? Should they get their own contexts?
# This could be done by adding `str(id(self))` to the slot name.
self.key = get_span_key(tracer_source_id=str(id(self)))
self._active_span_processor = MultiSpanProcessor()
self.sampler = sampler
self._atexit_handler = None
Expand All @@ -601,8 +596,9 @@ def get_tracer(
),
)

def get_current_span(self) -> Span:
return context_api.get_value(self.key) # type: ignore
@staticmethod
def get_current_span() -> Span:
return context_api.get_value(SPAN_KEY) # type: ignore

def add_span_processor(self, span_processor: SpanProcessor) -> None:
"""Registers a new :class:`SpanProcessor` for this `TracerSource`.
Expand Down
16 changes: 16 additions & 0 deletions opentelemetry-sdk/tests/trace/test_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,22 @@ def test_span_processor_for_source(self):
span2.span_processor, tracer_source._active_span_processor
)

def test_get_current_span_multiple_tracers(self):
"""In the case where there are multiple tracers,
get_current_span will return the same active span
for both tracers.
"""
tracer_1 = new_tracer()
tracer_2 = new_tracer()
root = tracer_1.start_span("root")
with tracer_1.use_span(root, True):
self.assertIs(tracer_1.get_current_span(), root)
self.assertIs(tracer_2.get_current_span(), root)

# outside of the loop, both should not reference a span.
self.assertIs(tracer_1.get_current_span(), None)
self.assertIs(tracer_2.get_current_span(), None)

def test_start_span_implicit(self):
tracer = new_tracer()

Expand Down

0 comments on commit f41f83d

Please sign in to comment.