Fix validity calculation for trace/span ID

.github/workflows/test.yml

@@ -10,7 +10,7 @@ env:
   # Otherwise, set variable to the commit of your branch on
   # opentelemetry-python-contrib which is compatible with these Core repo
   # changes.
-  CONTRIB_REPO_SHA: dde62cebffe519c35875af6d06fae053b3be65ec
+  CONTRIB_REPO_SHA: d2984f5242ed2250ad1c11b6164e2e8e11e2a804
 
 jobs:
   build:

CHANGELOG.md

@@ -26,6 +26,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#2138](https://github.com/open-telemetry/opentelemetry-python/pull/2138))
 - `opentelemetry-exporter-otlp`: Add `opentelemetry-otlp-proto-http` as dependency
   ([#2147](https://github.com/open-telemetry/opentelemetry-python/pull/2147))
+- 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
+            INVALID_TRACE_ID < trace_id <= _TRACE_ID_MAX_VALUE
+            and INVALID_SPAN_ID < 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)