diff --git a/.pylintrc b/.pylintrc index b23bef8e66..ea54273868 100644 --- a/.pylintrc +++ b/.pylintrc @@ -441,7 +441,8 @@ exclude-protected=_asdict, _fields, _replace, _source, - _make + _make, + _Span # List of valid names for the first argument in a class method. valid-classmethod-first-arg=cls diff --git a/exporter/opentelemetry-exporter-datadog/tests/test_datadog_exporter.py b/exporter/opentelemetry-exporter-datadog/tests/test_datadog_exporter.py index e1973e5ac6..27b4d0fa03 100644 --- a/exporter/opentelemetry-exporter-datadog/tests/test_datadog_exporter.py +++ b/exporter/opentelemetry-exporter-datadog/tests/test_datadog_exporter.py @@ -188,7 +188,7 @@ def test_translate_to_datadog(self): instrumentation_info = InstrumentationInfo(__name__, "0") otel_spans = [ - trace.Span( + trace._Span( name=span_names[0], context=span_context, parent=parent_context, @@ -196,14 +196,14 @@ def test_translate_to_datadog(self): instrumentation_info=instrumentation_info, resource=Resource({}), ), - trace.Span( + trace._Span( name=span_names[1], context=parent_context, parent=None, instrumentation_info=instrumentation_info, resource=resource_without_service, ), - trace.Span( + trace._Span( name=span_names[2], context=other_context, parent=None, @@ -289,7 +289,7 @@ def test_export(self): is_remote=False, ) - test_span = trace.Span("test_span", context=context) + test_span = trace._Span("test_span", context=context) test_span.start() test_span.end() @@ -492,8 +492,8 @@ def test_origin(self): ), ) - root_span = trace.Span(name="root", context=context, parent=None) - child_span = trace.Span( + root_span = trace._Span(name="root", context=context, parent=None) + child_span = trace._Span( name="child", context=context, parent=root_span ) root_span.start() @@ -528,7 +528,7 @@ def test_sampling_rate(self): ) sampler = sampling.TraceIdRatioBased(0.5) - span = trace.Span( + span = trace._Span( name="sampled", context=context, parent=None, sampler=sampler ) span.start() diff --git a/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py b/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py index 994cac2d60..3c045539fd 100644 --- a/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py +++ b/exporter/opentelemetry-exporter-datadog/tests/test_datadog_format.py @@ -104,7 +104,7 @@ def test_context_propagation(self): ) self.assertTrue(parent_context.is_remote) - child = trace.Span( + child = trace._Span( "child", trace_api.SpanContext( parent_context.trace_id, @@ -149,7 +149,7 @@ def test_sampling_priority_auto_reject(self): self.assertEqual(parent_context.trace_flags, constants.AUTO_REJECT) - child = trace.Span( + child = trace._Span( "child", trace_api.SpanContext( parent_context.trace_id, diff --git a/exporter/opentelemetry-exporter-jaeger/tests/test_jaeger_exporter.py b/exporter/opentelemetry-exporter-jaeger/tests/test_jaeger_exporter.py index c787081422..bb4920f7b3 100644 --- a/exporter/opentelemetry-exporter-jaeger/tests/test_jaeger_exporter.py +++ b/exporter/opentelemetry-exporter-jaeger/tests/test_jaeger_exporter.py @@ -36,7 +36,7 @@ def setUp(self): is_remote=False, ) - self._test_span = trace.Span("test_span", context=context) + self._test_span = trace._Span("test_span", context=context) self._test_span.start() self._test_span.end() @@ -187,7 +187,7 @@ def test_translate_to_jaeger(self): ] otel_spans = [ - trace.Span( + trace._Span( name=span_names[0], context=span_context, parent=parent_context, @@ -195,10 +195,12 @@ def test_translate_to_jaeger(self): links=(link,), kind=trace_api.SpanKind.CLIENT, ), - trace.Span( + trace._Span( name=span_names[1], context=parent_context, parent=None ), - trace.Span(name=span_names[2], context=other_context, parent=None), + trace._Span( + name=span_names[2], context=other_context, parent=None + ), ] otel_spans[0].start(start_time=start_times[0]) diff --git a/exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_trace_exporter.py b/exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_trace_exporter.py index d07fd053b4..3721139efc 100644 --- a/exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_trace_exporter.py +++ b/exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_trace_exporter.py @@ -120,7 +120,7 @@ def test_translate_to_collector(self): link_2 = trace_api.Link( context=parent_context, attributes=link_attributes ) - span_1 = trace.Span( + span_1 = trace._Span( name="test1", context=span_context, parent=parent_context, @@ -128,13 +128,13 @@ def test_translate_to_collector(self): links=(link_1,), kind=trace_api.SpanKind.CLIENT, ) - span_2 = trace.Span( + span_2 = trace._Span( name="test2", context=parent_context, parent=None, kind=trace_api.SpanKind.SERVER, ) - span_3 = trace.Span( + span_3 = trace._Span( name="test3", context=other_context, links=(link_2,), @@ -302,7 +302,7 @@ def test_export(self): trace_flags=TraceFlags(TraceFlags.SAMPLED), ) otel_spans = [ - trace.Span( + trace._Span( name="test1", context=span_context, kind=trace_api.SpanKind.CLIENT, diff --git a/exporter/opentelemetry-exporter-otlp/tests/test_otlp_trace_exporter.py b/exporter/opentelemetry-exporter-otlp/tests/test_otlp_trace_exporter.py index dcc0239798..e8c449c9df 100644 --- a/exporter/opentelemetry-exporter-otlp/tests/test_otlp_trace_exporter.py +++ b/exporter/opentelemetry-exporter-otlp/tests/test_otlp_trace_exporter.py @@ -45,7 +45,7 @@ from opentelemetry.proto.trace.v1.trace_pb2 import Span as OTLPSpan from opentelemetry.proto.trace.v1.trace_pb2 import Status from opentelemetry.sdk.resources import Resource as SDKResource -from opentelemetry.sdk.trace import Span, TracerProvider +from opentelemetry.sdk.trace import TracerProvider, _Span from opentelemetry.sdk.trace.export import ( SimpleExportSpanProcessor, SpanExportResult, @@ -123,7 +123,7 @@ def setUp(self): type(event_mock).name = PropertyMock(return_value="a") - self.span = Span( + self.span = _Span( "a", context=Mock( **{ diff --git a/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py index 6120f2dd6e..8d6d8bb860 100644 --- a/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py @@ -42,7 +42,7 @@ def setUp(self): is_remote=False, ) - self._test_span = trace.Span("test_span", context=context) + self._test_span = trace._Span("test_span", context=context) self._test_span.start() self._test_span.end() @@ -154,18 +154,22 @@ def test_export(self): ) otel_spans = [ - trace.Span( + trace._Span( name=span_names[0], context=span_context, parent=parent_context, events=(event,), links=(link,), ), - trace.Span( + trace._Span( name=span_names[1], context=parent_context, parent=None ), - trace.Span(name=span_names[2], context=other_context, parent=None), - trace.Span(name=span_names[3], context=other_context, parent=None), + trace._Span( + name=span_names[2], context=other_context, parent=None + ), + trace._Span( + name=span_names[3], context=other_context, parent=None + ), ] otel_spans[0].start(start_time=start_times[0]) @@ -328,7 +332,7 @@ def test_zero_padding(self): trace_id, parent_id, is_remote=False ) - otel_span = trace.Span( + otel_span = trace._Span( name=span_names[0], context=span_context, parent=parent_context, ) @@ -387,7 +391,7 @@ def test_max_tag_length(self): trace_flags=TraceFlags(TraceFlags.SAMPLED), ) - span = trace.Span(name="test-span", context=span_context,) + span = trace._Span(name="test-span", context=span_context,) span.start() span.resource = Resource({}) diff --git a/instrumentation/opentelemetry-instrumentation-celery/tests/test_utils.py b/instrumentation/opentelemetry-instrumentation-celery/tests/test_utils.py index 0842890e89..3df0f445b3 100644 --- a/instrumentation/opentelemetry-instrumentation-celery/tests/test_utils.py +++ b/instrumentation/opentelemetry-instrumentation-celery/tests/test_utils.py @@ -42,7 +42,7 @@ def test_set_attributes_from_context(self): "routing_key": "celery", } - span = trace.Span("name", mock.Mock(spec=trace_api.SpanContext)) + span = trace._Span("name", mock.Mock(spec=trace_api.SpanContext)) utils.set_attributes_from_context(span, context) self.assertEqual( @@ -78,7 +78,7 @@ def test_set_attributes_from_context_empty_keys(self): "retries": 0, } - span = trace.Span("name", mock.Mock(spec=trace_api.SpanContext)) + span = trace._Span("name", mock.Mock(spec=trace_api.SpanContext)) utils.set_attributes_from_context(span, context) self.assertEqual(len(span.attributes), 0) @@ -99,7 +99,7 @@ def fn_task(): # propagate and retrieve a Span task_id = "7c6731af-9533-40c3-83a9-25b58f0d837f" - span = trace.Span("name", mock.Mock(spec=trace_api.SpanContext)) + span = trace._Span("name", mock.Mock(spec=trace_api.SpanContext)) utils.attach_span(fn_task, task_id, span) span_after = utils.retrieve_span(fn_task, task_id) self.assertIs(span, span_after) @@ -112,7 +112,7 @@ def fn_task(): # propagate a Span task_id = "7c6731af-9533-40c3-83a9-25b58f0d837f" - span = trace.Span("name", mock.Mock(spec=trace_api.SpanContext)) + span = trace._Span("name", mock.Mock(spec=trace_api.SpanContext)) utils.attach_span(fn_task, task_id, span) # delete the Span utils.detach_span(fn_task, task_id) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index d283498612..0f366f2c07 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -12,6 +12,8 @@ ([#1153](https://github.com/open-telemetry/opentelemetry-python/pull/1153)) - Renaming metrics Batcher to Processor ([#1203](https://github.com/open-telemetry/opentelemetry-python/pull/1203)) +- Protect access to Span implementation + ([#1188](https://github.com/open-telemetry/opentelemetry-python/pull/1188)) ## Version 0.13b0 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 0134ec7e77..57f1fb6b77 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -357,6 +357,11 @@ class Span(trace_api.Span): this `Span`. """ + def __new__(cls, *args, **kwargs): + if cls is Span: + raise TypeError("Span must be instantiated via a tracer.") + return super().__new__(cls) + def __init__( self, name: str, @@ -663,6 +668,13 @@ def record_exception(self, exception: Exception) -> None: ) +class _Span(Span): + """Protected implementation of `opentelemetry.trace.Span`. + + This constructor should only be used internally. + """ + + class Tracer(trace_api.Tracer): """See `opentelemetry.trace.Tracer`. @@ -748,7 +760,7 @@ def start_span( # pylint: disable=too-many-locals # Only record if is_recording() is true if sampling_result.decision.is_recording(): # pylint:disable=protected-access - span = Span( + span = _Span( name=name, context=context, parent=parent_context, diff --git a/opentelemetry-sdk/tests/trace/export/test_export.py b/opentelemetry-sdk/tests/trace/export/test_export.py index 8c43f731b2..4ab3a3aa41 100644 --- a/opentelemetry-sdk/tests/trace/export/test_export.py +++ b/opentelemetry-sdk/tests/trace/export/test_export.py @@ -121,7 +121,7 @@ def test_simple_span_processor_not_sampled(self): def _create_start_and_end_span(name, span_processor): - span = trace.Span( + span = trace._Span( name, trace_api.SpanContext( 0xDEADBEEF, @@ -403,7 +403,7 @@ def test_export(self): # pylint: disable=no-self-use # Mocking stdout interferes with debugging and test reporting, mock on # the exporter instance instead. - span = trace.Span("span name", trace_api.INVALID_SPAN_CONTEXT) + span = trace._Span("span name", trace_api.INVALID_SPAN_CONTEXT) with mock.patch.object(exporter, "out") as mock_stdout: exporter.export([span]) mock_stdout.write.assert_called_once_with(span.to_json() + os.linesep) @@ -421,5 +421,5 @@ def formatter(span): # pylint: disable=unused-argument exporter = export.ConsoleSpanExporter( out=mock_stdout, formatter=formatter ) - exporter.export([trace.Span("span name", mock.Mock())]) + exporter.export([trace._Span("span name", mock.Mock())]) mock_stdout.write.assert_called_once_with(mock_span_str) diff --git a/opentelemetry-sdk/tests/trace/export/test_in_memory_span_exporter.py b/opentelemetry-sdk/tests/trace/export/test_in_memory_span_exporter.py index 4925bdb182..e45e8ee8fd 100644 --- a/opentelemetry-sdk/tests/trace/export/test_in_memory_span_exporter.py +++ b/opentelemetry-sdk/tests/trace/export/test_in_memory_span_exporter.py @@ -61,7 +61,7 @@ def test_shutdown(self): self.assertEqual(len(span_list), 3) def test_return_code(self): - span = trace.Span("name", mock.Mock(spec=trace_api.SpanContext)) + span = trace._Span("name", mock.Mock(spec=trace_api.SpanContext)) span_list = (span,) memory_exporter = InMemorySpanExporter() diff --git a/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py b/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py index 07b3010087..b825ea1a75 100644 --- a/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py +++ b/opentelemetry-sdk/tests/trace/propagation/test_b3_format.py @@ -33,8 +33,8 @@ def get_child_parent_new_carrier(old_carrier): ctx = FORMAT.extract(get_as_list, old_carrier) parent_context = trace_api.get_current_span(ctx).get_context() - parent = trace.Span("parent", parent_context) - child = trace.Span( + parent = trace._Span("parent", parent_context) + child = trace._Span( "child", trace_api.SpanContext( parent_context.trace_id, diff --git a/opentelemetry-sdk/tests/trace/test_implementation.py b/opentelemetry-sdk/tests/trace/test_implementation.py index 4582b06957..7e6ede1ae0 100644 --- a/opentelemetry-sdk/tests/trace/test_implementation.py +++ b/opentelemetry-sdk/tests/trace/test_implementation.py @@ -40,8 +40,8 @@ def test_tracer(self): def test_span(self): with self.assertRaises(Exception): # pylint: disable=no-value-for-parameter - span = trace.Span() + span = trace._Span() - span = trace.Span("name", INVALID_SPAN_CONTEXT) + span = trace._Span("name", INVALID_SPAN_CONTEXT) self.assertEqual(span.get_context(), INVALID_SPAN_CONTEXT) self.assertIs(span.is_recording(), True) diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 8c5544578d..2ea43659a8 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -404,13 +404,18 @@ def test_span_context_remote_flag(self): span = tracer.start_span("foo") self.assertFalse(span.context.is_remote) + def test_disallow_direct_span_creation(self): + with self.assertRaises(TypeError): + # pylint: disable=abstract-class-instantiated + trace.Span("name", mock.Mock(spec=trace_api.SpanContext)) + class TestSpan(unittest.TestCase): def setUp(self): self.tracer = new_tracer() def test_basic_span(self): - span = trace.Span("name", mock.Mock(spec=trace_api.SpanContext)) + span = trace._Span("name", mock.Mock(spec=trace_api.SpanContext)) self.assertEqual(span.name, "name") def test_attributes(self): @@ -657,7 +662,7 @@ def test_update_name(self): def test_start_span(self): """Start twice, end a not started""" - span = trace.Span("name", mock.Mock(spec=trace_api.SpanContext)) + span = trace._Span("name", mock.Mock(spec=trace_api.SpanContext)) # end not started span self.assertRaises(RuntimeError, span.end) @@ -683,7 +688,7 @@ def test_start_span(self): def test_span_override_start_and_end_time(self): """Span sending custom start_time and end_time values""" - span = trace.Span("name", mock.Mock(spec=trace_api.SpanContext)) + span = trace._Span("name", mock.Mock(spec=trace_api.SpanContext)) start_time = 123 span.start(start_time) self.assertEqual(start_time, span.start_time) @@ -782,7 +787,7 @@ def error_status_test(context): ) def test_record_exception(self): - span = trace.Span("name", mock.Mock(spec=trace_api.SpanContext)) + span = trace._Span("name", mock.Mock(spec=trace_api.SpanContext)) try: raise ValueError("invalid") except ValueError as err: @@ -922,7 +927,7 @@ def test_to_json(self): is_remote=False, trace_flags=trace_api.TraceFlags(trace_api.TraceFlags.SAMPLED), ) - span = trace.Span("span-name", context) + span = trace._Span("span-name", context) span.resource = Resource({}) self.assertEqual(