Skip to content

feat(stable_config): add >100mb check for stable config files #1432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vpellan wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(stable_config): add >100mb check for stable config files #1432

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eadbbbb
Add >100mb check for stable config files
vpellan Jan 7, 2026
23c5263
fix format and clippy
vpellan Jan 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 104 additions & 12 deletions libdd-library-config/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,14 +543,32 @@ impl Configurator {
debug_messages.push(format!("\tlocal: {path_local:?}"));
debug_messages.push(format!("\tfleet: {path_managed:?}"));
}

let local_config = match fs::File::open(path_local) {
Ok(file) => match self.parse_stable_config_file(file) {
LoggedResult::Ok(config, logs) => {
debug_messages.extend(logs);
config
Ok(file) => {
match file.metadata() {
Ok(metadata) => {
// Fail if the file is > 100mb
if metadata.len() > 1024 * 1024 * 100 {
let anyhow_error = anyhow::anyhow!("Local file is too large (> 100mb)");
let logged_result = LoggedResult::Err(anyhow_error);
return logged_result;
Copy link
Contributor

@anna-git anna-git Jan 7, 2026

Choose a reason for hiding this comment

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

is it by design that if one of them is too large, we bail out of stable config completely?
NIT: also was wondering if this check could be in a separate function which both local and fleet could reuse but not a big deal

Copy link

@mtoffl01 mtoffl01 Jan 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

@anna-git ,

if one of them is too large, we bail out of stable config completely

If I remember correctly, we discussed this limitation a few months ago. For libraries that do not embed libdatadog for stable config -- like java and go -- we can attempt to open each file individually, and handle errors separately. It's my understanding that, in the libdatadog architecture, if either file is too big, we return a single error, and when the library consumes this error, it bails. Hence the behavior you described above. Correct?

And to my memory, we said there isn't a clean way to separate the error handling right now without breaking APIs, yes?

I really don't like this solution, since both inputs will have very different user experiences and thus should be logically separated, but I also acknowledge if it's the best we can do right now. In this case, can we add a comment in here that references a jira card that describes the issue behavior as well as how we might fix it in the future?

Copy link
Contributor

@anna-git anna-git Jan 8, 2026

Choose a reason for hiding this comment

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

I think it wouldn't be a problem to return a config without the keys from the faulty file, and just add the validation error to the messages of LoggedResult. We could extend LoggedResult to contain error_messages and add the >100 mb error message in this vector. The tracers should read this vector always, and if they see it's filled up, log it as an error, so that it appears in error tracking as well. We just fill the error_messages, dont interrupt the function and just return an Ok() with whichever configs were read.
If no file could be read at all(very rare...), I guess we could reallyl return an Error..

}
}
Err(e) => {
return LoggedResult::Err(
anyhow::Error::from(e).context("failed to get file metadata"),
)
}
}
LoggedResult::Err(e) => return LoggedResult::Err(e),
},
match self.parse_stable_config_file(file) {
LoggedResult::Ok(config, logs) => {
debug_messages.extend(logs);
config
}
LoggedResult::Err(e) => return LoggedResult::Err(e),
}
}
Err(e) if e.kind() == io::ErrorKind::NotFound => StableConfig::default(),
Err(e) => {
return LoggedResult::Err(
Expand All @@ -559,13 +577,29 @@ impl Configurator {
}
};
let fleet_config = match fs::File::open(path_managed) {
Ok(file) => match self.parse_stable_config_file(file) {
LoggedResult::Ok(config, logs) => {
debug_messages.extend(logs);
config
Ok(file) => {
match file.metadata() {
Ok(metadata) => {
// Fail if the file is > 100mb
if metadata.len() > 1024 * 1024 * 100 {
let test = anyhow::anyhow!("Fleet file is too large (> 100mb)");
return LoggedResult::Err(test);
}
}
Err(e) => {
return LoggedResult::Err(
anyhow::Error::from(e).context("failed to get file metadata"),
)
}
}
LoggedResult::Err(e) => return LoggedResult::Err(e),
},
match self.parse_stable_config_file(file) {
LoggedResult::Ok(config, logs) => {
debug_messages.extend(logs);
config
}
LoggedResult::Err(e) => return LoggedResult::Err(e),
}
}
Err(e) if e.kind() == io::ErrorKind::NotFound => StableConfig::default(),
Err(e) => {
return LoggedResult::Err(
Expand Down Expand Up @@ -865,6 +899,64 @@ mod tests {
}
}

#[test]
fn test_large_local_file() {
let configurator = Configurator::new(true);
let mut temp_local_file = tempfile::NamedTempFile::new().unwrap();
let temp_fleet_file = tempfile::NamedTempFile::new().unwrap();
let large_file = vec![b'a'; 1024 * 1024 * 100 + 1];
temp_local_file.write_all(&large_file).unwrap();
let temp_local_path = temp_local_file.into_temp_path();
let temp_fleet_path = temp_fleet_file.into_temp_path();
let result = configurator.get_config_from_file(
temp_local_path.to_str().unwrap().as_ref(),
temp_fleet_path.to_str().unwrap().as_ref(),
&ProcessInfo {
args: vec![b"-jar HelloWorld.jar".to_vec()],
envp: vec![b"ENV=VAR".to_vec()],
language: b"java".to_vec(),
},
);
match result {
LoggedResult::Ok(..) => panic!("Expected error"),
LoggedResult::Err(e) => {
assert_eq!(
e.to_string(),
"Local file is too large (> 100mb)".to_string()
);
}
}
}

#[test]
fn test_large_fleet_file() {
let configurator = Configurator::new(true);
let temp_local_file = tempfile::NamedTempFile::new().unwrap();
let mut temp_fleet_file = tempfile::NamedTempFile::new().unwrap();
let large_file = vec![b'a'; 1024 * 1024 * 100 + 1];
temp_fleet_file.write_all(&large_file).unwrap();
let temp_local_path = temp_local_file.into_temp_path();
let temp_fleet_path = temp_fleet_file.into_temp_path();
let result = configurator.get_config_from_file(
temp_local_path.to_str().unwrap().as_ref(),
temp_fleet_path.to_str().unwrap().as_ref(),
&ProcessInfo {
args: vec![b"-jar HelloWorld.jar".to_vec()],
envp: vec![b"ENV=VAR".to_vec()],
language: b"java".to_vec(),
},
);
match result {
LoggedResult::Ok(..) => panic!("Expected error"),
Copy link
Contributor

@anna-git anna-git Jan 7, 2026

Choose a reason for hiding this comment

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

Suggested change
LoggedResult::Ok(..) => panic!("Expected error"),
LoggedResult::Ok(..) => panic!("Expected error for fleet file > 100mb, but got success instead"),

LoggedResult::Err(e) => {
assert_eq!(
e.to_string(),
"Fleet file is too large (> 100mb)".to_string()
);
}
}
}

#[test]
fn test_local_host_global_config() {
use LibraryConfigSource::*;
Expand Down
Loading