chore(core): clean up a bunch of code smells around supervisable worker implementations by tobz · Pull Request #1860 · DataDog/saluki

tobz · 2026-06-11T20:04:54Z

Summary

Classic cleanup PR.

In no particular order:

import and avoid the fully-qualified path to tokio::pin!
remove usages of pin! where we don't need worry about taking mutable borrows to get around "value moved here" issues
clean up a bunch of select! blocks that race against shutdown to have the shutdown branch at the top ("do simple things first" principle)
rework a couple spots to move the core loop into a standalone async function instead of inline in the select! block just to keep things tidier

Change Type

Bug fix
New feature
Non-functional (chore, refactoring, docs)
Performance

How did you test this PR?

Existing tests.

References

DADP-2

tobz · 2026-06-11T20:05:11Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

pr-commenter · 2026-06-11T20:12:31Z

Binary Size Analysis (Agent Data Plane)

Baseline: 0a909bd · Comparison: 6de0a10 · diff
Analysis Configuration: stripped binaries · Pass/Fail Threshold: +5%
Sizes: 40.20 MiB (baseline) vs 40.20 MiB (comparison)
Size Change: +4.48 KiB (+0.01%)

✅ Binary size difference within threshold

Changes by Module

Module	File Size	Symbols
`core`	+14.12 KiB	1721
`anon.57d96c4ede7bf83109ac6424a3f62ae8.567.llvm.17180955879123577895`	-13.99 KiB	1
`anon.855bd1f3aab73739536d50907e521fa7.475.llvm.9358083748291856240`	+13.90 KiB	1
`serde_json`	+7.05 KiB	31
`anon.b5248c7eec540d89c1bfd951917b92fa.162.llvm.7763143091765776288`	+6.62 KiB	1
`anon.261bb94b8d6de96de010fe9a3c608bbb.4.llvm.7628180700126258038`	-6.54 KiB	1
`tracing_appender`	-5.48 KiB	6
`bytes`	-5.47 KiB	11
`&mut serde_json`	-5.40 KiB	6
`anon.612f2e2698665f868ed84bb89aaecbca.455.llvm.1965253539978887558`	+4.65 KiB	1
`anon.eada8ea41377f3db7bbea3172a916f0a.8.llvm.5195435831375099930`	-4.56 KiB	1
`anon.0e1e4e06a12a3c063e051bbfece290e0.34.llvm.6266863115024496749`	+3.98 KiB	1
`anon.c25c913790dfde6cb211f8aa59520815.13.llvm.2062042948546246284`	-3.89 KiB	1
`anon.1c25688a6bdeca17d117dbd3065128ff.290.llvm.17579200716166551091`	+3.64 KiB	1
`anon.96a8c00c75a9e16ecc58214085e16bdf.64.llvm.14436002312095234804`	-3.64 KiB	1
`anon.0e1e4e06a12a3c063e051bbfece290e0.19.llvm.6266863115024496749`	+3.37 KiB	1
`anon.b49a9a5c39bf7e1ec31018b120e1e167.19.llvm.13269347254927954815`	-3.37 KiB	1
`tracing`	-3.11 KiB	4
`hyper`	+3.06 KiB	54
`serde_path_to_error`	+2.76 KiB	14

