Skip to content

Commit c3682b2

Browse files
committed
Correctly show executed command name in Chrome trace
1 parent 604c180 commit c3682b2

File tree

5 files changed

+54
-27
lines changed

5 files changed

+54
-27
lines changed

src/bootstrap/src/core/build_steps/compile.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2469,7 +2469,7 @@ pub fn stream_cargo(
24692469
let mut cmd = cargo.into_cmd();
24702470

24712471
#[cfg(feature = "tracing")]
2472-
let _run_span = crate::trace_cmd!(cmd);
2472+
let _run_span = crate::utils::tracing::trace_cmd(&cmd);
24732473

24742474
// Instruct Cargo to give us json messages on stdout, critically leaving
24752475
// stderr as piped so we can get those pretty colors.

src/bootstrap/src/core/builder/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -948,7 +948,7 @@ impl Step for Libdir {
948948
}
949949

950950
#[cfg(feature = "tracing")]
951-
pub const STEP_NAME_TARGET: &str = "STEP";
951+
pub const STEP_SPAN_TARGET: &str = "STEP";
952952

953953
impl<'a> Builder<'a> {
954954
fn get_step_descriptions(kind: Kind) -> Vec<StepDescription> {
@@ -1713,7 +1713,7 @@ You have to build a stage1 compiler for `{}` first, and then use it to build a s
17131713
let _span = {
17141714
// Keep the target and field names synchronized with `setup_tracing`.
17151715
let span = tracing::info_span!(
1716-
target: STEP_NAME_TARGET,
1716+
target: STEP_SPAN_TARGET,
17171717
// We cannot use a dynamic name here, so instead we record the actual step name
17181718
// in the step_name field.
17191719
"step",

src/bootstrap/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ mod utils;
4444

4545
pub use core::builder::PathSet;
4646
#[cfg(feature = "tracing")]
47-
pub use core::builder::STEP_NAME_TARGET;
47+
pub use core::builder::STEP_SPAN_TARGET;
4848
pub use core::config::flags::{Flags, Subcommand};
4949
pub use core::config::{ChangeId, Config};
5050

src/bootstrap/src/utils/exec.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ use build_helper::drop_bomb::DropBomb;
2626
use build_helper::exit;
2727

2828
use crate::core::config::DryRun;
29-
#[cfg(feature = "tracing")]
30-
use crate::trace_cmd;
3129
use crate::{PathBuf, t};
3230

3331
/// What should be done when the command fails.
@@ -76,9 +74,17 @@ pub struct CommandFingerprint {
7674
}
7775

7876
impl CommandFingerprint {
77+
#[cfg(feature = "tracing")]
78+
pub(crate) fn program_name(&self) -> String {
79+
Path::new(&self.program)
80+
.file_name()
81+
.map(|p| p.to_string_lossy().to_string())
82+
.unwrap_or_else(|| "<unknown command>".to_string())
83+
}
84+
7985
/// Helper method to format both Command and BootstrapCommand as a short execution line,
8086
/// without all the other details (e.g. environment variables).
81-
pub fn format_short_cmd(&self) -> String {
87+
pub(crate) fn format_short_cmd(&self) -> String {
8288
use std::fmt::Write;
8389

8490
let mut cmd = self.program.to_string_lossy().to_string();
@@ -676,7 +682,7 @@ impl ExecutionContext {
676682
let fingerprint = command.fingerprint();
677683

678684
#[cfg(feature = "tracing")]
679-
let span_guard = trace_cmd!(command);
685+
let span_guard = crate::utils::tracing::trace_cmd(command);
680686

681687
if let Some(cached_output) = self.command_cache.get(&fingerprint) {
682688
command.mark_as_executed();
@@ -768,7 +774,7 @@ impl ExecutionContext {
768774
}
769775

770776
#[cfg(feature = "tracing")]
771-
let span_guard = trace_cmd!(command);
777+
let span_guard = crate::utils::tracing::trace_cmd(command);
772778

773779
let start_time = Instant::now();
774780
let fingerprint = command.fingerprint();

src/bootstrap/src/utils/tracing.rs

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -48,19 +48,21 @@ macro_rules! error {
4848
}
4949
}
5050

51-
#[macro_export]
52-
macro_rules! trace_cmd {
53-
($cmd:expr) => {
54-
{
55-
::tracing::span!(
56-
target: "COMMAND",
57-
::tracing::Level::TRACE,
58-
"cmd",
59-
cmd = $cmd.fingerprint().format_short_cmd(),
60-
full_cmd = ?$cmd
61-
).entered()
62-
}
63-
};
51+
#[cfg(feature = "tracing")]
52+
const COMMAND_SPAN_TARGET: &str = "COMMAND";
53+
54+
#[cfg(feature = "tracing")]
55+
pub fn trace_cmd(command: &crate::BootstrapCommand) -> tracing::span::EnteredSpan {
56+
let fingerprint = command.fingerprint();
57+
tracing::span!(
58+
target: COMMAND_SPAN_TARGET,
59+
tracing::Level::TRACE,
60+
"cmd",
61+
cmd_name = fingerprint.program_name().to_string(),
62+
cmd = fingerprint.format_short_cmd(),
63+
full_cmd = ?command
64+
)
65+
.entered()
6466
}
6567

6668
// # Note on `tracing` usage in bootstrap
@@ -87,7 +89,8 @@ mod inner {
8789
use tracing_subscriber::registry::{LookupSpan, SpanRef};
8890
use tracing_subscriber::{EnvFilter, Layer};
8991

90-
use crate::STEP_NAME_TARGET;
92+
use crate::STEP_SPAN_TARGET;
93+
use crate::utils::tracing::COMMAND_SPAN_TARGET;
9194

9295
pub fn setup_tracing(profiling_enabled: bool) -> Option<TracingGuard> {
9396
let filter = EnvFilter::from_env("BOOTSTRAP_TRACING");
@@ -110,10 +113,14 @@ mod inner {
110113
.name_fn(Box::new(|event_or_span| match event_or_span {
111114
tracing_chrome::EventOrSpan::Event(e) => e.metadata().name().to_string(),
112115
tracing_chrome::EventOrSpan::Span(s) => {
113-
if s.metadata().target() == STEP_NAME_TARGET
116+
if s.metadata().target() == STEP_SPAN_TARGET
114117
&& let Some(extension) = s.extensions().get::<StepNameExtension>()
115118
{
116119
extension.0.clone()
120+
} else if s.metadata().target() == COMMAND_SPAN_TARGET
121+
&& let Some(extension) = s.extensions().get::<CommandNameExtension>()
122+
{
123+
extension.0.clone()
117124
} else {
118125
s.metadata().name().to_string()
119126
}
@@ -151,6 +158,8 @@ mod inner {
151158
message: Option<String>,
152159
/// Name of a recorded psna
153160
step_name: Option<String>,
161+
/// Short name of an executed command
162+
cmd_name: Option<String>,
154163
/// The rest of arbitrary event/span fields
155164
fields: Vec<(&'static str, String)>,
156165
}
@@ -163,6 +172,9 @@ mod inner {
163172
"step_name" => {
164173
self.step_name = Some(value.to_string());
165174
}
175+
"cmd_name" => {
176+
self.cmd_name = Some(value.to_string());
177+
}
166178
name => {
167179
self.fields.push((name, value.to_string()));
168180
}
@@ -187,9 +199,12 @@ mod inner {
187199
Enter,
188200
}
189201

190-
/// Holds the name of a step, stored in `tracing_subscriber`'s extensions.
202+
/// Holds the name of a step span, stored in `tracing_subscriber`'s extensions.
191203
struct StepNameExtension(String);
192204

205+
/// Holds the name of a command span, stored in `tracing_subscriber`'s extensions.
206+
struct CommandNameExtension(String);
207+
193208
#[derive(Default)]
194209
struct TracingPrinter {
195210
indent: std::sync::atomic::AtomicU32,
@@ -264,7 +279,7 @@ mod inner {
264279
// We handle steps specially. We instrument them dynamically in `Builder::ensure`,
265280
// and we want to have custom name for each step span. But tracing doesn't allow setting
266281
// dynamic span names. So we detect step spans here and override their name.
267-
if span.metadata().target() == STEP_NAME_TARGET {
282+
if span.metadata().target() == STEP_SPAN_TARGET {
268283
let name = field_values.and_then(|v| v.step_name.as_deref()).unwrap_or(span.name());
269284
write!(writer, "{name}")?;
270285

@@ -334,8 +349,14 @@ mod inner {
334349

335350
// We need to propagate the actual name of the span to the Chrome layer below, because
336351
// it cannot access field values. We do that through extensions.
337-
if let Some(step_name) = field_values.step_name.clone() {
352+
if attrs.metadata().target() == STEP_SPAN_TARGET
353+
&& let Some(step_name) = field_values.step_name.clone()
354+
{
338355
ctx.span(id).unwrap().extensions_mut().insert(StepNameExtension(step_name));
356+
} else if attrs.metadata().target() == COMMAND_SPAN_TARGET
357+
&& let Some(cmd_name) = field_values.cmd_name.clone()
358+
{
359+
ctx.span(id).unwrap().extensions_mut().insert(CommandNameExtension(cmd_name));
339360
}
340361
self.span_values.lock().unwrap().insert(id.clone(), field_values);
341362
}

0 commit comments

Comments
 (0)