From a800bc95a148be08a2cbab1c4227ca1fed0b138c Mon Sep 17 00:00:00 2001 From: Owais Lone Date: Thu, 27 May 2021 07:54:18 +0530 Subject: [PATCH] Introduce SpanLimits class to tracing SDK --- CHANGELOG.md | 4 + .../src/opentelemetry/sdk/trace/__init__.py | 75 +++++++---- opentelemetry-sdk/tests/trace/test_trace.py | 120 +++++++++++++----- 3 files changed, 136 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 587ca0a61b2..6a886acb8d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.2.0-0.21b0...HEAD) +### Added +- Allow span limits to be set programatically via TracerProvider. + ([#1877](https://github.com/open-telemetry/opentelemetry-python/pull/1877)) + ### Changed - Updated get_tracer to return an empty string when passed an invalid name ([#1854](https://github.com/open-telemetry/opentelemetry-python/pull/1854)) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index c202f5cbea1..66b4b383905 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -62,9 +62,12 @@ logger = logging.getLogger(__name__) -_DEFAULT_SPAN_EVENTS_LIMIT = 128 -_DEFAULT_SPAN_LINKS_LIMIT = 128 -_DEFAULT_SPAN_ATTRIBUTES_LIMIT = 128 +_DEFAULT_OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT = 128 +_DEFAULT_OTEL_SPAN_EVENT_COUNT_LIMIT = 128 +_DEFAULT_OTEL_SPAN_LINK_COUNT_LIMIT = 128 + + +_ENV_VALUE_UNSET = "unset" # pylint: disable=protected-access _TRACE_SAMPLER = sampling._get_from_env_or_default() @@ -499,19 +502,29 @@ def _format_links(links): return f_links -class _Limits: +class SpanLimits: """The limits that should be enforce on recorded data such as events, links, attributes etc. This class does not enforce any limits itself. It only provides an a way read limits from env, - default values and in future from user provided arguments. + default values and from user provided arguments. + + All limit arguments must be either a non-negative integer, ``None`` or ``SpanLimits.UNSET``. - All limit must be either a non-negative integer or ``None``. - Setting a limit to ``None`` will not set any limits for that field/type. + - All limit arguments are optional. + - If a limit argument is not set, the class will try to read it's value from the corresponding + environment variable. + - If the environment variable is not set, the default value for the limit is used. Args: + max_attributes: Maximum number of attributes that can be added to a Span. + Environment variable: OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT + Default: {_DEFAULT_SPAN_ATTRIBUTE_COUNT_LIMIT} max_events: Maximum number of events that can be added to a Span. + Environment variable: OTEL_SPAN_EVENT_COUNT_LIMIT + Default: {_DEFAULT_SPAN_EVENT_COUNT_LIMIT} max_links: Maximum number of links that can be added to a Span. - max_attributes: Maximum number of attributes that can be added to a Span. + Environment variable: OTEL_SPAN_LINK_COUNT_LIMIT + Default: {_DEFAULT_SPAN_LINK_COUNT_LIMIT} """ UNSET = -1 @@ -529,13 +542,17 @@ def __init__( self.max_attributes = self._from_env_if_absent( max_attributes, OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, - _DEFAULT_SPAN_ATTRIBUTES_LIMIT, + _DEFAULT_OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, ) self.max_events = self._from_env_if_absent( - max_events, OTEL_SPAN_EVENT_COUNT_LIMIT, _DEFAULT_SPAN_EVENTS_LIMIT + max_events, + OTEL_SPAN_EVENT_COUNT_LIMIT, + _DEFAULT_OTEL_SPAN_EVENT_COUNT_LIMIT, ) self.max_links = self._from_env_if_absent( - max_links, OTEL_SPAN_LINK_COUNT_LIMIT, _DEFAULT_SPAN_LINKS_LIMIT + max_links, + OTEL_SPAN_LINK_COUNT_LIMIT, + _DEFAULT_OTEL_SPAN_LINK_COUNT_LIMIT, ) def __repr__(self): @@ -556,7 +573,7 @@ def _from_env_if_absent( str_value = environ.get(env_var, "").strip().lower() if not str_value: return default - if str_value == "unset": + if str_value == _ENV_VALUE_UNSET: return None try: @@ -569,14 +586,16 @@ def _from_env_if_absent( return value -_UnsetLimits = _Limits( - max_attributes=_Limits.UNSET, - max_events=_Limits.UNSET, - max_links=_Limits.UNSET, +_UnsetLimits = SpanLimits( + max_attributes=SpanLimits.UNSET, + max_events=SpanLimits.UNSET, + max_links=SpanLimits.UNSET, ) -SPAN_ATTRIBUTE_COUNT_LIMIT = _Limits._from_env_if_absent( - None, OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, _DEFAULT_SPAN_ATTRIBUTES_LIMIT +SPAN_ATTRIBUTE_COUNT_LIMIT = SpanLimits._from_env_if_absent( + None, + OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, + _DEFAULT_OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, ) @@ -599,6 +618,7 @@ class Span(trace_api.Span, ReadableSpan): links: Links to other spans to be exported span_processor: `SpanProcessor` to invoke when starting and ending this `Span`. + limits: `SpanLimits` instance that was passed to the `TracerProvider` """ def __new__(cls, *args, **kwargs): @@ -623,6 +643,7 @@ def __init__( instrumentation_info: InstrumentationInfo = None, record_exception: bool = True, set_status_on_exception: bool = True, + limits=_UnsetLimits, ) -> None: super().__init__( name=name, @@ -637,6 +658,7 @@ def __init__( self._record_exception = record_exception self._set_status_on_exception = set_status_on_exception self._span_processor = span_processor + self._limits = limits self._lock = threading.Lock() _filter_attributes(attributes) @@ -847,10 +869,6 @@ class _Span(Span): by other mechanisms than through the `Tracer`. """ - def __init__(self, *args, limits=_UnsetLimits, **kwargs): - self._limits = limits - super().__init__(*args, **kwargs) - class Tracer(trace_api.Tracer): """See `opentelemetry.trace.Tracer`.""" @@ -864,13 +882,14 @@ def __init__( ], id_generator: IdGenerator, instrumentation_info: InstrumentationInfo, + span_limits: SpanLimits, ) -> None: self.sampler = sampler self.resource = resource self.span_processor = span_processor self.id_generator = id_generator self.instrumentation_info = instrumentation_info - self._limits = None + self._span_limits = span_limits @contextmanager def start_as_current_span( @@ -972,7 +991,7 @@ def start_span( # pylint: disable=too-many-locals instrumentation_info=self.instrumentation_info, record_exception=record_exception, set_status_on_exception=set_status_on_exception, - limits=self._limits, + limits=self._span_limits, ) span.start(start_time=start_time, parent_context=context) else: @@ -992,6 +1011,7 @@ def __init__( SynchronousMultiSpanProcessor, ConcurrentMultiSpanProcessor ] = None, id_generator: IdGenerator = None, + span_limits: SpanLimits = None, ): self._active_span_processor = ( active_span_processor or SynchronousMultiSpanProcessor() @@ -1002,7 +1022,7 @@ def __init__( self.id_generator = id_generator self._resource = resource self.sampler = sampler - self._limits = _Limits() + self._span_limits = span_limits or SpanLimits() self._atexit_handler = None if shutdown_on_exit: self._atexit_handler = atexit.register(self.shutdown) @@ -1019,7 +1039,7 @@ def get_tracer( if not instrumenting_module_name: # Reject empty strings too. instrumenting_module_name = "" logger.error("get_tracer called with missing module name.") - tracer = Tracer( + return Tracer( self.sampler, self.resource, self._active_span_processor, @@ -1027,9 +1047,8 @@ def get_tracer( InstrumentationInfo( instrumenting_module_name, instrumenting_library_version ), + self._span_limits, ) - tracer._limits = self._limits - return tracer def add_span_processor(self, span_processor: SpanProcessor) -> None: """Registers a new :class:`SpanProcessor` for this `TracerProvider`. diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index c9c523c3fab..84d57c199f5 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -40,8 +40,8 @@ from opentelemetry.util._time import _time_ns -def new_tracer() -> trace_api.Tracer: - return trace.TracerProvider().get_tracer(__name__) +def new_tracer(span_limits=None) -> trace_api.Tracer: + return trace.TracerProvider(span_limits=span_limits).get_tracer(__name__) class TestTracer(unittest.TestCase): @@ -539,7 +539,7 @@ def test_disallow_direct_span_creation(self): def test_surplus_span_links(self): # pylint: disable=protected-access - max_links = trace._Limits().max_links + max_links = trace.SpanLimits().max_links links = [ trace_api.Link(trace_api.SpanContext(0x1, idx, is_remote=False)) for idx in range(0, 16 + max_links) @@ -550,7 +550,7 @@ def test_surplus_span_links(self): def test_surplus_span_attributes(self): # pylint: disable=protected-access - max_attrs = trace._Limits().max_attributes + max_attrs = trace.SpanLimits().max_attributes attributes = {str(idx): idx for idx in range(0, 16 + max_attrs)} tracer = new_tracer() with tracer.start_as_current_span( @@ -1275,12 +1275,17 @@ class TestSpanLimits(unittest.TestCase): # pylint: disable=protected-access def test_limits_defaults(self): - limits = trace._Limits() + limits = trace.SpanLimits() self.assertEqual( - limits.max_attributes, trace._DEFAULT_SPAN_ATTRIBUTES_LIMIT + limits.max_attributes, + trace._DEFAULT_OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, + ) + self.assertEqual( + limits.max_events, trace._DEFAULT_OTEL_SPAN_EVENT_COUNT_LIMIT + ) + self.assertEqual( + limits.max_links, trace._DEFAULT_OTEL_SPAN_LINK_COUNT_LIMIT ) - self.assertEqual(limits.max_events, trace._DEFAULT_SPAN_EVENTS_LIMIT) - self.assertEqual(limits.max_links, trace._DEFAULT_SPAN_LINKS_LIMIT) def test_limits_values_code(self): max_attributes, max_events, max_links = ( @@ -1288,7 +1293,7 @@ def test_limits_values_code(self): randint(0, 10000), randint(0, 10000), ) - limits = trace._Limits( + limits = trace.SpanLimits( max_attributes=max_attributes, max_events=max_events, max_links=max_links, @@ -1311,21 +1316,12 @@ def test_limits_values_env(self): OTEL_SPAN_LINK_COUNT_LIMIT: str(max_links), }, ): - limits = trace._Limits() + limits = trace.SpanLimits() self.assertEqual(limits.max_attributes, max_attributes) self.assertEqual(limits.max_events, max_events) self.assertEqual(limits.max_links, max_links) - @mock.patch.dict( - "os.environ", - { - OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT: "10", - OTEL_SPAN_EVENT_COUNT_LIMIT: "20", - OTEL_SPAN_LINK_COUNT_LIMIT: "30", - }, - ) - def test_span_limits_env(self): - tracer = new_tracer() + def _test_span_limits(self, tracer): id_generator = RandomIdGenerator() some_links = [ trace_api.Link( @@ -1353,18 +1349,11 @@ def test_span_limits_env(self): self.assertEqual(len(root.attributes), 10) self.assertEqual(len(root.events), 20) - @mock.patch.dict( - "os.environ", - { - OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT: "unset", - OTEL_SPAN_EVENT_COUNT_LIMIT: "unset", - OTEL_SPAN_LINK_COUNT_LIMIT: "unset", - }, - ) - def test_span_no_limits_env(self): - num_links = int(trace._DEFAULT_SPAN_LINKS_LIMIT) + randint(1, 100) + def _test_span_no_limits(self, tracer): + num_links = int(trace._DEFAULT_OTEL_SPAN_LINK_COUNT_LIMIT) + randint( + 1, 100 + ) - tracer = new_tracer() id_generator = RandomIdGenerator() some_links = [ trace_api.Link( @@ -1379,18 +1368,79 @@ def test_span_no_limits_env(self): with tracer.start_as_current_span("root", links=some_links) as root: self.assertEqual(len(root.links), num_links) - num_events = int(trace._DEFAULT_SPAN_EVENTS_LIMIT) + randint(1, 100) + num_events = int(trace._DEFAULT_OTEL_SPAN_EVENT_COUNT_LIMIT) + randint( + 1, 100 + ) with tracer.start_as_current_span("root") as root: for idx in range(num_events): root.add_event("my_event_{}".format(idx)) self.assertEqual(len(root.events), num_events) - num_attributes = int(trace._DEFAULT_SPAN_ATTRIBUTES_LIMIT) + randint( - 1, 100 - ) + num_attributes = int( + trace._DEFAULT_OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT + ) + randint(1, 100) with tracer.start_as_current_span("root") as root: for idx in range(num_attributes): root.set_attribute("my_attribute_{}".format(idx), 0) self.assertEqual(len(root.attributes), num_attributes) + + @mock.patch.dict( + "os.environ", + { + OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT: "10", + OTEL_SPAN_EVENT_COUNT_LIMIT: "20", + OTEL_SPAN_LINK_COUNT_LIMIT: "30", + }, + ) + def test_span_limits_env(self): + self._test_span_limits(new_tracer()) + + @mock.patch.dict( + "os.environ", + { + OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT: "10", + OTEL_SPAN_EVENT_COUNT_LIMIT: "20", + OTEL_SPAN_LINK_COUNT_LIMIT: "30", + }, + ) + def test_span_limits_default_to_env(self): + self._test_span_limits( + new_tracer( + span_limits=trace.SpanLimits( + max_attributes=None, max_events=None, max_links=None + ) + ) + ) + + def test_span_limits_code(self): + self._test_span_limits( + new_tracer( + span_limits=trace.SpanLimits( + max_attributes=10, max_events=20, max_links=30 + ) + ) + ) + + @mock.patch.dict( + "os.environ", + { + OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT: "unset", + OTEL_SPAN_EVENT_COUNT_LIMIT: "unset", + OTEL_SPAN_LINK_COUNT_LIMIT: "unset", + }, + ) + def test_span_no_limits_env(self): + self._test_span_no_limits(new_tracer()) + + def test_span_no_limits_code(self): + self._test_span_no_limits( + new_tracer( + span_limits=trace.SpanLimits( + max_attributes=trace.SpanLimits.UNSET, + max_links=trace.SpanLimits.UNSET, + max_events=trace.SpanLimits.UNSET, + ) + ) + )