Open
Conversation
Signed-off-by: Bob Weinand <[email protected]>
…ge on wordpress Batch ack sending too Signed-off-by: Bob Weinand <[email protected]>
The sidecar's write_shm_file() serializes buffered_integrations as HashSet<Integration>, but the PHP-side reader in components-rs deserialized it as HashSet<String>. This mismatch has existed since the SHM cache was introduced in 91222ad ("feat: reduce telemetry sent", #3316), but was latent: config_sent was set true before any integrations appeared in SHM, and integration dedup (the only other consumer) was silently broken with the sidecar deduplicating server-side as a fallback. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Signed-off-by: Bob Weinand <[email protected]>
bwoebi
force-pushed
the
bob/thread-sidecar
branch
from
1efe005 to
e673db0
Compare
|
✨ Fix all issues with BitsAI or with Cursor
|
While the sidecar has been a process global for the longest time, it's also unfair with respect to limits and not compatible with the newly introduced outbox. Having a connection per thread ensures that every thread can have up to 100 queued items. It also reduces contention on the SidecarTransport, which was indeed adding quite a bit of latency due to threads accumulating in futex_wait for very short requests under moderate load. Moving the background sender stats to its own thread in preparation of dropping the Mutex on the SidecarTransport fully. Signed-off-by: Bob Weinand <[email protected]>
bwoebi
force-pushed
the
bob/thread-sidecar
branch
from
e673db0 to
1b4b37f
Compare
Leiyks
reviewed
Apr 2, 2026
| dd_signal_data.uc = uc; | ||
|
|
||
| if (ddtrace_sidecar) { | ||
| if (ddtrace_sidecar_for_signal) { |
Contributor
There was a problem hiding this comment.
Suggested change
| if (ddtrace_sidecar_for_signal) { | |
| if (atomic_load(&ddtrace_sidecar_for_signal)) { |
Comment on lines
537
to
538
| #endif | ||
| if (ddtrace_sidecar_active_mode == DD_SIDECAR_CONNECTION_THREAD && |
Contributor
There was a problem hiding this comment.
Suggested change
| #endif | |
| atomic_store(&ddtrace_sidecar_for_signal, NULL); | |
| if (ddtrace_sidecar_active_mode == DD_SIDECAR_CONNECTION_THREAD && |
Probably best to clear this here instead of later to avoid any case of use-after-free don't you think ? 🤔
Comment on lines
+874
to
+881
| void ddtrace_sidecar_gshutdown(void) { | ||
| if (DDTRACE_G(sidecar)) { | ||
| // Drain any accumulated background-sender metrics before the transport goes away. | ||
| ddtrace_telemetry_flush_bgs_metrics_final(); | ||
| ddog_sidecar_transport_drop(DDTRACE_G(sidecar)); | ||
| DDTRACE_G(sidecar) = NULL; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Shoudn't this also clear ddtrace_sidecar_for_signal ?
bwoebi
force-pushed
the
bob/batch-endpoint
branch
from
8ecdabc to
3503499
Compare
bwoebi
force-pushed
the
bob/batch-endpoint
branch
2 times, most recently
from
0db877a to
83d3af1
Compare
Signed-off-by: Bob Weinand <[email protected]>
bwoebi
force-pushed
the
bob/thread-sidecar
branch
from
d58b820 to
7b5db07
Compare
bwoebi
force-pushed
the
bob/batch-endpoint
branch
2 times, most recently
from
9b92ee4 to
5cf8ebb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While the sidecar has been a process global for the longest time, it's also unfair with respect to limits and not compatible with the newly introduced outbox.
Having a connection per thread ensures that every thread can have up to 100 queued items. It also reduces contention on the SidecarTransport, which was indeed adding quite a bit of latency due to threads accumulating in futex_wait for very short requests under moderate load.
Moving the background sender stats to its own thread in preparation of dropping the Mutex on the SidecarTransport fully.