From 0a770c91eba03a02bd3fbb9abee4437dcc14a089 Mon Sep 17 00:00:00 2001 From: Lucas Kent Date: Sat, 2 Sep 2023 06:51:43 +1000 Subject: [PATCH] Add samply as a windsock profiler (#1315) --- .github/workflows/windsock_benches.yaml | 5 ++ shotover-proxy/benches/windsock/cassandra.rs | 2 +- shotover-proxy/benches/windsock/kafka.rs | 2 +- shotover-proxy/benches/windsock/profilers.rs | 29 +++++++- .../benches/windsock/profilers/samply.rs | 67 +++++++++++++++++++ shotover-proxy/benches/windsock/redis.rs | 2 +- 6 files changed, 101 insertions(+), 6 deletions(-) create mode 100644 shotover-proxy/benches/windsock/profilers/samply.rs diff --git a/.github/workflows/windsock_benches.yaml b/.github/workflows/windsock_benches.yaml index 12f91af3d..2586e0ba1 100644 --- a/.github/workflows/windsock_benches.yaml +++ b/.github/workflows/windsock_benches.yaml @@ -32,8 +32,13 @@ jobs: save-if: ${{ github.ref == 'refs/heads/main' }} - name: Ensure that custom benches run run: | + # This isnt needed locally because we run profilers via sudo. + # But for some reason on CI even with sudo we didnt have access to perf events. + echo '1' | sudo tee /proc/sys/kernel/perf_event_paranoid + # run some extra cases that arent handled by nextest cargo windsock --bench-length-seconds 5 --operations-per-second 100 --profilers flamegraph --name cassandra,compression=none,driver=scylla,operation=read_i64,protocol=v4,shotover=standard,topology=single + cargo windsock --bench-length-seconds 5 --operations-per-second 100 --profilers samply --name cassandra,compression=none,driver=scylla,operation=read_i64,protocol=v4,shotover=standard,topology=single cargo windsock --bench-length-seconds 5 --operations-per-second 100 --profilers sys_monitor --name kafka,shotover=standard,size=1B,topology=single # windsock/examples/cassandra.rs - this can stay here until windsock is moved to its own repo diff --git a/shotover-proxy/benches/windsock/cassandra.rs b/shotover-proxy/benches/windsock/cassandra.rs index 052bf1cf2..3da61a33f 100644 --- a/shotover-proxy/benches/windsock/cassandra.rs +++ b/shotover-proxy/benches/windsock/cassandra.rs @@ -559,7 +559,7 @@ impl Bench for CassandraBench { Shotover::None => None, Shotover::ForcedMessageParsed => todo!(), }; - profiler.run(&shotover); + profiler.run(&shotover).await; self.execute_run(address, ¶meters).await; diff --git a/shotover-proxy/benches/windsock/kafka.rs b/shotover-proxy/benches/windsock/kafka.rs index 1d1e3d4d3..9f9826527 100644 --- a/shotover-proxy/benches/windsock/kafka.rs +++ b/shotover-proxy/benches/windsock/kafka.rs @@ -139,7 +139,7 @@ impl Bench for KafkaBench { Shotover::None => "127.0.0.1:9092", }; - profiler.run(&shotover); + profiler.run(&shotover).await; self.execute_run(broker_address, ¶meters).await; diff --git a/shotover-proxy/benches/windsock/profilers.rs b/shotover-proxy/benches/windsock/profilers.rs index 0f45d5592..78d685c68 100644 --- a/shotover-proxy/benches/windsock/profilers.rs +++ b/shotover-proxy/benches/windsock/profilers.rs @@ -1,3 +1,4 @@ +use self::samply::Samply; use crate::common::Shotover; use aws_throwaway::Ec2Instance; use std::{collections::HashMap, path::PathBuf}; @@ -5,14 +6,17 @@ use test_helpers::{flamegraph::Perf, shotover_process::BinProcess}; use tokio::sync::mpsc::UnboundedReceiver; use windsock::Profiling; +mod samply; mod sar; pub struct ProfilerRunner { bench_name: String, run_flamegraph: bool, + run_samply: bool, run_sys_monitor: bool, results_path: PathBuf, perf: Option, + samply: Option, sys_monitor: Option>, } @@ -21,6 +25,7 @@ impl ProfilerRunner { let run_flamegraph = profiling .profilers_to_use .contains(&"flamegraph".to_owned()); + let run_samply = profiling.profilers_to_use.contains(&"samply".to_owned()); let run_sys_monitor = profiling .profilers_to_use .contains(&"sys_monitor".to_owned()); @@ -29,13 +34,15 @@ impl ProfilerRunner { bench_name, run_flamegraph, run_sys_monitor, + run_samply, results_path: profiling.results_path, perf: None, + samply: None, sys_monitor: None, } } - pub fn run(&mut self, shotover: &Option) { + pub async fn run(&mut self, shotover: &Option) { self.perf = if self.run_flamegraph { if let Some(shotover) = &shotover { Some(Perf::new( @@ -48,6 +55,15 @@ impl ProfilerRunner { } else { None }; + self.samply = if self.run_samply { + if let Some(shotover) = &shotover { + Some(Samply::run(self.results_path.clone(), shotover.child().id().unwrap()).await) + } else { + panic!("samply not supported when benching without shotover") + } + } else { + None + }; self.sys_monitor = if self.run_sys_monitor { Some(sar::run_sar_local()) } else { @@ -56,7 +72,7 @@ impl ProfilerRunner { } pub fn shotover_profile(&self) -> Option<&'static str> { - if self.run_flamegraph { + if self.run_flamegraph || self.run_samply { Some("profiling") } else { None @@ -69,6 +85,9 @@ impl Drop for ProfilerRunner { if let Some(perf) = self.perf.take() { perf.flamegraph(); } + if let Some(samply) = self.samply.take() { + samply.wait(); + } if let Some(mut rx) = self.sys_monitor.take() { sar::insert_sar_results_to_bench_archive(&self.bench_name, "", sar::parse_sar(&mut rx)); } @@ -118,6 +137,10 @@ pub fn supported_profilers(shotover: Shotover) -> Vec { if let Shotover::None = shotover { vec!["sys_monitor".to_owned()] } else { - vec!["flamegraph".to_owned(), "sys_monitor".to_owned()] + vec![ + "flamegraph".to_owned(), + "samply".to_owned(), + "sys_monitor".to_owned(), + ] } } diff --git a/shotover-proxy/benches/windsock/profilers/samply.rs b/shotover-proxy/benches/windsock/profilers/samply.rs new file mode 100644 index 000000000..28d7cad24 --- /dev/null +++ b/shotover-proxy/benches/windsock/profilers/samply.rs @@ -0,0 +1,67 @@ +use std::path::PathBuf; +use tokio::task::JoinHandle; + +pub struct Samply(JoinHandle<()>); + +impl Samply { + pub async fn run(results_path: PathBuf, pid: u32) -> Samply { + run_command( + "cargo", + &[ + "install", + "samply", + "--git", + "https://github.com/rukai/samply", + "--rev", + "af14e56ac9e809a445e4f0bd80fcd4fefc45b552", + ], + ) + .await; + let output_file = results_path.join("samply.json"); + let home = std::env::var("HOME").unwrap(); + + // Run `sudo ls` so that we can get sudo to stop asking for password for a while. + // It also lets us block until the user has entered the password, otherwise the rest of the test would continue before `perf` has started. + // TODO: It would be more robust to get a single root prompt and then keep feeding commands into it. + // 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; + })) + } + + pub fn wait(self) { + futures::executor::block_on(self.0).unwrap(); + } +} + +async fn run_command(command: &str, args: &[&str]) -> String { + let output = tokio::process::Command::new(command) + .args(args) + .output() + .await + .unwrap(); + + let stdout = String::from_utf8(output.stdout).unwrap(); + let stderr = String::from_utf8(output.stderr).unwrap(); + if !output.status.success() { + panic!("command {command} {args:?} failed:\nstdout:\n{stdout}\nstderr:\n{stderr}") + } + stdout +} diff --git a/shotover-proxy/benches/windsock/redis.rs b/shotover-proxy/benches/windsock/redis.rs index e7e6dc8b8..404e5af3c 100644 --- a/shotover-proxy/benches/windsock/redis.rs +++ b/shotover-proxy/benches/windsock/redis.rs @@ -212,7 +212,7 @@ impl Bench for RedisBench { ), Shotover::None => None, }; - profiler.run(&shotover); + profiler.run(&shotover).await; self.execute_run(address, ¶meters).await;