Skip to content

Commit

Permalink
api: adding trace.get_current_span (open-telemetry#552)
Browse files Browse the repository at this point in the history
The span context is no longer coupled with the tracer itself.
As such, providing a get_current_span method bound to the
trace api module rather than a specific tracer is semantically
correct, and removes a hurdle where someone who wants to retrieve
the current trace would have to create a tracer to do so.

renaming and exporting get_span_in_context to get_current_span,
as the intention of the API is similar, and reduces unneeded
aliasing and duplication.

set_span_in_context is not renamed, as set_current_span would have
implied that the span would then be active in the default context,
which is only true after attaching the resulting context returned
by set_span_in_context. Keeping that name at least implies some
asymmetric behavior from get_current_span.

After discussion in the SIG, we decided to remove the
legacy get_current_span APIs from Tracer and TracerProvider
to reduce long-term confusion of how to idiomatically retrieve
the span.

Co-authored-by: alrex <aboten@lightstep.com>
Co-authored-by: Hector Hernandez <39923391+hectorhdzg@users.noreply.github.com>
Co-authored-by: Leighton Chen <lechen@microsoft.com>
Co-authored-by: Diego Hurtado <ocelotl@users.noreply.github.com>
Co-authored-by: Mauricio Vásquez <mauricio@kinvolk.io>
Co-authored-by: Andrew Xue <aaxue@google.com>
Co-authored-by: Cheng-Lung Sung <clsung@gmail.com>
  • Loading branch information
8 people committed Jun 11, 2020
1 parent a514d21 commit 0316c1f
Show file tree
Hide file tree
Showing 24 changed files with 522 additions and 448 deletions.
3 changes: 2 additions & 1 deletion docs/api/trace.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ Submodules

trace.sampling
trace.status
trace.span

Module contents
---------------

.. automodule:: opentelemetry.trace
.. automodule:: opentelemetry.trace
7 changes: 7 additions & 0 deletions docs/api/trace.span.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
opentelemetry.trace.span
========================

.. automodule:: opentelemetry.trace.span
:members:
:undoc-members:
:show-inheritance:
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ async def on_request_start(
)

trace_config_ctx.token = context_api.attach(
trace.propagation.set_span_in_context(trace_config_ctx.span)
trace.set_span_in_context(trace_config_ctx.span)
)

propagators.inject(type(params.headers).__setitem__, params.headers)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@

from opentelemetry import trace
from opentelemetry.context import Context
from opentelemetry.trace.propagation import (
get_span_from_context,
set_span_in_context,
)
from opentelemetry.trace import get_current_span, set_span_in_context
from opentelemetry.trace.propagation.httptextformat import (
Getter,
HTTPTextFormat,
Expand Down Expand Up @@ -88,7 +85,7 @@ def inject(
carrier: HTTPTextFormatT,
context: typing.Optional[Context] = None,
) -> None:
span = get_span_from_context(context=context)
span = get_current_span(context)
sampled = (trace.TraceFlags.SAMPLED & span.context.trace_flags) != 0
set_in_carrier(
carrier, self.TRACE_ID_KEY, format_trace_id(span.context.trace_id),
Expand Down
15 changes: 6 additions & 9 deletions ext/opentelemetry-ext-datadog/tests/test_datadog_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@
from opentelemetry import trace as trace_api
from opentelemetry.ext.datadog import constants, propagator
from opentelemetry.sdk import trace
from opentelemetry.trace.propagation import (
get_span_from_context,
set_span_in_context,
)
from opentelemetry.trace import get_current_span, set_span_in_context

FORMAT = propagator.DatadogFormat()

Expand All @@ -45,7 +42,7 @@ def test_malformed_headers(self):
"""Test with no Datadog headers"""
malformed_trace_id_key = FORMAT.TRACE_ID_KEY + "-x"
malformed_parent_id_key = FORMAT.PARENT_ID_KEY + "-x"
context = get_span_from_context(
context = get_current_span(
FORMAT.extract(
get_as_list,
{
Expand All @@ -66,7 +63,7 @@ def test_missing_trace_id(self):
}

ctx = FORMAT.extract(get_as_list, carrier)
span_context = get_span_from_context(ctx).get_context()
span_context = get_current_span(ctx).get_context()
self.assertEqual(span_context.trace_id, trace_api.INVALID_TRACE_ID)

def test_missing_parent_id(self):
Expand All @@ -76,12 +73,12 @@ def test_missing_parent_id(self):
}

ctx = FORMAT.extract(get_as_list, carrier)
span_context = get_span_from_context(ctx).get_context()
span_context = get_current_span(ctx).get_context()
self.assertEqual(span_context.span_id, trace_api.INVALID_SPAN_ID)

def test_context_propagation(self):
"""Test the propagation of Datadog headers."""
parent_context = get_span_from_context(
parent_context = get_current_span(
FORMAT.extract(
get_as_list,
{
Expand Down Expand Up @@ -138,7 +135,7 @@ def test_context_propagation(self):

def test_sampling_priority_auto_reject(self):
"""Test sampling priority rejected."""
parent_context = get_span_from_context(
parent_context = get_current_span(
FORMAT.extract(
get_as_list,
{
Expand Down
16 changes: 5 additions & 11 deletions ext/opentelemetry-ext-grpc/tests/test_server_interceptor.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,13 @@ def test_span_lifetime(self):
"""Check that the span is active for the duration of the call."""

interceptor = server_interceptor()
tracer = self.tracer_provider.get_tracer(__name__)

# To capture the current span at the time the handler is called
active_span_in_handler = None

def handler(request, context):
nonlocal active_span_in_handler
# The current span is shared among all the tracers.
active_span_in_handler = tracer.get_current_span()
active_span_in_handler = trace.get_current_span()
return b""

server = grpc.server(
Expand All @@ -112,13 +110,13 @@ def handler(request, context):
port = server.add_insecure_port("[::]:0")
channel = grpc.insecure_channel("localhost:{:d}".format(port))

active_span_before_call = tracer.get_current_span()
active_span_before_call = trace.get_current_span()
try:
server.start()
channel.unary_unary("")(b"")
finally:
server.stop(None)
active_span_after_call = tracer.get_current_span()
active_span_after_call = trace.get_current_span()

self.assertIsNone(active_span_before_call)
self.assertIsNone(active_span_after_call)
Expand All @@ -128,15 +126,13 @@ def handler(request, context):
def test_sequential_server_spans(self):
"""Check that sequential RPCs get separate server spans."""

tracer = self.tracer_provider.get_tracer(__name__)

interceptor = server_interceptor()

# Capture the currently active span in each thread
active_spans_in_handler = []

def handler(request, context):
active_spans_in_handler.append(tracer.get_current_span())
active_spans_in_handler.append(trace.get_current_span())
return b""

server = grpc.server(
Expand Down Expand Up @@ -175,8 +171,6 @@ def test_concurrent_server_spans(self):
context.
"""

tracer = self.tracer_provider.get_tracer(__name__)

interceptor = server_interceptor()

# Capture the currently active span in each thread
Expand All @@ -185,7 +179,7 @@ def test_concurrent_server_spans(self):

def handler(request, context):
latch()
active_spans_in_handler.append(tracer.get_current_span())
active_spans_in_handler.append(trace.get_current_span())
return b""

server = grpc.server(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,7 @@
from opentelemetry import propagators
from opentelemetry.ext.opentracing_shim import util
from opentelemetry.ext.opentracing_shim.version import __version__
from opentelemetry.trace import DefaultSpan
from opentelemetry.trace.propagation import (
get_span_from_context,
set_span_in_context,
)
from opentelemetry.trace import DefaultSpan, set_span_in_context

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -473,7 +469,7 @@ def active(self):
shim and is likely to be handled in future versions.
"""

span = self._tracer.unwrap().get_current_span()
span = trace_api.get_current_span()
if span is None:
return None

Expand Down Expand Up @@ -703,6 +699,10 @@ def get_as_list(dict_object, key):

propagator = propagators.get_global_httptextformat()
ctx = propagator.extract(get_as_list, carrier)
otel_context = get_span_from_context(ctx).get_context()
span = trace_api.get_current_span(ctx)
if span is not None:
otel_context = span.get_context()
else:
otel_context = trace_api.INVALID_SPAN_CONTEXT

return SpanContextShim(otel_context)
19 changes: 18 additions & 1 deletion ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@
from opentelemetry import propagators, trace
from opentelemetry.ext.opentracing_shim import util
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.test.mock_httptextformat import MockHTTPTextFormat
from opentelemetry.test.mock_httptextformat import (
MockHTTPTextFormat,
NOOPHTTPTextFormat,
)


class TestShim(TestCase):
Expand Down Expand Up @@ -515,6 +518,20 @@ def test_extract_http_headers(self):
self.assertEqual(ctx.unwrap().trace_id, 1220)
self.assertEqual(ctx.unwrap().span_id, 7478)

def test_extract_empty_context_returns_invalid_context(self):
"""In the case where the propagator cannot extract a
SpanContext, extract should return and invalid span context.
"""
_old_propagator = propagators.get_global_httptextformat()
propagators.set_global_httptextformat(NOOPHTTPTextFormat())
try:
carrier = {}

ctx = self.shim.extract(opentracing.Format.HTTP_HEADERS, carrier)
self.assertEqual(ctx.unwrap(), trace.INVALID_SPAN_CONTEXT)
finally:
propagators.set_global_httptextformat(_old_propagator)

def test_extract_text_map(self):
"""Test `extract()` method for Format.TEXT_MAP."""

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ def hello():
from opentelemetry import context, propagators, trace
from opentelemetry.ext.wsgi.version import __version__
from opentelemetry.instrumentation.utils import http_status_to_canonical_code
from opentelemetry.trace.propagation import get_span_from_context
from opentelemetry.trace.status import Status, StatusCanonicalCode

_HTTP_VERSION_PREFIX = "HTTP/"
Expand Down
2 changes: 2 additions & 0 deletions opentelemetry-api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
([#751](https://github.com/open-telemetry/opentelemetry-python/pull/751))
- Rename Measure to ValueRecorder in metrics
([#761](https://github.com/open-telemetry/opentelemetry-python/pull/761))
- Adding trace.get_current_span, Removing Tracer.get_current_span
([#552](https://github.com/open-telemetry/opentelemetry-python/pull/552))
- Rename Observer to ValueObserver
([#764](https://github.com/open-telemetry/opentelemetry-python/pull/764))

Expand Down
Loading

0 comments on commit 0316c1f

Please sign in to comment.