Skip to content

feat: Make transport channel capacity configurable#1040

Open
mvanhorn wants to merge 7 commits intogetsentry:masterfrom
mvanhorn:feat/configurable-transport-channel-capacity
Open

feat: Make transport channel capacity configurable#1040
mvanhorn wants to merge 7 commits intogetsentry:masterfrom
mvanhorn:feat/configurable-transport-channel-capacity

Conversation

@mvanhorn
Copy link
Copy Markdown
Contributor

@mvanhorn mvanhorn commented Mar 27, 2026

Summary

Adds transport-level with_channel_capacity constructors so users can tune the bounded channel size used by the transport thread. The default remains 30, preserving existing behavior.

The original design added a transport_channel_capacity field to ClientOptions. Per @szokeasaurusrex's review, refactored to additive transport-level methods so ClientOptions stays minimal and the feature is opt-in at the transport boundary.

Why this matters

In high-throughput scenarios (many transactions with single spans each), the hardcoded capacity of 30 can saturate quickly, leading to dropped envelopes. Identified in #923, tracked in #994. Making it configurable lets users trade memory for reliability based on their workload.

Changes

  • sentry/src/transports/thread.rs: TransportThread::new(send) keeps its original signature. New TransportThread::with_capacity(send, channel_capacity) accepts a custom capacity, clamped to a minimum of 1 to avoid rendezvous channels.
  • sentry/src/transports/tokio_thread.rs: Same pattern for the async transport variant.
  • sentry/src/transports/curl.rs: Added CurlHttpTransport::with_channel_capacity(options, channel_capacity). new(options) unchanged.
  • sentry/src/transports/reqwest.rs: Added ReqwestHttpTransport::with_channel_capacity(options, channel_capacity). new(options) / with_client(options, client) unchanged.
  • sentry/src/transports/ureq.rs: Added UreqHttpTransport::with_channel_capacity(options, channel_capacity). new(options) / with_agent(options, agent) unchanged.

Usage

let opts = ClientOptions {
    transport: Some(Arc::new(move |opts| {
        Arc::new(ReqwestHttpTransport::with_channel_capacity(opts, 256))
    })),
    ..Default::default()
};

Testing

  • cargo check --workspace --all-features passes
  • cargo fmt --all clean
  • cargo clippy --workspace --all-features clean
  • Default behavior unchanged (capacity stays at 30 unless explicitly configured)

Closes #994

This contribution was developed with AI assistance (Claude Code).

Add `transport_channel_capacity` field to `ClientOptions` that controls
the bounded sync channel size used by the transport thread. Defaults to
30 (preserving current behavior). Users in high-throughput scenarios can
increase this to reduce the chance of dropped envelopes.

Closes getsentry#994

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread sentry-core/src/clientoptions.rs Outdated
Comment thread sentry/src/transports/thread.rs Outdated
Comment thread sentry-core/src/clientoptions.rs Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 13, 2026

Codecov Report

❌ Patch coverage is 8.67347% with 179 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.97%. Comparing base (a57b91c) to head (6d1c9ff).
⚠️ Report is 81 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1040      +/-   ##
==========================================
+ Coverage   73.81%   73.97%   +0.16%     
==========================================
  Files          64       67       +3     
  Lines        7538     7959     +421     
==========================================
+ Hits         5564     5888     +324     
- Misses       1974     2071      +97     

@szokeasaurusrex
Copy link
Copy Markdown
Member

Hi @mvanhorn, thanks for the contribution! Before I proceed to a full review, please address the CI failures and the AI review agent comments. If any of the AI review comments are inaccurate, please comment on them and mark as resolved. Thanks!

- Add transport_channel_capacity to manual Debug implementation
- Clamp channel capacity to minimum of 1 to prevent rendezvous
  channel from silently dropping envelopes via try_send
- Run cargo fmt to fix lint CI failure
@mvanhorn
Copy link
Copy Markdown
Contributor Author

Addressed in 9c8f904:

  • Added transport_channel_capacity to the manual Debug impl
  • Clamped capacity to min 1 to prevent rendezvous channel (try_send would silently drop)
  • Ran cargo fmt

Copy link
Copy Markdown
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

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

Thanks again for the contribution. I think we should revise the public API shape to avoid public API breakages before merging this change.

