Skip to content

Commit 1da1c37

Browse files
committed
chore(config): type the memory limit as bytes and fix doc prose
The memory limit is a byte size the source may express either as a bare integer (bytes) or a suffixed string ('512MB'). The Saluki-only source modeled it as a String, so a bare-integer value failed to deserialize and aborted configuration loading at startup. Model it as bytes: the control field becomes a u64 (matching the other byte-size fields), and the source parses it as a ByteSize, which accepts both forms. Also reword doc comments flagged by the prose linter.
1 parent e842466 commit 1da1c37

2 files changed

Lines changed: 29 additions & 11 deletions

File tree

lib/agent-data-plane-config-system/src/saluki_only.rs

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
//!
1616
//! # The one invariant (read before adding or debugging a value)
1717
//!
18-
//! This struct's shape mirrors the source key hierarchy exactly. Each field is populated by plain
18+
//! This struct mirrors the source key hierarchy exactly. Each field is populated by plain
1919
//! serde from the merged map, so its path here must equal the real config-key path, with no
2020
//! `rename` and (aside from a documented multi-key `alias`) nothing papering over a mismatch:
2121
//!
@@ -34,7 +34,7 @@
3434
//! type coerces the source form (durations are [`DurationString`], not `Duration`), and that
3535
//! `seed` copies it to the model destination the component reads.
3636
//! 2. Value arrives wrong only when the key is absent: the default bug is not here. The default
37-
//! lives in the model struct's `Default` in `agent-data-plane-config`, because these fields are
37+
//! lives in the model `Default` in `agent-data-plane-config`, because these fields are
3838
//! `Option<T>` and `seed` writes only when set. Fix the model `Default`, not this struct.
3939
//! 3. Then add or extend the round-trip test below (set the real key, assert the model field). A
4040
//! missing test is why a silent transport failure was not caught.
@@ -49,6 +49,7 @@ use std::time::Duration;
4949
use agent_data_plane_config::control::ListenAddress;
5050
use agent_data_plane_config::domains::traces::{OttlErrorMode, OttlFilter, OttlTransform};
5151
use agent_data_plane_config::SalukiConfiguration;
52+
use bytesize::ByteSize;
5253
use saluki_config::DurationString;
5354
use serde::Deserialize;
5455

@@ -68,8 +69,10 @@ pub struct SalukiOnly {
6869
pub remote_agent_string_interner_size_bytes: Option<usize>,
6970
/// Checks IPC endpoint (`checks_ipc_endpoint`).
7071
pub checks_ipc_endpoint: Option<String>,
71-
/// Process memory limit, a byte-size string such as `512MB` (`memory_limit`).
72-
pub memory_limit: Option<String>,
72+
/// Process memory limit (`memory_limit`), given as a bare integer number of bytes or a
73+
/// byte-size string such as `512MB`. `ByteSize` accepts both forms, so a numeric value does not
74+
/// fail the load.
75+
pub memory_limit: Option<ByteSize>,
7376
/// Memory-accounting slop fraction (`memory_slop_factor`).
7477
pub memory_slop_factor: Option<f64>,
7578
/// Encoder flush timeout, in seconds (`flush_timeout_secs`).
@@ -337,8 +340,8 @@ impl SalukiOnly {
337340
if let Some(v) = self.data_plane.checks.enabled {
338341
config.control.checks = v;
339342
}
340-
if let Some(v) = self.memory_limit.clone() {
341-
config.control.memory_limit = v;
343+
if let Some(v) = self.memory_limit {
344+
config.control.memory_limit = v.as_u64();
342345
}
343346
if let Some(v) = self.memory_slop_factor {
344347
config.control.memory_slop_factor = v;
@@ -575,7 +578,7 @@ mod tests {
575578
assert_eq!(config.control.stop_timeout, 45);
576579
assert!(config.control.standalone_mode);
577580
assert!(config.control.checks);
578-
assert_eq!(config.control.memory_limit, "512MB");
581+
assert_eq!(config.control.memory_limit, ByteSize::mb(512).as_u64());
579582
assert_eq!(config.control.memory_slop_factor, 0.3);
580583
assert_eq!(config.control.ipc.remote_agent_string_interner_size_bytes, 4096);
581584

@@ -641,9 +644,25 @@ mod tests {
641644
assert_eq!(config.domains.checks.ipc_endpoint.0, "localhost:5006");
642645
}
643646

647+
/// `memory_limit` is a byte size the source may express as a bare integer (bytes) or a suffixed
648+
/// string. Both must deserialize to the same byte count; a bare integer previously failed the
649+
/// whole config load.
650+
#[test]
651+
fn memory_limit_accepts_a_bare_integer_or_a_string() {
652+
for (value, expected) in [
653+
(json!({ "memory_limit": 1 }), 1),
654+
(json!({ "memory_limit": "512MB" }), ByteSize::mb(512).as_u64()),
655+
] {
656+
let saluki_only: SalukiOnly = serde_json::from_value(value).expect("memory_limit deserializes");
657+
let mut config = SalukiConfiguration::default();
658+
saluki_only.seed(&mut config);
659+
assert_eq!(config.control.memory_limit, expected);
660+
}
661+
}
662+
644663
/// An absent key leaves the model default in place (the common case). `seed` writes only present
645664
/// options, so this exercises the `Option`-in-source / default-in-model split. A wrong value
646-
/// here means the model struct's `Default` is wrong, not this struct.
665+
/// here means the model `Default` is wrong, not this struct.
647666
#[test]
648667
fn absent_keys_leave_model_defaults() {
649668
let saluki_only: SalukiOnly = serde_json::from_value(json!({})).expect("empty source deserializes");

lib/agent-data-plane-config/src/control.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,8 @@ pub struct ControlConfiguration {
5858
/// schema)
5959
pub stop_timeout: u64,
6060

61-
/// Process memory ceiling as a byte-size string such as `512MB`. (not in Datadog Agent config
62-
/// schema)
63-
pub memory_limit: String,
61+
/// Process memory ceiling, in bytes. (not in Datadog Agent config schema)
62+
pub memory_limit: u64,
6463

6564
/// Fraction of the memory limit held back as headroom during memory accounting. (not in Datadog
6665
/// Agent config schema)

0 commit comments

Comments
 (0)