Detailed Symbol Changes

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +242% +14.6Ki  +246% +14.6Ki    h2::proto::connection::DynConnection<B>::recv_frame::hbdc5a4d075a959a7
  [NEW] +13.9Ki  [NEW] +13.8Ki    _<core::future::poll_fn::PollFn<F> as core::future::future::Future>::poll::hcf869cbb66344762
  [NEW] +13.9Ki  [NEW]     +82    anon.855bd1f3aab73739536d50907e521fa7.475.llvm.9358083748291856240
  [NEW] +9.80Ki  [NEW] +9.58Ki    _<saluki_app::dynamic_api::DynamicAPIBuilder as saluki_core::runtime::supervisor::Supervisable>::initialize::_{{closure}}::_{{closure}}::h7b5e822556b66a8e
  [NEW] +6.62Ki  [NEW]     +79    anon.b5248c7eec540d89c1bfd951917b92fa.162.llvm.7763143091765776288
  [NEW] +6.31Ki  [NEW] +6.22Ki    matchit::router::Router<T>::insert::hf7fce6842df2cba8
  [NEW] +5.27Ki  [NEW]     +33    core::ptr::drop_in_place<http_body_util::combinators::map_err::MapErr<http_body_util::combinators::map_err::MapErr<http_body_util::combinators::map_frame::MapFrame<tonic::service::interceptor::ResponseBody<tonic::body::Body>,tonic::codec::decode::Streaming<datadog_protos::agent_include::datadog::model::v1::FetchEntityResponse>::new<tonic::service::interceptor::ResponseBody<tonic::body::Body>,tonic_prost::codec::ProstDecoder<datadog_protos::agent_include::datadog::model::v1::FetchEntityResponse>>::{{closure}}>,tonic::codec::decode::Streaming<datadog_protos::agent_include::datadog::model::v1::FetchEntityResponse>::new<tonic::service::interceptor::ResponseBody<tonic::body::Body>,tonic_prost::codec::ProstDecoder<datadog_protos::agent_include::datadog::model::v1::FetchEntityResponse>>::{{closure}}>,tonic::status::Status::map_error<tonic::status::Status>>>::h2271be4c7af4df87
  [NEW] +4.65Ki  [NEW]     +74    anon.612f2e2698665f868ed84bb89aaecbca.455.llvm.1965253539978887558
  [NEW] +3.98Ki  [NEW]     +96    anon.0e1e4e06a12a3c063e051bbfece290e0.34.llvm.6266863115024496749
  [NEW] +3.64Ki  [NEW]     +16    anon.1c25688a6bdeca17d117dbd3065128ff.290.llvm.17579200716166551091
  [DEL] -3.64Ki  [DEL]     -16    anon.96a8c00c75a9e16ecc58214085e16bdf.64.llvm.14436002312095234804
  [DEL] -3.89Ki  [DEL]     -96    anon.c25c913790dfde6cb211f8aa59520815.13.llvm.2062042948546246284
  -0.1% -4.03Ki  -0.1% -4.37Ki    [10492 Others]
  [DEL] -4.56Ki  [DEL]     -74    anon.eada8ea41377f3db7bbea3172a916f0a.8.llvm.5195435831375099930
  [DEL] -4.81Ki  [DEL] -4.65Ki    _<&mut serde_json::de::Deserializer<R> as serde_core::de::Deserializer>::deserialize_any::ha85cb34e7d1ce272
  [DEL] -5.27Ki  [DEL]     -33    core::ptr::drop_in_place<http_body_util::combinators::map_err::MapErr<http_body_util::combinators::map_err::MapErr<http_body_util::combinators::map_frame::MapFrame<tonic::service::interceptor::ResponseBody<tonic::body::Body>,tonic::codec::decode::Streaming<datadog_protos::agent_include::datadog::model::v1::HostnameReply>::new<tonic::service::interceptor::ResponseBody<tonic::body::Body>,tonic_prost::codec::ProstDecoder<datadog_protos::agent_include::datadog::model::v1::HostnameReply>>::{{closure}}>,tonic::codec::decode::Streaming<datadog_protos::agent_include::datadog::model::v1::HostnameReply>::new<tonic::service::interceptor::ResponseBody<tonic::body::Body>,tonic_prost::codec::ProstDecoder<datadog_protos::agent_include::datadog::model::v1::HostnameReply>>::{{closure}}>,tonic::status::Status::map_error<tonic::status::Status>>>::hd69af6e5120caecd
  [DEL] -6.21Ki  [DEL] -6.13Ki    matchit::tree::Node<T>::insert::h446939f1baa4b32b
  [DEL] -6.54Ki  [DEL]     -79    anon.261bb94b8d6de96de010fe9a3c608bbb.4.llvm.7628180700126258038
  [DEL] -11.4Ki  [DEL] -11.1Ki    _<saluki_app::dynamic_api::DynamicAPIBuilder as saluki_core::runtime::supervisor::Supervisable>::initialize::_{{closure}}::_{{closure}}::h9f55c463d434eed2
  [DEL] -13.9Ki  [DEL] -13.8Ki    _<core::future::poll_fn::PollFn<F> as core::future::future::Future>::poll::h5545d86b3a3dd6d4
  [DEL] -14.0Ki  [DEL]     -82    anon.57d96c4ede7bf83109ac6424a3f62ae8.567.llvm.17180955879123577895
  +0.0% +4.48Ki  +0.0% +4.12Ki    TOTAL

