From 063346b03ca880c59e4d3026d5087a134b9ec871 Mon Sep 17 00:00:00 2001 From: ellttben Date: Tue, 9 Jul 2024 17:41:31 +0100 Subject: [PATCH] Switch to a direct deserialize implementation for json kv attributes encoding. Factor pattern parsing code between MDC and key_value targets --- src/encode/json.rs | 48 +++++++------ src/encode/pattern/mod.rs | 145 ++++++++++++++++---------------------- 2 files changed, 89 insertions(+), 104 deletions(-) diff --git a/src/encode/json.rs b/src/encode/json.rs index 53efae9c..b520351c 100644 --- a/src/encode/json.rs +++ b/src/encode/json.rs @@ -82,7 +82,7 @@ impl JsonEncoder { thread_id: thread_id::get(), mdc: Mdc, #[cfg(feature = "log_kv")] - attributes: kv::get_attributes(record.key_values())?, + attributes: kv::Attributes(record.key_values()), }; message.serialize(&mut serde_json::Serializer::new(&mut *w))?; w.write_all(NEWLINE.as_bytes())?; @@ -114,7 +114,7 @@ struct Message<'a> { thread_id: usize, mdc: Mdc, #[cfg(feature = "log_kv")] - attributes: kv::Map, + attributes: kv::Attributes<'a>, } fn ser_display(v: &T, s: S) -> Result @@ -174,29 +174,35 @@ impl Deserialize for JsonEncoderDeserializer { #[cfg(feature = "log_kv")] mod kv { use log::kv::VisitSource; - use std::collections::BTreeMap; + use serde::ser::{self, Error, SerializeMap}; - pub(crate) type Map = BTreeMap; + pub(crate) struct Attributes<'a>(pub &'a dyn log::kv::Source); - pub(crate) fn get_attributes(source: &dyn log::kv::Source) -> anyhow::Result { - struct Visitor { - inner: Map, + pub(crate) struct SerializerVisitor(pub T); + + impl<'kvs, T: ser::SerializeMap> VisitSource<'kvs> for SerializerVisitor { + fn visit_pair( + &mut self, + key: log::kv::Key<'kvs>, + value: log::kv::Value<'kvs>, + ) -> Result<(), log::kv::Error> { + self.0 + .serialize_entry(key.as_str(), &value.to_string()) + .map_err(|e| log::kv::Error::boxed(e.to_string()))?; + Ok(()) } - impl<'kvs> VisitSource<'kvs> for Visitor { - fn visit_pair( - &mut self, - key: log::kv::Key<'kvs>, - value: log::kv::Value<'kvs>, - ) -> Result<(), log::kv::Error> { - self.inner.insert(format!("{key}"), format!("{value}")); - Ok(()) - } + } + + impl<'a> ser::Serialize for Attributes<'a> { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + let map = serializer.serialize_map(Some(self.0.count()))?; + let mut visitor = SerializerVisitor(map); + self.0.visit(&mut visitor).map_err(S::Error::custom)?; + visitor.0.end() } - let mut visitor = Visitor { - inner: BTreeMap::new(), - }; - source.visit(&mut visitor)?; - Ok(visitor.inner) } } diff --git a/src/encode/pattern/mod.rs b/src/encode/pattern/mod.rs index 7c77f50f..577022e8 100644 --- a/src/encode/pattern/mod.rs +++ b/src/encode/pattern/mod.rs @@ -144,6 +144,8 @@ use crate::encode::{ #[cfg(feature = "config_parsing")] use crate::config::{Deserialize, Deserializers}; +use self::parser::Formatter; + mod parser; thread_local!( @@ -505,87 +507,25 @@ impl<'a> From> for Chunk { "P" | "pid" => no_args(&formatter.args, parameters, FormattedChunk::ProcessId), "i" | "tid" => no_args(&formatter.args, parameters, FormattedChunk::SystemThreadId), "t" | "target" => no_args(&formatter.args, parameters, FormattedChunk::Target), - "X" | "mdc" => { - if formatter.args.len() > 2 { - return Chunk::Error("expected at most two arguments".to_owned()); - } - - let key = match formatter.args.first() { - Some(arg) => { - if let Some(arg) = arg.first() { - match arg { - Piece::Text(key) => key.to_owned(), - Piece::Error(ref e) => return Chunk::Error(e.clone()), - _ => return Chunk::Error("invalid MDC key".to_owned()), - } - } else { - return Chunk::Error("invalid MDC key".to_owned()); - } - } - None => return Chunk::Error("missing MDC key".to_owned()), - }; - - let default = match formatter.args.get(1) { - Some(arg) => { - if let Some(arg) = arg.first() { - match arg { - Piece::Text(key) => key.to_owned(), - Piece::Error(ref e) => return Chunk::Error(e.clone()), - _ => return Chunk::Error("invalid MDC default".to_owned()), - } - } else { - return Chunk::Error("invalid MDC default".to_owned()); - } - } - None => "", - }; - - Chunk::Formatted { - chunk: FormattedChunk::Mdc(key.into(), default.into()), + "X" | "mdc" => match kv_parsing(&formatter) { + Err(e) => Chunk::Error(format!("MDC: {e}")), + Ok((key, default)) => Chunk::Formatted { + chunk: FormattedChunk::Mdc(key, default), params: parameters, - } - } + }, + }, #[cfg(feature = "log_kv")] - "K" | "key_value" => { - if formatter.args.len() > 2 { - return Chunk::Error("expected at most two arguments".to_owned()); - } - - let key = match formatter.args.first() { - Some(arg) => { - if let Some(arg) = arg.first() { - match arg { - Piece::Text(key) => key.to_owned(), - Piece::Error(ref e) => return Chunk::Error(e.clone()), - _ => return Chunk::Error("invalid log::kv key".to_owned()), - } - } else { - return Chunk::Error("invalid log::kv key".to_owned()); - } - } - None => return Chunk::Error("missing log::kv key".to_owned()), - }; - - let default = match formatter.args.get(1) { - Some(arg) => { - if let Some(arg) = arg.first() { - match arg { - Piece::Text(value) => value.to_owned(), - Piece::Error(ref e) => return Chunk::Error(e.clone()), - _ => return Chunk::Error("invalid log::kv default".to_owned()), - } - } else { - return Chunk::Error("invalid log::kv default".to_owned()); - } - } - None => "", - }; - - Chunk::Formatted { - chunk: FormattedChunk::Kv(key.into(), default.into()), + "K" | "key_value" => match kv_parsing(&formatter) { + Err(e) => Chunk::Error(format!("key_value: {e}")), + Ok((key, default)) => Chunk::Formatted { + chunk: FormattedChunk::Kv(key, default), params: parameters, - } - } + }, + }, + #[cfg(not(feature = "log_kv"))] + "K" | "key_value" => Chunk::Error( + "The log_kv feature is required to parse the key_value argument".to_owned(), + ), "" => { if formatter.args.len() != 1 { return Chunk::Error("expected exactly one argument".to_owned()); @@ -618,6 +558,43 @@ fn no_args(arg: &[Vec], params: Parameters, chunk: FormattedChunk) -> Chu } } +fn kv_parsing<'a>(formatter: &'a Formatter) -> Result<(String, String), &'a str> { + if formatter.args.len() > 2 { + return Err("expected at most two arguments"); + } + + let key = match formatter.args.first() { + Some(arg) => { + if let Some(arg) = arg.first() { + match arg { + Piece::Text(key) => key.to_owned(), + Piece::Error(ref e) => return Err(e), + _ => return Err("invalid key"), + } + } else { + return Err("invalid key"); + } + } + None => return Err("missing key"), + }; + + let default = match formatter.args.get(1) { + Some(arg) => { + if let Some(arg) = arg.first() { + match arg { + Piece::Text(key) => key.to_owned(), + Piece::Error(ref e) => return Err(e), + _ => return Err("invalid default"), + } + } else { + return Err("invalid default"); + } + } + None => "", + }; + Ok((key.into(), default.into())) +} + #[derive(Clone, Eq, PartialEq, Hash, Debug)] enum Timezone { Utc, @@ -812,7 +789,9 @@ mod tests { #[cfg(feature = "simple_writer")] use std::thread; - use super::*; + #[cfg(feature = "log_kv")] + use super::Parser; + use super::{Chunk, PatternEncoder}; #[cfg(feature = "simple_writer")] use crate::encode::writer::simple::SimpleWriter; #[cfg(feature = "simple_writer")] @@ -1141,11 +1120,11 @@ mod tests { "expected at most two arguments", ), ("[{K({l user_id):<5.5}]", "expected '}'"), - ("[{K({l} user_id):<5.5}]", "invalid log::kv key"), - ("[{K:<5.5}]", "missing log::kv key"), + ("[{K({l} user_id):<5.5}]", "key_value: invalid key"), + ("[{K:<5.5}]", "key_value: missing key"), ("[{K(user_id)({l):<5.5}]", "expected '}'"), - ("[{K(user_id)({l}):<5.5}]", "invalid log::kv default"), - ("[{K(user_id)():<5.5} {M}]", "invalid log::kv default"), + ("[{K(user_id)({l}):<5.5}]", "key_value: invalid default"), + ("[{K(user_id)():<5.5} {M}]", "key_value: invalid default"), ]; for (pattern, error_msg) in tests {