Repository navigation
feat: do not entirely disable connection pooling for periodic connections - #2440
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
More details
Both HTTP backends set the same five-second idle limit for periodic clients. The changed call sites use the new periodic policy consistently.
🤖 Datadog Autotest · Commit 8d26774 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
|
✅ All CI checks and tests passed. 🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 1444009 | Docs | View more details | Give us feedback! |
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
Periodic clients (telemetry flushes, remote config polling, etc.) used to disable connection pooling entirely, which meant every request paid a full TLS handshake. That is quite costly for agentless traffic (on the order of magnitude of 0.5s per connection), and short-lived apps send several separate requests in a short span of time. Instead of disabling pooling, `new_client_periodic` now pools connections with a small idle timeout (5s), much smaller than typical keep-alive timeouts on the receiving end, so we still avoid reusing a connection the receiver may have closed.
BenchmarksComparisonBenchmark execution time: 2026-09-02 09:09:36 Comparing candidate commit 1444009 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 135 metrics, 1 unstable metrics.
|
8d26774 to
929afab
Compare
|
/merge |
|
View all feedbacks in Devflow UI.
It will be processed automatically as soon as GitHub reports it as mergeable. View in MergeQueue UI.
The expected merge time in
|
## Description This PR removes unnecessary flushing logic for telemetry happening at app closing. #19368 switched to the native Rust telemetry implementation. I suspect that the previous underlying stop() implementation didn't flush/sent app-closing event automatically, but the new native telemetry worker does on shutdown (both flush and sending `app-closing`), so: 1. It's meaningless to have the `send_app_closing` tweak anymore on `stop()`: unless we change the telemetry worker in libdatadog, we can't opt out of `app-closing`. I kept the parameter because I feel like this ought to be fixed and not just ignored, but it was already ineffective. Happy to do the follow-up work in libdatadog to provide a way to stop the worker without sending the `app-closing` event if requested. 2. It's useless to flush in the Rust wrapping code of dd-trace-py before calling to libdatadog's `stop()`: the worker shutdown code already takes care of the flush 3. It's useless to explicitly flush in the Python implementation before calling to Rust (for the same reason) The additional flush is creating the following batches for a short-lived app that does nothing: ``` Sent ["app-started"] Sent ["app-dependencies-loaded", "app-heartbeat"] Sent ["app-closing"] ``` While the new version does the expected ``` Sent ["app-started"] Sent ["app-dependencies-loaded", "app-closing"] ``` This has been discovered as part of investigating #19915, and does help getting back a few dozen ms of shutdown time (50 to 100ms locally, once pooling has been enabled - see (DataDog/libdatadog#2440)) by saving one HTTPS request. ## Testing Tested locally on the repro of #19915. Included a regression test. ## Risks No major risk identified. ## Additional Notes See https://datadoghq.atlassian.net/browse/APMS-20441 Co-authored-by: yann.hamdaoui <yann.hamdaoui@datadoghq.com>
…ker, libdd-data-pipeline, li... (#2482) # Release proposal for libdd-capabilities-impl, libdd-crashtracker, libdd-data-pipeline, libdd-ddsketch, libdd-ffe, libdd-http-client, libdd-ipc, libdd-library-config, libdd-live-debugger, libdd-otel-thread-ctx, libdd-remote-config, libdd-shared-runtime, libdd-telemetry, libdd-trace-utils, libdd-tracer-flare and their dependencies This PR contains version bumps based on public API changes and commits since last release. ###⚠️ Crates left out of this proposal affected by its major bumps These publishable workspace crates are not part of this release but their dependency requirement was rewritten on this branch while their published version still requires the old major. If they are a dependency on your deployment not including them in the release could result in duplicate packages or symbol incompatibility. - `libdd-common` `5.2.0` → `6.0.0` affects: `libdd-profiling`, `libdd-sampling` - `libdd-trace-utils` `11.0.0` → `12.0.0` affects: `libdd-sampling` ## libdd-capabilities **Next version:** `3.0.1` **Semver bump:** `patch` **Tag:** `libdd-capabilities-v3.0.1` ### Commits - refactor: migrate HTTP & networking deps to workspace level (phase 4bis) (#2350) - feat: do not entirely disable connection pooling for periodic connections (#2440) ## libdd-common **Next version:** `6.0.0` **Semver bump:** `major` **Tag:** `libdd-common-v6.0.0` ### Commits - refactor: migrate HTTP & networking deps to workspace level (phase 4bis) (#2350) - feat: do not entirely disable connection pooling for periodic connections (#2440) - feat(data-pipeline)!: add agentless stats export (#2309) - feat(data-pipeline): add runtime-independent agentless sending (#2389) - feat(common)!: add HTTPS_PROXY support for hyper_backend (#2421) ## libdd-ipc-macros **Next version:** `1.0.1` **Semver bump:** `patch` **Tag:** `libdd-ipc-macros-v1.0.1` ### Commits - feat(sidecar)!: support appsec helper-rust integration with sidecar (#2310) ## libdd-otel-thread-ctx **Next version:** `1.1.0` **Semver bump:** `minor` **Tag:** `libdd-otel-thread-ctx-v1.1.0` ### Commits - feat(otel-thread-ctx): add update-and-attach operation (#2443) ## libdd-tinybytes **Next version:** `1.1.3` **Semver bump:** `patch` **Tag:** `libdd-tinybytes-v1.1.3` ### Commits - refactor: migrate HTTP & networking deps to workspace level (phase 4bis) (#2350) ## libdd-capabilities-impl **Next version:** `5.0.0` **Semver bump:** `major` **Tag:** `libdd-capabilities-impl-v5.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^5.1.1 → ^6.0.0 ### Commits - refactor: migrate HTTP & networking deps to workspace level (phase 4bis) (#2350) - feat: do not entirely disable connection pooling for periodic connections (#2440) - feat(data-pipeline): add runtime-independent agentless sending (#2389) ## libdd-http-client **Next version:** `2.0.0` **Semver bump:** `major` **Tag:** `libdd-http-client-v2.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^5.1.1 → ^6.0.0 ### Commits - refactor: migrate HTTP & networking deps to workspace level (phase 4bis) (#2350) - feat(data-pipeline): add runtime-independent agentless sending (#2389) - feat(common)!: add HTTPS_PROXY support for hyper_backend (#2421) ## libdd-remote-config **Next version:** `5.0.0` **Semver bump:** `major` **Tag:** `libdd-remote-config-v5.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^5.2.0 → ^6.0.0 ### Commits - fix(remote-config): refresh fetcher identity (#2469) - refactor: migrate HTTP & networking deps to workspace level (phase 4bis) (#2350) - fix(remote-config): reuse injected sleep capability (#2429) ## libdd-shared-runtime **Next version:** `4.0.0` **Semver bump:** `major` **Tag:** `libdd-shared-runtime-v4.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^5.2.0 → ^6.0.0 ### Commits - fix(shared-runtime): allow disabling worker fork restart (#2464) ## libdd-trace-utils **Next version:** `12.0.0` **Semver bump:** `major` **Tag:** `libdd-trace-utils-v12.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^5.2.0 → ^6.0.0 ### Commits - feat(trace-utils): add v1-native JSON log encoder brick (#2371) - feat(trace-utils): add v1-native agentless JSON encoder brick (#2370) - refactor: migrate HTTP & networking deps to workspace level (phase 4bis) (#2350) - fix(trace-stats): read OTel HTTP names for the status and method dimensions (#2323) - feat(data-pipeline): emit native trace export telemetry (#2338) - feat(data-pipeline)!: add agentless stats export (#2309) - fix(trace-utils): use vec map dedup when serializing (#2422) - feat(data-pipeline): add runtime-independent agentless sending (#2389) - fix(compression): align zstd behavior across targets (#2400) - feat(trace-utils)!: add from owned to SpanText (#2403) ## libdd-ffe **Next version:** `2.0.0` **Semver bump:** `major` **Tag:** `libdd-ffe-v2.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^5.1.1 → ^6.0.0 - `libdd-remote-config`: ^3.0.0 → ^4.1.0 - `libdd-trace-protobuf`: ^4.0.1 → ^5.0.0 ### Commits - refactor: migrate HTTP & networking deps to workspace level (phase 4bis) (#2350) - feat(ffe): support arbitrary semver core parts (#2413) - feat(ffe)!: send the split serial id on exposure events [EX-3425] (#2402) - feat(ffe): expose observeFullEvaluationData config-level FFI getter (#2373) - fix(ffe): report rejected flags as parse errors (#2339) - test: skip/shorten slow miri jobs (#2331) ## libdd-dogstatsd-client **Next version:** `6.0.0` **Semver bump:** `major` **Tag:** `libdd-dogstatsd-client-v6.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^5.2.0 → ^6.0.0 ### Commits - refactor: migrate HTTP & networking deps to workspace level (phase 4bis) (#2350) ## libdd-telemetry **Next version:** `8.0.0` **Semver bump:** `major` **Tag:** `libdd-telemetry-v8.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^5.2.0 → ^6.0.0 ### Commits - refactor: migrate HTTP & networking deps to workspace level (phase 4bis) (#2350) - refactor(telemetry): avoid doing two separate http requests in stop telemetry (#2435) ## libdd-trace-obfuscation **Next version:** `8.0.0` **Semver bump:** `major` **Tag:** `libdd-trace-obfuscation-v8.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^5.2.0 → ^6.0.0 - `libdd-trace-utils`: ^11.0.0 → ^12.0.0 ### Commits - fix(trace-obfuscation): scan all span meta for credit-card obfuscation (#2472) - refactor: migrate HTTP & networking deps to workspace level (phase 4bis) (#2350) - feat(data-pipeline)!: Obfuscate v04 spans in agentless context (#2418) ## libdd-tracer-flare **Next version:** `3.0.0` **Semver bump:** `major` **Tag:** `libdd-tracer-flare-v3.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^5.2.0 → ^6.0.0 - `libdd-trace-utils`: ^11.0.0 → ^12.0.0 ### Commits - refactor: migrate HTTP & networking deps to workspace level (phase 4bis) (#2350) ## libdd-crashtracker **Next version:** `3.0.0` **Semver bump:** `major` **Tag:** `libdd-crashtracker-v3.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^5.2.0 → ^6.0.0 ### Commits - refactor: migrate HTTP & networking deps to workspace level (phase 4bis) (#2350) - fix(crashtracking): filter out frames above faulting frame (#2428) - feat(sidecar)!: support appsec helper-rust integration with sidecar (#2310) - chore(crashtracking): use RAII remote ptrace API (#2416) - chore(crashtracking): bump libdd-libunwind-sys to v1.0.3 (#2414) ## libdd-data-pipeline-core **Next version:** `1.0.0` **Semver bump:** `major` **Tag:** `libdd-data-pipeline-core-v1.0.0` **Warning:** this is an initial release. Please verify that the version and commits included are correct. ## libdd-trace-stats **Next version:** `9.0.0` **Semver bump:** `major` **Tag:** `libdd-trace-stats-v9.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^5.2.0 → ^6.0.0 - `libdd-trace-obfuscation`: ^7.0.0 → ^8.0.0 - `libdd-trace-utils`: ^11.0.0 → ^12.0.0 ### Commits - refactor: migrate HTTP & networking deps to workspace level (phase 4bis) (#2350) - fix(trace-stats): read OTel HTTP names for the status and method dimensions (#2323) - feat(data-pipeline)!: add agentless stats export (#2309) - feat(trace-utils)!: add from owned to SpanText (#2403) ## libdd-data-pipeline **Next version:** `10.0.0` **Semver bump:** `major` **Tag:** `libdd-data-pipeline-v10.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^5.2.0 → ^6.0.0 - `libdd-trace-obfuscation`: ^7.0.0 → ^8.0.0 - `libdd-trace-stats`: ^8.0.0 → ^9.0.0 - `libdd-trace-utils`: ^11.0.0 → ^12.0.0 ### Commits - feat(trace-utils): add v1-native JSON log encoder brick (#2371) - chore: prepare crate for publishing (#2466) - refactor: migrate HTTP & networking deps to workspace level (phase 4bis) (#2350) - fix(trace-stats): read OTel HTTP names for the status and method dimensions (#2323) - fix(data-pipeline): pass obfuscation config to OTLP stats (#2444) - feat(data-pipeline): emit native trace export telemetry (#2338) - feat(data-pipeline): add fork-safe OTLP gRPC trace transport (#2273) - feat(data-pipeline)!: add agentless stats export (#2309) - feat(data-pipeline)!: Obfuscate v04 spans in agentless context (#2418) - feat(data-pipeline): add runtime-independent agentless sending (#2389) - feat(trace-utils)!: add from owned to SpanText (#2403) ## libdd-ipc **Next version:** `2.0.0` **Semver bump:** `major` **Tag:** `libdd-ipc-v2.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^5.2.0 → ^6.0.0 - `libdd-trace-stats`: ^8.0.0 → ^9.0.0 ### Commits - fix(ipc): drop the signal feature from libdd-ipc (#2431) ## libdd-live-debugger **Next version:** `1.0.0` **Semver bump:** `major` **Tag:** `libdd-live-debugger-v1.0.0` **Warning:** this is an initial release. Please verify that the version and commits included are correct. [EX-3425]: https://datadoghq.atlassian.net/browse/EX-3425?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: hoolioh <107922352+hoolioh@users.noreply.github.com>
…as well (#2449) # What does this PR do? Follow-up of #2440. Rename `new_no_connection_pooling` to reflect the more nuanced semantics of the function. Apply the same treatment to the equivalent functions in libdd-http-client and libdd-agent-client. # Motivation See #2440. # How to test the change? Tested locally on the repro mentioned in #2440. Co-authored-by: yann.hamdaoui <yann.hamdaoui@datadoghq.com>
What does this PR do?
This PR slightly changes the HTTP behavior for periodic connections (most flushes to the agent/agentless intake). Instead of disabling connection pooling to avoid data races entirely, it restricts the lifetime of pooled connections.
This PR aims to be a backward-compatible hotfix. A subsequent PR is coming with renaming (since
no_connection_poolingisn't really true anymore) and mirroring the change in libdd-http-client and libdd-agent-client as well.Motivation
Mitigates APMS-20441 / DataDog/dd-trace-py#19915: some telemetry events can generate several requests for short-lived scripts. In the agentless case, this means opening several new HTTPS connection in a row, which is slow.
Additional Notes
Applying the setting to all periodic connections sounds reasonable, as multiple requests could reasonably be issued for other things than telemetry. It should impact single-request workflows.
How to test the change?
This was tested with the repro in 20441, reducing the shutdown delay to 0.750ms locally, which indicates there's indeed a single HTTPS connection (vs double before the change).