Skip to content

Commit c7c9655

Browse files
authored
chore: add some more instrumentation (#11157)
1 parent 4c41b1e commit c7c9655

File tree

10 files changed

+91
-43
lines changed

10 files changed

+91
-43
lines changed

crates/common/src/compile.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
use crate::{
44
TestFunctionExt,
5-
preprocessor::TestOptimizerPreprocessor,
5+
preprocessor::DynamicTestLinkingPreprocessor,
66
reports::{ReportKind, report_kind},
77
shell,
88
term::SpinnerReporter,
@@ -146,13 +146,15 @@ impl ProjectCompiler {
146146
self.dynamic_test_linking = preprocess;
147147
self
148148
}
149+
149150
/// Compiles the project.
151+
#[instrument(target = "forge::compile", skip_all)]
150152
pub fn compile<C: Compiler<CompilerContract = Contract>>(
151153
mut self,
152154
project: &Project<C>,
153155
) -> Result<ProjectCompileOutput<C>>
154156
where
155-
TestOptimizerPreprocessor: Preprocessor<C>,
157+
DynamicTestLinkingPreprocessor: Preprocessor<C>,
156158
{
157159
self.project_root = project.root().to_path_buf();
158160

@@ -180,7 +182,7 @@ impl ProjectCompiler {
180182
let mut compiler =
181183
foundry_compilers::project::ProjectCompiler::with_sources(project, sources)?;
182184
if preprocess {
183-
compiler = compiler.with_preprocessor(TestOptimizerPreprocessor);
185+
compiler = compiler.with_preprocessor(DynamicTestLinkingPreprocessor);
184186
}
185187
compiler.compile().map_err(Into::into)
186188
})
@@ -196,7 +198,6 @@ impl ProjectCompiler {
196198
/// let prj = config.project().unwrap();
197199
/// ProjectCompiler::new().compile_with(|| Ok(prj.compile()?)).unwrap();
198200
/// ```
199-
#[instrument(target = "forge::compile", skip_all)]
200201
fn compile_with<C: Compiler<CompilerContract = Contract>, F>(
201202
self,
202203
f: F,
@@ -519,7 +520,7 @@ pub fn compile_target<C: Compiler<CompilerContract = Contract>>(
519520
quiet: bool,
520521
) -> Result<ProjectCompileOutput<C>>
521522
where
522-
TestOptimizerPreprocessor: Preprocessor<C>,
523+
DynamicTestLinkingPreprocessor: Preprocessor<C>,
523524
{
524525
ProjectCompiler::new().quiet(quiet).files([target_path.into()]).compile(project)
525526
}

crates/common/src/preprocessor/mod.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,17 @@ fn span_to_range(source_map: &SourceMap, span: Span) -> Range<usize> {
2525
source_map.span_to_source(span).unwrap().1
2626
}
2727

28+
/// Preprocessor that replaces static bytecode linking in tests and scripts (`new Contract`) with
29+
/// dynamic linkage through (`Vm.create*`).
30+
///
31+
/// This allows for more efficient caching when iterating on tests.
32+
///
33+
/// See <https://github.com/foundry-rs/foundry/pull/10010>.
2834
#[derive(Debug)]
29-
pub struct TestOptimizerPreprocessor;
35+
pub struct DynamicTestLinkingPreprocessor;
3036

31-
impl Preprocessor<SolcCompiler> for TestOptimizerPreprocessor {
37+
impl Preprocessor<SolcCompiler> for DynamicTestLinkingPreprocessor {
38+
#[instrument(name = "DynamicTestLinkingPreprocessor::preprocess", skip_all)]
3239
fn preprocess(
3340
&self,
3441
_solc: &SolcCompiler,
@@ -97,7 +104,7 @@ impl Preprocessor<SolcCompiler> for TestOptimizerPreprocessor {
97104
}
98105
}
99106

100-
impl Preprocessor<MultiCompiler> for TestOptimizerPreprocessor {
107+
impl Preprocessor<MultiCompiler> for DynamicTestLinkingPreprocessor {
101108
fn preprocess(
102109
&self,
103110
compiler: &MultiCompiler,

crates/evm/evm/src/executors/fuzz/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ impl FuzzedExecutor {
177177
reason: None,
178178
counterexample: None,
179179
logs: fuzz_result.logs,
180-
labeled_addresses: call.labels,
180+
labels: call.labels,
181181
traces: last_run_traces,
182182
breakpoints: last_run_breakpoints,
183183
gas_report_traces: traces.into_iter().map(|a| a.arena).collect(),

crates/evm/fuzz/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ pub struct FuzzTestResult {
206206
pub logs: Vec<Log>,
207207

208208
/// Labeled addresses
209-
pub labeled_addresses: AddressHashMap<String>,
209+
pub labels: AddressHashMap<String>,
210210

211211
/// Exemplary traces for a fuzz run of the test function
212212
///

crates/evm/traces/src/decoder/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ impl CallTraceDecoder {
167167
INIT.get_or_init(Self::init)
168168
}
169169

170+
#[instrument(name = "CallTraceDecoder::init", level = "debug")]
170171
fn init() -> Self {
171172
Self {
172173
contracts: Default::default(),

crates/evm/traces/src/identifier/signatures.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ impl Serialize for SignaturesCache {
8383

8484
impl SignaturesCache {
8585
/// Loads the cache from a file.
86-
#[instrument(target = "evm::traces")]
86+
#[instrument(target = "evm::traces", name = "SignaturesCache::load")]
8787
pub fn load(path: &Path) -> Self {
8888
trace!(target: "evm::traces", ?path, "reading signature cache");
8989
fs::read_json_file(path)
@@ -94,7 +94,7 @@ impl SignaturesCache {
9494
}
9595

9696
/// Saves the cache to a file.
97-
#[instrument(target = "evm::traces", skip(self))]
97+
#[instrument(target = "evm::traces", name = "SignaturesCache::save", skip(self))]
9898
pub fn save(&self, path: &Path) {
9999
if let Some(parent) = path.parent()
100100
&& let Err(err) = std::fs::create_dir_all(parent)

crates/forge/src/cmd/coverage.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ impl CoverageArgs {
185185
}
186186

187187
/// Builds the coverage report.
188-
#[instrument(name = "prepare", skip_all)]
188+
#[instrument(name = "Coverage::prepare", skip_all)]
189189
fn prepare(
190190
&self,
191191
project_paths: &ProjectPathsConfig,
@@ -258,6 +258,7 @@ impl CoverageArgs {
258258
}
259259

260260
/// Runs tests, collects coverage data and generates the final report.
261+
#[instrument(name = "Coverage::collect", skip_all)]
261262
async fn collect(
262263
mut self,
263264
root: &Path,
@@ -329,10 +330,17 @@ impl CoverageArgs {
329330
}
330331

331332
// Output final reports.
333+
self.report(&report)?;
334+
335+
Ok(())
336+
}
337+
338+
#[instrument(name = "Coverage::report", skip_all)]
339+
fn report(&mut self, report: &CoverageReport) -> Result<()> {
332340
for reporter in &mut self.reporters {
333-
reporter.report(&report)?;
341+
let _guard = debug_span!("reporter.report", kind=%reporter.name()).entered();
342+
reporter.report(report)?;
334343
}
335-
336344
Ok(())
337345
}
338346

crates/forge/src/cmd/test/mod.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ impl TestArgs {
208208
/// Returns sources which include any tests to be executed.
209209
/// If no filters are provided, sources are filtered by existence of test/invariant methods in
210210
/// them, If filters are provided, sources are additionally filtered by them.
211+
#[instrument(target = "forge::test", skip_all)]
211212
pub fn get_sources_to_compile(
212213
&self,
213214
config: &Config,
@@ -612,9 +613,7 @@ impl TestArgs {
612613

613614
// Clear the addresses and labels from previous runs.
614615
decoder.clear_addresses();
615-
decoder
616-
.labels
617-
.extend(result.labeled_addresses.iter().map(|(k, v)| (*k, v.clone())));
616+
decoder.labels.extend(result.labels.iter().map(|(k, v)| (*k, v.clone())));
618617

619618
// Identify addresses and decode traces.
620619
let mut decoded_traces = Vec::with_capacity(result.traces.len());

crates/forge/src/coverage.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ pub use foundry_evm::coverage::*;
1515

1616
/// A coverage reporter.
1717
pub trait CoverageReporter {
18+
/// Returns a debug string for the reporter.
19+
fn name(&self) -> &'static str;
20+
1821
/// Returns `true` if the reporter needs source maps for the final report.
1922
fn needs_source_maps(&self) -> bool {
2023
false
@@ -62,6 +65,10 @@ impl CoverageSummaryReporter {
6265
}
6366

6467
impl CoverageReporter for CoverageSummaryReporter {
68+
fn name(&self) -> &'static str {
69+
"summary"
70+
}
71+
6572
fn report(&mut self, report: &CoverageReport) -> eyre::Result<()> {
6673
for (path, summary) in report.summary_by_file() {
6774
self.total.merge(&summary);
@@ -108,6 +115,10 @@ impl LcovReporter {
108115
}
109116

110117
impl CoverageReporter for LcovReporter {
118+
fn name(&self) -> &'static str {
119+
"lcov"
120+
}
121+
111122
fn report(&mut self, report: &CoverageReport) -> eyre::Result<()> {
112123
let mut out = std::io::BufWriter::new(fs::create_file(&self.path)?);
113124

@@ -184,6 +195,10 @@ impl CoverageReporter for LcovReporter {
184195
pub struct DebugReporter;
185196

186197
impl CoverageReporter for DebugReporter {
198+
fn name(&self) -> &'static str {
199+
"debug"
200+
}
201+
187202
fn report(&mut self, report: &CoverageReport) -> eyre::Result<()> {
188203
for (path, items) in report.items_by_file() {
189204
sh_println!("Uncovered for {}:", path.display())?;
@@ -237,6 +252,10 @@ impl BytecodeReporter {
237252
}
238253

239254
impl CoverageReporter for BytecodeReporter {
255+
fn name(&self) -> &'static str {
256+
"bytecode"
257+
}
258+
240259
fn needs_source_maps(&self) -> bool {
241260
true
242261
}

crates/forge/src/result.rs

Lines changed: 38 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,8 @@ pub struct TestResult {
407407
pub line_coverage: Option<HitMaps>,
408408

409409
/// Labeled addresses
410-
pub labeled_addresses: AddressHashMap<String>,
410+
#[serde(rename = "labeled_addresses")] // Backwards compatibility.
411+
pub labels: AddressHashMap<String>,
411412

412413
#[serde(with = "foundry_common::serde_helpers::duration")]
413414
pub duration: Duration,
@@ -472,11 +473,20 @@ impl fmt::Display for TestResult {
472473
}
473474
}
474475

476+
macro_rules! extend {
477+
($a:expr, $b:expr, $trace_kind:expr) => {
478+
$a.logs.extend($b.logs);
479+
$a.labels.extend($b.labels);
480+
$a.traces.extend($b.traces.map(|traces| ($trace_kind, traces)));
481+
$a.merge_coverages($b.line_coverage);
482+
};
483+
}
484+
475485
impl TestResult {
476486
/// Creates a new test result starting from test setup results.
477487
pub fn new(setup: &TestSetup) -> Self {
478488
Self {
479-
labeled_addresses: setup.labels.clone(),
489+
labels: setup.labels.clone(),
480490
logs: setup.logs.clone(),
481491
traces: setup.traces.clone(),
482492
line_coverage: setup.coverage.clone(),
@@ -491,13 +501,25 @@ impl TestResult {
491501

492502
/// Creates a test setup result.
493503
pub fn setup_result(setup: TestSetup) -> Self {
504+
let TestSetup {
505+
address: _,
506+
fuzz_fixtures: _,
507+
logs,
508+
labels,
509+
traces,
510+
coverage,
511+
deployed_libs: _,
512+
reason,
513+
skipped,
514+
deployment_failure: _,
515+
} = setup;
494516
Self {
495-
status: if setup.skipped { TestStatus::Skipped } else { TestStatus::Failure },
496-
reason: setup.reason,
497-
logs: setup.logs,
498-
traces: setup.traces,
499-
line_coverage: setup.coverage,
500-
labeled_addresses: setup.labels,
517+
status: if skipped { TestStatus::Skipped } else { TestStatus::Failure },
518+
reason,
519+
logs,
520+
traces,
521+
line_coverage: coverage,
522+
labels,
501523
..Default::default()
502524
}
503525
}
@@ -525,11 +547,7 @@ impl TestResult {
525547
self.kind =
526548
TestKind::Unit { gas: raw_call_result.gas_used.wrapping_sub(raw_call_result.stipend) };
527549

528-
// Record logs, labels, traces and merge coverages.
529-
self.logs.extend(raw_call_result.logs);
530-
self.labeled_addresses.extend(raw_call_result.labels);
531-
self.traces.extend(raw_call_result.traces.map(|traces| (TraceKind::Execution, traces)));
532-
self.merge_coverages(raw_call_result.line_coverage);
550+
extend!(self, raw_call_result, TraceKind::Execution);
533551

534552
self.status = match success {
535553
true => TestStatus::Success,
@@ -557,10 +575,7 @@ impl TestResult {
557575
};
558576

559577
// Record logs, labels, traces and merge coverages.
560-
self.logs.extend(result.logs);
561-
self.labeled_addresses.extend(result.labeled_addresses);
562-
self.traces.extend(result.traces.map(|traces| (TraceKind::Execution, traces)));
563-
self.merge_coverages(result.line_coverage);
578+
extend!(self, result, TraceKind::Execution);
564579

565580
self.status = if result.skipped {
566581
TestStatus::Skipped
@@ -667,10 +682,7 @@ impl TestResult {
667682

668683
/// Merges the given raw call result into `self`.
669684
pub fn extend(&mut self, call_result: RawCallResult) {
670-
self.logs.extend(call_result.logs);
671-
self.labeled_addresses.extend(call_result.labels);
672-
self.traces.extend(call_result.traces.map(|traces| (TraceKind::Execution, traces)));
673-
self.merge_coverages(call_result.line_coverage);
685+
extend!(self, call_result, TraceKind::Execution);
674686
}
675687

676688
/// Merges the given coverage result into `self`.
@@ -825,9 +837,10 @@ impl TestSetup {
825837
}
826838

827839
pub fn extend(&mut self, raw: RawCallResult, trace_kind: TraceKind) {
828-
self.logs.extend(raw.logs);
829-
self.labels.extend(raw.labels);
830-
self.traces.extend(raw.traces.map(|traces| (trace_kind, traces)));
831-
HitMaps::merge_opt(&mut self.coverage, raw.line_coverage);
840+
extend!(self, raw, trace_kind);
841+
}
842+
843+
pub fn merge_coverages(&mut self, other_coverage: Option<HitMaps>) {
844+
HitMaps::merge_opt(&mut self.coverage, other_coverage);
832845
}
833846
}

0 commit comments

Comments
 (0)