Skip to content

Commit 47d8862

Browse files
pietroalbinipietroalbini
committed
logs: allow restricting the amount of storage used by logs
1 parent 02adbfb commit 47d8862

File tree

21 files changed

+197
-21
lines changed

21 files changed

+197
-21
lines changed

‎config.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ local-crates = []
2929
[sandbox]
3030
# Maximum amount of RAM allowed during builds
3131
memory-limit = "1536M" # 1.5G
32+
# Restrictions on the amount of information stored in build logs
33+
build-log-max-size = "5M"
34+
build-log-max-lines = 10000
3235

3336

3437
# These sections allows to customize how crater treats specific crates/repos

‎src/agent/results.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::agent::api::AgentApi;
2+
use crate::config::Config;
23
use crate::crates::{Crate, GitHubRepo};
34
use crate::experiments::Experiment;
45
use crate::logs::{self, LogStorage};
@@ -49,12 +50,13 @@ impl<'a> WriteResults for ResultsUploader<'a> {
4950
toolchain: &Toolchain,
5051
krate: &Crate,
5152
existing_logs: Option<LogStorage>,
53+
config: &Config,
5254
f: F,
5355
) -> Fallible<TestResult>
5456
where
5557
F: FnOnce() -> Fallible<TestResult>,
5658
{
57-
let storage = existing_logs.unwrap_or_else(|| LogStorage::new(LevelFilter::Info));
59+
let storage = existing_logs.unwrap_or_else(|| LogStorage::new(LevelFilter::Info, config));
5860
let result = logs::capture(&storage, f)?;
5961
let output = storage.to_string();
6062

‎src/config.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ pub struct DemoCrates {
6767
#[serde(rename_all = "kebab-case")]
6868
pub struct SandboxConfig {
6969
pub memory_limit: Size,
70+
pub build_log_max_size: Size,
71+
pub build_log_max_lines: usize,
7072
}
7173

7274
#[derive(Clone, Serialize, Deserialize)]
@@ -241,6 +243,8 @@ impl Default for Config {
241243
local_crates: HashMap::new(),
242244
sandbox: SandboxConfig {
243245
memory_limit: Size::Gigabytes(2),
246+
build_log_max_size: Size::Megabytes(1),
247+
build_log_max_lines: 1000,
244248
},
245249
server: ServerConfig {
246250
bot_acl: Vec::new(),
@@ -275,6 +279,8 @@ mod tests {
275279
"local-crates = []\n",
276280
"[sandbox]\n",
277281
"memory-limit = \"2G\"\n",
282+
"build-log-max-size = \"2M\"\n",
283+
"build-log-max-lines = 1000\n",
278284
"[crates]\n",
279285
"lazy_static = { skip = true }\n",
280286
"[github-repos]\n",

‎src/lib.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
#![recursion_limit = "256"]
2-
#![allow(clippy::needless_pass_by_value, clippy::new_ret_no_self)]
2+
#![allow(
3+
clippy::needless_pass_by_value,
4+
clippy::new_ret_no_self,
5+
clippy::too_many_arguments
6+
)]
37

48
#[cfg_attr(test, macro_use)]
59
extern crate toml;

‎src/logs/mod.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
mod storage;
22

3-
use crate::prelude::*;
43
use log::{Log, Metadata, Record};
54
use std::cell::RefCell;
65
use std::sync::Once;
@@ -60,9 +59,9 @@ impl Log for MultiLogger {
6059
}
6160
}
6261

63-
pub fn capture<F, R, L>(storage: &L, f: F) -> Fallible<R>
62+
pub fn capture<F, R, L>(storage: &L, f: F) -> R
6463
where
65-
F: FnOnce() -> Fallible<R>,
64+
F: FnOnce() -> R,
6665
L: Log + Clone + 'static,
6766
{
6867
let storage = Box::new(storage.clone());

‎src/logs/storage.rs

Lines changed: 123 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,40 @@
1+
use crate::config::Config;
12
use log::{Level, LevelFilter, Log, Metadata, Record};
23
use std::fmt;
34
use std::sync::{Arc, Mutex};
45

56
#[derive(Clone)]
7+
#[cfg_attr(test, derive(Debug, PartialEq, Eq))]
68
struct StoredRecord {
79
level: Level,
810
message: String,
911
}
1012

1113
struct InnerStorage {
1214
records: Vec<StoredRecord>,
15+
size: usize,
16+
truncated: bool,
1317
}
1418

1519
#[derive(Clone)]
1620
pub struct LogStorage {
1721
inner: Arc<Mutex<InnerStorage>>,
1822
min_level: LevelFilter,
23+
max_size: usize,
24+
max_lines: usize,
1925
}
2026

2127
impl LogStorage {
22-
pub(crate) fn new(min_level: LevelFilter) -> Self {
28+
pub(crate) fn new(min_level: LevelFilter, config: &Config) -> Self {
2329
LogStorage {
2430
inner: Arc::new(Mutex::new(InnerStorage {
2531
records: Vec::new(),
32+
truncated: false,
33+
size: 0,
2634
})),
2735
min_level,
36+
max_size: config.sandbox.build_log_max_size.to_bytes(),
37+
max_lines: config.sandbox.build_log_max_lines,
2838
}
2939
}
3040

@@ -33,8 +43,12 @@ impl LogStorage {
3343
LogStorage {
3444
inner: Arc::new(Mutex::new(InnerStorage {
3545
records: inner.records.clone(),
46+
truncated: inner.truncated,
47+
size: inner.size,
3648
})),
3749
min_level: self.min_level,
50+
max_size: self.max_size,
51+
max_lines: self.max_lines,
3852
}
3953
}
4054
}
@@ -49,9 +63,30 @@ impl Log for LogStorage {
4963
return;
5064
}
5165
let mut inner = self.inner.lock().unwrap();
66+
if inner.truncated {
67+
return;
68+
}
69+
if inner.records.len() >= self.max_lines {
70+
inner.records.push(StoredRecord {
71+
level: Level::Warn,
72+
message: "too many lines in the log, truncating it".into(),
73+
});
74+
inner.truncated = true;
75+
return;
76+
}
77+
let message = record.args().to_string();
78+
if inner.size + message.len() >= self.max_size {
79+
inner.records.push(StoredRecord {
80+
level: Level::Warn,
81+
message: "too much data in the log, truncating it".into(),
82+
});
83+
inner.truncated = true;
84+
return;
85+
}
86+
inner.size += message.len();
5287
inner.records.push(StoredRecord {
5388
level: record.level(),
54-
message: record.args().to_string(),
89+
message,
5590
});
5691
}
5792

@@ -67,3 +102,89 @@ impl fmt::Display for LogStorage {
67102
Ok(())
68103
}
69104
}
105+
106+
#[cfg(test)]
107+
mod tests {
108+
use super::{LogStorage, StoredRecord};
109+
use crate::config::Config;
110+
use crate::logs;
111+
use crate::prelude::*;
112+
use crate::utils::size::Size;
113+
use log::{Level, LevelFilter};
114+
115+
#[test]
116+
fn test_log_storage() {
117+
logs::init_test();
118+
let config = Config::default();
119+
120+
let storage = LogStorage::new(LevelFilter::Info, &config);
121+
logs::capture(&storage, || {
122+
info!("an info record");
123+
warn!("a warn record");
124+
trace!("a trace record");
125+
});
126+
127+
assert_eq!(
128+
storage.inner.lock().unwrap().records,
129+
vec![
130+
StoredRecord {
131+
level: Level::Info,
132+
message: "an info record".to_string(),
133+
},
134+
StoredRecord {
135+
level: Level::Warn,
136+
message: "a warn record".to_string(),
137+
},
138+
]
139+
);
140+
}
141+
142+
#[test]
143+
fn test_too_much_content() {
144+
logs::init_test();
145+
146+
let mut config = Config::default();
147+
config.sandbox.build_log_max_size = Size::Kilobytes(4);
148+
149+
let storage = LogStorage::new(LevelFilter::Info, &config);
150+
logs::capture(&storage, || {
151+
let content = (0..Size::Kilobytes(8).to_bytes())
152+
.map(|_| '.')
153+
.collect::<String>();
154+
info!("{}", content);
155+
});
156+
157+
let inner = storage.inner.lock().unwrap();
158+
assert_eq!(inner.records.len(), 1);
159+
assert!(inner
160+
.records
161+
.last()
162+
.unwrap()
163+
.message
164+
.contains("too much data"));
165+
}
166+
167+
#[test]
168+
fn test_too_many_lines() {
169+
logs::init_test();
170+
171+
let mut config = Config::default();
172+
config.sandbox.build_log_max_lines = 100;
173+
174+
let storage = LogStorage::new(LevelFilter::Info, &config);
175+
logs::capture(&storage, || {
176+
for _ in 0..200 {
177+
info!("a line");
178+
}
179+
});
180+
181+
let inner = storage.inner.lock().unwrap();
182+
assert_eq!(inner.records.len(), 101);
183+
assert!(inner
184+
.records
185+
.last()
186+
.unwrap()
187+
.message
188+
.contains("too many lines"));
189+
}
190+
}

‎src/report/archives.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,25 +127,25 @@ mod tests {
127127
// Fill some dummy results into the database
128128
let results = DatabaseDB::new(&db);
129129
results
130-
.record_result(&ex, &ex.toolchains[0], &crate1, None, || {
130+
.record_result(&ex, &ex.toolchains[0], &crate1, None, &config, || {
131131
info!("tc1 crate1");
132132
Ok(TestResult::TestPass)
133133
})
134134
.unwrap();
135135
results
136-
.record_result(&ex, &ex.toolchains[1], &crate1, None, || {
136+
.record_result(&ex, &ex.toolchains[1], &crate1, None, &config, || {
137137
info!("tc2 crate1");
138138
Ok(TestResult::BuildFail(FailureReason::Unknown))
139139
})
140140
.unwrap();
141141
results
142-
.record_result(&ex, &ex.toolchains[0], &crate2, None, || {
142+
.record_result(&ex, &ex.toolchains[0], &crate2, None, &config, || {
143143
info!("tc1 crate2");
144144
Ok(TestResult::TestPass)
145145
})
146146
.unwrap();
147147
results
148-
.record_result(&ex, &ex.toolchains[1], &crate2, None, || {
148+
.record_result(&ex, &ex.toolchains[1], &crate2, None, &config, || {
149149
info!("tc2 crate2");
150150
Ok(TestResult::TestPass)
151151
})

‎src/results/db.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::config::Config;
12
use crate::crates::{Crate, GitHubRepo};
23
use crate::db::{Database, QueryUtils};
34
use crate::experiments::Experiment;
@@ -169,12 +170,13 @@ impl<'a> WriteResults for DatabaseDB<'a> {
169170
toolchain: &Toolchain,
170171
krate: &Crate,
171172
existing_logs: Option<LogStorage>,
173+
config: &Config,
172174
f: F,
173175
) -> Fallible<TestResult>
174176
where
175177
F: FnOnce() -> Fallible<TestResult>,
176178
{
177-
let storage = existing_logs.unwrap_or_else(|| LogStorage::new(LevelFilter::Info));
179+
let storage = existing_logs.unwrap_or_else(|| LogStorage::new(LevelFilter::Info, config));
178180
let result = logs::capture(&storage, f)?;
179181
let output = storage.to_string();
180182
self.store_result(ex, krate, toolchain, result, output.as_bytes())?;
@@ -299,7 +301,7 @@ mod tests {
299301

300302
// Record a result with a message in it
301303
results
302-
.record_result(&ex, &MAIN_TOOLCHAIN, &krate, None, || {
304+
.record_result(&ex, &MAIN_TOOLCHAIN, &krate, None, &config, || {
303305
info!("hello world");
304306
Ok(TestResult::TestPass)
305307
})
@@ -336,7 +338,7 @@ mod tests {
336338

337339
// Add another result
338340
results
339-
.record_result(&ex, &TEST_TOOLCHAIN, &krate, None, || {
341+
.record_result(&ex, &TEST_TOOLCHAIN, &krate, None, &config, || {
340342
info!("Another log message!");
341343
Ok(TestResult::TestFail(FailureReason::Unknown))
342344
})

‎src/results/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ mod db;
22
#[cfg(test)]
33
mod dummy;
44

5+
use crate::config::Config;
56
use crate::crates::{Crate, GitHubRepo};
67
use crate::experiments::Experiment;
78
use crate::logs::LogStorage;
@@ -43,6 +44,7 @@ pub trait WriteResults {
4344
toolchain: &Toolchain,
4445
krate: &Crate,
4546
existing_logs: Option<LogStorage>,
47+
config: &Config,
4648
f: F,
4749
) -> Fallible<TestResult>
4850
where

‎src/runner/graph.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ impl TasksGraph {
191191
ex: &Experiment,
192192
db: &DB,
193193
state: &RunnerState,
194+
config: &Config,
194195
error: &F,
195196
result: TestResult,
196197
) -> Fallible<()> {
@@ -199,11 +200,13 @@ impl TasksGraph {
199200
.neighbors_directed(node, Direction::Incoming)
200201
.collect::<Vec<_>>();
201202
for child in children.drain(..) {
202-
self.mark_as_failed(child, ex, db, state, error, result)?;
203+
self.mark_as_failed(child, ex, db, state, config, error, result)?;
203204
}
204205

205206
match self.graph[node] {
206-
Node::Task { ref task, .. } => task.mark_as_failed(ex, db, state, error, result)?,
207+
Node::Task { ref task, .. } => {
208+
task.mark_as_failed(ex, db, state, config, error, result)?
209+
}
207210
Node::CrateCompleted | Node::Root => return Ok(()),
208211
}
209212

0 commit comments

Comments
 (0)