Copilot

Pull request overview

This PR is a refactor/cleanup across several Supervisable worker implementations and async loops, primarily standardizing pin! usage and improving shutdown responsiveness/legibility in select! blocks.

Changes:

Replace tokio::pin! fully-qualified macro usage with imported pin!, and remove pinning where unnecessary.
Reorder multiple select! blocks to prioritize shutdown branches and simplify control flow.
Extract a couple of inline select! loop bodies into standalone async functions for readability (notably dynamic_api and resource telemetry).

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
lib/saluki-env/src/workload/collectors/cgroups.rs	Use imported `pin!` for the spawned blocking poller handle.
lib/saluki-core/src/topology/blueprint.rs	Import/use `pin!`; reorder `select!` branches to prioritize shutdown.
lib/saluki-core/src/observability/metrics/mod.rs	Remove unnecessary pinning; simplify `select!` shutdown vs flush.
lib/saluki-core/src/health/worker.rs	Add rationale/TODO comment around shutdown handling and receiver return ordering.
lib/saluki-components/src/transforms/aggregate/mod.rs	Import/use `pin!` for flush future.
lib/saluki-components/src/encoders/datadog/traces/mod.rs	Import/use `pin!` for flush timeout future.
lib/saluki-components/src/encoders/datadog/stats/mod.rs	Import/use `pin!` for flush timeout future.
lib/saluki-components/src/encoders/datadog/metrics/mod.rs	Import/use `pin!` for flush timeout future.
lib/saluki-components/src/encoders/buffered_incremental/mod.rs	Import/use `pin!` for flush timeout future.
lib/saluki-components/src/destinations/dsd_stats/mod.rs	Consolidate time imports and use imported `pin!` for `collection_done`.
lib/saluki-app/src/metrics/mod.rs	Reorder `select!` to prioritize shutdown.
lib/saluki-app/src/metrics/api.rs	Minor formatting/whitespace tidy after processing loop.
lib/saluki-app/src/logging/api.rs	Import/use `pin!` for timeout + shutdown futures in loop.
lib/saluki-app/src/dynamic_api.rs	Rework shutdown/error handling to be at the top-level `select!`; simplify route event loop.
lib/saluki-app/src/accounting.rs	Extract resource-group telemetry loop into standalone async function; swap to `FastHashMap`.
bin/correctness/panoramic/src/runner.rs	Import/use `pin!` for the test run future.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tobz · 2026-06-11T20:14:41Z

+    loop {
+        ResourceGroupRegistry::global().visit_resource_groups(|group_name, stats| {
+            let group_metrics = match metrics.get_mut(group_name) {
+                Some(group_metrics) => group_metrics,
+                None => metrics
+                    .entry(group_name.to_string())
+                    .or_insert_with(|| ResourceGroupMetrics::new(group_name)),
+            };
+
+            group_metrics.update(stats);
+        });
+
+        sleep(Duration::from_secs(1)).await;
+    }


I actually like the fact we do the initial emission earlier.

Suggestion denieddddd.

pr-commenter · 2026-06-11T20:29:53Z

Regression Detector (Agent Data Plane)

Run ID: 7cbeea99-a4fe-4af6-8fce-4916840a2193
Baseline: 0a909bd2 · Comparison: 6de0a102 · diff

Optimization Goals: ✅ No significant changes detected

Fine details of change detection per experiment (35)

Experiments configured erratic: true are tagged (ignored) and skipped when determining which experiments regressed or improved. Experiments which are detected as erratic at runtime are tagged (erratic) to flag that the run's sample dispersion was high, but their regression / improvement signal still counts.

