From 9fedfbc3ac25839e0e2680ce4ca2d84c349a7715 Mon Sep 17 00:00:00 2001 From: Alireza Ghasemi Date: Wed, 7 May 2025 22:35:35 +0330 Subject: [PATCH 1/5] Allow passing a custom random number generator to RandomIdGenerator --- .../opentelemetry/sdk/trace/id_generator.py | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/id_generator.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/id_generator.py index cd1f89bcde2..ccd269d665f 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/id_generator.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/id_generator.py @@ -14,6 +14,8 @@ import abc import random +from random import Random +from typing import Optional from opentelemetry import trace @@ -45,16 +47,28 @@ def generate_trace_id(self) -> int: class RandomIdGenerator(IdGenerator): """The default ID generator for TracerProvider which randomly generates all bits when generating IDs. + + Args: + rng: A random number generator. Defaults to the global random instance. + It is recommended to use a fresh `Random()` instead of the default + to avoid potential conflicts with the global random instance + (duplicate ids across multiple processes when a constant global + random seed is set). """ + def __init__(self, rng: Optional[Random] = None) -> None: + if rng is None: + rng = random # Just a hack to preserve backward compatibility, otherwise it does not quite match the type hint. + self._rng = rng + def generate_span_id(self) -> int: - span_id = random.getrandbits(64) + span_id = self._rng.getrandbits(64) while span_id == trace.INVALID_SPAN_ID: - span_id = random.getrandbits(64) + span_id = self._rng.getrandbits(64) return span_id def generate_trace_id(self) -> int: - trace_id = random.getrandbits(128) + trace_id = self._rng.getrandbits(128) while trace_id == trace.INVALID_TRACE_ID: - trace_id = random.getrandbits(128) + trace_id = self._rng.getrandbits(128) return trace_id From dc5133bf4c6ce784f95988e979dd8069d64cd01c Mon Sep 17 00:00:00 2001 From: Alireza Ghasemi Date: Wed, 7 May 2025 23:24:11 +0330 Subject: [PATCH 2/5] Update change-logs --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22e56015b89..c0d83ffaa15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +- Allow passing a custom random number generator to `trace.RandomIdGenerator`. + ([#4571](https://github.com/open-telemetry/opentelemetry-python/pull/4571)) - Fix intermittent `Connection aborted` error when using otlp/http exporters ([#4477](https://github.com/open-telemetry/opentelemetry-python/pull/4477)) - opentelemetry-sdk: use stable code attributes: `code.function` -> `code.function.name`, `code.lineno` -> `code.line.number`, `code.filepath` -> `code.file.path` From 7af0a739880cb02db0435a55e8f049416ab0fc6a Mon Sep 17 00:00:00 2001 From: Alireza Ghasemi Date: Fri, 9 May 2025 19:34:25 +0330 Subject: [PATCH 3/5] Add random uniformness heads-up document --- opentelemetry-sdk/src/opentelemetry/sdk/trace/id_generator.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/id_generator.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/id_generator.py index ccd269d665f..93974595734 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/id_generator.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/id_generator.py @@ -53,7 +53,8 @@ class RandomIdGenerator(IdGenerator): It is recommended to use a fresh `Random()` instead of the default to avoid potential conflicts with the global random instance (duplicate ids across multiple processes when a constant global - random seed is set). + random seed is set). In case of a custom implementation, it should + be uniform, as some samplers rely on this randomness to make sampling decisions. """ def __init__(self, rng: Optional[Random] = None) -> None: From 4ab2049d9d650ca65c76a73dfa001acb7de26d7c Mon Sep 17 00:00:00 2001 From: Alireza Ghasemi Date: Mon, 22 Sep 2025 20:15:59 +0330 Subject: [PATCH 4/5] Add unit tests --- opentelemetry-sdk/tests/trace/test_trace.py | 52 ++++++++++----------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 7b23c11fa1f..50c33d5e644 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -20,7 +20,7 @@ import unittest from importlib import reload from logging import ERROR, WARNING -from random import randint +from random import randint, Random from time import time_ns from typing import Optional from unittest import mock @@ -2168,29 +2168,29 @@ class TestRandomIdGenerator(unittest.TestCase): _TRACE_ID_MAX_VALUE = 2**128 - 1 _SPAN_ID_MAX_VALUE = 2**64 - 1 - @patch( - "random.getrandbits", - side_effect=[trace_api.INVALID_SPAN_ID, 0x00000000DEADBEF0], - ) - def test_generate_span_id_avoids_invalid(self, mock_getrandbits): - generator = RandomIdGenerator() - span_id = generator.generate_span_id() - - self.assertNotEqual(span_id, trace_api.INVALID_SPAN_ID) - mock_getrandbits.assert_any_call(64) - self.assertEqual(mock_getrandbits.call_count, 2) - - @patch( - "random.getrandbits", - side_effect=[ - trace_api.INVALID_TRACE_ID, - 0x000000000000000000000000DEADBEEF, - ], - ) - def test_generate_trace_id_avoids_invalid(self, mock_getrandbits): - generator = RandomIdGenerator() - trace_id = generator.generate_trace_id() + def setUp(self): + self.generators = { + 'rng=None': RandomIdGenerator(), + 'rng=Random()': RandomIdGenerator(rng=Random()), + 'rng=Random(42)': RandomIdGenerator(rng=Random(x=42)) + } - self.assertNotEqual(trace_id, trace_api.INVALID_TRACE_ID) - mock_getrandbits.assert_any_call(128) - self.assertEqual(mock_getrandbits.call_count, 2) + def test_generate_span_id_avoids_invalid(self): + for msg, generator in self.generators.items(): + with self.subTest(msg=msg), \ + patch.object(generator._rng, "getrandbits", side_effect=[trace_api.INVALID_SPAN_ID, 0x00000000DEADBEF0]) as mock_getrandbits: + span_id = generator.generate_span_id() + + self.assertNotEqual(span_id, trace_api.INVALID_SPAN_ID) + mock_getrandbits.assert_any_call(64) + self.assertEqual(mock_getrandbits.call_count, 2) + + def test_generate_trace_id_avoids_invalid(self): + for name, generator in self.generators.items(): + with self.subTest(msg=name), \ + patch.object(generator._rng, "getrandbits", side_effect=[trace_api.INVALID_SPAN_ID, 0x00000000DEADBEF0]) as mock_getrandbits: + trace_id = generator.generate_trace_id() + + self.assertNotEqual(trace_id, trace_api.INVALID_TRACE_ID) + mock_getrandbits.assert_any_call(128) + self.assertEqual(mock_getrandbits.call_count, 2) From 914a30fd447bc222e5bcc9e266ada206e7ed4db3 Mon Sep 17 00:00:00 2001 From: Alireza Ghasemi Date: Mon, 22 Sep 2025 20:18:01 +0330 Subject: [PATCH 5/5] Fixup --- opentelemetry-sdk/tests/trace/test_trace.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 50c33d5e644..188fa3aa386 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -2188,7 +2188,7 @@ def test_generate_span_id_avoids_invalid(self): def test_generate_trace_id_avoids_invalid(self): for name, generator in self.generators.items(): with self.subTest(msg=name), \ - patch.object(generator._rng, "getrandbits", side_effect=[trace_api.INVALID_SPAN_ID, 0x00000000DEADBEF0]) as mock_getrandbits: + patch.object(generator._rng, "getrandbits", side_effect=[trace_api.INVALID_TRACE_ID, 0x000000000000000000000000DEADBEEF]) as mock_getrandbits: trace_id = generator.generate_trace_id() self.assertNotEqual(trace_id, trace_api.INVALID_TRACE_ID)