From a4f7a3198756af453a657db7aa44ae0d38e0e6d9 Mon Sep 17 00:00:00 2001 From: Antoine Tenart Date: Wed, 3 Jul 2024 09:15:04 +0200 Subject: [PATCH 1/6] tools: benchmark: enable parallel build and be less verbose There's no reason to enable verbose build while running the benchmarking scripts: if there's a build issue one can run `make bench` manually. Also make the build use more than a single CPU to speed up things. Signed-off-by: Antoine Tenart --- tools/benchmark_event_output.sh | 2 +- tools/benchmark_event_parsing.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/benchmark_event_output.sh b/tools/benchmark_event_output.sh index 7fe6802fd..77b19181e 100755 --- a/tools/benchmark_event_output.sh +++ b/tools/benchmark_event_output.sh @@ -1,6 +1,6 @@ #!/bin/bash -make bench V=1 +make bench -j$(nproc) time_single_single=0 time_single_multi=0 diff --git a/tools/benchmark_event_parsing.sh b/tools/benchmark_event_parsing.sh index d83a5756f..ddffb4be7 100755 --- a/tools/benchmark_event_parsing.sh +++ b/tools/benchmark_event_parsing.sh @@ -1,6 +1,6 @@ #!/bin/bash -make bench V=1 +make bench -j$(nproc) time_first=0 time_1m=0 From b969edc0a5f19502bdede7ded6e9278eaa599df0 Mon Sep 17 00:00:00 2001 From: Antoine Tenart Date: Tue, 2 Jul 2024 18:01:40 +0200 Subject: [PATCH 2/6] events: use event_fmt in the core event output logic Using `display()` is meant for external users. The event and its sub-sections (& types) can directly access the internal `event_fmt` helper. Signed-off-by: Antoine Tenart --- retis-events/src/events.rs | 42 +++++++++++++++++--------------- retis-events/src/skb_tracking.rs | 3 ++- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/retis-events/src/events.rs b/retis-events/src/events.rs index a33b060df..abc8f4d6d 100644 --- a/retis-events/src/events.rs +++ b/retis-events/src/events.rs @@ -124,48 +124,52 @@ impl EventFmt for Event { fn event_fmt(&self, f: &mut std::fmt::Formatter, format: &DisplayFormat) -> std::fmt::Result { // First format the first event line starting with the always-there // {common} section, followed by the {kernel} or {user} one. - write!( - f, - "{}", - self.0.get(&SectionId::Common).unwrap().display(format) - )?; + self.0 + .get(&SectionId::Common) + .unwrap() + .event_fmt(f, format)?; if let Some(kernel) = self.0.get(&SectionId::Kernel) { - write!(f, " {}", kernel.display(format))?; + write!(f, " ")?; + kernel.event_fmt(f, format)?; } else if let Some(user) = self.0.get(&SectionId::Userspace) { - write!(f, " {}", user.display(format))?; + write!(f, " ")?; + user.event_fmt(f, format)?; } // If we do have tracking and/or drop sections, put them there too. // Special case the global tracking information from here for now. if let Some(tracking) = self.0.get(&SectionId::Tracking) { - write!(f, " {}", tracking.display(format))?; + write!(f, " ")?; + tracking.event_fmt(f, format)?; } else if let Some(skb_tracking) = self.0.get(&SectionId::SkbTracking) { - write!(f, " {}", skb_tracking.display(format))?; + write!(f, " ")?; + skb_tracking.event_fmt(f, format)?; } if let Some(skb_drop) = self.0.get(&SectionId::SkbDrop) { - write!(f, " {}", skb_drop.display(format))?; + write!(f, " ")?; + skb_drop.event_fmt(f, format)?; } + // Separator between each following sections. + let sep = if format.multiline { "\n " } else { " " }; + // If we have a stack trace, show it. if let Some(kernel) = self.get_section::(SectionId::Kernel) { if let Some(stack) = &kernel.stack_trace { - write!( - f, - "{}{}", - if format.multiline { '\n' } else { ' ' }, - stack.display(format) - )?; + write!(f, "{sep}")?; + stack.event_fmt(f, format)?; } } - let sep = if format.multiline { "\n " } else { " " }; - // Finally show all sections. (SectionId::Skb.to_u8()..SectionId::_MAX.to_u8()) .collect::>() .iter() .filter_map(|id| self.0.get(&SectionId::from_u8(*id).unwrap())) - .try_for_each(|section| write!(f, "{sep}{}", section.display(format)))?; + .try_for_each(|section| { + write!(f, "{sep}")?; + section.event_fmt(f, format) + })?; Ok(()) } diff --git a/retis-events/src/skb_tracking.rs b/retis-events/src/skb_tracking.rs index 4dfb9d33c..178725b3f 100644 --- a/retis-events/src/skb_tracking.rs +++ b/retis-events/src/skb_tracking.rs @@ -86,7 +86,8 @@ impl Ord for TrackingInfo { impl EventFmt for TrackingInfo { fn event_fmt(&self, f: &mut fmt::Formatter, format: &DisplayFormat) -> fmt::Result { - write!(f, "{} n {}", self.skb.display(format), self.idx) + self.skb.event_fmt(f, format)?; + write!(f, " n {}", self.idx) } } From 5226eeae41215b0b6360fb803a0cad94740b6598 Mon Sep 17 00:00:00 2001 From: Antoine Tenart Date: Tue, 2 Jul 2024 18:08:57 +0200 Subject: [PATCH 3/6] events: kernel: do not hardcode indent level for stack traces Signed-off-by: Antoine Tenart --- retis-events/src/kernel.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/retis-events/src/kernel.rs b/retis-events/src/kernel.rs index ff345e6a8..623923342 100644 --- a/retis-events/src/kernel.rs +++ b/retis-events/src/kernel.rs @@ -46,7 +46,7 @@ impl EventFmt for StackTrace { let last = self.0.len() - 1; if format.multiline { self.0.iter().enumerate().try_for_each(|(i, sym)| { - write!(f, " {sym}")?; + write!(f, "{sym}")?; if i != last { writeln!(f)?; } From b04d3a4c24056236335009b96e75fb1bc7f0655a Mon Sep 17 00:00:00 2001 From: Antoine Tenart Date: Tue, 2 Jul 2024 18:09:46 +0200 Subject: [PATCH 4/6] events: skb: avoid allocating strings in event_fmt Signed-off-by: Antoine Tenart --- retis-events/src/skb.rs | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/retis-events/src/skb.rs b/retis-events/src/skb.rs index c749e3f01..53beee188 100644 --- a/retis-events/src/skb.rs +++ b/retis-events/src/skb.rs @@ -65,16 +65,11 @@ impl EventFmt for SkbEvent { if let Some(eth) = &self.eth { space.write(f)?; - let ethertype = match etype_str(eth.etype) { - Some(s) => format!(" {}", s), - None => String::new(), - }; - - write!( - f, - "{} > {} ethertype{} ({:#06x})", - eth.src, eth.dst, ethertype, eth.etype - )?; + write!(f, "{} > {} ethertype", eth.src, eth.dst)?; + if let Some(etype) = etype_str(eth.etype) { + write!(f, " {etype}")?; + } + write!(f, " ({:#06x})", eth.etype)?; } if let Some(arp) = &self.arp { @@ -153,18 +148,17 @@ impl EventFmt for SkbEvent { } } - let protocol = match protocol_str(ip.protocol) { - Some(s) => format!(" {}", s), - None => String::new(), - }; - // In some rare cases the IP header might not be fully filled yet, // length might be unset. if ip.len != 0 { write!(f, " len {}", ip.len)?; } - write!(f, " proto{} ({})", protocol, ip.protocol)?; + if let Some(proto) = protocol_str(ip.protocol) { + write!(f, " proto {}", proto)?; + } + + write!(f, " ({})", ip.protocol)?; } if let Some(tcp) = &self.tcp { From d5350df262241ea1c9b1e6c29534763f5465ccac Mon Sep 17 00:00:00 2001 From: Antoine Tenart Date: Tue, 2 Jul 2024 18:13:10 +0200 Subject: [PATCH 5/6] events: add our own formatter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add our own formatter and an associated configuration struct. The scope of a configuration object is expected to be within a single `display()` call. Using our own formatter provides better flexibility over how we print our events and better performances. Flexibility improvements: - Indentation is handled at the formatter level. Thanks to this sub-sections and event types can now print multi-line events. - Delimiter can be used while printing parts of an event. Performances improvements: - An internal buffer is used which helps reducing write calls as events usually have many write!() call in sequences. - This buffer is also used to only apply the formatting logic (eg. indentation) once in a while, reducing the costs. Before the series, 1M_time_single_single: 988812µs 1M_time_single_multi: 995100µs 1M_time_series_single: 1043459µs 1M_time_series_multi: 1039526µs After the series, 1M_time_single_single: 842520µs 1M_time_single_multi: 877022µs 1M_time_series_single: 971998µs 1M_time_series_multi: 997519µs Also the benchmark does not use stdout() which has a lock, so benefits should be slightly better. Signed-off-by: Antoine Tenart --- retis-events/src/common.rs | 6 +- retis-events/src/ct.rs | 6 +- retis-events/src/display.rs | 201 +++++++++++++++++++++++++++++-- retis-events/src/events.rs | 9 +- retis-events/src/kernel.rs | 6 +- retis-events/src/nft.rs | 4 +- retis-events/src/ovs.rs | 18 +-- retis-events/src/skb.rs | 4 +- retis-events/src/skb_drop.rs | 4 +- retis-events/src/skb_tracking.rs | 6 +- retis-events/src/user.rs | 4 +- retis/src/collect/collector.rs | 2 +- retis/src/core/events/bpf.rs | 8 +- 13 files changed, 228 insertions(+), 50 deletions(-) diff --git a/retis-events/src/common.rs b/retis-events/src/common.rs index 58e92b680..1cc8067da 100644 --- a/retis-events/src/common.rs +++ b/retis-events/src/common.rs @@ -2,7 +2,7 @@ use std::fmt; use chrono::{DateTime, Utc}; -use crate::{event_section, event_type, *}; +use crate::*; /// Startup event section. Contains global information about a collection as a /// whole, with data gathered at collection startup time. @@ -15,7 +15,7 @@ pub struct StartupEvent { } impl EventFmt for StartupEvent { - fn event_fmt(&self, f: &mut fmt::Formatter, _: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, _: &DisplayFormat) -> fmt::Result { write!(f, "Retis version {}", self.retis_version) } } @@ -44,7 +44,7 @@ pub struct CommonEvent { } impl EventFmt for CommonEvent { - fn event_fmt(&self, f: &mut fmt::Formatter, format: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, format: &DisplayFormat) -> fmt::Result { match format.time_format { TimeFormat::MonotonicTimestamp => write!(f, "{}", self.timestamp)?, TimeFormat::UtcDate => match format.monotonic_offset { diff --git a/retis-events/src/ct.rs b/retis-events/src/ct.rs index ea64a00d8..3b878e8ca 100644 --- a/retis-events/src/ct.rs +++ b/retis-events/src/ct.rs @@ -1,7 +1,7 @@ use std::fmt; use super::*; -use crate::{event_section, event_type}; +use crate::{event_section, event_type, Formatter}; #[event_type] #[derive(Default)] @@ -128,7 +128,7 @@ pub struct CtConnEvent { } impl EventFmt for CtEvent { - fn event_fmt(&self, f: &mut fmt::Formatter, _: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, _: &DisplayFormat) -> fmt::Result { use CtState::*; match self.state { Established => write!(f, "ct_state ESTABLISHED ")?, @@ -152,7 +152,7 @@ impl EventFmt for CtEvent { } impl CtEvent { - fn format_conn(conn: &CtConnEvent, f: &mut fmt::Formatter) -> fmt::Result { + fn format_conn(conn: &CtConnEvent, f: &mut Formatter) -> fmt::Result { match (&conn.orig.proto, &conn.reply.proto) { (CtProto::Tcp(tcp_orig), CtProto::Tcp(tcp_reply)) => { write!( diff --git a/retis-events/src/display.rs b/retis-events/src/display.rs index ab564d1c7..577453214 100644 --- a/retis-events/src/display.rs +++ b/retis-events/src/display.rs @@ -1,4 +1,9 @@ -use std::fmt; +use std::{ + fmt::{self, Write}, + result, str, +}; + +use log::warn; use super::TimeSpec; @@ -45,6 +50,165 @@ impl DisplayFormat { } } +/// `Formatter` implements `std::fmt::Write` and controls how events are being +/// displayed. This is similar to `std::fmt::Formatter` but with our own +/// constraints. +/// +/// It supports the following capabilities: indentation and itemization. Each of +/// those are always context-based: the capabilities and their configuration can +/// change over time and might end based on input (eg. itemization). +pub struct Formatter<'a, 'inner> { + inner: &'a mut fmt::Formatter<'inner>, + pub conf: FormatterConf, + /// Indentation level (in spaces). + level: usize, + /// True if the next input is the start of a block (aka. first call to + /// `flush_buf`). + first: bool, + /// True if the next input is the start of a line. + start: bool, + /// Buffer holding the output before being flushed. + buf: String, +} + +impl<'a, 'inner> Formatter<'a, 'inner> { + pub fn new( + inner: &'a mut fmt::Formatter<'inner>, + conf: FormatterConf, + ) -> Formatter<'a, 'inner> { + let level = conf.level; + + Self { + inner, + conf, + level, + first: true, + start: true, + buf: String::with_capacity(4096usize), + } + } + + /// Directly implement write_fmt to avoid the need of an explicit + /// `use fmt::Write` by every user. See the `std::write` documentation. + #[inline] + pub fn write_fmt(&mut self, args: fmt::Arguments<'_>) -> result::Result<(), fmt::Error> { + ::write_fmt(self, args) + } + + pub fn flush_buf(&mut self) -> result::Result<(), fmt::Error> { + let first = self.first; + match self.buf.is_empty() { + true => return Ok(()), + false => self.first = false, + } + + let mut lines = self.buf.split('\n'); + + // Compute the prefix including the itemization char, if any. + let mut prefix = " ".repeat(self.level); + if first && self.level >= 2 { + if let Some(item) = self.conf.item { + prefix.replace_range(self.level - 2..self.level - 1, &item.to_string()); + } + } + + if let Some(line) = lines.next() { + if self.start { + self.start = false; + self.inner.write_str(&prefix)?; + } + self.inner.write_str(line)?; + } + + // Reset the itemization char, if any. + if first && self.level >= 2 && self.conf.item.is_some() { + prefix = " ".repeat(self.level); + } + + lines.try_for_each(|line| { + self.inner.write_char('\n')?; + self.inner.write_str(&prefix)?; + self.inner.write_str(line) + })?; + + if self.buf.ends_with('\n') { + self.inner.write_char('\n')?; + self.start = true; + } + + self.buf.clear(); + Ok(()) + } +} + +impl fmt::Write for Formatter<'_, '_> { + fn write_str(&mut self, s: &str) -> result::Result<(), fmt::Error> { + if self.conf.level != self.level { + if !self.buf.is_empty() { + self.flush_buf()?; + } + self.level = self.conf.level; + } + + self.buf.push_str(s); + Ok(()) + } +} + +impl Drop for Formatter<'_, '_> { + fn drop(&mut self) { + if !self.buf.is_empty() { + self.flush_buf().expect("Could not flush Formatter buffer"); + } + } +} + +/// Configuration for the `Formatter`. It can be shared between multiple +/// `EventDisplay::display` calls but its scope is restricted to a single call. +/// This means a base configuration can be shared for multiple +/// `EventDisplay::display` call but any modification made within an +/// `EventDisplay::display` call won't be visibile outside. +#[derive(Clone, Default)] +pub struct FormatterConf { + level: usize, + saved_levels: Vec, + item: Option, +} + +impl FormatterConf { + pub fn new() -> Self { + Self::with_level(0) + } + + pub fn with_level(level: usize) -> Self { + Self { + level, + ..Default::default() + } + } + + /// Increase the indentation level by `diff`. + pub fn inc_level(&mut self, diff: usize) { + self.saved_levels.push(self.level); + self.level += diff; + } + + /// Reset the indentation level to its previous value. + pub fn reset_level(&mut self) { + match self.saved_levels.pop() { + Some(level) => { + self.level = level; + } + None => warn!("Cannot reset the indentation level"), + } + } + + /// Set an itemization char to be printed at the start of output, or None. + pub fn set_item(&mut self, item: Option) { + self.item = item; + } +} + /// Trait controlling how an event or an event section (or any custom type /// inside it) is displayed. It works by providing an helper returning an /// implementation of the std::fmt::Display trait, which can be used later to @@ -52,7 +216,11 @@ impl DisplayFormat { /// arguments, unlike a plain std::fmt::Display implementation. pub trait EventDisplay<'a>: EventFmt { /// Display the event using the default event format. - fn display(&'a self, format: &'a DisplayFormat) -> Box; + fn display( + &'a self, + format: &'a DisplayFormat, + conf: &'a FormatterConf, + ) -> Box; } /// Trait controlling how an event or an event section (or any custom type @@ -64,26 +232,33 @@ pub trait EventDisplay<'a>: EventFmt { /// members if any. pub trait EventFmt { /// Default formatting of an event. - fn event_fmt(&self, f: &mut fmt::Formatter, format: &DisplayFormat) -> fmt::Result; + fn event_fmt(&self, f: &mut Formatter, format: &DisplayFormat) -> fmt::Result; } impl<'a, T> EventDisplay<'a> for T where T: EventFmt, { - fn display(&'a self, format: &'a DisplayFormat) -> Box { + fn display( + &'a self, + format: &'a DisplayFormat, + conf: &'a FormatterConf, + ) -> Box { struct DefaultDisplay<'a, U> { myself: &'a U, format: &'a DisplayFormat, + conf: &'a FormatterConf, } impl fmt::Display for DefaultDisplay<'_, U> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - self.myself.event_fmt(f, self.format) + self.myself + .event_fmt(&mut Formatter::new(f, self.conf.clone()), self.format) } } Box::new(DefaultDisplay { myself: self, format, + conf, }) } } @@ -95,7 +270,7 @@ where /// /// ``` /// use std::fmt; -/// use retis_events::DelimWriter; +/// use retis_events::{Formatter, FormatterConf, DelimWriter}; /// /// struct Flags { /// opt1: bool, @@ -103,15 +278,17 @@ where /// } /// impl fmt::Display for Flags { /// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { -/// write!(f, "flags")?; +/// let mut f = Formatter::new(f, FormatterConf::new()); +/// +/// write!(&mut f, "flags")?; /// let mut space = DelimWriter::new(' '); /// if self.opt1 { -/// space.write(f)?; -/// write!(f, "opt1"); +/// space.write(&mut f)?; +/// write!(&mut f, "opt1"); /// } /// if self.opt2 { -/// space.write(f)?; -/// write!(f, "opt2")?; +/// space.write(&mut f)?; +/// write!(&mut f, "opt2")?; /// } /// Ok(()) /// } @@ -129,7 +306,7 @@ impl DelimWriter { } /// If it's not the first time it's called, write the delimiter. - pub fn write(&mut self, f: &mut fmt::Formatter) -> fmt::Result { + pub fn write(&mut self, f: &mut Formatter) -> fmt::Result { match self.first { true => self.first = false, false => write!(f, "{}", self.delim)?, diff --git a/retis-events/src/events.rs b/retis-events/src/events.rs index abc8f4d6d..cc1ccfeb7 100644 --- a/retis-events/src/events.rs +++ b/retis-events/src/events.rs @@ -121,7 +121,7 @@ impl Event { } impl EventFmt for Event { - fn event_fmt(&self, f: &mut std::fmt::Formatter, format: &DisplayFormat) -> std::fmt::Result { + fn event_fmt(&self, f: &mut Formatter, format: &DisplayFormat) -> std::fmt::Result { // First format the first event line starting with the always-there // {common} section, followed by the {kernel} or {user} one. self.0 @@ -151,16 +151,20 @@ impl EventFmt for Event { } // Separator between each following sections. - let sep = if format.multiline { "\n " } else { " " }; + let sep = if format.multiline { '\n' } else { ' ' }; // If we have a stack trace, show it. if let Some(kernel) = self.get_section::(SectionId::Kernel) { if let Some(stack) = &kernel.stack_trace { + f.conf.inc_level(4); write!(f, "{sep}")?; stack.event_fmt(f, format)?; + f.conf.reset_level(); } } + f.conf.inc_level(2); + // Finally show all sections. (SectionId::Skb.to_u8()..SectionId::_MAX.to_u8()) .collect::>() @@ -171,6 +175,7 @@ impl EventFmt for Event { section.event_fmt(f, format) })?; + f.conf.reset_level(); Ok(()) } } diff --git a/retis-events/src/kernel.rs b/retis-events/src/kernel.rs index 623923342..59df5690b 100644 --- a/retis-events/src/kernel.rs +++ b/retis-events/src/kernel.rs @@ -1,7 +1,7 @@ use std::fmt; use super::*; -use crate::{event_section, event_type}; +use crate::{event_section, event_type, Formatter}; #[event_section("kernel")] pub struct KernelEvent { @@ -14,7 +14,7 @@ pub struct KernelEvent { } impl EventFmt for KernelEvent { - fn event_fmt(&self, f: &mut fmt::Formatter, _: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, _: &DisplayFormat) -> fmt::Result { write!( f, "[{}] {}", @@ -42,7 +42,7 @@ impl StackTrace { } impl EventFmt for StackTrace { - fn event_fmt(&self, f: &mut fmt::Formatter, format: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, format: &DisplayFormat) -> fmt::Result { let last = self.0.len() - 1; if format.multiline { self.0.iter().enumerate().try_for_each(|(i, sym)| { diff --git a/retis-events/src/nft.rs b/retis-events/src/nft.rs index 21e31b5f5..bbc2d8cfd 100644 --- a/retis-events/src/nft.rs +++ b/retis-events/src/nft.rs @@ -1,7 +1,7 @@ use std::{fmt, str}; use super::*; -use crate::event_section; +use crate::{event_section, Formatter}; /// Nft event section #[event_section("nft")] @@ -17,7 +17,7 @@ pub struct NftEvent { } impl EventFmt for NftEvent { - fn event_fmt(&self, f: &mut fmt::Formatter, _: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, _: &DisplayFormat) -> fmt::Result { write!( f, "table {} ({}) chain {} ({})", diff --git a/retis-events/src/ovs.rs b/retis-events/src/ovs.rs index c90e92abf..f7d348889 100644 --- a/retis-events/src/ovs.rs +++ b/retis-events/src/ovs.rs @@ -4,7 +4,7 @@ use anyhow::{bail, Result}; use serde::{de::Error as Derror, ser::Error as Serror, Deserialize, Deserializer, Serializer}; use super::*; -use crate::{event_section, event_type}; +use crate::{event_section, event_type, Formatter}; ///The OVS Event #[derive(PartialEq)] @@ -16,7 +16,7 @@ pub struct OvsEvent { } impl EventFmt for OvsEvent { - fn event_fmt(&self, f: &mut fmt::Formatter, format: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, format: &DisplayFormat) -> fmt::Result { self.event.event_fmt(f, format) } } @@ -58,7 +58,7 @@ pub enum OvsEventType { } impl EventFmt for OvsEventType { - fn event_fmt(&self, f: &mut fmt::Formatter, format: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, format: &DisplayFormat) -> fmt::Result { use OvsEventType::*; let disp: &dyn EventFmt = match self { Upcall(e) => e, @@ -102,7 +102,7 @@ pub struct UpcallEvent { } impl EventFmt for UpcallEvent { - fn event_fmt(&self, f: &mut fmt::Formatter, _: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, _: &DisplayFormat) -> fmt::Result { write!( f, "upcall{} port {} cpu {}", @@ -134,7 +134,7 @@ pub struct UpcallEnqueueEvent { } impl EventFmt for UpcallEnqueueEvent { - fn event_fmt(&self, f: &mut fmt::Formatter, _: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, _: &DisplayFormat) -> fmt::Result { write!( f, "upcall_enqueue{} ({}/{}) q {} ret {}", @@ -158,7 +158,7 @@ pub struct UpcallReturnEvent { } impl EventFmt for UpcallReturnEvent { - fn event_fmt(&self, f: &mut fmt::Formatter, _: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, _: &DisplayFormat) -> fmt::Result { write!( f, "upcall_ret ({}/{}) ret {}", @@ -218,7 +218,7 @@ impl OperationEvent { } impl EventFmt for OperationEvent { - fn event_fmt(&self, f: &mut fmt::Formatter, _: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, _: &DisplayFormat) -> fmt::Result { write!( f, "flow_{} q {} ts {} ({})", @@ -250,7 +250,7 @@ pub struct RecvUpcallEvent { } impl EventFmt for RecvUpcallEvent { - fn event_fmt(&self, f: &mut fmt::Formatter, _: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, _: &DisplayFormat) -> fmt::Result { // FIXME: there are more fields. write!( f, @@ -276,7 +276,7 @@ pub struct ActionEvent { } impl EventFmt for ActionEvent { - fn event_fmt(&self, f: &mut fmt::Formatter, _: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, _: &DisplayFormat) -> fmt::Result { if self.recirc_id != 0 { write!(f, "[recirc_id {:#x}] ", self.recirc_id)?; } diff --git a/retis-events/src/skb.rs b/retis-events/src/skb.rs index 53beee188..55e5f83d8 100644 --- a/retis-events/src/skb.rs +++ b/retis-events/src/skb.rs @@ -4,7 +4,7 @@ use super::{ net::{etype_str, protocol_str, RawPacket}, *, }; -use crate::{event_section, event_type}; +use crate::{event_section, event_type, Formatter}; /// Skb event section. #[event_section("skb")] @@ -38,7 +38,7 @@ pub struct SkbEvent { } impl EventFmt for SkbEvent { - fn event_fmt(&self, f: &mut fmt::Formatter, _: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, _: &DisplayFormat) -> fmt::Result { let mut len = 0; let mut space = DelimWriter::new(' '); diff --git a/retis-events/src/skb_drop.rs b/retis-events/src/skb_drop.rs index 97f61ad8d..ebdd89e22 100644 --- a/retis-events/src/skb_drop.rs +++ b/retis-events/src/skb_drop.rs @@ -1,7 +1,7 @@ use std::fmt; use super::*; -use crate::event_section; +use crate::{event_section, Formatter}; /// Skb drop event section. #[event_section("skb-drop")] @@ -14,7 +14,7 @@ pub struct SkbDropEvent { } impl EventFmt for SkbDropEvent { - fn event_fmt(&self, f: &mut fmt::Formatter, _: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, _: &DisplayFormat) -> fmt::Result { match &self.subsys { None => write!(f, "drop (reason {})", self.drop_reason), Some(name) => write!(f, "drop (reason {name}/{})", self.drop_reason), diff --git a/retis-events/src/skb_tracking.rs b/retis-events/src/skb_tracking.rs index 178725b3f..78ad8dca9 100644 --- a/retis-events/src/skb_tracking.rs +++ b/retis-events/src/skb_tracking.rs @@ -6,7 +6,7 @@ use std::{ }; use super::*; -use crate::event_section; +use crate::{event_section, Formatter}; /// Tracking event section. /// For more information of how the tracking logic is designed and how it can be @@ -47,7 +47,7 @@ impl SkbTrackingEvent { } impl EventFmt for SkbTrackingEvent { - fn event_fmt(&self, f: &mut fmt::Formatter, _: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, _: &DisplayFormat) -> fmt::Result { write!(f, "#{:x} (skb {:x})", self.tracking_id(), self.skb) } } @@ -85,7 +85,7 @@ impl Ord for TrackingInfo { } impl EventFmt for TrackingInfo { - fn event_fmt(&self, f: &mut fmt::Formatter, format: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, format: &DisplayFormat) -> fmt::Result { self.skb.event_fmt(f, format)?; write!(f, " n {}", self.idx) } diff --git a/retis-events/src/user.rs b/retis-events/src/user.rs index e2de5ad55..da2c094eb 100644 --- a/retis-events/src/user.rs +++ b/retis-events/src/user.rs @@ -1,7 +1,7 @@ use std::fmt; use super::*; -use crate::event_section; +use crate::{event_section, Formatter}; #[event_section("userspace")] pub struct UserEvent { @@ -21,7 +21,7 @@ pub struct UserEvent { } impl EventFmt for UserEvent { - fn event_fmt(&self, f: &mut fmt::Formatter, _: &DisplayFormat) -> fmt::Result { + fn event_fmt(&self, f: &mut Formatter, _: &DisplayFormat) -> fmt::Result { write!(f, "[u] {}", self.symbol)?; if let Some((_, bin)) = self.path.rsplit_once('/') { write!(f, " ({})", bin)?; diff --git a/retis/src/collect/collector.rs b/retis/src/collect/collector.rs index e132a260c..906a1d62b 100644 --- a/retis/src/collect/collector.rs +++ b/retis/src/collect/collector.rs @@ -618,7 +618,7 @@ mod tests { struct TestEvent {} impl EventFmt for TestEvent { - fn event_fmt(&self, f: &mut std::fmt::Formatter, _: &DisplayFormat) -> std::fmt::Result { + fn event_fmt(&self, f: &mut Formatter, _: &DisplayFormat) -> std::fmt::Result { write!(f, "test event section") } } diff --git a/retis/src/core/events/bpf.rs b/retis/src/core/events/bpf.rs index 4bd659df3..0f63e58de 100644 --- a/retis/src/core/events/bpf.rs +++ b/retis/src/core/events/bpf.rs @@ -17,11 +17,7 @@ use anyhow::{anyhow, bail, Result}; use log::{error, log, Level}; use plain::Plain; -use crate::{ - events::{CommonEvent, TaskEvent, *}, - helpers::signals::Running, - raw_event_section, -}; +use crate::{events::*, helpers::signals::Running, raw_event_section}; /// Raw event sections for common. pub(super) const COMMON_SECTION_CORE: u64 = 0; @@ -623,7 +619,7 @@ mod tests { } impl EventFmt for TestEvent { - fn event_fmt(&self, f: &mut std::fmt::Formatter, _: &DisplayFormat) -> std::fmt::Result { + fn event_fmt(&self, f: &mut Formatter, _: &DisplayFormat) -> std::fmt::Result { write!( f, "field0: {:?} field1: {:?} field2: {:?}", From 44eace34c7bde47dfeded91c2d1f76d2b6f81a69 Mon Sep 17 00:00:00 2001 From: Antoine Tenart Date: Tue, 2 Jul 2024 18:20:06 +0200 Subject: [PATCH 6/6] process: display: convert to using our own formatter Signed-off-by: Antoine Tenart --- retis/src/process/display.rs | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/retis/src/process/display.rs b/retis/src/process/display.rs index 05cfce5d5..25db81bac 100644 --- a/retis/src/process/display.rs +++ b/retis/src/process/display.rs @@ -33,11 +33,13 @@ impl PrintEvent { format.monotonic_offset = Some(common.clock_monotonic_offset); } - let event = format!("{}", e.display(format)); + let mut event = format!("{}", e.display(format, &FormatterConf::new())); if !event.is_empty() { + event.push('\n'); + if format.multiline { + event.push('\n'); + } self.writer.write_all(event.as_bytes())?; - self.writer - .write_all(if format.multiline { b"\n\n" } else { b"\n" })?; } } PrintEventFormat::Json => { @@ -67,36 +69,27 @@ impl PrintSeries { Self { writer, format } } - fn indent(n_spaces: usize, lines: String) -> String { - if n_spaces == 0 || lines.is_empty() { - return lines; - } - - let mut res = Vec::new(); - let mut delim = "↳ "; - for line in lines.split('\n') { - res.push(format!("{}{}{}", " ".repeat(n_spaces), delim, line)); - delim = " "; - } - - res.join("\n") - } - /// Process events one by one (format & print). pub(crate) fn process_one(&mut self, series: &EventSeries) -> Result<()> { let mut content = String::new(); match self.format { PrintEventFormat::Text(ref mut format) => { - let mut indent = 0; + let mut fconf = FormatterConf::new(); + let mut first = true; + for event in series.events.iter() { if let Some(common) = event.get_section::(SectionId::Startup) { format.monotonic_offset = Some(common.clock_monotonic_offset); } - content.push_str(&Self::indent(indent, format!("{}", event.display(format)))); + content.push_str(&format!("{}", event.display(format, &fconf))); if !content.is_empty() { content.push('\n'); - indent = 2; + if first { + first = false; + fconf.inc_level(4); + fconf.set_item(Some('↳')); + } } }