experiment	goal	Δ mean %	links
otlp_ingest_logs_5mb_memory (ignored)	memory	⚪ +14.44	metrics profiles logs
otlp_ingest_metrics_5mb_memory	memory	⚪ +5.00	metrics profiles logs
otlp_ingest_metrics_5mb_cpu (erratic)	cpu	⚪ +3.62	metrics profiles logs
otlp_ingest_traces_5mb_throughput	throughput	⚪ -1.38	metrics profiles logs
otlp_ingest_logs_5mb_cpu (ignored)	cpu	⚪ +1.01	metrics profiles logs
dsd_uds_500mb_3k_contexts_cpu (erratic)	cpu	⚪ +0.71	metrics profiles logs
quality_gates_rss_dsd_ultraheavy	memory	⚪ +0.26	metrics profiles logs
quality_gates_rss_idle	memory	⚪ +0.16	metrics profiles logs
quality_gates_rss_dsd_low	memory	⚪ +0.15	metrics profiles logs
otlp_ingest_traces_ottl_transform_5mb_cpu (erratic)	cpu	⚪ +0.05	metrics profiles logs
dsd_uds_500mb_3k_contexts_throughput	throughput	⚪ -0.04	metrics profiles logs
dsd_uds_100mb_3k_contexts_memory	memory	⚪ +0.02	metrics profiles logs
otlp_ingest_logs_5mb_throughput (ignored)	throughput	⚪ -0.02	metrics profiles logs
dsd_uds_100mb_3k_contexts_throughput	throughput	⚪ -0.00	metrics profiles logs
dsd_uds_1mb_3k_contexts_throughput	throughput	⚪ -0.00	metrics profiles logs
dsd_uds_512kb_3k_contexts_throughput	throughput	⚪ -0.00	metrics profiles logs
dsd_uds_10mb_3k_contexts_throughput	throughput	⚪ +0.01	metrics profiles logs
otlp_ingest_metrics_5mb_throughput	throughput	⚪ +0.02	metrics profiles logs
dsd_uds_10mb_3k_contexts_memory	memory	⚪ -0.05	metrics profiles logs
quality_gates_rss_dsd_heavy	memory	⚪ -0.05	metrics profiles logs
quality_gates_rss_dsd_medium	memory	⚪ -0.06	metrics profiles logs
otlp_ingest_traces_ottl_filtering_5mb_throughput	throughput	⚪ +0.09	metrics profiles logs
otlp_ingest_traces_ottl_transform_5mb_memory	memory	⚪ -0.17	metrics profiles logs
otlp_ingest_traces_ottl_filtering_5mb_memory	memory	⚪ -0.29	metrics profiles logs
otlp_ingest_traces_5mb_memory	memory	⚪ -0.30	metrics profiles logs
otlp_ingest_traces_ottl_transform_5mb_throughput	throughput	⚪ +0.56	metrics profiles logs
dsd_uds_500mb_3k_contexts_memory	memory	⚪ -0.66	metrics profiles logs
dsd_uds_1mb_3k_contexts_memory	memory	⚪ -0.66	metrics profiles logs
dsd_uds_512kb_3k_contexts_memory	memory	⚪ -0.76	metrics profiles logs
otlp_ingest_traces_5mb_cpu (erratic)	cpu	⚪ -1.46	metrics profiles logs
dsd_uds_100mb_3k_contexts_cpu (erratic)	cpu	⚪ -1.50	metrics profiles logs
otlp_ingest_traces_ottl_filtering_5mb_cpu (erratic)	cpu	⚪ -2.21	metrics profiles logs
dsd_uds_512kb_3k_contexts_cpu (erratic)	cpu	⚪ -2.81	metrics profiles logs
dsd_uds_10mb_3k_contexts_cpu (erratic)	cpu	🟢 -6.54	metrics profiles logs
dsd_uds_1mb_3k_contexts_cpu (erratic)	cpu	🟢 -10.58	metrics profiles logs

Bounds Checks: ✅ Passed (5)

experiment	check	replicates	observed	links
quality_gates_rss_dsd_heavy	memory_usage	10/10	✅ 132 MiB ≤ 140 MiB	metrics profiles logs
quality_gates_rss_dsd_low	memory_usage	10/10	✅ 42.4 MiB ≤ 50 MiB	metrics profiles logs
quality_gates_rss_dsd_medium	memory_usage	10/10	✅ 65.8 MiB ≤ 75 MiB	metrics profiles logs
quality_gates_rss_dsd_ultraheavy	memory_usage	10/10	✅ 191 MiB ≤ 200 MiB	metrics profiles logs
quality_gates_rss_idle	memory_usage	10/10	✅ 28.3 MiB ≤ 40 MiB	metrics profiles logs

Explanation

