Introduce SpanLimits class to tracing SDK

owais · owais · commit 8194e99811a9 · 2021-05-27T08:10:23.000+05:30
diff --git 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
@@ -65,6 +65,7 @@
 _DEFAULT_SPAN_EVENTS_LIMIT = 128
 _DEFAULT_SPAN_LINKS_LIMIT = 128
 _DEFAULT_SPAN_ATTRIBUTES_LIMIT = 128
+_ENV_VALUE_UNSET = "unset"
 
 # pylint: disable=protected-access
 _TRACE_SAMPLER = sampling._get_from_env_or_default()
@@ -499,7 +500,7 @@ 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,
@@ -556,7 +557,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,13 +570,13 @@ 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(
+SPAN_ATTRIBUTE_COUNT_LIMIT = SpanLimits._from_env_if_absent(
     None, OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, _DEFAULT_SPAN_ATTRIBUTES_LIMIT
 )
 
@@ -599,6 +600,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 +625,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 +640,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 +851,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 +864,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 +973,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 +993,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 +1004,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,17 +1021,16 @@ 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,
             self.id_generator,
             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
@@ -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,7 +1275,7 @@ 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
         )
@@ -1288,7 +1288,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 +1311,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 +1344,9 @@ 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):
+    def _test_span_no_limits(self, tracer):
         num_links = int(trace._DEFAULT_SPAN_LINKS_LIMIT) + randint(1, 100)
 
-        tracer = new_tracer()
         id_generator = RandomIdGenerator()
         some_links = [
             trace_api.Link(
@@ -1394,3 +1376,62 @@ def test_span_no_limits_env(self):
                 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,
+                )
+            )
+        )
