Direct user where to find samply output (#1473)

shotover-proxy/benches/windsock/profilers.rs

@@ -56,7 +56,14 @@ impl ProfilerRunner {
  };
  self.samply = if self.run_samply {
  if let Some(shotover) = &shotover {
- Some(Samply::run(self.results_path.clone(), shotover.child().id().unwrap()).await)
+ Some(
+ Samply::run(
+ self.bench_name.clone(),
+ self.results_path.clone(),
+ shotover.child().id().unwrap(),
+ )
+ .await,
+ )
  } else {
  panic!("samply not supported when benching without shotover")
  }

shotover-proxy/benches/windsock/profilers/samply.rs

@@ -1,10 +1,15 @@
 use std::path::PathBuf;
 use tokio::task::JoinHandle;
+use windsock::ReportArchive;
 
-pub struct Samply(JoinHandle<()>);
+pub struct Samply {
+ bench_name: String,
+ handle: JoinHandle<()>,
+ output_file: String,
+}
 
 impl Samply {
- pub async fn run(results_path: PathBuf, pid: u32) -> Samply {
+ pub async fn run(bench_name: String, results_path: PathBuf, pid: u32) -> Samply {
  run_command(
  "cargo",
  &[
@@ -13,11 +18,16 @@ impl Samply {
  "--git",
  "https://github.com/mstange/samply",
  "--rev",
- "4c8d5eb164e44c4eda1b29de116f5ea546d64c65",
+ "59a03a716cadab7835e3a961d0107ec797e458ec",
  ],
  )
  .await;
- let output_file = results_path.join("samply.json");
+ let output_file = results_path
+ .join("samply.json")
+ .as_os_str()
+ .to_str()
+ .unwrap()
+ .to_owned();
  let home = std::env::var("HOME").unwrap();
 
  // Run `sudo ls` so that we can get sudo to stop asking for password for a while.
@@ -26,28 +36,38 @@ impl Samply {
  // But that would require a bunch of work so its out of scope for now.
  run_command("sudo", &["ls"]).await;
 
- Samply(tokio::spawn(async move {
- run_command(
- "sudo",
- &[
- // specify the full path as CI has trouble finding it for some reason.
- &format!("{home}/.cargo/bin/samply"),
- "record",
- "-p",
- &pid.to_string(),
- "--save-only",
- "--profile-name",
- results_path.file_name().unwrap().to_str().unwrap(),
- "--output",
- output_file.as_os_str().to_str().unwrap(),
- ],
- )
- .await;
- }))
+ Samply {
+ bench_name,
+ output_file: output_file.clone(),
+ handle: tokio::spawn(async move {
+ run_command(
+ "sudo",
+ &[
+ // specify the full path as CI has trouble finding it for some reason.
+ &format!("{home}/.cargo/bin/samply"),
+ "record",
+ "-p",
+ &pid.to_string(),
+ "--save-only",
+ "--profile-name",
+ results_path.file_name().unwrap().to_str().unwrap(),
+ "--output",
+ &output_file,
+ ],
+ )
+ .await;
+ }),
+ }
  }
 
  pub fn wait(self) {
- futures::executor::block_on(self.0).unwrap();
+ futures::executor::block_on(self.handle).unwrap();
+ let mut report = ReportArchive::load(&self.bench_name).unwrap();
+ report.info_messages.push(format!(
+ "To open profiler results run: samply load {}",
+ self.output_file
+ ));
+ report.save();
  }
 }
 

windsock/src/report.rs

@@ -8,25 +8,46 @@ use tokio::sync::mpsc::UnboundedReceiver;
 
 #[derive(Debug, Serialize, Deserialize)]
 pub enum Report {
+ /// Indicates the warmup is over and the benchmark has begun.
+ /// Any Completed/Errored Events received before this are considered warmups and discarded.
  Start,
+
+ /// Indicates a response came back from the service.
+ /// The Duration should be the time between the request being sent and the response being received
  QueryCompletedIn(Duration),
+
+ /// Indicates an an error response came back from the service.
  QueryErrored {
+ /// The time between the request being sent and the response being received
  completed_in: Duration,
+ /// The error message received from the service or the local error that occured while trying to communicate with the service.
  message: String,
  },
+
+ /// Indicates a pubsub produce ack came back from the service.
+ /// The Duration should be the time between the request being sent and the response being received
  ProduceCompletedIn(Duration),
+
+ /// Indicates a pubsub produce error response came back from the service.
  ProduceErrored {
  completed_in: Duration,
  message: String,
  },
+ /// Indicates a pubsub consume response comes back from the service.
  ConsumeCompleted,
- ConsumeErrored {
- message: String,
- },
+
+ /// Indicates pubsub consume error response came back from the service.
+ ConsumeErrored { message: String },
+
+ /// Indicates a second has passed for the benchmarker
  SecondPassed(Duration),
- /// contains the time that the test ran for
+
+ /// Contains the time that the test ran for
  FinishedIn(Duration),
 
+ /// Adds a note that will be visible to the user when viewing the benchmark results.
+ AddInfoMessage(String),
+
  /// Ignore all other reports and use the ManualReport as the only source of benchmark metrics.
  /// Do not use this under normal circumstances.
  /// Instead this should only be used if you have an independent benchmarker that you want to call from windsock and include in windsocks results.
@@ -112,7 +133,8 @@ pub struct ReportArchive {
  pub(crate) operations_report: Option<OperationsReport>,
  pub(crate) pubsub_report: Option<PubSubReport>,
  pub metrics: Vec<Metric>,
- pub(crate) error_messages: Vec<String>,
+ pub error_messages: Vec<String>,
+ pub info_messages: Vec<String>,
 }
 
 #[derive(Clone, Debug, Serialize, Deserialize, Default)]
@@ -341,12 +363,14 @@ pub(crate) async fn report_builder(
  let mut total_operation_time = Duration::from_secs(0);
  let mut total_produce_time = Duration::from_secs(0);
  let mut error_messages = vec![];
+ let mut info_messages = vec![];
 
  while let Some(report) = rx.recv().await {
  match report {
  Report::Start => {
  started = Some(OffsetDateTime::now_utc());
  }
+ Report::AddInfoMessage(message) => info_messages.push(message),
  Report::QueryCompletedIn(duration) => {
  let report = operations_report.get_or_insert_with(OperationsReport::default);
  if started.is_some() {
@@ -490,6 +514,7 @@ pub(crate) async fn report_builder(
  tags,
  pubsub_report,
  error_messages,
+ info_messages,
  operations_report,
  metrics: vec![],
  };

windsock/src/tables.rs

@@ -735,8 +735,7 @@ fn base(reports: &[ReportColumn], table_type: &str) {
  );
  println!("{}", style(error).red().bold());
  for (i, message) in report.current.error_messages.iter().enumerate() {
- let line = format!(" {i}. {message}");
- println!("{}", line);
+ println!(" {i}. {message}");
  }
  }
 
@@ -748,8 +747,7 @@ fn base(reports: &[ReportColumn], table_type: &str) {
  );
  println!("{}", style(error).red().bold());
  for (i, message) in report.current.error_messages.iter().enumerate() {
- let line = format!(" {i}. {message}");
- println!("{}", line);
+ println!(" {i}. {message}");
  }
  }
  }
@@ -766,9 +764,17 @@ fn base(reports: &[ReportColumn], table_type: &str) {
  !x.current.running_in_release
  || x.baseline
  .as_ref()
- .map(|x| !x.error_messages.is_empty())
+ .map(|x| !x.running_in_release)
+ .unwrap_or(false)
+ });
+ let info_found = reports.iter().any(|x| {
+ !x.current.info_messages.is_empty()
+ || x.baseline
+ .as_ref()
+ .map(|x| !x.info_messages.is_empty())
  .unwrap_or(false)
  });
+
  if errors_found && not_running_in_release_found {
  // ensure these two sections are kept apart
  println!();
@@ -793,6 +799,34 @@ fn base(reports: &[ReportColumn], table_type: &str) {
  }
  }
  }
+
+ #[allow(clippy::nonminimal_bool)]
+ if info_found
+ && (not_running_in_release_found || (errors_found && !not_running_in_release_found))
+ {
+ // ensure these two sections are kept apart
+ println!();
+ }
+
+ for report in reports {
+ if !report.current.info_messages.is_empty() {
+ let error = format!("notes for {}", report.current.tags.get_name());
+ println!("{}", style(error).blue().bold());
+ for (i, message) in report.current.info_messages.iter().enumerate() {
+ println!(" {i}. {message}");
+ }
+ }
+
+ if let Some(baseline) = &report.baseline {
+ if !baseline.info_messages.is_empty() {
+ let error = format!("notes for baseline {}", report.current.tags.get_name());
+ println!("{}", style(error).blue().bold());
+ for (i, message) in report.current.info_messages.iter().enumerate() {
+ println!(" {i}. {message}");
+ }
+ }
+ }
+ }
 }
 
 fn duration_ms(duration: Duration) -> String {