The current implementation introduces public API breakage by:

  • adding a new public field to ClientOptions
  • changing the signatures of the public transport thread constructors

I would like to avoid those breakages here and instead expose this as additive API:

  • add additive with_capacity(...) constructors on the public transport thread types, i.e. StdTransportThread::with_capacity(send, channel_capacity) and TokioTransportThread::with_capacity(send, channel_capacity)
  • keep the existing new(...) signatures unchanged, but make those constructors delegate to with_capacity(..., 30)
  • add transport-specific constructors such as ReqwestHttpTransport::with_channel_capacity(options, channel_capacity) (and similarly for curl and ureq), which would use those new with_capacity(...) thread constructors internally
  • document use through ClientOptions.transport

That still gives users a way to override the transport queue capacity via the existing TransportFactory mechanism, without changing ClientOptions yet.

For example, initializing the SDK with a custom capacity could look like this:

let opts = ClientOptions {
    transport: Some(Arc::new(move |opts| {
        Arc::new(ReqwestHttpTransport::with_channel_capacity(opts, 256))
    })),
    ..Default::default()
};

If you are open to it, please refactor the PR in that direction. If not, let me know and I can take it over as a follow-up.

…el_capacity

Per review: avoid public API breakages in ClientOptions and the transport
constructors. Replace the 'transport_channel_capacity: usize' field and the
changed TransportThread/transport constructor signatures with purely
additive APIs:

- TransportThread::new(send) restored to original signature, delegates to
  with_capacity(send, 30). TransportThread::with_capacity(send, capacity)
  is the new entry point for custom capacity.
- Same pattern for tokio_thread::TransportThread.
- ReqwestHttpTransport/CurlHttpTransport/UreqHttpTransport gain
  with_channel_capacity(options, capacity). Existing new/with_client/
  with_agent keep the default capacity of 30.
- Remove transport_channel_capacity from ClientOptions (field, Default,
  and Debug impl).

Users that need to override capacity configure it via
ClientOptions.transport with a factory returning the desired transport,
e.g. Arc::new(move |opts| Arc::new(ReqwestHttpTransport::with_channel_capacity(opts, 256))).
@mvanhorn
Copy link
Copy Markdown
Contributor Author

Thanks for the direction @szokeasaurusrex. Refactored to the additive shape you described in fbff3ea:

  • TransportThread::new(send) restored to the original signature. New TransportThread::with_capacity(send, channel_capacity) is the entry point for custom capacity (clamped to a minimum of 1 to avoid the rendezvous channel). Same pattern in tokio_thread::TransportThread.
  • ReqwestHttpTransport::with_channel_capacity(options, channel_capacity) added. new(options) / with_client(options, client) keep the default capacity of 30. Same for CurlHttpTransport and UreqHttpTransport (with_agent likewise unchanged).
  • Removed transport_channel_capacity from ClientOptions (field, Default, and the manual Debug impl).

Your example works unchanged:

let opts = ClientOptions {
    transport: Some(Arc::new(move |opts| {
        Arc::new(ReqwestHttpTransport::with_channel_capacity(opts, 256))
    })),
    ..Default::default()
};

cargo check --workspace --all-features, cargo fmt --all, and cargo clippy --workspace --all-features all pass.

Comment thread sentry/src/transports/curl.rs Outdated
Copy link
Copy Markdown

@cursor cursor Bot left a comment

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 fbff3ea. Configure here.

Comment thread sentry/src/transports/reqwest.rs Outdated
@lcian lcian removed their request for review April 15, 2026 09:49
@mvanhorn
Copy link
Copy Markdown
Contributor Author

@szokeasaurusrex - the refactor in fbff3ea matches the additive-API shape you requested. I've also replied to and resolved the two stale bot comments from Apr 15, which were analyzing against the original design. Verified locally: cargo build --features reqwest succeeds and the sentry crate tests pass. Ready for your full review whenever you have time.

@szokeasaurusrex
Copy link
Copy Markdown
Member

@mvanhorn I have this PR on my list of things to review. I am busy with another project at the moment but will try to review this within the next week or so

@mvanhorn
Copy link
Copy Markdown
Contributor Author

Thanks @szokeasaurusrex - no rush, whenever you get a chance.

Copy link
Copy Markdown
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

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

