Fix validity calculation for trace/span ID

CHANGELOG.md

@@ -20,6 +20,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#2101](https://github.com/open-telemetry/opentelemetry-python/pull/2101))
 - Fix incorrect headers parsing via environment variables
   ([#2103](https://github.com/open-telemetry/opentelemetry-python/pull/2103))
+- Fix validity calculation for trace and span IDs
+  ([#2145](https://github.com/open-telemetry/opentelemetry-python/pull/2145))
 
 ## [1.5.0-0.24b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.5.0-0.24b0) - 2021-08-26
 

opentelemetry-api/src/opentelemetry/trace/span.py

@@ -387,7 +387,8 @@ def values(self) -> typing.ValuesView[str]:
 
 
 DEFAULT_TRACE_STATE = TraceState.get_default()
-_TRACE_ID_HEX_LENGTH = 2 ** 128 - 1
+_TRACE_ID_MAX_VALUE = 2 ** 128 - 1
+_SPAN_ID_MAX_VALUE = 2 ** 64 - 1
 
 
 class SpanContext(
@@ -420,9 +421,8 @@ def __new__(
             trace_state = DEFAULT_TRACE_STATE
 
         is_valid = (
-            trace_id != INVALID_TRACE_ID
-            and span_id != INVALID_SPAN_ID
-            and trace_id < _TRACE_ID_HEX_LENGTH
+            0 < trace_id <= _TRACE_ID_MAX_VALUE
+            and 0 < span_id <= _SPAN_ID_MAX_VALUE
         )
 
         return tuple.__new__(

opentelemetry-api/tests/trace/test_span_context.py

@@ -43,3 +43,47 @@ def test_span_context_pickle(self):
             trace_state=trace.DEFAULT_TRACE_STATE,
         )
         self.assertFalse(invalid_sc.is_valid)
+
+    def test_trace_id_validity(self):
+        trace_id_max_value = int("f" * 32, 16)
+        span_id = 1
+
+        # valid trace IDs
+        sc = trace.SpanContext(trace_id_max_value, span_id, is_remote=False)
+        self.assertTrue(sc.is_valid)
+
+        sc = trace.SpanContext(1, span_id, is_remote=False)
+        self.assertTrue(sc.is_valid)
+
+        # invalid trace IDs
+        sc = trace.SpanContext(0, span_id, is_remote=False)
+        self.assertFalse(sc.is_valid)
+
+        sc = trace.SpanContext(-1, span_id, is_remote=False)
+        self.assertFalse(sc.is_valid)
+
+        sc = trace.SpanContext(
+            trace_id_max_value + 1, span_id, is_remote=False
+        )
+        self.assertFalse(sc.is_valid)
+
+    def test_span_id_validity(self):
+        span_id_max = int("f" * 16, 16)
+        trace_id = 1
+
+        # valid span IDs
+        sc = trace.SpanContext(trace_id, span_id_max, is_remote=False)
+        self.assertTrue(sc.is_valid)
+
+        sc = trace.SpanContext(trace_id, 1, is_remote=False)
+        self.assertTrue(sc.is_valid)
+
+        # invalid span IDs
+        sc = trace.SpanContext(trace_id, 0, is_remote=False)
+        self.assertFalse(sc.is_valid)
+
+        sc = trace.SpanContext(trace_id, -1, is_remote=False)
+        self.assertFalse(sc.is_valid)
+
+        sc = trace.SpanContext(trace_id, span_id_max + 1, is_remote=False)
+        self.assertFalse(sc.is_valid)