SpanLimit: Treat empty value env vars as unset

Fixes open-telemetry#2052
owais · Aug 18, 2021 · 891233f · 891233f
1 parent 061e72c
commit 891233f
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 10 deletions.
diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/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
 

diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/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):