SpanLimit: Treat empty value env vars as unset

Fixes #2052

Author: owais

CHANGELOG.md

@@ -20,6 +20,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#2044](https://github.com/open-telemetry/opentelemetry-python/pull/2044))
 - `opentelemetry-sdk` Fixed bugs (#2041, #2042 & #2045) in Span Limits
   ([#2044](https://github.com/open-telemetry/opentelemetry-python/pull/2044))
+- `opentelemetry-sdk` Treat limit even vars set to empty values as unset/unlimited.
+  ([#2044](https://github.com/open-telemetry/opentelemetry-python/pull/2054))
 
 ## [0.23.1](https://github.com/open-telemetry/opentelemetry-python/pull/1987) - 2021-07-26
 

opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py

@@ -67,7 +67,7 @@
 _DEFAULT_OTEL_LINK_ATTRIBUTE_COUNT_LIMIT = 128
 
 
-_ENV_VALUE_UNSET = "unset"
+_ENV_VALUE_UNSET = ""
 
 # pylint: disable=protected-access
 _TRACE_SAMPLER = sampling._get_from_env_or_default()
@@ -533,7 +533,7 @@ class SpanLimits:
     - All limit arguments are optional.
     - If a limit argument is not set, the class will try to read its value from the corresponding
       environment variable.
-    - If the environment variable is not set, the default value for the limit is used.
+    - If the environment variable is not set, the default value, if any, will be used.
 
     Args:
         max_attributes: Maximum number of attributes that can be added to a Span.
@@ -553,7 +553,7 @@ class SpanLimits:
             the specified length will be truncated.
     """
 
-    UNSET = -1
+    UNSET = "unset"
 
     def __init__(
         self,
@@ -609,15 +609,18 @@ def __repr__(self):
     def _from_env_if_absent(
         cls, value: Optional[int], env_var: str, default: Optional[int] = None
     ) -> Optional[int]:
-        if value is cls.UNSET:
+        if value == cls.UNSET:
             return None
 
         err_msg = "{0} must be a non-negative integer but got {}"
 
+        # if no value is provided for the limit, try to load it from env
         if value is None:
-            str_value = environ.get(env_var, "").strip().lower()
-            if not str_value:
+            # return default value if env var is not set
+            if env_var not in environ:
                 return default
+
+            str_value = environ.get(env_var, "").strip().lower()
             if str_value == _ENV_VALUE_UNSET:
                 return None
 

opentelemetry-sdk/tests/trace/test_trace.py

@@ -1533,10 +1533,10 @@ def test_span_limits_code(self):
     @mock.patch.dict(
         "os.environ",
         {
-            OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT: "unset",
-            OTEL_SPAN_EVENT_COUNT_LIMIT: "unset",
-            OTEL_SPAN_LINK_COUNT_LIMIT: "unset",
-            OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT: "unset",
+            OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT: "",
+            OTEL_SPAN_EVENT_COUNT_LIMIT: "",
+            OTEL_SPAN_LINK_COUNT_LIMIT: "",
+            OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT: "",
         },
     )
     def test_span_no_limits_env(self):