From 2f0ef468afb4a943080bf348fe5a9e44d7925587 Mon Sep 17 00:00:00 2001
From: Owais Lone <owais@lone.pw>
Date: Sat, 8 May 2021 15:14:25 +0530
Subject: [PATCH] Allow span limits to be unset

Ths spec recommends most span limits to default to 128 and allow users
to change it via environment variables. It does not dictate that limits
should always be set though. So users should be able to remove these
limits. This commit accepts `"none"` as a valid value for the limit
config environment variables.
---
 CHANGELOG.md                                   |  3 +++
 .../src/opentelemetry/sdk/trace/__init__.py    | 18 ++++++++++++++----
 .../src/opentelemetry/sdk/util/__init__.py     |  9 +++++----
 opentelemetry-sdk/tests/test_util.py           | 16 ++++++++++++++++
 4 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6e889bccc13..c08ec7f13a4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 - Added example for running Django with auto instrumentation.
   ([#1803](https://github.com/open-telemetry/opentelemetry-python/pull/1803))
+- Allow span limits (OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, OTEL_SPAN_EVENT_COUNT_LIMIT, OTEL_SPAN_LINK_COUNT_LIMIT)
+  to be set to "none".
+  ([#1830](https://github.com/open-telemetry/opentelemetry-python/pull/1830))
 
 ### Changed
 - Fixed OTLP gRPC exporter silently failing if scheme is not specified in endpoint.
diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
index 625b8519ef5..67b4396a2f7 100644
--- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
+++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
@@ -58,12 +58,22 @@
 
 logger = logging.getLogger(__name__)
 
-SPAN_ATTRIBUTE_COUNT_LIMIT = int(
-    environ.get(OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, 128)
+
+def _get_limit_from_env(limit_name, default) -> Optional[int]:
+    value = environ.get(limit_name, default).strip().lower()
+    if value == "none":
+        return None
+    return int(value)
+
+
+SPAN_ATTRIBUTE_COUNT_LIMIT = _get_limit_from_env(
+    OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, "128"
 )
 
-_SPAN_EVENT_COUNT_LIMIT = int(environ.get(OTEL_SPAN_EVENT_COUNT_LIMIT, 128))
-_SPAN_LINK_COUNT_LIMIT = int(environ.get(OTEL_SPAN_LINK_COUNT_LIMIT, 128))
+_SPAN_EVENT_COUNT_LIMIT = _get_limit_from_env(
+    OTEL_SPAN_EVENT_COUNT_LIMIT, "128"
+)
+_SPAN_LINK_COUNT_LIMIT = _get_limit_from_env(OTEL_SPAN_LINK_COUNT_LIMIT, "128")
 _VALID_ATTR_VALUE_TYPES = (bool, str, int, float)
 # pylint: disable=protected-access
 _TRACE_SAMPLER = sampling._get_from_env_or_default()
diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py
index 1bb8ed264fc..694a0a5b1bd 100644
--- a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py
+++ b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py
@@ -97,10 +97,11 @@ class BoundedDict(MutableMapping):
     """
 
     def __init__(self, maxlen):
-        if not isinstance(maxlen, int):
-            raise ValueError
-        if maxlen < 0:
-            raise ValueError
+        if maxlen is not None:
+            if not isinstance(maxlen, int):
+                raise ValueError
+            if maxlen < 0:
+                raise ValueError
         self.maxlen = maxlen
         self.dropped = 0
         self._dict = OrderedDict()  # type: OrderedDict
diff --git a/opentelemetry-sdk/tests/test_util.py b/opentelemetry-sdk/tests/test_util.py
index cacc77dbd98..5c13bf2c7ed 100644
--- a/opentelemetry-sdk/tests/test_util.py
+++ b/opentelemetry-sdk/tests/test_util.py
@@ -134,6 +134,14 @@ def test_extend_drop(self):
         self.assertEqual(len(blist), list_len)
         self.assertEqual(blist.dropped, len(other_list))
 
+    def test_no_limit(self):
+        blist = BoundedList(maxlen=None)
+        for num in range(100):
+            blist.append(num)
+
+        for num in range(100):
+            self.assertEqual(blist[num], num)
+
 
 class TestBoundedDict(unittest.TestCase):
     base = collections.OrderedDict(
@@ -211,3 +219,11 @@ def test_bounded_dict(self):
 
         with self.assertRaises(KeyError):
             _ = bdict["new-name"]
+
+    def test_no_limit(self):
+        bdict = BoundedDict(maxlen=None)
+        for num in range(100):
+            bdict[num] = num
+
+        for num in range(100):
+            self.assertEqual(bdict[num], num)