From 02c31a9f0f00c3319674d5d24db1860affc5001d Mon Sep 17 00:00:00 2001 From: Ammar Abou Zor Date: Thu, 29 Aug 2024 15:35:03 +0200 Subject: [PATCH 1/8] DLT SomeIP: Update dependencies & Fix build errors * Dependencies of DLT and SomeIP changed to Kevin forks * Code copied in the positions form Kevin's PR except section --- application/apps/indexer/Cargo.lock | 15 +++++----- application/apps/indexer/Cargo.toml | 4 ++- application/apps/indexer/parsers/Cargo.toml | 4 ++- .../apps/indexer/parsers/src/dlt/fmt.rs | 29 +++++++++++++++++++ application/apps/indexer/session/Cargo.toml | 4 ++- 5 files changed, 45 insertions(+), 11 deletions(-) diff --git a/application/apps/indexer/Cargo.lock b/application/apps/indexer/Cargo.lock index e7cbfaebfa..453a5d379e 100644 --- a/application/apps/indexer/Cargo.lock +++ b/application/apps/indexer/Cargo.lock @@ -650,9 +650,8 @@ dependencies = [ [[package]] name = "dlt-core" -version = "0.14.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f6fd69a328826e883613fbbcf468a33a53df1198c3e39bae7ad079c449431bfd" +version = "0.16.0" +source = "git+https://github.com/kruss/dlt-core.git?branch=dlt_network_traces#525f591862437ca15c56639143f205a956d01b6a" dependencies = [ "buf_redux 0.8.4 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder", @@ -1686,9 +1685,9 @@ dependencies = [ [[package]] name = "quick-xml" -version = "0.22.0" +version = "0.23.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8533f14c8382aaad0d592c812ac3b826162128b65662331e1127b45c3d18536b" +checksum = "11bafc859c6815fbaffbbbf4229ecb767ac913fecb27f9ad4343662e9ef099ea" dependencies = [ "memchr", ] @@ -2085,12 +2084,12 @@ dependencies = [ [[package]] name = "someip-payload" -version = "0.1.1" -source = "git+https://github.com/esrlabs/someip-payload#9a58a561a3d284e37a25ea2dc52188c70694682d" +version = "0.1.3" +source = "git+https://github.com/kruss/someip-payload.git?branch=robustness#ccd100907b38f60d9e1ac657250111b2e0a69518" dependencies = [ "byteorder", "log", - "quick-xml 0.22.0", + "quick-xml 0.23.1", "regex", "thiserror", "ux", diff --git a/application/apps/indexer/Cargo.toml b/application/apps/indexer/Cargo.toml index ea5581935d..3998b4b67a 100644 --- a/application/apps/indexer/Cargo.toml +++ b/application/apps/indexer/Cargo.toml @@ -22,7 +22,9 @@ thiserror = "1.0" lazy_static = "1.4" tokio = { version = "1", features = ["full"] } tokio-stream = "0.1" -dlt-core = "0.14" +# dlt-core = "0.14" +# TODO https://github.com/esrlabs/dlt-core/pull/24 +dlt-core = { git = "https://github.com/kruss/dlt-core.git", branch = "dlt_network_traces" } crossbeam-channel = "0.5" futures = "0.3" tokio-util = "0.7" diff --git a/application/apps/indexer/parsers/Cargo.toml b/application/apps/indexer/parsers/Cargo.toml index c2ac11d3ac..8645323e88 100644 --- a/application/apps/indexer/parsers/Cargo.toml +++ b/application/apps/indexer/parsers/Cargo.toml @@ -18,7 +18,9 @@ rand.workspace = true # someip-messages = { path = "../../../../../someip"} someip-messages = { git = "https://github.com/esrlabs/someip" } # someip-payload = { path = "../../../../../someip-payload" } -someip-payload = { git = "https://github.com/esrlabs/someip-payload" } +# someip-payload = { git = "https://github.com/esrlabs/someip-payload" } +# TODO +someip-payload = { git = "https://github.com/kruss/someip-payload.git", branch = "robustness" } [dev-dependencies] stringreader = "0.1.1" diff --git a/application/apps/indexer/parsers/src/dlt/fmt.rs b/application/apps/indexer/parsers/src/dlt/fmt.rs index 52941eee2b..5da5c3af5f 100644 --- a/application/apps/indexer/parsers/src/dlt/fmt.rs +++ b/application/apps/indexer/parsers/src/dlt/fmt.rs @@ -281,6 +281,17 @@ impl<'a> Serialize for FormattableMessage<'a> { None => state.serialize_field("payload", "[Unknown CtrlCommand]")?, } } + PayloadContent::NetworkTrace(slices) => { + state.serialize_field("app-id", &ext_header_app_id)?; + state.serialize_field("context-id", &ext_header_context_id)?; + state.serialize_field("message-type", &ext_header_msg_type)?; + let arg_string = slices + .iter() + .map(|slice| format!("{:02X?}", slice)) + .collect::>() + .join("|"); + state.serialize_field("payload", &arg_string)?; + } } state.end() } @@ -386,6 +397,19 @@ impl<'a> FormattableMessage<'a> { payload_string, )) } + PayloadContent::NetworkTrace(slices) => { + let payload_string = slices + .iter() + .map(|slice| format!("{:02X?}", slice)) + .collect::>() + .join("|"); + Ok(PrintableMessage::new( + ext_h_app_id, + eh_ctx_id, + ext_h_msg_type, + payload_string, + )) + } } } @@ -559,6 +583,11 @@ impl<'a> fmt::Display for FormattableMessage<'a> { None => write!(f, "[Unknown CtrlCommand]"), } } + PayloadContent::NetworkTrace(slices) => { + self.write_app_id_context_id_and_message_type(f)?; + //TODO AAZ: Return error here with the needed bytes + todo!(); + } } } } diff --git a/application/apps/indexer/session/Cargo.toml b/application/apps/indexer/session/Cargo.toml index 803f87bd4c..8ae61602b4 100644 --- a/application/apps/indexer/session/Cargo.toml +++ b/application/apps/indexer/session/Cargo.toml @@ -8,7 +8,9 @@ edition = "2021" blake3 = "1.3" crossbeam-channel.workspace = true dirs.workspace = true -dlt-core.workspace = true +# dlt-core.workspace = true +# TODO https://github.com/esrlabs/dlt-core/pull/24 +dlt-core = { git = "https://github.com/kruss/dlt-core.git", branch = "dlt_network_traces", features=["statistics"] } envvars = "0.1" file-tools = { path = "../addons/file-tools" } futures.workspace = true From 43f217c191ca3dc3544e51cc743b1988e8f356c2 Mon Sep 17 00:00:00 2001 From: Ammar Abou Zor Date: Thu, 29 Aug 2024 18:04:01 +0200 Subject: [PATCH 2/8] DLT SomeIP: Implement Parse Log Message can fail. * Replace `Display` trait bound with `to_text()` method which can fail * Add check for trait to explicitly tell if their parsing can fail at compile time. * Reaction on these Errors still missing in processors. --- .../indexer/indexer_cli/src/interactive.rs | 27 +++++- .../apps/indexer/parsers/src/dlt/fmt.rs | 88 +++++++++++++++---- .../apps/indexer/parsers/src/dlt/mod.rs | 21 +++-- application/apps/indexer/parsers/src/lib.rs | 74 +++++++++++++++- .../apps/indexer/parsers/src/someip.rs | 8 +- application/apps/indexer/parsers/src/text.rs | 8 +- .../session/src/handlers/observing/mod.rs | 25 +++++- 7 files changed, 215 insertions(+), 36 deletions(-) diff --git a/application/apps/indexer/indexer_cli/src/interactive.rs b/application/apps/indexer/indexer_cli/src/interactive.rs index a12215d399..505e3e6f11 100644 --- a/application/apps/indexer/indexer_cli/src/interactive.rs +++ b/application/apps/indexer/indexer_cli/src/interactive.rs @@ -1,6 +1,6 @@ use crate::{duration_report, Instant}; use futures::{pin_mut, stream::StreamExt}; -use parsers::{dlt::DltParser, MessageStreamItem, ParseYield}; +use parsers::{dlt::DltParser, LogMessage, MessageStreamItem, ParseYield}; use processor::grabber::LineRange; use rustyline::{error::ReadlineError, DefaultEditor}; use session::session::Session; @@ -56,10 +56,12 @@ pub(crate) async fn handle_interactive_session(input: Option) { } item = msg_stream.next() => { match item { - Some((_, MessageStreamItem::Item(ParseYield::Message(msg)))) => { + Some((_, MessageStreamItem::Item(ParseYield::Message(item)))) => { + let msg = get_msg_todo(item); println!("msg: {msg}"); } - Some((_, MessageStreamItem::Item(ParseYield::MessageAndAttachment((msg, attachment))))) => { + Some((_, MessageStreamItem::Item(ParseYield::MessageAndAttachment((item, attachment))))) => { + let msg = get_msg_todo(item); println!("msg: {msg}, attachment: {attachment:?}"); } Some((_, MessageStreamItem::Item(ParseYield::Attachment(attachment)))) => { @@ -194,3 +196,22 @@ async fn collect_user_input(tx: mpsc::UnboundedSender) -> JoinHandle<() println!("done with readline loop"); }) } + +//TODO AAZ: This should handle the cases when we have nested parsers + Saving the messages into +///the faulty messages stack if the nested parsers couldn't resolve it. +pub fn get_msg_todo(item: impl LogMessage) -> String { + let text_res = item.to_text(); + if item.can_error() { + //TODO AAZ: Manage someip parser case for now + let mut msg = text_res.msg; + if let Some(err_info) = text_res.error { + msg = format!( + "{msg}: TODO: These bytes should be parsed with someip {:?}", + err_info.remain_bytes + ); + } + msg + } else { + text_res.msg + } +} diff --git a/application/apps/indexer/parsers/src/dlt/fmt.rs b/application/apps/indexer/parsers/src/dlt/fmt.rs index 5da5c3af5f..25e63aba75 100644 --- a/application/apps/indexer/parsers/src/dlt/fmt.rs +++ b/application/apps/indexer/parsers/src/dlt/fmt.rs @@ -32,6 +32,8 @@ use std::{ str, }; +use crate::{LogMessage, ParseLogError, ToTextResult}; + const DLT_COLUMN_SENTINAL: char = '\u{0004}'; const DLT_ARGUMENT_SENTINAL: char = '\u{0005}'; const DLT_NEWLINE_SENTINAL_SLICE: &[u8] = &[0x6]; @@ -415,7 +417,7 @@ impl<'a> FormattableMessage<'a> { fn write_app_id_context_id_and_message_type( &self, - f: &mut fmt::Formatter, + f: &mut impl std::fmt::Write, ) -> Result<(), fmt::Error> { match self.message.extended_header.as_ref() { Some(ext) => { @@ -443,7 +445,7 @@ impl<'a> FormattableMessage<'a> { &self, id: u32, data: &[u8], - f: &mut fmt::Formatter, + f: &mut impl std::fmt::Write, ) -> fmt::Result { trace!("format_nonverbose_data"); let mut fibex_info_added = false; @@ -535,7 +537,16 @@ impl<'a> FormattableMessage<'a> { } } -impl<'a> fmt::Display for FormattableMessage<'a> { +impl LogMessage for FormattableMessage<'_> { + const CAN_ERROR: bool = true; + + fn to_writer(&self, writer: &mut W) -> Result { + let bytes = self.message.as_bytes(); + let len = bytes.len(); + writer.write_all(&bytes)?; + Ok(len) + } + /// will format dlt Message with those fields: /// ********* storage-header ******** /// date-time @@ -552,48 +563,87 @@ impl<'a> fmt::Display for FormattableMessage<'a> { /// context-id /// /// payload - fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> { + fn to_text(&self) -> ToTextResult { + use std::fmt::Write; + + let mut msg = String::new(); + let f = &mut msg; + // unwrap is safe here because writing to a string never fails + // TODO AAZ: Change this if we want to continue with this implementation. if let Some(h) = &self.message.storage_header { let tz = self.options.map(|o| o.tz); match tz { Some(Some(tz)) => { - write_tz_string(f, &h.timestamp, &tz)?; - write!(f, "{DLT_COLUMN_SENTINAL}{}", h.ecu_id)?; + write_tz_string(f, &h.timestamp, &tz).unwrap(); + write!(f, "{DLT_COLUMN_SENTINAL}{}", h.ecu_id).unwrap(); } - _ => write!(f, "{}", DltStorageHeader(h))?, + _ => write!(f, "{}", DltStorageHeader(h)).unwrap(), }; } let header = DltStandardHeader(&self.message.header); - write!(f, "{DLT_COLUMN_SENTINAL}",)?; - write!(f, "{header}")?; - write!(f, "{DLT_COLUMN_SENTINAL}",)?; + write!(f, "{DLT_COLUMN_SENTINAL}",).unwrap(); + write!(f, "{header}").unwrap(); + write!(f, "{DLT_COLUMN_SENTINAL}",).unwrap(); match &self.message.payload { PayloadContent::Verbose(arguments) => { - self.write_app_id_context_id_and_message_type(f)?; + self.write_app_id_context_id_and_message_type(f).unwrap(); arguments .iter() .try_for_each(|arg| write!(f, "{}{}", DLT_ARGUMENT_SENTINAL, DltArgument(arg))) + .unwrap(); + } + PayloadContent::NonVerbose(id, data) => { + self.format_nonverbose_data(*id, data, f).unwrap(); } - PayloadContent::NonVerbose(id, data) => self.format_nonverbose_data(*id, data, f), PayloadContent::ControlMsg(ctrl_id, _data) => { - self.write_app_id_context_id_and_message_type(f)?; + self.write_app_id_context_id_and_message_type(f).unwrap(); match service_id_lookup(ctrl_id.value()) { - Some((name, _desc)) => write!(f, "[{name}]"), - None => write!(f, "[Unknown CtrlCommand]"), + Some((name, _desc)) => { + write!(f, "[{name}]").unwrap(); + } + None => { + write!(f, "[Unknown CtrlCommand]").unwrap(); + } } } PayloadContent::NetworkTrace(slices) => { - self.write_app_id_context_id_and_message_type(f)?; - //TODO AAZ: Return error here with the needed bytes - todo!(); + self.write_app_id_context_id_and_message_type(f).unwrap(); + let is_someip = self + .message + .extended_header + .as_ref() + .is_some_and(|ext_header| { + matches!( + ext_header.message_type, + MessageType::NetworkTrace(NetworkTraceType::Ipc) + | MessageType::NetworkTrace(NetworkTraceType::Someip) + ) + }); + + if is_someip { + if let Some(slice) = slices.get(1) { + //TODO AAZ: Introduce solve error hint enum that have someip in it. + let err = ParseLogError::new( + slice.to_owned(), + crate::ParseErrorType::Other("Need Some IP".into()), + ); + return ToTextResult::new(msg, Some(err)); + } + } + + slices.iter().for_each(|slice| { + write!(f, "{}{:02X?}", DLT_ARGUMENT_SENTINAL, slice).unwrap() + }); } } + + return msg.into(); } } fn write_tz_string( - f: &mut Formatter, + f: &mut impl std::fmt::Write, time_stamp: &DltTimeStamp, tz: &Tz, ) -> Result<(), fmt::Error> { diff --git a/application/apps/indexer/parsers/src/dlt/mod.rs b/application/apps/indexer/parsers/src/dlt/mod.rs index dae82f4ce4..34f8990841 100644 --- a/application/apps/indexer/parsers/src/dlt/mod.rs +++ b/application/apps/indexer/parsers/src/dlt/mod.rs @@ -17,15 +17,6 @@ use std::{io::Write, ops::Range}; use self::{attachment::FtScanner, fmt::FormatOptions}; -impl LogMessage for FormattableMessage<'_> { - fn to_writer(&self, writer: &mut W) -> Result { - let bytes = self.message.as_bytes(); - let len = bytes.len(); - writer.write_all(&bytes)?; - Ok(len) - } -} - #[derive(Debug, Serialize)] pub struct RawMessage { pub content: Vec, @@ -48,20 +39,32 @@ impl std::fmt::Display for RawMessage { } impl LogMessage for RangeMessage { + const CAN_ERROR: bool = false; + /// A RangeMessage only has range information and cannot serialize to bytes fn to_writer(&self, writer: &mut W) -> Result { writer.write_u64::(self.range.start as u64)?; writer.write_u64::(self.range.end as u64)?; Ok(8 + 8) } + + fn to_text(&self) -> crate::ToTextResult { + self.into() + } } impl LogMessage for RawMessage { + const CAN_ERROR: bool = false; + fn to_writer(&self, writer: &mut W) -> Result { let len = self.content.len(); writer.write_all(&self.content)?; Ok(len) } + + fn to_text(&self) -> crate::ToTextResult { + self.into() + } } #[derive(Default)] diff --git a/application/apps/indexer/parsers/src/lib.rs b/application/apps/indexer/parsers/src/lib.rs index 1b7fce8efe..68f1fb2b37 100644 --- a/application/apps/indexer/parsers/src/lib.rs +++ b/application/apps/indexer/parsers/src/lib.rs @@ -78,10 +78,82 @@ pub enum ByteRepresentation { Range((usize, usize)), } -pub trait LogMessage: Display + Serialize { +#[derive(Debug, Clone)] +pub enum ParseErrorType { + Fmt(String), + Other(String), +} + +#[derive(Debug, Clone)] +pub struct ParseLogError { + pub remain_bytes: Vec, + pub error_type: ParseErrorType, +} + +impl ParseLogError { + pub fn new(remain_bytes: Vec, error_type: ParseErrorType) -> Self { + Self { + remain_bytes, + error_type, + } + } +} + +impl Display for ParseLogError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match &self.error_type { + ParseErrorType::Other(msg) | ParseErrorType::Fmt(msg) => write!(f, "{msg}"), + } + } +} + +impl From for ParseLogError { + fn from(value: std::fmt::Error) -> Self { + Self { + remain_bytes: Vec::new(), + error_type: ParseErrorType::Fmt(value.to_string()), + } + } +} + +impl std::error::Error for ParseLogError {} + +#[derive(Debug, Clone)] +pub struct ToTextResult { + pub msg: String, + pub error: Option, +} + +impl ToTextResult { + pub fn new(msg: String, error: Option) -> Self { + Self { msg, error } + } +} + +impl From for ToTextResult +where + T: Display, +{ + fn from(value: T) -> Self { + Self::new(value.to_string(), None) + } +} + +pub trait LogMessage: Serialize { + /// Indicates that parsing this struct to text can error. + const CAN_ERROR: bool; + + //TODO AAZ: Measure this an remove if rust already optimize the code without it. + fn can_error(&self) -> bool { + ::CAN_ERROR + } + /// Serializes a message directly into a Writer /// returns the size of the serialized message fn to_writer(&self, writer: &mut W) -> Result; + + /// Get the text representation of this message. + fn to_text(&self) -> ToTextResult; } #[derive(Debug)] diff --git a/application/apps/indexer/parsers/src/someip.rs b/application/apps/indexer/parsers/src/someip.rs index 1601017d78..438c383b6c 100644 --- a/application/apps/indexer/parsers/src/someip.rs +++ b/application/apps/indexer/parsers/src/someip.rs @@ -1,4 +1,4 @@ -use crate::{Error, LogMessage, ParseYield, Parser}; +use crate::{Error, LogMessage, ParseYield, Parser, ToTextResult}; use std::{borrow::Cow, fmt, fmt::Display, io::Write, path::PathBuf}; use someip_messages::*; @@ -325,10 +325,16 @@ impl SomeipLogMessage { } impl LogMessage for SomeipLogMessage { + const CAN_ERROR: bool = false; + fn to_writer(&self, writer: &mut W) -> Result { writer.write_all(&self.bytes)?; Ok(self.bytes.len()) } + + fn to_text(&self) -> ToTextResult { + self.into() + } } impl Display for SomeipLogMessage { diff --git a/application/apps/indexer/parsers/src/text.rs b/application/apps/indexer/parsers/src/text.rs index 6a09a52b9c..59f3880237 100644 --- a/application/apps/indexer/parsers/src/text.rs +++ b/application/apps/indexer/parsers/src/text.rs @@ -1,4 +1,4 @@ -use crate::{Error, LogMessage, ParseYield, Parser}; +use crate::{Error, LogMessage, ParseYield, Parser, ToTextResult}; use serde::Serialize; use std::{fmt, io::Write}; @@ -16,11 +16,17 @@ impl fmt::Display for StringMessage { } impl LogMessage for StringMessage { + const CAN_ERROR: bool = false; + fn to_writer(&self, writer: &mut W) -> Result { let len = self.content.len(); writer.write_all(self.content.as_bytes())?; Ok(len) } + + fn to_text(&self) -> ToTextResult { + self.into() + } } impl Parser for StringTokenizer diff --git a/application/apps/indexer/session/src/handlers/observing/mod.rs b/application/apps/indexer/session/src/handlers/observing/mod.rs index 48d8b1a250..bf531dcd86 100644 --- a/application/apps/indexer/session/src/handlers/observing/mod.rs +++ b/application/apps/indexer/session/src/handlers/observing/mod.rs @@ -139,16 +139,18 @@ async fn run_producer, S: ByteSource>( Next::Item(item) => { match item { MessageStreamItem::Item(ParseYield::Message(item)) => { + let msg = get_msg_todo(item); state - .write_session_file(source_id, format!("{item}\n")) + .write_session_file(source_id, format!("{msg}\n")) .await?; } MessageStreamItem::Item(ParseYield::MessageAndAttachment(( item, attachment, ))) => { + let msg = get_msg_todo(item); state - .write_session_file(source_id, format!("{item}\n")) + .write_session_file(source_id, format!("{msg}\n")) .await?; state.add_attachment(attachment)?; } @@ -202,3 +204,22 @@ async fn run_producer, S: ByteSource>( debug!("listen done"); Ok(None) } + +///TODO AAZ: This should handle the cases when we have nested parsers + Saving the messages into +///the faulty messages stack if the nested parsers couldn't resolve it. +pub fn get_msg_todo(item: impl LogMessage) -> String { + let text_res = item.to_text(); + if item.can_error() { + //TODO AAZ: Manage someip parser case for now + let mut msg = text_res.msg; + if let Some(err_info) = text_res.error { + msg = format!( + "{msg}: TODO: These bytes should be parsed with someip {:?}", + err_info.remain_bytes + ); + } + msg + } else { + text_res.msg + } +} From e621efd31a96ac60efc2627d9f51dc1f44f491b3 Mon Sep 17 00:00:00 2001 From: Ammar Abou Zor Date: Fri, 30 Aug 2024 12:09:30 +0200 Subject: [PATCH 3/8] DLT SomeIP: Introduce error hint --- application/apps/indexer/parsers/src/dlt/fmt.rs | 4 ++-- application/apps/indexer/parsers/src/lib.rs | 16 +++++++++++++++- .../session/src/handlers/observing/mod.rs | 2 +- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/application/apps/indexer/parsers/src/dlt/fmt.rs b/application/apps/indexer/parsers/src/dlt/fmt.rs index 25e63aba75..924bdca588 100644 --- a/application/apps/indexer/parsers/src/dlt/fmt.rs +++ b/application/apps/indexer/parsers/src/dlt/fmt.rs @@ -32,7 +32,7 @@ use std::{ str, }; -use crate::{LogMessage, ParseLogError, ToTextResult}; +use crate::{LogMessage, ParseLogError, ResolveErrorHint, ToTextResult}; const DLT_COLUMN_SENTINAL: char = '\u{0004}'; const DLT_ARGUMENT_SENTINAL: char = '\u{0005}'; @@ -623,10 +623,10 @@ impl LogMessage for FormattableMessage<'_> { if is_someip { if let Some(slice) = slices.get(1) { - //TODO AAZ: Introduce solve error hint enum that have someip in it. let err = ParseLogError::new( slice.to_owned(), crate::ParseErrorType::Other("Need Some IP".into()), + Some(ResolveErrorHint::SomeIP), ); return ToTextResult::new(msg, Some(err)); } diff --git a/application/apps/indexer/parsers/src/lib.rs b/application/apps/indexer/parsers/src/lib.rs index 68f1fb2b37..9fa77a5949 100644 --- a/application/apps/indexer/parsers/src/lib.rs +++ b/application/apps/indexer/parsers/src/lib.rs @@ -84,17 +84,30 @@ pub enum ParseErrorType { Other(String), } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +/// Gives Hint about how this error can be resolved by processor +pub enum ResolveErrorHint { + /// The message needs to be parsed with SomeIP Parser. + SomeIP, +} + #[derive(Debug, Clone)] pub struct ParseLogError { pub remain_bytes: Vec, pub error_type: ParseErrorType, + pub resolve_hint: Option, } impl ParseLogError { - pub fn new(remain_bytes: Vec, error_type: ParseErrorType) -> Self { + pub fn new( + remain_bytes: Vec, + error_type: ParseErrorType, + resolve_hint: Option, + ) -> Self { Self { remain_bytes, error_type, + resolve_hint, } } } @@ -112,6 +125,7 @@ impl From for ParseLogError { Self { remain_bytes: Vec::new(), error_type: ParseErrorType::Fmt(value.to_string()), + resolve_hint: None, } } } diff --git a/application/apps/indexer/session/src/handlers/observing/mod.rs b/application/apps/indexer/session/src/handlers/observing/mod.rs index bf531dcd86..ca959999f3 100644 --- a/application/apps/indexer/session/src/handlers/observing/mod.rs +++ b/application/apps/indexer/session/src/handlers/observing/mod.rs @@ -1,4 +1,4 @@ -use std::path::PathBuf; +use std::{collections::HashMap, path::PathBuf}; use crate::{ operations::{OperationAPI, OperationResult}, From d6cfc004c45b17aa8e910327706e2f3d1120f4ac Mon Sep 17 00:00:00 2001 From: Ammar Abou Zor Date: Fri, 30 Aug 2024 14:05:55 +0200 Subject: [PATCH 4/8] DLT SomeIP: Resolve parsing Error in sessions * Implement Error Resolver on sessions to try parsing failed parse items depending on ErrorHint from the error * Saving unresolved to a store isn't implemented. * Add inline attribute to can_error to make sure the compiler will figure out this condition at compile time. --- application/apps/indexer/parsers/src/lib.rs | 1 + .../apps/indexer/session/src/handlers/mod.rs | 1 + .../session/src/handlers/observing/mod.rs | 68 +++++++++++++++---- .../indexer/session/src/handlers/parse_err.rs | 42 ++++++++++++ .../apps/rustcore/rs-bindings/Cargo.lock | 15 ++-- 5 files changed, 105 insertions(+), 22 deletions(-) create mode 100644 application/apps/indexer/session/src/handlers/parse_err.rs diff --git a/application/apps/indexer/parsers/src/lib.rs b/application/apps/indexer/parsers/src/lib.rs index 9fa77a5949..6e3267afa2 100644 --- a/application/apps/indexer/parsers/src/lib.rs +++ b/application/apps/indexer/parsers/src/lib.rs @@ -158,6 +158,7 @@ pub trait LogMessage: Serialize { const CAN_ERROR: bool; //TODO AAZ: Measure this an remove if rust already optimize the code without it. + #[inline(always)] fn can_error(&self) -> bool { ::CAN_ERROR } diff --git a/application/apps/indexer/session/src/handlers/mod.rs b/application/apps/indexer/session/src/handlers/mod.rs index f0301268e7..b08527557d 100644 --- a/application/apps/indexer/session/src/handlers/mod.rs +++ b/application/apps/indexer/session/src/handlers/mod.rs @@ -2,6 +2,7 @@ pub mod export_raw; pub mod extract; pub mod observe; mod observing; +pub mod parse_err; pub mod search; pub mod search_values; pub mod sleep; diff --git a/application/apps/indexer/session/src/handlers/observing/mod.rs b/application/apps/indexer/session/src/handlers/observing/mod.rs index ca959999f3..55e1a6ec92 100644 --- a/application/apps/indexer/session/src/handlers/observing/mod.rs +++ b/application/apps/indexer/session/src/handlers/observing/mod.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, path::PathBuf}; +use std::path::PathBuf; use crate::{ operations::{OperationAPI, OperationResult}, @@ -24,6 +24,8 @@ use tokio::{ }; use tokio_stream::StreamExt; +use super::parse_err::ParseErrorReslover; + enum Next { Item(MessageStreamItem), Timeout, @@ -77,6 +79,7 @@ async fn run_source_intern( rx_sde: Option, rx_tail: Option>>, ) -> OperationResult<()> { + let mut parse_err_resolver = ParseErrorReslover::new(); match parser { ParserType::SomeIp(settings) => { let someip_parser = match &settings.fibex_file_paths { @@ -86,11 +89,27 @@ async fn run_source_intern( None => SomeipParser::new(), }; let producer = MessageProducer::new(someip_parser, source, rx_sde); - run_producer(operation_api, state, source_id, producer, rx_tail).await + run_producer( + operation_api, + state, + source_id, + producer, + rx_tail, + &mut parse_err_resolver, + ) + .await } ParserType::Text => { let producer = MessageProducer::new(StringTokenizer {}, source, rx_sde); - run_producer(operation_api, state, source_id, producer, rx_tail).await + run_producer( + operation_api, + state, + source_id, + producer, + rx_tail, + &mut parse_err_resolver, + ) + .await } ParserType::Dlt(settings) => { let fmt_options = Some(FormatOptions::from(settings.tz.as_ref())); @@ -101,7 +120,22 @@ async fn run_source_intern( settings.with_storage_header, ); let producer = MessageProducer::new(dlt_parser, source, rx_sde); - run_producer(operation_api, state, source_id, producer, rx_tail).await + let someip_parse = match &settings.fibex_file_paths { + Some(paths) => { + SomeipParser::from_fibex_files(paths.into_iter().map(PathBuf::from).collect()) + } + None => SomeipParser::new(), + }; + parse_err_resolver.with_someip_parser(someip_parse); + run_producer( + operation_api, + state, + source_id, + producer, + rx_tail, + &mut parse_err_resolver, + ) + .await } } } @@ -112,6 +146,7 @@ async fn run_producer, S: ByteSource>( source_id: u16, mut producer: MessageProducer, mut rx_tail: Option>>, + parse_err_resolver: &mut ParseErrorReslover, ) -> OperationResult<()> { use log::debug; state.set_session_file(None).await?; @@ -139,7 +174,7 @@ async fn run_producer, S: ByteSource>( Next::Item(item) => { match item { MessageStreamItem::Item(ParseYield::Message(item)) => { - let msg = get_msg_todo(item); + let msg = get_log_text(item, parse_err_resolver); state .write_session_file(source_id, format!("{msg}\n")) .await?; @@ -148,7 +183,7 @@ async fn run_producer, S: ByteSource>( item, attachment, ))) => { - let msg = get_msg_todo(item); + let msg = get_log_text(item, parse_err_resolver); state .write_session_file(source_id, format!("{msg}\n")) .await?; @@ -205,18 +240,23 @@ async fn run_producer, S: ByteSource>( Ok(None) } -///TODO AAZ: This should handle the cases when we have nested parsers + Saving the messages into -///the faulty messages stack if the nested parsers couldn't resolve it. -pub fn get_msg_todo(item: impl LogMessage) -> String { +/// Get the text message of [`LogMessage`], resolving parse text errors if possible, +/// TODO: Otherwise it should save the error to the faulty messages store, which need to be +/// implemented as well :) +pub fn get_log_text(item: impl LogMessage, err_resolver: &mut ParseErrorReslover) -> String { let text_res = item.to_text(); if item.can_error() { - //TODO AAZ: Manage someip parser case for now let mut msg = text_res.msg; if let Some(err_info) = text_res.error { - msg = format!( - "{msg}: TODO: These bytes should be parsed with someip {:?}", - err_info.remain_bytes - ); + match err_resolver.resolve_err(&err_info) { + Some(resloved_msg) => { + msg.push_str(&resloved_msg); + } + None => { + //TODO: Item with error details should be reported faulty messages store. + msg = format!("{msg}: Unknow Error bytes: {:?}", err_info.remain_bytes); + } + } } msg } else { diff --git a/application/apps/indexer/session/src/handlers/parse_err.rs b/application/apps/indexer/session/src/handlers/parse_err.rs new file mode 100644 index 0000000000..1bf4100fbd --- /dev/null +++ b/application/apps/indexer/session/src/handlers/parse_err.rs @@ -0,0 +1,42 @@ +use parsers::{someip::SomeipParser, LogMessage, ParseLogError, Parser}; + +#[derive(Default)] +pub struct ParseErrorReslover { + someip_praser: Option, +} + +impl ParseErrorReslover { + pub fn new() -> Self { + Self::default() + } + + /// Sets SomeIP parser on the resolver + pub fn with_someip_parser(&mut self, someip_praser: SomeipParser) -> &mut Self { + self.someip_praser = Some(someip_praser); + self + } + + /// Tries to resolve the given error returning the parsed string if succeeded. + pub fn resolve_err(&mut self, error: &ParseLogError) -> Option { + match error.resolve_hint { + Some(parsers::ResolveErrorHint::SomeIP) => self + .someip_praser + .as_mut() + .and_then(|parser| { + parser + .parse(&error.remain_bytes, None) + .ok() + .and_then(|res| res.1) + }) + .and_then(|a| match a { + // TODO: Handle someip parser errors after prototyping. + parsers::ParseYield::Message(msg) => Some(msg.to_text().msg), + parsers::ParseYield::Attachment(_) => None, + parsers::ParseYield::MessageAndAttachment((msg, _att)) => { + Some(msg.to_text().msg) + } + }), + None => None, + } + } +} diff --git a/application/apps/rustcore/rs-bindings/Cargo.lock b/application/apps/rustcore/rs-bindings/Cargo.lock index 56cf3e3c13..a7559efd54 100644 --- a/application/apps/rustcore/rs-bindings/Cargo.lock +++ b/application/apps/rustcore/rs-bindings/Cargo.lock @@ -698,9 +698,8 @@ dependencies = [ [[package]] name = "dlt-core" -version = "0.14.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f6fd69a328826e883613fbbcf468a33a53df1198c3e39bae7ad079c449431bfd" +version = "0.16.0" +source = "git+https://github.com/kruss/dlt-core.git?branch=dlt_network_traces#525f591862437ca15c56639143f205a956d01b6a" dependencies = [ "buf_redux 0.8.4 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder", @@ -2026,9 +2025,9 @@ dependencies = [ [[package]] name = "quick-xml" -version = "0.22.0" +version = "0.23.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8533f14c8382aaad0d592c812ac3b826162128b65662331e1127b45c3d18536b" +checksum = "11bafc859c6815fbaffbbbf4229ecb767ac913fecb27f9ad4343662e9ef099ea" dependencies = [ "memchr", ] @@ -2476,12 +2475,12 @@ dependencies = [ [[package]] name = "someip-payload" -version = "0.1.1" -source = "git+https://github.com/esrlabs/someip-payload#9a58a561a3d284e37a25ea2dc52188c70694682d" +version = "0.1.3" +source = "git+https://github.com/kruss/someip-payload.git?branch=robustness#ccd100907b38f60d9e1ac657250111b2e0a69518" dependencies = [ "byteorder", "log", - "quick-xml 0.22.0", + "quick-xml 0.23.1", "regex", "thiserror", "ux", From 29bca92c3cdc3d69cd033de480c1bc67c3a35f62 Mon Sep 17 00:00:00 2001 From: Ammar Abou Zor Date: Fri, 30 Aug 2024 16:01:34 +0200 Subject: [PATCH 5/8] DLT SomeIP: Handling formatting Errors & Clippy * Add more comment to the section where we are writing to a string about error handling + ignore the error instead of panicking. * Clippy fixes --- .../apps/indexer/parsers/src/dlt/fmt.rs | 48 +++++++++---------- .../session/src/handlers/observing/mod.rs | 2 +- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/application/apps/indexer/parsers/src/dlt/fmt.rs b/application/apps/indexer/parsers/src/dlt/fmt.rs index 924bdca588..a9c26dd948 100644 --- a/application/apps/indexer/parsers/src/dlt/fmt.rs +++ b/application/apps/indexer/parsers/src/dlt/fmt.rs @@ -28,7 +28,7 @@ use log::trace; use serde::ser::{Serialize, SerializeStruct, Serializer}; use std::{ - fmt::{self, Formatter}, + fmt::{self, Formatter, Write}, str, }; @@ -564,51 +564,51 @@ impl LogMessage for FormattableMessage<'_> { /// /// payload fn to_text(&self) -> ToTextResult { - use std::fmt::Write; - let mut msg = String::new(); - let f = &mut msg; - // unwrap is safe here because writing to a string never fails - // TODO AAZ: Change this if we want to continue with this implementation. + // Taken from Documentation: string formatting is considered an infallible operation. + // Thus we can ignore `fmt::Error` errors. + // Link from Clippy: 'https://rust-lang.github.io/rust-clippy/master/index.html#/format_push_string' + // TODO: Consider another way of concatenating the string after prototyping. if let Some(h) = &self.message.storage_header { let tz = self.options.map(|o| o.tz); match tz { Some(Some(tz)) => { - write_tz_string(f, &h.timestamp, &tz).unwrap(); - write!(f, "{DLT_COLUMN_SENTINAL}{}", h.ecu_id).unwrap(); + let _ = write_tz_string(&mut msg, &h.timestamp, &tz); + let _ = write!(msg, "{DLT_COLUMN_SENTINAL}{}", h.ecu_id); + } + _ => { + let _ = write!(msg, "{}", DltStorageHeader(h)); } - _ => write!(f, "{}", DltStorageHeader(h)).unwrap(), }; } let header = DltStandardHeader(&self.message.header); - write!(f, "{DLT_COLUMN_SENTINAL}",).unwrap(); - write!(f, "{header}").unwrap(); - write!(f, "{DLT_COLUMN_SENTINAL}",).unwrap(); + write!(msg, "{DLT_COLUMN_SENTINAL}",).unwrap(); + write!(msg, "{header}").unwrap(); + write!(msg, "{DLT_COLUMN_SENTINAL}",).unwrap(); match &self.message.payload { PayloadContent::Verbose(arguments) => { - self.write_app_id_context_id_and_message_type(f).unwrap(); - arguments - .iter() - .try_for_each(|arg| write!(f, "{}{}", DLT_ARGUMENT_SENTINAL, DltArgument(arg))) - .unwrap(); + let _ = self.write_app_id_context_id_and_message_type(&mut msg); + arguments.iter().for_each(|arg| { + let _ = write!(msg, "{}{}", DLT_ARGUMENT_SENTINAL, DltArgument(arg)); + }); } PayloadContent::NonVerbose(id, data) => { - self.format_nonverbose_data(*id, data, f).unwrap(); + let _ = self.format_nonverbose_data(*id, data, &mut msg); } PayloadContent::ControlMsg(ctrl_id, _data) => { - self.write_app_id_context_id_and_message_type(f).unwrap(); + let _ = self.write_app_id_context_id_and_message_type(&mut msg); match service_id_lookup(ctrl_id.value()) { Some((name, _desc)) => { - write!(f, "[{name}]").unwrap(); + let _ = write!(msg, "[{name}]"); } None => { - write!(f, "[Unknown CtrlCommand]").unwrap(); + let _ = write!(msg, "[Unknown CtrlCommand]"); } } } PayloadContent::NetworkTrace(slices) => { - self.write_app_id_context_id_and_message_type(f).unwrap(); + let _ = self.write_app_id_context_id_and_message_type(&mut msg); let is_someip = self .message .extended_header @@ -633,12 +633,12 @@ impl LogMessage for FormattableMessage<'_> { } slices.iter().for_each(|slice| { - write!(f, "{}{:02X?}", DLT_ARGUMENT_SENTINAL, slice).unwrap() + let _ = write!(msg, "{}{:02X?}", DLT_ARGUMENT_SENTINAL, slice); }); } } - return msg.into(); + msg.into() } } diff --git a/application/apps/indexer/session/src/handlers/observing/mod.rs b/application/apps/indexer/session/src/handlers/observing/mod.rs index 55e1a6ec92..a216b07594 100644 --- a/application/apps/indexer/session/src/handlers/observing/mod.rs +++ b/application/apps/indexer/session/src/handlers/observing/mod.rs @@ -122,7 +122,7 @@ async fn run_source_intern( let producer = MessageProducer::new(dlt_parser, source, rx_sde); let someip_parse = match &settings.fibex_file_paths { Some(paths) => { - SomeipParser::from_fibex_files(paths.into_iter().map(PathBuf::from).collect()) + SomeipParser::from_fibex_files(paths.iter().map(PathBuf::from).collect()) } None => SomeipParser::new(), }; From 46ba96ebbde101bd438a3974ba882aaa297c0ce3 Mon Sep 17 00:00:00 2001 From: Ammar Abou Zor Date: Fri, 30 Aug 2024 16:17:29 +0200 Subject: [PATCH 6/8] DLT SomeIP: Gather Error parsing in shared module Set used Error parsing functions in one module to be shared in indexer CLI too. --- .../indexer/indexer_cli/src/interactive.rs | 31 +++++-------------- .../apps/indexer/session/src/handlers/mod.rs | 1 - .../session/src/handlers/observing/mod.rs | 27 +--------------- application/apps/indexer/session/src/lib.rs | 1 + .../session/src/{handlers => }/parse_err.rs | 24 ++++++++++++++ 5 files changed, 34 insertions(+), 50 deletions(-) rename application/apps/indexer/session/src/{handlers => }/parse_err.rs (61%) diff --git a/application/apps/indexer/indexer_cli/src/interactive.rs b/application/apps/indexer/indexer_cli/src/interactive.rs index 505e3e6f11..28e2d15e24 100644 --- a/application/apps/indexer/indexer_cli/src/interactive.rs +++ b/application/apps/indexer/indexer_cli/src/interactive.rs @@ -1,9 +1,12 @@ use crate::{duration_report, Instant}; use futures::{pin_mut, stream::StreamExt}; -use parsers::{dlt::DltParser, LogMessage, MessageStreamItem, ParseYield}; +use parsers::{dlt::DltParser, MessageStreamItem, ParseYield}; use processor::grabber::LineRange; use rustyline::{error::ReadlineError, DefaultEditor}; -use session::session::Session; +use session::{ + parse_err::{get_log_text, ParseErrorReslover}, + session::Session, +}; use sources::{ factory::{DltParserSettings, FileFormat, ObserveOptions, ParserType}, producer::MessageProducer, @@ -46,6 +49,7 @@ pub(crate) async fn handle_interactive_session(input: Option) { let udp_source = UdpSource::new(RECEIVER, vec![]).await.unwrap(); let dlt_parser = DltParser::new(None, None, None, false); let mut dlt_msg_producer = MessageProducer::new(dlt_parser, udp_source, None); + let mut parse_reslover = ParseErrorReslover::new(); let msg_stream = dlt_msg_producer.as_stream(); pin_mut!(msg_stream); loop { @@ -57,11 +61,11 @@ pub(crate) async fn handle_interactive_session(input: Option) { item = msg_stream.next() => { match item { Some((_, MessageStreamItem::Item(ParseYield::Message(item)))) => { - let msg = get_msg_todo(item); + let msg = get_log_text(item, &mut parse_reslover); println!("msg: {msg}"); } Some((_, MessageStreamItem::Item(ParseYield::MessageAndAttachment((item, attachment))))) => { - let msg = get_msg_todo(item); + let msg = get_log_text(item, &mut parse_reslover); println!("msg: {msg}, attachment: {attachment:?}"); } Some((_, MessageStreamItem::Item(ParseYield::Attachment(attachment)))) => { @@ -196,22 +200,3 @@ async fn collect_user_input(tx: mpsc::UnboundedSender) -> JoinHandle<() println!("done with readline loop"); }) } - -//TODO AAZ: This should handle the cases when we have nested parsers + Saving the messages into -///the faulty messages stack if the nested parsers couldn't resolve it. -pub fn get_msg_todo(item: impl LogMessage) -> String { - let text_res = item.to_text(); - if item.can_error() { - //TODO AAZ: Manage someip parser case for now - let mut msg = text_res.msg; - if let Some(err_info) = text_res.error { - msg = format!( - "{msg}: TODO: These bytes should be parsed with someip {:?}", - err_info.remain_bytes - ); - } - msg - } else { - text_res.msg - } -} diff --git a/application/apps/indexer/session/src/handlers/mod.rs b/application/apps/indexer/session/src/handlers/mod.rs index b08527557d..f0301268e7 100644 --- a/application/apps/indexer/session/src/handlers/mod.rs +++ b/application/apps/indexer/session/src/handlers/mod.rs @@ -2,7 +2,6 @@ pub mod export_raw; pub mod extract; pub mod observe; mod observing; -pub mod parse_err; pub mod search; pub mod search_values; pub mod sleep; diff --git a/application/apps/indexer/session/src/handlers/observing/mod.rs b/application/apps/indexer/session/src/handlers/observing/mod.rs index a216b07594..95cae72988 100644 --- a/application/apps/indexer/session/src/handlers/observing/mod.rs +++ b/application/apps/indexer/session/src/handlers/observing/mod.rs @@ -2,6 +2,7 @@ use std::path::PathBuf; use crate::{ operations::{OperationAPI, OperationResult}, + parse_err::{get_log_text, ParseErrorReslover}, state::SessionStateAPI, tail, }; @@ -24,8 +25,6 @@ use tokio::{ }; use tokio_stream::StreamExt; -use super::parse_err::ParseErrorReslover; - enum Next { Item(MessageStreamItem), Timeout, @@ -239,27 +238,3 @@ async fn run_producer, S: ByteSource>( debug!("listen done"); Ok(None) } - -/// Get the text message of [`LogMessage`], resolving parse text errors if possible, -/// TODO: Otherwise it should save the error to the faulty messages store, which need to be -/// implemented as well :) -pub fn get_log_text(item: impl LogMessage, err_resolver: &mut ParseErrorReslover) -> String { - let text_res = item.to_text(); - if item.can_error() { - let mut msg = text_res.msg; - if let Some(err_info) = text_res.error { - match err_resolver.resolve_err(&err_info) { - Some(resloved_msg) => { - msg.push_str(&resloved_msg); - } - None => { - //TODO: Item with error details should be reported faulty messages store. - msg = format!("{msg}: Unknow Error bytes: {:?}", err_info.remain_bytes); - } - } - } - msg - } else { - text_res.msg - } -} diff --git a/application/apps/indexer/session/src/lib.rs b/application/apps/indexer/session/src/lib.rs index 45a70490c1..42dd87eb3c 100644 --- a/application/apps/indexer/session/src/lib.rs +++ b/application/apps/indexer/session/src/lib.rs @@ -2,6 +2,7 @@ pub mod events; mod handlers; pub mod operations; +pub mod parse_err; pub mod paths; pub mod progress; pub mod session; diff --git a/application/apps/indexer/session/src/handlers/parse_err.rs b/application/apps/indexer/session/src/parse_err.rs similarity index 61% rename from application/apps/indexer/session/src/handlers/parse_err.rs rename to application/apps/indexer/session/src/parse_err.rs index 1bf4100fbd..d413bb1121 100644 --- a/application/apps/indexer/session/src/handlers/parse_err.rs +++ b/application/apps/indexer/session/src/parse_err.rs @@ -40,3 +40,27 @@ impl ParseErrorReslover { } } } + +/// Get the text message of [`LogMessage`], resolving parse text errors if possible, +/// TODO: Otherwise it should save the error to the faulty messages store, which need to be +/// implemented as well :) +pub fn get_log_text(item: impl LogMessage, err_resolver: &mut ParseErrorReslover) -> String { + let text_res = item.to_text(); + if item.can_error() { + let mut msg = text_res.msg; + if let Some(err_info) = text_res.error { + match err_resolver.resolve_err(&err_info) { + Some(resloved_msg) => { + msg.push_str(&resloved_msg); + } + None => { + //TODO: Item with error details should be reported faulty messages store. + msg = format!("{msg}: Unknow Error bytes: {:?}", err_info.remain_bytes); + } + } + } + msg + } else { + text_res.msg + } +} From c6a7b8fa7616261c194338e905454f958e6146ae Mon Sep 17 00:00:00 2001 From: Ammar Abou Zor Date: Sat, 31 Aug 2024 09:05:14 +0200 Subject: [PATCH 7/8] DLT SomeIP: Check for constant boolean directly * Check for constant bool `CAN_ERROR` directly method without the need to an extra method to access it. * Remove the unneeded inline method --- application/apps/indexer/parsers/src/lib.rs | 7 +------ application/apps/indexer/session/src/parse_err.rs | 4 ++-- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/application/apps/indexer/parsers/src/lib.rs b/application/apps/indexer/parsers/src/lib.rs index 6e3267afa2..1d7122da9e 100644 --- a/application/apps/indexer/parsers/src/lib.rs +++ b/application/apps/indexer/parsers/src/lib.rs @@ -154,15 +154,10 @@ where } pub trait LogMessage: Serialize { + //TODO AAZ: Measure this an remove if rust already optimize the code without it. /// Indicates that parsing this struct to text can error. const CAN_ERROR: bool; - //TODO AAZ: Measure this an remove if rust already optimize the code without it. - #[inline(always)] - fn can_error(&self) -> bool { - ::CAN_ERROR - } - /// Serializes a message directly into a Writer /// returns the size of the serialized message fn to_writer(&self, writer: &mut W) -> Result; diff --git a/application/apps/indexer/session/src/parse_err.rs b/application/apps/indexer/session/src/parse_err.rs index d413bb1121..a1a9094441 100644 --- a/application/apps/indexer/session/src/parse_err.rs +++ b/application/apps/indexer/session/src/parse_err.rs @@ -44,9 +44,9 @@ impl ParseErrorReslover { /// Get the text message of [`LogMessage`], resolving parse text errors if possible, /// TODO: Otherwise it should save the error to the faulty messages store, which need to be /// implemented as well :) -pub fn get_log_text(item: impl LogMessage, err_resolver: &mut ParseErrorReslover) -> String { +pub fn get_log_text(item: T, err_resolver: &mut ParseErrorReslover) -> String { let text_res = item.to_text(); - if item.can_error() { + if T::CAN_ERROR { let mut msg = text_res.msg; if let Some(err_info) = text_res.error { match err_resolver.resolve_err(&err_info) { From 6df329991cb7554a4a150420a669b93b5caf3eab Mon Sep 17 00:00:00 2001 From: Ammar Abou Zor Date: Mon, 2 Sep 2024 13:22:52 +0200 Subject: [PATCH 8/8] DLT SomeIP: Make solution more broad supporting templating * Resolving log messages provides an enum with text or template which need to be resolved. * Sessions can try to resolve all the chunks from the templates * Faulty messages are removed for now to keep the focus here on multiple parsers. * Renaming modules, types and methods --- .../indexer/indexer_cli/src/interactive.rs | 8 +- .../apps/indexer/parsers/src/dlt/fmt.rs | 18 +- .../apps/indexer/parsers/src/dlt/mod.rs | 8 +- application/apps/indexer/parsers/src/lib.rs | 168 ++++++++++-------- .../apps/indexer/parsers/src/someip.rs | 6 +- application/apps/indexer/parsers/src/text.rs | 6 +- .../session/src/handlers/observing/mod.rs | 12 +- application/apps/indexer/session/src/lib.rs | 2 +- .../apps/indexer/session/src/parse_err.rs | 66 ------- .../session/src/parse_rest_resolver.rs | 64 +++++++ 10 files changed, 188 insertions(+), 170 deletions(-) delete mode 100644 application/apps/indexer/session/src/parse_err.rs create mode 100644 application/apps/indexer/session/src/parse_rest_resolver.rs diff --git a/application/apps/indexer/indexer_cli/src/interactive.rs b/application/apps/indexer/indexer_cli/src/interactive.rs index 28e2d15e24..49955ee6c4 100644 --- a/application/apps/indexer/indexer_cli/src/interactive.rs +++ b/application/apps/indexer/indexer_cli/src/interactive.rs @@ -4,7 +4,7 @@ use parsers::{dlt::DltParser, MessageStreamItem, ParseYield}; use processor::grabber::LineRange; use rustyline::{error::ReadlineError, DefaultEditor}; use session::{ - parse_err::{get_log_text, ParseErrorReslover}, + parse_rest_resolver::{resolve_log_msg, ParseRestReslover}, session::Session, }; use sources::{ @@ -49,7 +49,7 @@ pub(crate) async fn handle_interactive_session(input: Option) { let udp_source = UdpSource::new(RECEIVER, vec![]).await.unwrap(); let dlt_parser = DltParser::new(None, None, None, false); let mut dlt_msg_producer = MessageProducer::new(dlt_parser, udp_source, None); - let mut parse_reslover = ParseErrorReslover::new(); + let mut parse_reslover = ParseRestReslover::new(); let msg_stream = dlt_msg_producer.as_stream(); pin_mut!(msg_stream); loop { @@ -61,11 +61,11 @@ pub(crate) async fn handle_interactive_session(input: Option) { item = msg_stream.next() => { match item { Some((_, MessageStreamItem::Item(ParseYield::Message(item)))) => { - let msg = get_log_text(item, &mut parse_reslover); + let msg = resolve_log_msg(item, &mut parse_reslover); println!("msg: {msg}"); } Some((_, MessageStreamItem::Item(ParseYield::MessageAndAttachment((item, attachment))))) => { - let msg = get_log_text(item, &mut parse_reslover); + let msg = resolve_log_msg(item, &mut parse_reslover); println!("msg: {msg}, attachment: {attachment:?}"); } Some((_, MessageStreamItem::Item(ParseYield::Attachment(attachment)))) => { diff --git a/application/apps/indexer/parsers/src/dlt/fmt.rs b/application/apps/indexer/parsers/src/dlt/fmt.rs index a9c26dd948..e645debd14 100644 --- a/application/apps/indexer/parsers/src/dlt/fmt.rs +++ b/application/apps/indexer/parsers/src/dlt/fmt.rs @@ -32,7 +32,7 @@ use std::{ str, }; -use crate::{LogMessage, ParseLogError, ResolveErrorHint, ToTextResult}; +use crate::{LogMessage, LogMessageContent, ResolveParseHint, TemplateLogMsg, TemplateLogMsgChunk}; const DLT_COLUMN_SENTINAL: char = '\u{0004}'; const DLT_ARGUMENT_SENTINAL: char = '\u{0005}'; @@ -538,8 +538,6 @@ impl<'a> FormattableMessage<'a> { } impl LogMessage for FormattableMessage<'_> { - const CAN_ERROR: bool = true; - fn to_writer(&self, writer: &mut W) -> Result { let bytes = self.message.as_bytes(); let len = bytes.len(); @@ -563,7 +561,7 @@ impl LogMessage for FormattableMessage<'_> { /// context-id /// /// payload - fn to_text(&self) -> ToTextResult { + fn try_resolve(&self) -> LogMessageContent { let mut msg = String::new(); // Taken from Documentation: string formatting is considered an infallible operation. // Thus we can ignore `fmt::Error` errors. @@ -623,12 +621,14 @@ impl LogMessage for FormattableMessage<'_> { if is_someip { if let Some(slice) = slices.get(1) { - let err = ParseLogError::new( - slice.to_owned(), - crate::ParseErrorType::Other("Need Some IP".into()), - Some(ResolveErrorHint::SomeIP), + let template = TemplateLogMsg::new( + vec![ + TemplateLogMsgChunk::Text(msg), + TemplateLogMsgChunk::Bytes(slice.to_owned()), + ], + vec![ResolveParseHint::SomeIP], ); - return ToTextResult::new(msg, Some(err)); + return LogMessageContent::Template(template); } } diff --git a/application/apps/indexer/parsers/src/dlt/mod.rs b/application/apps/indexer/parsers/src/dlt/mod.rs index 34f8990841..fef9164e39 100644 --- a/application/apps/indexer/parsers/src/dlt/mod.rs +++ b/application/apps/indexer/parsers/src/dlt/mod.rs @@ -39,8 +39,6 @@ impl std::fmt::Display for RawMessage { } impl LogMessage for RangeMessage { - const CAN_ERROR: bool = false; - /// A RangeMessage only has range information and cannot serialize to bytes fn to_writer(&self, writer: &mut W) -> Result { writer.write_u64::(self.range.start as u64)?; @@ -48,21 +46,19 @@ impl LogMessage for RangeMessage { Ok(8 + 8) } - fn to_text(&self) -> crate::ToTextResult { + fn try_resolve(&self) -> crate::LogMessageContent { self.into() } } impl LogMessage for RawMessage { - const CAN_ERROR: bool = false; - fn to_writer(&self, writer: &mut W) -> Result { let len = self.content.len(); writer.write_all(&self.content)?; Ok(len) } - fn to_text(&self) -> crate::ToTextResult { + fn try_resolve(&self) -> crate::LogMessageContent { self.into() } } diff --git a/application/apps/indexer/parsers/src/lib.rs b/application/apps/indexer/parsers/src/lib.rs index 1d7122da9e..82d97923c5 100644 --- a/application/apps/indexer/parsers/src/lib.rs +++ b/application/apps/indexer/parsers/src/lib.rs @@ -78,99 +78,125 @@ pub enum ByteRepresentation { Range((usize, usize)), } -#[derive(Debug, Clone)] -pub enum ParseErrorType { - Fmt(String), - Other(String), +#[derive(Debug)] +pub enum MessageStreamItem { + Item(ParseYield), + Skipped, + Incomplete, + Empty, + Done, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -/// Gives Hint about how this error can be resolved by processor -pub enum ResolveErrorHint { - /// The message needs to be parsed with SomeIP Parser. - SomeIP, +pub trait LogMessage: Serialize { + /// Serializes a message directly into a Writer + /// returns the size of the serialized message + fn to_writer(&self, writer: &mut W) -> Result; + + /// Tries to resolve the message to get its text representation. + /// + /// TODO: This function should return another optional field, containing information + /// about errors, warning ... + fn try_resolve(&self) -> LogMessageContent; } #[derive(Debug, Clone)] -pub struct ParseLogError { - pub remain_bytes: Vec, - pub error_type: ParseErrorType, - pub resolve_hint: Option, -} - -impl ParseLogError { - pub fn new( - remain_bytes: Vec, - error_type: ParseErrorType, - resolve_hint: Option, - ) -> Self { - Self { - remain_bytes, - error_type, - resolve_hint, - } - } +/// Represents The content of a log message after trying to resolve it. +pub enum LogMessageContent { + Text(String), + Template(TemplateLogMsg), } -impl Display for ParseLogError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match &self.error_type { - ParseErrorType::Other(msg) | ParseErrorType::Fmt(msg) => write!(f, "{msg}"), - } - } +#[derive(Debug, Clone)] +/// Represents an unresolved log messages that contains chunks that needs to be resolved. +pub struct TemplateLogMsg { + chunks: Vec, + resolve_hints: Vec, } -impl From for ParseLogError { - fn from(value: std::fmt::Error) -> Self { - Self { - remain_bytes: Vec::new(), - error_type: ParseErrorType::Fmt(value.to_string()), - resolve_hint: None, - } - } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +/// Gives Hint about how the payload rest can be resolved +pub enum ResolveParseHint { + /// The message needs to be parsed with SomeIP Parser. + SomeIP, } -impl std::error::Error for ParseLogError {} - #[derive(Debug, Clone)] -pub struct ToTextResult { - pub msg: String, - pub error: Option, -} - -impl ToTextResult { - pub fn new(msg: String, error: Option) -> Self { - Self { msg, error } - } +/// Represents a chunk in [`TemplateLogMsg`] +pub enum TemplateLogMsgChunk { + /// Resolved Chunk + Text(String), + /// Unresolved Chunk + Bytes(Vec), } -impl From for ToTextResult +impl From for LogMessageContent where T: Display, { fn from(value: T) -> Self { - Self::new(value.to_string(), None) + Self::Text(value.to_string()) } } -pub trait LogMessage: Serialize { - //TODO AAZ: Measure this an remove if rust already optimize the code without it. - /// Indicates that parsing this struct to text can error. - const CAN_ERROR: bool; +impl TemplateLogMsg { + pub fn new(chunks: Vec, resolve_hints: Vec) -> Self { + Self { + chunks, + resolve_hints, + } + } - /// Serializes a message directly into a Writer - /// returns the size of the serialized message - fn to_writer(&self, writer: &mut W) -> Result; + pub fn get_resolve_hints(&self) -> Vec { + self.resolve_hints.to_vec() + } - /// Get the text representation of this message. - fn to_text(&self) -> ToTextResult; -} + /// Applies the given [`FnMut`] on the unresolved chunks, replacing them with texts if succeed. + /// This function will return a String once chunks get resolved. + /// + /// * `parse_fn`: Function to apply on the unresolved chunks. + pub fn try_resolve(&mut self, mut parse_fn: F) -> Option + where + F: FnMut(&[u8]) -> Option, + { + let mut all_resolved = true; + for ch in self.chunks.iter_mut() { + match ch { + TemplateLogMsgChunk::Text(_) => continue, + TemplateLogMsgChunk::Bytes(bytes) => match parse_fn(&bytes) { + Some(resolved) => *ch = TemplateLogMsgChunk::Text(resolved), + None => all_resolved = false, + }, + } + } -#[derive(Debug)] -pub enum MessageStreamItem { - Item(ParseYield), - Skipped, - Incomplete, - Empty, - Done, + if all_resolved { + self.chunks + .iter() + .map(|ch| match ch { + TemplateLogMsgChunk::Text(msg) => msg, + TemplateLogMsgChunk::Bytes(_) => panic!("All must be resolved"), + }) + .cloned() + .reduce(|mut acc, msg| { + acc.push_str(&msg); + acc + }) + } else { + None + } + } + + /// Concatenates the chunks to a string, replacing the unresolved chunks with their bytes + /// representation. + pub fn resolve_lossy(self) -> String { + self.chunks + .into_iter() + .fold(String::new(), |mut acc, ch| match ch { + TemplateLogMsgChunk::Text(msg) => { + acc.push_str(&msg); + acc + } + TemplateLogMsgChunk::Bytes(bytes) => format!("{acc} {bytes:?}"), + }) + } } diff --git a/application/apps/indexer/parsers/src/someip.rs b/application/apps/indexer/parsers/src/someip.rs index 438c383b6c..79e3bc66ab 100644 --- a/application/apps/indexer/parsers/src/someip.rs +++ b/application/apps/indexer/parsers/src/someip.rs @@ -1,4 +1,4 @@ -use crate::{Error, LogMessage, ParseYield, Parser, ToTextResult}; +use crate::{Error, LogMessage, LogMessageContent, ParseYield, Parser}; use std::{borrow::Cow, fmt, fmt::Display, io::Write, path::PathBuf}; use someip_messages::*; @@ -325,14 +325,12 @@ impl SomeipLogMessage { } impl LogMessage for SomeipLogMessage { - const CAN_ERROR: bool = false; - fn to_writer(&self, writer: &mut W) -> Result { writer.write_all(&self.bytes)?; Ok(self.bytes.len()) } - fn to_text(&self) -> ToTextResult { + fn try_resolve(&self) -> LogMessageContent { self.into() } } diff --git a/application/apps/indexer/parsers/src/text.rs b/application/apps/indexer/parsers/src/text.rs index 59f3880237..a59aad423e 100644 --- a/application/apps/indexer/parsers/src/text.rs +++ b/application/apps/indexer/parsers/src/text.rs @@ -1,4 +1,4 @@ -use crate::{Error, LogMessage, ParseYield, Parser, ToTextResult}; +use crate::{Error, LogMessage, LogMessageContent, ParseYield, Parser}; use serde::Serialize; use std::{fmt, io::Write}; @@ -16,15 +16,13 @@ impl fmt::Display for StringMessage { } impl LogMessage for StringMessage { - const CAN_ERROR: bool = false; - fn to_writer(&self, writer: &mut W) -> Result { let len = self.content.len(); writer.write_all(self.content.as_bytes())?; Ok(len) } - fn to_text(&self) -> ToTextResult { + fn try_resolve(&self) -> LogMessageContent { self.into() } } diff --git a/application/apps/indexer/session/src/handlers/observing/mod.rs b/application/apps/indexer/session/src/handlers/observing/mod.rs index 95cae72988..3dd96eb903 100644 --- a/application/apps/indexer/session/src/handlers/observing/mod.rs +++ b/application/apps/indexer/session/src/handlers/observing/mod.rs @@ -2,7 +2,7 @@ use std::path::PathBuf; use crate::{ operations::{OperationAPI, OperationResult}, - parse_err::{get_log_text, ParseErrorReslover}, + parse_rest_resolver::{resolve_log_msg, ParseRestReslover}, state::SessionStateAPI, tail, }; @@ -78,7 +78,7 @@ async fn run_source_intern( rx_sde: Option, rx_tail: Option>>, ) -> OperationResult<()> { - let mut parse_err_resolver = ParseErrorReslover::new(); + let mut parse_err_resolver = ParseRestReslover::new(); match parser { ParserType::SomeIp(settings) => { let someip_parser = match &settings.fibex_file_paths { @@ -119,6 +119,7 @@ async fn run_source_intern( settings.with_storage_header, ); let producer = MessageProducer::new(dlt_parser, source, rx_sde); + let someip_parse = match &settings.fibex_file_paths { Some(paths) => { SomeipParser::from_fibex_files(paths.iter().map(PathBuf::from).collect()) @@ -126,6 +127,7 @@ async fn run_source_intern( None => SomeipParser::new(), }; parse_err_resolver.with_someip_parser(someip_parse); + run_producer( operation_api, state, @@ -145,7 +147,7 @@ async fn run_producer, S: ByteSource>( source_id: u16, mut producer: MessageProducer, mut rx_tail: Option>>, - parse_err_resolver: &mut ParseErrorReslover, + parse_err_resolver: &mut ParseRestReslover, ) -> OperationResult<()> { use log::debug; state.set_session_file(None).await?; @@ -173,7 +175,7 @@ async fn run_producer, S: ByteSource>( Next::Item(item) => { match item { MessageStreamItem::Item(ParseYield::Message(item)) => { - let msg = get_log_text(item, parse_err_resolver); + let msg = resolve_log_msg(item, parse_err_resolver); state .write_session_file(source_id, format!("{msg}\n")) .await?; @@ -182,7 +184,7 @@ async fn run_producer, S: ByteSource>( item, attachment, ))) => { - let msg = get_log_text(item, parse_err_resolver); + let msg = resolve_log_msg(item, parse_err_resolver); state .write_session_file(source_id, format!("{msg}\n")) .await?; diff --git a/application/apps/indexer/session/src/lib.rs b/application/apps/indexer/session/src/lib.rs index 42dd87eb3c..9e3d0d32bd 100644 --- a/application/apps/indexer/session/src/lib.rs +++ b/application/apps/indexer/session/src/lib.rs @@ -2,7 +2,7 @@ pub mod events; mod handlers; pub mod operations; -pub mod parse_err; +pub mod parse_rest_resolver; pub mod paths; pub mod progress; pub mod session; diff --git a/application/apps/indexer/session/src/parse_err.rs b/application/apps/indexer/session/src/parse_err.rs deleted file mode 100644 index a1a9094441..0000000000 --- a/application/apps/indexer/session/src/parse_err.rs +++ /dev/null @@ -1,66 +0,0 @@ -use parsers::{someip::SomeipParser, LogMessage, ParseLogError, Parser}; - -#[derive(Default)] -pub struct ParseErrorReslover { - someip_praser: Option, -} - -impl ParseErrorReslover { - pub fn new() -> Self { - Self::default() - } - - /// Sets SomeIP parser on the resolver - pub fn with_someip_parser(&mut self, someip_praser: SomeipParser) -> &mut Self { - self.someip_praser = Some(someip_praser); - self - } - - /// Tries to resolve the given error returning the parsed string if succeeded. - pub fn resolve_err(&mut self, error: &ParseLogError) -> Option { - match error.resolve_hint { - Some(parsers::ResolveErrorHint::SomeIP) => self - .someip_praser - .as_mut() - .and_then(|parser| { - parser - .parse(&error.remain_bytes, None) - .ok() - .and_then(|res| res.1) - }) - .and_then(|a| match a { - // TODO: Handle someip parser errors after prototyping. - parsers::ParseYield::Message(msg) => Some(msg.to_text().msg), - parsers::ParseYield::Attachment(_) => None, - parsers::ParseYield::MessageAndAttachment((msg, _att)) => { - Some(msg.to_text().msg) - } - }), - None => None, - } - } -} - -/// Get the text message of [`LogMessage`], resolving parse text errors if possible, -/// TODO: Otherwise it should save the error to the faulty messages store, which need to be -/// implemented as well :) -pub fn get_log_text(item: T, err_resolver: &mut ParseErrorReslover) -> String { - let text_res = item.to_text(); - if T::CAN_ERROR { - let mut msg = text_res.msg; - if let Some(err_info) = text_res.error { - match err_resolver.resolve_err(&err_info) { - Some(resloved_msg) => { - msg.push_str(&resloved_msg); - } - None => { - //TODO: Item with error details should be reported faulty messages store. - msg = format!("{msg}: Unknow Error bytes: {:?}", err_info.remain_bytes); - } - } - } - msg - } else { - text_res.msg - } -} diff --git a/application/apps/indexer/session/src/parse_rest_resolver.rs b/application/apps/indexer/session/src/parse_rest_resolver.rs new file mode 100644 index 0000000000..ceb9582495 --- /dev/null +++ b/application/apps/indexer/session/src/parse_rest_resolver.rs @@ -0,0 +1,64 @@ +use parsers::{someip::SomeipParser, LogMessage, Parser, TemplateLogMsg}; + +#[derive(Default)] +pub struct ParseRestReslover { + someip_praser: Option, +} + +impl ParseRestReslover { + pub fn new() -> Self { + Self::default() + } + + /// Sets SomeIP parser on the resolver + pub fn with_someip_parser(&mut self, someip_praser: SomeipParser) -> &mut Self { + self.someip_praser = Some(someip_praser); + self + } + + /// Tries to resolve the given error returning the parsed string if succeeded. + pub fn resolve_log_template(&mut self, template: &mut TemplateLogMsg) -> Option { + for hint in template.get_resolve_hints() { + match hint { + parsers::ResolveParseHint::SomeIP => { + if let Some(p) = self.someip_praser.as_mut() { + let res = template.try_resolve(|bytes| { + let p_yield = p.parse(bytes, None).ok()?.1?; + match p_yield { + parsers::ParseYield::Message(item) => match item.try_resolve() { + parsers::LogMessageContent::Text(msg) => Some(msg), + // Ignore nested errors for now + parsers::LogMessageContent::Template(_) => None, + }, + // Ignore other parse types for now + parsers::ParseYield::Attachment(_) => None, + parsers::ParseYield::MessageAndAttachment(_) => None, + } + }); + + if res.is_some() { + return res; + } + } + } + } + } + None + } +} + +/// Get the text message of [`LogMessage`], resolving its rest payloads if existed when possible, +/// TODO: Otherwise it should save the error to the faulty messages store, which need to be +/// implemented as well :) +pub fn resolve_log_msg(item: T, err_resolver: &mut ParseRestReslover) -> String { + match item.try_resolve() { + parsers::LogMessageContent::Text(msg) => msg, + parsers::LogMessageContent::Template(mut template) => { + if let Some(resolved) = err_resolver.resolve_log_template(&mut template) { + return resolved; + } + //TODO: Add message to the faulty messages once implemented. + template.resolve_lossy() + } + } +}