Thanks for your patience for my review. I added some small comments; this looks pretty good overall though!

Comment thread sentry/src/transports/tokio_thread.rs Outdated
/// `send` blocks. `channel_capacity` is clamped to a minimum of 1 to
/// avoid a rendezvous channel, which would silently drop envelopes under
/// `try_send`.
pub fn with_capacity<SendFn, SendFuture>(mut send: SendFn, channel_capacity: usize) -> Self
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

m: Let's make this pub(crate) for now to limit public API surface. If folks want to have this as a public API in the future, we can expose it at that time.

Suggested change
pub fn with_capacity<SendFn, SendFuture>(mut send: SendFn, channel_capacity: usize) -> Self
pub(crate) fn with_capacity<SendFn, SendFuture>(mut send: SendFn, channel_capacity: usize) -> Self

Comment thread sentry/src/transports/thread.rs Outdated
/// `send` blocks. `channel_capacity` is clamped to a minimum of 1 to
/// avoid a rendezvous channel, which would silently drop envelopes under
/// `try_send`.
pub fn with_capacity<SendFn>(mut send: SendFn, channel_capacity: usize) -> Self
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

m: Let's make this pub(crate) for now to limit public API surface. If folks want to have this as a public API in the future, we can expose it at that time.

Suggested change
pub fn with_capacity<SendFn>(mut send: SendFn, channel_capacity: usize) -> Self
pub(crate) fn with_capacity<SendFn>(mut send: SendFn, channel_capacity: usize) -> Self

Comment thread sentry/src/transports/curl.rs Outdated
/// Creates a new Transport.
pub fn new(options: &ClientOptions) -> Self {
Self::new_internal(options, None)
Self::new_internal(options, None, 30)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

m: As we use the number 30 as the default channel capacity in all the transports, we should extract it to a constant that we reuse in all of them.

…TY const

Per @szokeasaurusrex review: with_capacity narrowed to pub(crate)
on both TransportThread types; the 30 literal extracted to a shared
pub(crate) const reused across curl, reqwest, and ureq transports.

Includes: Authorized scope (mod.rs) + correct extension (ureq.rs has
the same literal). Mechanical change verified by build + fmt; no
runtime effect needs test coverage.
@mvanhorn
Copy link
Copy Markdown
Contributor Author

mvanhorn commented May 5, 2026

Addressed in 6d1c9ff:

  • Both TransportThread::with_capacity are now pub(crate) per your suggestion.
  • Extracted the 30 default to pub(crate) const DEFAULT_CHANNEL_CAPACITY: usize = 30 in transports/mod.rs and reused it across curl, reqwest, and ureq transports (plus both thread modules' new defaults). Open to moving the const to transports/thread.rs instead if that reads cleaner -- happy to follow your preference.

Verified cargo fmt --check, cargo build --features reqwest, and cargo build --features curl locally before pushing.

Copy link
Copy Markdown
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

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

Hey, thanks for addressing those! I just have one more thought about the clamping, then I think this is good to merge

where
SendFn: FnMut(Envelope, &mut RateLimiter) + Send + 'static,
{
let (sender, receiver) = sync_channel(channel_capacity.max(1));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should honor 0 here instead of clamping it. This is an advanced, opt-in transport API, and sync_channel(0) has defined rendezvous/no-buffer semantics. A capacity of 1 can still drop most events under bursts; it is not a general safeguard, only a different buffering policy. If someone explicitly passes 0, treating that as “no queued buffering” seems reasonable.

I tested this end-to-end with the clamp removed and with_channel_capacity(..., 0): sending 10 rapid events accepted 1 and dropped 9. That matches the channel semantics: zero capacity does not necessarily drop everything; it accepts an envelope when try_send happens while the transport thread is already waiting on the receiver. If we keep support for 0, the doc comment should describe that behavior rather than saying it would silently drop envelopes generally.

// NOTE: returning RateLimiter here, otherwise we are in borrow hell
SendFuture: std::future::Future<Output = RateLimiter>,
{
let (sender, receiver) = sync_channel(channel_capacity.max(1));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same concept applies here; I tested both transport thread variants with the clamp removed, and both accepted 1 of 10 rapid events with capacity 0 rather than dropping everything.

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.

Make transport channel capacity configurable

2 participants