A change is flagged as a regression when |Δ mean %| > 5.00% in the regressing direction for its optimization goal AND SMP marks the experiment as a regression (is_regression: true). Improvements use the matching criteria for the improving direction. Experiments configured erratic: true (tagged (ignored)) are skipped outright; experiments detected as erratic at runtime (tagged (erratic)) still count, since that flag describes sample dispersion rather than directional certainty. The Δ mean % cell is colored accordingly: 🟢 = improvement, 🔴 = regression, ⚪ = neutral. Reduction in CPU or memory is an improvement; reduction in ingress throughput is a regression.

webern

Looks good, and AI says "I don't see any threading correctness issues that need to be raised." 👍

…er implementations

…er implementations (#1860) fb4ea4a

…er implementations (#1860)

tobz mentioned this pull request Jun 11, 2026

chore(common): unify multiple shutdown synchronization types #1856

Merged

4 tasks

tobz marked this pull request as ready for review June 11, 2026 20:10

tobz requested a review from a team as a code owner June 11, 2026 20:10

Copilot AI review requested due to automatic review settings June 11, 2026 20:10

Copilot started reviewing on behalf of tobz June 11, 2026 20:10 View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

Base automatically changed from tobz/shutdown-primitives-cleanup to main June 12, 2026 01:22

tobz force-pushed the tobz/pin-worker-init-clean-up branch from f9fc6c3 to 623626e Compare June 12, 2026 12:54

aqian01 approved these changes Jun 15, 2026

View reviewed changes

tobz force-pushed the tobz/pin-worker-init-clean-up branch from 623626e to a80937c Compare June 15, 2026 18:23

This was referenced Jun 15, 2026

enhancement(core): add configurable per-child restart types for supervisors #1873

Merged

enhancement(core): add support for dynamically added child processes in Supervisor #1874

Merged

webern approved these changes Jun 16, 2026

View reviewed changes

tobz force-pushed the tobz/pin-worker-init-clean-up branch 2 times, most recently from cdc8126 to 6e7c11b Compare June 17, 2026 13:30

chore(core): clean up a bunch of code smells around supervisable work…

eac1843

…er implementations

more cleanup

6de0a10

tobz force-pushed the tobz/pin-worker-init-clean-up branch from 6e7c11b to 6de0a10 Compare June 17, 2026 16:26

tobz merged commit fb4ea4a into main Jun 17, 2026
82 checks passed

tobz deleted the tobz/pin-worker-init-clean-up branch June 17, 2026 19:29

dd-octo-sts Bot pushed a commit that referenced this pull request Jun 17, 2026

chore(core): clean up a bunch of code smells around supervisable work…

639e82d

…er implementations (#1860) fb4ea4a

tobz mentioned this pull request Jun 29, 2026

chore(core): integrate component tasks via supervision trees #1942

Merged

4 tasks

tobz added a commit that referenced this pull request Jun 30, 2026

chore(core): clean up a bunch of code smells around supervisable work…

720890c

…er implementations (#1860)

tobz mentioned this pull request Jul 6, 2026

chore(core): unify how we define identifiers for subsystems/components #2029

Merged

7 tasks

Navigation Menu

Uh oh!

chore(core): clean up a bunch of code smells around supervisable worker implementations - #1860

chore(core): clean up a bunch of code smells around supervisable worker implementations#1860
tobz merged 2 commits into
mainfrom
tobz/pin-worker-init-clean-up

tobz commented Jun 11, 2026 •

edited

Loading

Uh oh!

tobz commented Jun 11, 2026 •

edited

Loading

Uh oh!

pr-commenter Bot commented Jun 11, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

tobz Jun 11, 2026

Uh oh!

This comment has been minimized.

pr-commenter Bot commented Jun 11, 2026 •

edited

Loading

Uh oh!

webern left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

tobz commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Change Type

How did you test this PR?

References

Uh oh!

tobz commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pr-commenter Bot commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Binary Size Analysis (Agent Data Plane)

✅ Binary size difference within threshold

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

tobz Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

pr-commenter Bot commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Regression Detector (Agent Data Plane)

Optimization Goals: ✅ No significant changes detected

Uh oh!

webern left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tobz commented Jun 11, 2026 •

edited

Loading

tobz commented Jun 11, 2026 •

edited

Loading

pr-commenter Bot commented Jun 11, 2026 •

edited

Loading

pr-commenter Bot commented Jun 11, 2026 •

edited

Loading