enhancement(agent-data-plane): switch to HttpClient from saluki-io for data plane API client to reduce binary size#1224
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
HttpClient from saluki-io for data plane API client to reduce binary size
Binary Size Analysis (Agent Data Plane)Target: 90900eb (baseline) vs cd3a756 (comparison) diff
|
| Module | File Size | Symbols |
|---|---|---|
core |
-200.77 KiB | 10253 |
reqwest |
-174.27 KiB | 112 |
hyper_util |
-127.88 KiB | 385 |
h2 |
-120.43 KiB | 405 |
hyper |
-63.01 KiB | 368 |
[sections] |
-62.65 KiB | 7 |
rustls |
-26.00 KiB | 59 |
futures_util |
-19.63 KiB | 68 |
webpki |
-19.58 KiB | 12 |
hyper_rustls |
-19.46 KiB | 29 |
iri_string |
-19.24 KiB | 16 |
http |
-16.91 KiB | 430 |
rustls_platform_verifier |
-13.88 KiB | 9 |
alloc |
-13.60 KiB | 1073 |
saluki_io::net::client |
+11.87 KiB | 41 |
tower |
-8.96 KiB | 359 |
std |
-7.68 KiB | 251 |
hashbrown |
-7.62 KiB | 387 |
ipnet |
-6.80 KiB | 8 |
tokio_rustls |
-6.27 KiB | 36 |
Detailed Symbol Changes
FILE SIZE VM SIZE
-------------- --------------
[NEW] +1.79Mi [NEW] +1.79Mi std::thread::local::LocalKey<T>::with::he517c09e5477efe1
[NEW] +114Ki [NEW] +113Ki agent_data_plane::cli::run::create_topology::_{{closure}}::h4a1b0b37b7e2d03e
[NEW] +84.6Ki [NEW] +84.5Ki agent_data_plane::internal::control_plane::spawn_control_plane::_{{closure}}::h78c38079d35f4f77
[NEW] +64.2Ki [NEW] +64.0Ki saluki_components::common::datadog::io::run_endpoint_io_loop::_{{closure}}::h1c7964dcd8ecbead
[NEW] +57.8Ki [NEW] +57.7Ki agent_data_plane::cli::run::handle_run_command::_{{closure}}::hebabd4c0d26708e2
[NEW] +49.5Ki [NEW] +49.3Ki saluki_app::bootstrap::AppBootstrapper::bootstrap::_{{closure}}::h15f926ef5659e580
[NEW] +47.5Ki [NEW] +47.3Ki moka::sync::base_cache::Inner<K,V,S>::do_run_pending_tasks::hfd5cba4fa7f4623d
[NEW] +46.4Ki [NEW] +46.3Ki h2::proto::connection::Connection<T,P,B>::poll::h100071ec98c95c21
[NEW] +46.1Ki [NEW] +46.0Ki h2::proto::connection::Connection<T,P,B>::poll::h60f979f739385b46
[DEL] -46.1Ki [DEL] -45.9Ki _<saluki_components::forwarders::otlp::OtlpForwarder as saluki_core::components::forwarders::Forwarder>::run::_{{closure}}::h16dd847b7336eb06
[DEL] -46.1Ki [DEL] -46.0Ki h2::proto::connection::Connection<T,P,B>::poll::h8d9d6127b67107aa
[DEL] -46.1Ki [DEL] -46.0Ki h2::proto::connection::Connection<T,P,B>::poll::hd483507182f7e6fa
[DEL] -46.4Ki [DEL] -46.3Ki h2::proto::connection::Connection<T,P,B>::poll::h722d9ba49acb2559
[DEL] -47.5Ki [DEL] -47.3Ki moka::sync::base_cache::Inner<K,V,S>::do_run_pending_tasks::hb0ccfb0439ae00e1
[DEL] -49.0Ki [DEL] -48.9Ki saluki_app::bootstrap::AppBootstrapper::bootstrap::_{{closure}}::h7b5b0f2785656dcb
[DEL] -57.9Ki [DEL] -57.7Ki agent_data_plane::cli::run::handle_run_command::_{{closure}}::h517a19a4b350019e
[DEL] -64.2Ki [DEL] -64.0Ki saluki_components::common::datadog::io::run_endpoint_io_loop::_{{closure}}::h08fc84c201eab0a3
[DEL] -84.5Ki [DEL] -84.4Ki agent_data_plane::internal::control_plane::spawn_control_plane::_{{closure}}::h23810f4dc340e6f1
[DEL] -113Ki [DEL] -113Ki agent_data_plane::cli::run::create_topology::_{{closure}}::h2ba97c4b54b0c30b
-5.6% -857Ki -5.6% -708Ki [26239 Others]
[DEL] -1.79Mi [DEL] -1.79Mi std::thread::local::LocalKey<T>::with::h6c9a8fa0b613775b
-3.4% -948Ki -3.3% -799Ki TOTAL
Regression Detector (Agent Data Plane)Regression Detector ResultsRun ID: 0cd37a4b-c03a-4056-96cf-5d4d69e4a7c6 Baseline: 90900eb Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | otlp_ingest_logs_5mb_cpu | % cpu utilization | +0.77 | [-4.47, +6.00] | 1 | (metrics) (profiles) (logs) |
| ➖ | otlp_ingest_logs_5mb_throughput | ingress throughput | -0.02 | [-0.14, +0.10] | 1 | (metrics) (profiles) (logs) |
| ➖ | otlp_ingest_logs_5mb_memory | memory utilization | -4.42 | [-4.76, -4.09] | 1 | (metrics) (profiles) (logs) |
Fine details of change detection per experiment
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | dsd_uds_512kb_3k_contexts_cpu | % cpu utilization | +9.84 | [-47.17, +66.84] | 1 | (metrics) (profiles) (logs) |
| ➖ | dsd_uds_1mb_3k_contexts_cpu | % cpu utilization | +9.04 | [-44.64, +62.71] | 1 | (metrics) (profiles) (logs) |
| ➖ | dsd_uds_100mb_3k_contexts_cpu | % cpu utilization | +1.37 | [-4.06, +6.80] | 1 | (metrics) (profiles) (logs) |
| ➖ | otlp_ingest_logs_5mb_cpu | % cpu utilization | +0.77 | [-4.47, +6.00] | 1 | (metrics) (profiles) (logs) |
| ➖ | otlp_ingest_metrics_5mb_memory | memory utilization | +0.64 | [+0.49, +0.80] | 1 | (metrics) (profiles) (logs) |
| ➖ | otlp_ingest_traces_ottl_5mb_cpu | % cpu utilization | +0.55 | [-1.93, +3.02] | 1 | (metrics) (profiles) (logs) |
| ➖ | quality_gates_rss_dsd_heavy | memory utilization | +0.06 | [-0.07, +0.20] | 1 | (metrics) (profiles) (logs) |
| ➖ | dsd_uds_100mb_3k_contexts_throughput | ingress throughput | +0.01 | [-0.03, +0.06] | 1 | (metrics) (profiles) (logs) |
| ➖ | dsd_uds_10mb_3k_contexts_throughput | ingress throughput | +0.01 | [-0.14, +0.16] | 1 | (metrics) (profiles) (logs) |
| ➖ | otlp_ingest_traces_5mb_throughput | ingress throughput | +0.00 | [-0.02, +0.02] | 1 | (metrics) (profiles) (logs) |
| ➖ | dsd_uds_1mb_3k_contexts_throughput | ingress throughput | -0.00 | [-0.06, +0.06] | 1 | (metrics) (profiles) (logs) |
| ➖ | otlp_ingest_traces_ottl_5mb_throughput | ingress throughput | -0.00 | [-0.02, +0.02] | 1 | (metrics) (profiles) (logs) |
| ➖ | dsd_uds_512kb_3k_contexts_throughput | ingress throughput | -0.01 | [-0.06, +0.05] | 1 | (metrics) (profiles) (logs) |
| ➖ | otlp_ingest_logs_5mb_throughput | ingress throughput | -0.02 | [-0.14, +0.10] | 1 | (metrics) (profiles) (logs) |
| ➖ | otlp_ingest_metrics_5mb_throughput | ingress throughput | -0.03 | [-0.16, +0.10] | 1 | (metrics) (profiles) (logs) |
| ➖ | dsd_uds_10mb_3k_contexts_cpu | % cpu utilization | -0.04 | [-29.68, +29.61] | 1 | (metrics) (profiles) (logs) |
| ➖ | otlp_ingest_metrics_5mb_cpu | % cpu utilization | -0.46 | [-6.20, +5.29] | 1 | (metrics) (profiles) (logs) |
| ➖ | dsd_uds_500mb_3k_contexts_cpu | % cpu utilization | -0.47 | [-1.77, +0.83] | 1 | (metrics) (profiles) (logs) |
| ➖ | otlp_ingest_traces_5mb_memory | memory utilization | -0.55 | [-0.79, -0.31] | 1 | (metrics) (profiles) (logs) |
| ➖ | quality_gates_rss_dsd_medium | memory utilization | -0.60 | [-0.80, -0.41] | 1 | (metrics) (profiles) (logs) |
| ➖ | quality_gates_rss_dsd_ultraheavy | memory utilization | -0.65 | [-0.77, -0.52] | 1 | (metrics) (profiles) (logs) |
| ➖ | otlp_ingest_traces_5mb_cpu | % cpu utilization | -0.85 | [-2.94, +1.24] | 1 | (metrics) (profiles) (logs) |
| ➖ | dsd_uds_500mb_3k_contexts_memory | memory utilization | -1.37 | [-1.53, -1.20] | 1 | (metrics) (profiles) (logs) |
| ➖ | otlp_ingest_traces_ottl_5mb_memory | memory utilization | -1.52 | [-1.85, -1.20] | 1 | (metrics) (profiles) (logs) |
| ➖ | quality_gates_rss_idle | memory utilization | -1.58 | [-1.63, -1.54] | 1 | (metrics) (profiles) (logs) |
| ➖ | dsd_uds_512kb_3k_contexts_memory | memory utilization | -1.63 | [-1.80, -1.45] | 1 | (metrics) (profiles) (logs) |
| ➖ | dsd_uds_500mb_3k_contexts_throughput | ingress throughput | -1.67 | [-1.81, -1.53] | 1 | (metrics) (profiles) (logs) |
| ➖ | dsd_uds_1mb_3k_contexts_memory | memory utilization | -1.69 | [-1.86, -1.52] | 1 | (metrics) (profiles) (logs) |
| ➖ | dsd_uds_100mb_3k_contexts_memory | memory utilization | -1.71 | [-1.90, -1.53] | 1 | (metrics) (profiles) (logs) |
| ➖ | dsd_uds_10mb_3k_contexts_memory | memory utilization | -1.80 | [-1.98, -1.61] | 1 | (metrics) (profiles) (logs) |
| ➖ | quality_gates_rss_dsd_low | memory utilization | -2.13 | [-2.33, -1.94] | 1 | (metrics) (profiles) (logs) |
| ➖ | otlp_ingest_logs_5mb_memory | memory utilization | -4.42 | [-4.76, -4.09] | 1 | (metrics) (profiles) (logs) |
Bounds Checks: ✅ Passed
| perf | experiment | bounds_check_name | replicates_passed | links |
|---|---|---|---|---|
| ✅ | quality_gates_rss_dsd_heavy | memory_usage | 10/10 | (metrics) (profiles) (logs) |
| ✅ | quality_gates_rss_dsd_low | memory_usage | 10/10 | (metrics) (profiles) (logs) |
| ✅ | quality_gates_rss_dsd_medium | memory_usage | 10/10 | (metrics) (profiles) (logs) |
| ✅ | quality_gates_rss_dsd_ultraheavy | memory_usage | 10/10 | (metrics) (profiles) (logs) |
| ✅ | quality_gates_rss_idle | memory_usage | 10/10 | (metrics) (profiles) (logs) |
Explanation
Confidence level: 90.00%
Effect size tolerance: |Δ mean %| ≥ 5.00%
Performance changes are noted in the perf column of each table:
- ✅ = significantly better comparison variant performance
- ❌ = significantly worse comparison variant performance
- ➖ = no significant change in performance
A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".
For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:
-
Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
-
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
-
Its configuration does not mark it "erratic".
There was a problem hiding this comment.
Pull request overview
Switches the agent data plane HTTP client stack to use the in-repo saluki-io HTTP client/connector patterns (including Unix domain sockets), while adding TLS support for optionally skipping certificate verification and tightening OTLP gRPC connection behavior.
Changes:
- Add an opt-in “dangerous” TLS mode that disables server certificate verification.
- Refactor HTTP connector transport to unify TCP + Unix domain socket paths under a single
Transporttype to reduce monomorphization/binary size. - Add a connect timeout to the OTLP gRPC channel setup.
Reviewed changes
Copilot reviewed 4 out of 11 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lib/saluki-tls/src/lib.rs | Adds a “dangerous” certificate verifier option to the TLS config builder. |
| lib/saluki-io/src/net/client/http/conn.rs | Introduces a unified Transport and adds Unix-socket routing in the connector layer. |
| lib/saluki-io/src/net/client/http/client.rs | Exposes Unix-socket configuration on the public HTTP client builder. |
| lib/saluki-components/src/forwarders/otlp/mod.rs | Adds a connect timeout to OTLP gRPC endpoint initialization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let normalized_endpoint = normalize_endpoint(&self.core_agent_otlp_grpc_endpoint); | ||
| let core_agent_grpc_channel = Channel::from_shared(normalized_endpoint) | ||
| .error_context("Failed to construct gRPC channel due to an invalid endpoint.")? | ||
| .connect_timeout(Duration::from_secs(5)) |
There was a problem hiding this comment.
This is a one-off change that is unrelated to the PR title, yes... but what it's doing here is matching the usage of another Tonic callsite in terms of the generated code: since everything inside of Channel is monomorphized, this code was previously something like Foo<Bar<Blah<...>>> and other usages were TimeoutLayer<FooBar<Blah<...>>>. This meant a ton of duplicated code generation.
Is this a fragile optimization? Yes. Nothing breaks if the other Tonic usages change the options they enable, though... the code size just goes back up slightly.
This is also just a good change to make, since timeouts are good. 🤷
2e44e2a to
58d384c
Compare
33d24d9 to
cbcff87
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let result = self | ||
| .inner | ||
| .ready() | ||
| .await | ||
| .map_err(GenericError::from_boxed)? | ||
| .call(req) | ||
| .await; | ||
|
|
||
| check_connection_state(captured_conn); | ||
|
|
||
| result | ||
| result.map_err(GenericError::from_boxed) |
There was a problem hiding this comment.
GenericError is a type alias to anyhow::Error and doesn’t provide a from_boxed constructor. These map_err(GenericError::from_boxed) calls will not compile; convert the BoxError via Into::into / anyhow::Error::from instead (and keep the ? propagation consistent).
There was a problem hiding this comment.
.. wrong? It demonstrably compiles. 🤦🏻
| let mut config = if self.danger_accept_invalid_certs { | ||
| let crypto_provider = CryptoProvider::get_default() | ||
| .map(Arc::clone) | ||
| .ok_or(generic_error!("Default TLS root certificate store not initialized.")) | ||
| })?; | ||
| .ok_or_else(|| generic_error!("Default cryptography provider not yet installed."))?; | ||
| let verifier = Arc::new(AcceptAllServerCertVerifier { |
There was a problem hiding this comment.
ClientTLSConfigBuilder::build can now fail when danger_accept_invalid_certs is enabled and no default CryptoProvider has been installed. The # Errors doc comment should mention this additional failure mode so callers aren’t surprised.
…r data plane API client to reduce binary size
58d384c to
cd3a756
Compare

Summary
This PR switches from
reqwestto our ownHttpClientinagent-data-planein order to meaningfully reduce the ADP binary size by eliminating a ton of duplicate codegen due to monomorphization.Quoting the Rust Compiler Development Guide:
The bulk of monomorphization cost in ADP, outside of standard library types, is related to the
hyperstack, and by extension, things that depend onhyper:tonic,reqwest, etc. In particular,reqwestis very heavy in terms of binary size because of all the additional things it pulls in and how they monomorphize.This PR replaces
reqwestwith our ownHttpClientin order to shrink the binary size by reducing the penalty we pay through monomorphization: while the dependency graph stays almost exactly the same, we drop so many additional generated copies of monomorphized code that we drastically shrink the binary, to the tune of ~1MiB, or roughly 5%.We had to do some light refactoring to switch to
HttpClient, as well as implementing some additional logic to match whatreqwestprovided, but this was minimal: support for UDS inHttpClient, and a no-verify certificate verifier to allow self-signed certificates.Change Type
How did you test this PR?
Existing tests.
References
AGTMETRICS-400