Skip to content

opentelemetry-sdk: make 'consume_measurement' lock free#5321

Merged
lzchen merged 3 commits into
open-telemetry:mainfrom
herin049:fix/consume-measurement-deadlock-regression
Jun 18, 2026
Merged

opentelemetry-sdk: make 'consume_measurement' lock free#5321
lzchen merged 3 commits into
open-telemetry:mainfrom
herin049:fix/consume-measurement-deadlock-regression

Conversation

@herin049

@herin049 herin049 commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

This PR fixes a deadlock scenario with consume_measurement() in SynchronousMeasurementConsumer by changing _reader_storages to use COW semantics.

The original PR #4863 also introduced a significant performance regression as seen in the benchmarks here: https://opentelemetry.io/docs/languages/python/benchmarks/

This fix brings the performance close to the performance before these changes were introduced:

  ┌───────────────────────────────────────────────────────────────────────┬──────────────┬───────────┬────────┬─────────┐
  │                               Benchmark                               │ without (ns) │ with (ns) │ change │ speedup │
  ├───────────────────────────────────────────────────────────────────────┼──────────────┼───────────┼────────┼─────────┤
  │ test_counter_add[0-cumulative]                                        │         2167 │      1208 │ −44.3% │   1.79× │
  ├───────────────────────────────────────────────────────────────────────┼──────────────┼───────────┼────────┼─────────┤
  │ test_counter_add[10-cumulative]                                       │         2458 │      1500 │ −39.0% │   1.64× │
  ├───────────────────────────────────────────────────────────────────────┼──────────────┼───────────┼────────┼─────────┤
  │ test_counter_add[0-delta]                                             │         2167 │      1208 │ −44.3% │   1.79× │
  ├───────────────────────────────────────────────────────────────────────┼──────────────┼───────────┼────────┼─────────┤
  │ test_counter_add[10-delta]                                            │         2458 │      1500 │ −39.0% │   1.64× │
  ├───────────────────────────────────────────────────────────────────────┼──────────────┼───────────┼────────┼─────────┤
  │ test_up_down_counter_add[0]                                           │         2125 │      1208 │ −43.2% │   1.76× │
  ├───────────────────────────────────────────────────────────────────────┼──────────────┼───────────┼────────┼─────────┤
  │ test_up_down_counter_add[10]                                          │         2458 │      1500 │ −39.0% │   1.64× │
  ├───────────────────────────────────────────────────────────────────────┼──────────────┼───────────┼────────┼─────────┤
  │ test_counter_add_with_meter_configurator_rules[1]                     │         2167 │      1208 │ −44.3% │   1.79× │
  ├───────────────────────────────────────────────────────────────────────┼──────────────┼───────────┼────────┼─────────┤

Fixes #5313

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

uv run tox -e py314t-test-opentelemetry-sdk

Does This PR Require a Contrib Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@herin049
herin049 requested a review from a team as a code owner June 18, 2026 03:24
@JP-MY

JP-MY commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

@herin049 Thanks for the quick fix, I had a look and the change seems good. Sorry about any issues caused by the last PR.

@github-project-automation github-project-automation Bot moved this to Approved PRs in Python PR digest Jun 18, 2026
@MikeGoldsmith MikeGoldsmith changed the title opentelemetry-sdk: make 'consume_measurment' lock free opentelemetry-sdk: make 'consume_measurement' lock free Jun 18, 2026
DylanRussell
DylanRussell reviewed Jun 18, 2026
View reviewed changes
Comment thread opentelemetry-sdk/tests/metrics/test_measurement_consumer.py
@lzchen
lzchen added this pull request to the merge queue Jun 18, 2026
Merged via the queue into open-telemetry:main with commit 30bc92b Jun 18, 2026
519 checks passed
@github-project-automation github-project-automation Bot moved this from Approved PRs to Done in Python PR digest Jun 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xrmx xrmx xrmx approved these changes
@MikeGoldsmith MikeGoldsmith MikeGoldsmith approved these changes
@DylanRussell DylanRussell DylanRussell approved these changes
@lzchen lzchen lzchen approved these changes
+1 more reviewer
@JP-MY JP-MY JP-MY left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

Python PR digest
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug in PR#4863 causes deadlocks when GC duration is instrumented

6 participants

@herin049 @JP-MY @xrmx @MikeGoldsmith @DylanRussell @lzchen