From a85b2bb554ef11145bdbc05665c38b99d5970e13 Mon Sep 17 00:00:00 2001 From: Alex Boten <223565+codeboten@users.noreply.github.com> Date: Fri, 12 Jun 2026 13:09:46 -0700 Subject: [PATCH 1/6] opentelemetry-sdk: reduce lock contention in attributes There are a few improvements here: - set_attributes was double locking (once in set_attributes and once in __setitem__. This change updates the call to a private _setitem_locked for set_attributes once the lock has been obtained - __iter__ doesn't use a lock if the BoundedAttributes's instantiated as immutable - _worker_awaken was being set even when it's already set, adding a check in emit to avoid unnecessary calls Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com> --- .../src/opentelemetry/attributes/__init__.py | 43 +++++++------ .../benchmarks/trace/test_benchmark_trace.py | 60 +++++++++++++++++++ .../sdk/_shared_internal/__init__.py | 2 +- .../src/opentelemetry/sdk/trace/__init__.py | 6 +- 4 files changed, 89 insertions(+), 22 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/attributes/__init__.py b/opentelemetry-api/src/opentelemetry/attributes/__init__.py index 86e899d7516..c8a5d95f959 100644 --- a/opentelemetry-api/src/opentelemetry/attributes/__init__.py +++ b/opentelemetry-api/src/opentelemetry/attributes/__init__.py @@ -278,28 +278,31 @@ def __setitem__(self, key: str, value: types.AnyValue) -> None: if getattr(self, "_immutable", False): # type: ignore raise TypeError with self._lock: - if self.maxlen is not None and self.maxlen == 0: - self.dropped += 1 - return + self._setitem_locked(key, value) - if self._extended_attributes: - value = _clean_extended_attribute( - key, value, self.max_value_len - ) - else: - value = _clean_attribute(key, value, self.max_value_len) # type: ignore - if value is None: - return + def _setitem_locked(self, key: str, value: types.AnyValue) -> None: + if self.maxlen is not None and self.maxlen == 0: + self.dropped += 1 + return + + if self._extended_attributes: + value = _clean_extended_attribute( + key, value, self.max_value_len + ) + else: + value = _clean_attribute(key, value, self.max_value_len) # type: ignore + if value is None: + return - if key in self._dict: - del self._dict[key] - elif self.maxlen is not None and len(self._dict) == self.maxlen: - if not isinstance(self._dict, OrderedDict): - self._dict = OrderedDict(self._dict) - self._dict.popitem(last=False) # type: ignore - self.dropped += 1 + if key in self._dict: + del self._dict[key] + elif self.maxlen is not None and len(self._dict) == self.maxlen: + if not isinstance(self._dict, OrderedDict): + self._dict = OrderedDict(self._dict) + self._dict.popitem(last=False) # type: ignore + self.dropped += 1 - self._dict[key] = value # type: ignore + self._dict[key] = value # type: ignore def __delitem__(self, key: str) -> None: if getattr(self, "_immutable", False): # type: ignore @@ -308,6 +311,8 @@ def __delitem__(self, key: str) -> None: del self._dict[key] def __iter__(self): + if self._immutable: + return iter(self._dict) with self._lock: return iter(list(self._dict)) diff --git a/opentelemetry-sdk/benchmarks/trace/test_benchmark_trace.py b/opentelemetry-sdk/benchmarks/trace/test_benchmark_trace.py index 137314d3b0f..4a4fd6329bc 100644 --- a/opentelemetry-sdk/benchmarks/trace/test_benchmark_trace.py +++ b/opentelemetry-sdk/benchmarks/trace/test_benchmark_trace.py @@ -1,6 +1,7 @@ # Copyright The OpenTelemetry Authors # SPDX-License-Identifier: Apache-2.0 +import threading import tracemalloc from functools import lru_cache @@ -17,6 +18,11 @@ _TracerConfig, sampling, ) +from opentelemetry.sdk.trace.export import BatchSpanProcessor +from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( + InMemorySpanExporter, +) +from opentelemetry.sdk.util import BoundedList from opentelemetry.sdk.util.instrumentation import _scope_name_matches_glob from opentelemetry.trace import SpanContext, TraceFlags @@ -239,3 +245,57 @@ def benchmark_iter(): list(attrs) benchmark(benchmark_iter) + + +@pytest.mark.parametrize("num_items", [1, 10, 50, 128]) +def test_bounded_list_iterator(benchmark, num_items): + blist = BoundedList.from_seq(num_items, range(num_items)) + + peaks = [] + for _ in range(200): + tracemalloc.start() + list(blist) + _, peak = tracemalloc.get_traced_memory() + tracemalloc.stop() + peaks.append(peak) + benchmark.extra_info["mean_alloc_bytes"] = sum(peaks) / len(peaks) + + def benchmark_iter(): + list(blist) + + benchmark(benchmark_iter) + + +# Total span work is fixed at TOTAL_SPANS regardless of thread count so that +# ideal (GIL-free) parallelism would produce a flat wall-clock time across +# thread counts. Any increase instead reveals GIL + lock contention. +_TOTAL_SPANS = 128 +_ATTRS_PER_SPAN = {f"key{i}": f"value{i}" for i in range(50)} + + +@pytest.mark.parametrize("num_threads", [1, 2, 4, 8]) +def test_gil_contention_batch_processor(benchmark, num_threads): + exporter = InMemorySpanExporter() + provider = TracerProvider(sampler=sampling.DEFAULT_ON) + # max_export_batch_size=16 ensures the export threshold is crossed + # during the benchmark so _worker_awaken.set() contention is exercised. + provider.add_span_processor( + BatchSpanProcessor(exporter, max_export_batch_size=16) + ) + tracer = provider.get_tracer("bench") + spans_per_thread = _TOTAL_SPANS // num_threads + + def worker(): + for _ in range(spans_per_thread): + span = tracer.start_span("s") + span.set_attributes(_ATTRS_PER_SPAN) + span.end() + + def benchmark_fn(): + threads = [threading.Thread(target=worker) for _ in range(num_threads)] + for t in threads: + t.start() + for t in threads: + t.join() + + benchmark(benchmark_fn) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_shared_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_shared_internal/__init__.py index bf816e287e5..73d55d8d75b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_shared_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_shared_internal/__init__.py @@ -206,7 +206,7 @@ def emit(self, data: Telemetry) -> None: self._metrics.drop_items(1) # This will drop a log from the right side if the queue is at _max_queue_size. self._queue.appendleft(data) - if len(self._queue) >= self._max_export_batch_size: + if len(self._queue) >= self._max_export_batch_size and not self._worker_awaken.is_set(): self._worker_awaken.set() def shutdown(self, timeout_millis: int = 30000): diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index e3e42e28bec..a37f1fdad93 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -891,8 +891,9 @@ def set_attributes( logger.warning("Setting attribute on ended span.") return - for key, value in attributes.items(): - self._attributes[key] = value + with self._attributes._lock: + for key, value in attributes.items(): + self._attributes._setitem_locked(key, value) def set_attribute(self, key: str, value: types.AttributeValue) -> None: with self._lock: @@ -990,6 +991,7 @@ def end(self, end_time: int | None = None) -> None: return self._end_time = end_time if end_time is not None else time_ns() + self._attributes._immutable = True if self._record_end_metrics: self._record_end_metrics() From 94832e36a25884f6e6a43f0b004dc75509313396 Mon Sep 17 00:00:00 2001 From: Alex Boten <223565+codeboten@users.noreply.github.com> Date: Mon, 15 Jun 2026 10:28:37 -0700 Subject: [PATCH 2/6] move code out of hotpath Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com> --- .../src/opentelemetry/attributes/__init__.py | 35 ++++++++++++++----- .../src/opentelemetry/sdk/trace/__init__.py | 4 +-- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/attributes/__init__.py b/opentelemetry-api/src/opentelemetry/attributes/__init__.py index c8a5d95f959..e052a034e2d 100644 --- a/opentelemetry-api/src/opentelemetry/attributes/__init__.py +++ b/opentelemetry-api/src/opentelemetry/attributes/__init__.py @@ -277,23 +277,40 @@ def __getitem__(self, key: str) -> types.AnyValue: def __setitem__(self, key: str, value: types.AnyValue) -> None: if getattr(self, "_immutable", False): # type: ignore raise TypeError - with self._lock: - self._setitem_locked(key, value) - - def _setitem_locked(self, key: str, value: types.AnyValue) -> None: if self.maxlen is not None and self.maxlen == 0: - self.dropped += 1 + with self._lock: + self.dropped += 1 return - if self._extended_attributes: - value = _clean_extended_attribute( - key, value, self.max_value_len - ) + value = _clean_extended_attribute(key, value, self.max_value_len) else: value = _clean_attribute(key, value, self.max_value_len) # type: ignore if value is None: return + with self._lock: + self._setitem_locked(key, value) + def _set_many(self, attributes: "types._ExtendedAttributes") -> None: + if getattr(self, "_immutable", False): # type: ignore + raise TypeError + if self.maxlen is not None and self.maxlen == 0: + with self._lock: + self.dropped += len(attributes) + return + cleaned = [] + for key, value in attributes.items(): + if self._extended_attributes: + cv = _clean_extended_attribute(key, value, self.max_value_len) + else: + cv = _clean_attribute(key, value, self.max_value_len) # type: ignore + if cv is None: + continue + cleaned.append((key, cv)) + with self._lock: + for key, cv in cleaned: + self._setitem_locked(key, cv) + + def _setitem_locked(self, key: str, value: types.AnyValue) -> None: if key in self._dict: del self._dict[key] elif self.maxlen is not None and len(self._dict) == self.maxlen: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index a37f1fdad93..1f657e74d12 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -891,9 +891,7 @@ def set_attributes( logger.warning("Setting attribute on ended span.") return - with self._attributes._lock: - for key, value in attributes.items(): - self._attributes._setitem_locked(key, value) + self._attributes._set_many(attributes) def set_attribute(self, key: str, value: types.AttributeValue) -> None: with self._lock: From ff02c077cbe6c73a1839c9cc27ccbf87d60e96df Mon Sep 17 00:00:00 2001 From: Alex Boten <223565+codeboten@users.noreply.github.com> Date: Mon, 15 Jun 2026 11:22:15 -0700 Subject: [PATCH 3/6] lint Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com> --- .../benchmarks/trace/test_benchmark_trace.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/benchmarks/trace/test_benchmark_trace.py b/opentelemetry-sdk/benchmarks/trace/test_benchmark_trace.py index 4a4fd6329bc..bac15bcc79a 100644 --- a/opentelemetry-sdk/benchmarks/trace/test_benchmark_trace.py +++ b/opentelemetry-sdk/benchmarks/trace/test_benchmark_trace.py @@ -293,9 +293,9 @@ def worker(): def benchmark_fn(): threads = [threading.Thread(target=worker) for _ in range(num_threads)] - for t in threads: - t.start() - for t in threads: - t.join() + for thread in threads: + thread.start() + for thread in threads: + thread.join() benchmark(benchmark_fn) From e35c12129cc5675dbd9d26f398316491563d2020 Mon Sep 17 00:00:00 2001 From: Alex Boten <223565+codeboten@users.noreply.github.com> Date: Mon, 15 Jun 2026 11:23:32 -0700 Subject: [PATCH 4/6] changelog Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com> --- .changelog/5298.changed | 1 + 1 file changed, 1 insertion(+) create mode 100644 .changelog/5298.changed diff --git a/.changelog/5298.changed b/.changelog/5298.changed new file mode 100644 index 00000000000..ee6c7821d21 --- /dev/null +++ b/.changelog/5298.changed @@ -0,0 +1 @@ +opentelemetry-sdk: reduce lock contention in attributes \ No newline at end of file From 08995ce791c753217937996ad562439961c01edd Mon Sep 17 00:00:00 2001 From: Alex Boten <223565+codeboten@users.noreply.github.com> Date: Tue, 16 Jun 2026 13:00:29 -0700 Subject: [PATCH 5/6] apply feedback, fix lint Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com> --- opentelemetry-api/src/opentelemetry/attributes/__init__.py | 2 +- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/attributes/__init__.py b/opentelemetry-api/src/opentelemetry/attributes/__init__.py index e052a034e2d..15519ce0c2c 100644 --- a/opentelemetry-api/src/opentelemetry/attributes/__init__.py +++ b/opentelemetry-api/src/opentelemetry/attributes/__init__.py @@ -290,7 +290,7 @@ def __setitem__(self, key: str, value: types.AnyValue) -> None: with self._lock: self._setitem_locked(key, value) - def _set_many(self, attributes: "types._ExtendedAttributes") -> None: + def _set_items(self, attributes: "types._ExtendedAttributes") -> None: if getattr(self, "_immutable", False): # type: ignore raise TypeError if self.maxlen is not None and self.maxlen == 0: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 1f657e74d12..81cefb5ccfc 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -891,7 +891,7 @@ def set_attributes( logger.warning("Setting attribute on ended span.") return - self._attributes._set_many(attributes) + self._attributes._set_items(attributes) # pylint: disable=protected-access def set_attribute(self, key: str, value: types.AttributeValue) -> None: with self._lock: @@ -989,7 +989,7 @@ def end(self, end_time: int | None = None) -> None: return self._end_time = end_time if end_time is not None else time_ns() - self._attributes._immutable = True + self._attributes._immutable = True # pylint: disable=protected-access if self._record_end_metrics: self._record_end_metrics() From f47948f8d71bddf741c024cc306576c0f9395c07 Mon Sep 17 00:00:00 2001 From: Alex Boten <223565+codeboten@users.noreply.github.com> Date: Tue, 16 Jun 2026 13:19:10 -0700 Subject: [PATCH 6/6] ruff format Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com> --- .../src/opentelemetry/sdk/_shared_internal/__init__.py | 5 ++++- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_shared_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_shared_internal/__init__.py index 73d55d8d75b..3e2b8a263a4 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_shared_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_shared_internal/__init__.py @@ -206,7 +206,10 @@ def emit(self, data: Telemetry) -> None: self._metrics.drop_items(1) # This will drop a log from the right side if the queue is at _max_queue_size. self._queue.appendleft(data) - if len(self._queue) >= self._max_export_batch_size and not self._worker_awaken.is_set(): + if ( + len(self._queue) >= self._max_export_batch_size + and not self._worker_awaken.is_set() + ): self._worker_awaken.set() def shutdown(self, timeout_millis: int = 30000): diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 81cefb5ccfc..d292fed8e55 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -891,7 +891,7 @@ def set_attributes( logger.warning("Setting attribute on ended span.") return - self._attributes._set_items(attributes) # pylint: disable=protected-access + self._attributes._set_items(attributes) # pylint: disable=protected-access def set_attribute(self, key: str, value: types.AttributeValue) -> None: with self._lock: @@ -989,7 +989,7 @@ def end(self, end_time: int | None = None) -> None: return self._end_time = end_time if end_time is not None else time_ns() - self._attributes._immutable = True # pylint: disable=protected-access + self._attributes._immutable = True # pylint: disable=protected-access if self._record_end_metrics: self._record_end_metrics()