Skip to content

Gracefully handle sidecar broken pipes#3370

Merged
bwoebi merged 2 commits intomasterfrom
bob/sidecar-reconnect
Aug 6, 2025
Merged

Gracefully handle sidecar broken pipes#3370
bwoebi merged 2 commits intomasterfrom
bob/sidecar-reconnect

Conversation

@bwoebi
Copy link
Copy Markdown
Collaborator

@bwoebi bwoebi commented Aug 1, 2025

This now reconnects immediately, gracefully restoring telemetry and remote config state (i.e. resending service / env etc. tags), making sure the queue_id exists on the sidecar side.

This should avoid significant data loss scenarios and infinite broken pipe loops.

@bwoebi bwoebi requested review from a team as code owners August 1, 2025 20:21
@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented Aug 1, 2025

Benchmarks [ tracer ]

Benchmark execution time: 2025-08-05 21:44:54

Comparing candidate commit e1e9649 in PR branch bob/sidecar-reconnect with baseline commit dc8ab47 in branch master.

Found 1 performance improvements and 2 performance regressions! Performance is the same for 175 metrics, 0 unstable metrics.

scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache

  • 🟩 execution_time [-6.659µs; -4.241µs] or [-3.187%; -2.030%]

scenario:PDOBench/benchPDOBaseline

  • 🟥 execution_time [+2.657µs; +4.656µs] or [+4.323%; +7.575%]

scenario:TraceSerializationBench/benchSerializeTrace

  • 🟥 execution_time [+36.514µs; +62.086µs] or [+2.749%; +4.674%]

Comment thread components-rs/sidecar.rs
// Generally the Send buffer ought to be big enough for instantaneous transmission
_ = stream.set_write_timeout(Some(Duration::from_millis(100)));
_ = stream.set_read_timeout(Some(Duration::from_secs(10)));
// We do not put reconnect_fn into sidecar_connect, as the reconnect shall not reconnect again on error to prevent recursion
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good comment 👍🏻

Copy link
Copy Markdown
Collaborator

@morrisonlevi morrisonlevi left a comment

Choose a reason for hiding this comment

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

Looks good.

This now reconnects immediately, gracefully restoring telemetry and remote config state (i.e. resending service / env etc. tags), making sure the queue_id exists on the sidecar side.

This should avoid significant data loss scenarios and infinite broken pipe loops.

Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
@bwoebi bwoebi force-pushed the bob/sidecar-reconnect branch from 72eef1e to 5075b3c Compare August 5, 2025 18:07
Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.82%. Comparing base (dc8ab47) to head (e1e9649).
⚠️ Report is 464 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3370      +/-   ##
==========================================
- Coverage   61.95%   61.82%   -0.13%     
==========================================
  Files         140      140              
  Lines       12388    12388              
  Branches     1622     1622              
==========================================
- Hits         7675     7659      -16     
- Misses       3999     4015      +16     
  Partials      714      714              

see 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc8ab47...e1e9649. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bwoebi bwoebi merged commit 21ba23e into master Aug 6, 2025
1842 of 1878 checks passed
@bwoebi bwoebi deleted the bob/sidecar-reconnect branch August 6, 2025 17:37
@github-actions github-actions Bot added this to the 1.12.0 milestone Aug 6, 2025
@realFlowControl realFlowControl mentioned this pull request Sep 10, 2025
2 tasks
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.

3 participants