Skip to content

[WIP] perf: serialize telemetry batches on shared thread pool - #1883

Open
jpnurmi wants to merge 3 commits into
jpnurmi/ref/sentry-telemetryfrom
jpnurmi/perf/thread-pool
Open

[WIP] perf: serialize telemetry batches on shared thread pool#1883
jpnurmi wants to merge 3 commits into
jpnurmi/ref/sentry-telemetryfrom
jpnurmi/perf/thread-pool

Conversation

@jpnurmi

@jpnurmi jpnurmi commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Warning

WIP    🚧🔨⏳⛔

  • Introduce a thread pool that runs tasks in parallel and invokes completion callbacks in submission order.
  • Add an internal telemetry module for coordinating logs and metrics startup, shutdown, and flush.
  • Let the telemetry module own a shared serialization pool for enabled telemetry batchers.
  • Use the pool for log and metric batch serialization while keeping completion ordered through the batcher.
image

before-vs-after.ftrace.zip

Close: #1862

@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Fails
🚫 Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

### Features

- serialize telemetry batches on shared thread pool ([#1883](https://github.com/getsentry/sentry-native/pull/1883))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description or adding a skip-changelog label.

Generated by 🚫 dangerJS against ba24ac6

Comment thread src/sentry_telemetry.c
Comment thread src/sentry_batcher.c
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.54545% with 90 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.04%. Comparing base (ff2f6a7) to head (ba24ac6).

Additional details and impacted files
@@                       Coverage Diff                        @@
##           jpnurmi/ref/sentry-telemetry    #1883      +/-   ##
================================================================
+ Coverage                         69.80%   76.04%   +6.23%     
================================================================
  Files                                94       94              
  Lines                             22133    22545     +412     
  Branches                           3930     4018      +88     
================================================================
+ Hits                              15451    17144    +1693     
+ Misses                             4719     4515     -204     
+ Partials                           1963      886    -1077     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jpnurmi
jpnurmi force-pushed the jpnurmi/perf/thread-pool branch from 2996a56 to ec39dc8 Compare July 22, 2026 14:55
Comment thread src/backends/sentry_backend_inproc.c
Comment thread src/sentry_batcher.c
@jpnurmi
jpnurmi force-pushed the jpnurmi/perf/thread-pool branch from ec39dc8 to 829189b Compare July 22, 2026 15:14
Comment thread src/backends/sentry_backend_breakpad.cpp
@jpnurmi
jpnurmi force-pushed the jpnurmi/perf/thread-pool branch from 829189b to 406b054 Compare July 22, 2026 15:47
Comment thread src/sentry_batcher.c
Comment thread src/sentry_telemetry.c
@jpnurmi
jpnurmi force-pushed the jpnurmi/perf/thread-pool branch from 406b054 to 03159d6 Compare July 22, 2026 16:02
Comment thread src/sentry_batcher.c
Comment thread src/sentry_batcher.c
Comment thread src/sentry_transport.c
Comment thread src/sentry_telemetry.c
@jpnurmi
jpnurmi force-pushed the jpnurmi/perf/thread-pool branch from e417bed to 12f4872 Compare July 24, 2026 10:19
Comment thread src/sentry_telemetry.c
Comment thread src/sentry_batcher.c
@jpnurmi
jpnurmi force-pushed the jpnurmi/perf/thread-pool branch 4 times, most recently from 33f0d1a to d39154d Compare August 3, 2026 08:43
@jpnurmi
jpnurmi changed the base branch from master to jpnurmi/ref/transport-crash-dump August 3, 2026 08:55
@jpnurmi
jpnurmi force-pushed the jpnurmi/perf/thread-pool branch from d39154d to 9dee343 Compare August 3, 2026 09:39
@jpnurmi
jpnurmi force-pushed the jpnurmi/perf/thread-pool branch 2 times, most recently from 265b68d to 8c2f921 Compare August 3, 2026 12:53
Comment thread src/sentry_sync.c
Base automatically changed from jpnurmi/ref/transport-crash-dump to master August 3, 2026 16:22
@jpnurmi
jpnurmi force-pushed the jpnurmi/perf/thread-pool branch 2 times, most recently from 8e78e16 to e92e866 Compare August 3, 2026 16:28
@jpnurmi
jpnurmi changed the base branch from master to jpnurmi/ref/sentry-telemetry August 3, 2026 16:28
jpnurmi added 2 commits August 3, 2026 20:25
Add a bounded thread pool that runs tasks in parallel and invokes completion
callbacks in submission order.

Add unit coverage for ordered parallel execution.
Let the telemetry lifecycle own a shared serialization pool for enabled
telemetry batchers.

Use the pool for log and metric batch serialization while keeping completion
ordered through the batcher flush lifecycle.
@jpnurmi
jpnurmi force-pushed the jpnurmi/perf/thread-pool branch from e92e866 to 9793aaf Compare August 3, 2026 18:27
Comment thread src/sentry_batcher.c
Make sentry__threadpool_submit consume task data consistently by
calling the cleanup callback when submission fails. This matches the
existing task ownership model and keeps callers from having to duplicate
cleanup on rejection.
Comment thread src/sentry_telemetry.c

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ba24ac6. Configure here.

Comment thread src/sentry_batcher.c
|| sentry__atomic_fetch(&task->state)
== SENTRY_BATCH_TASK_RUNNING;
}
unlock_tasks(batcher);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spinlock held across heavy dump work

Medium Severity

batch_task_dump_pending runs batch_func and sentry__run_write_envelope while dump_pending_all still holds task_lock. Pool workers use a plain sentry__spin_lock to publish READY, so they busy-spin for the whole serialization and disk write. That stretches crash-flush latency and makes the RUNNING-wait timeout much easier to hit.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ba24ac6. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Logs: optimize performance

1 participant