Skip to content

Commit 063346b

Browse files
ellttBenellttBen
committed
Switch to a direct deserialize implementation for json kv attributes encoding. Factor pattern parsing code between MDC and key_value targets
1 parent 60f2b20 commit 063346b

File tree

2 files changed

+89
-104
lines changed

2 files changed

+89
-104
lines changed

‎src/encode/json.rs‎

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ impl JsonEncoder {
8282
thread_id: thread_id::get(),
8383
mdc: Mdc,
8484
#[cfg(feature = "log_kv")]
85-
attributes: kv::get_attributes(record.key_values())?,
85+
attributes: kv::Attributes(record.key_values()),
8686
};
8787
message.serialize(&mut serde_json::Serializer::new(&mut *w))?;
8888
w.write_all(NEWLINE.as_bytes())?;
@@ -114,7 +114,7 @@ struct Message<'a> {
114114
thread_id: usize,
115115
mdc: Mdc,
116116
#[cfg(feature = "log_kv")]
117-
attributes: kv::Map,
117+
attributes: kv::Attributes<'a>,
118118
}
119119

120120
fn ser_display<T, S>(v: &T, s: S) -> Result<S::Ok, S::Error>
@@ -174,29 +174,35 @@ impl Deserialize for JsonEncoderDeserializer {
174174
#[cfg(feature = "log_kv")]
175175
mod kv {
176176
use log::kv::VisitSource;
177-
use std::collections::BTreeMap;
177+
use serde::ser::{self, Error, SerializeMap};
178178

179-
pub(crate) type Map = BTreeMap<String, String>;
179+
pub(crate) struct Attributes<'a>(pub &'a dyn log::kv::Source);
180180

181-
pub(crate) fn get_attributes(source: &dyn log::kv::Source) -> anyhow::Result<Map> {
182-
struct Visitor {
183-
inner: Map,
181+
pub(crate) struct SerializerVisitor<T: ser::SerializeMap>(pub T);
182+
183+
impl<'kvs, T: ser::SerializeMap> VisitSource<'kvs> for SerializerVisitor<T> {
184+
fn visit_pair(
185+
&mut self,
186+
key: log::kv::Key<'kvs>,
187+
value: log::kv::Value<'kvs>,
188+
) -> Result<(), log::kv::Error> {
189+
self.0
190+
.serialize_entry(key.as_str(), &value.to_string())
191+
.map_err(|e| log::kv::Error::boxed(e.to_string()))?;
192+
Ok(())
184193
}
185-
impl<'kvs> VisitSource<'kvs> for Visitor {
186-
fn visit_pair(
187-
&mut self,
188-
key: log::kv::Key<'kvs>,
189-
value: log::kv::Value<'kvs>,
190-
) -> Result<(), log::kv::Error> {
191-
self.inner.insert(format!("{key}"), format!("{value}"));
192-
Ok(())
193-
}
194+
}
195+
196+
impl<'a> ser::Serialize for Attributes<'a> {
197+
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
198+
where
199+
S: serde::Serializer,
200+
{
201+
let map = serializer.serialize_map(Some(self.0.count()))?;
202+
let mut visitor = SerializerVisitor(map);
203+
self.0.visit(&mut visitor).map_err(S::Error::custom)?;
204+
visitor.0.end()
194205
}
195-
let mut visitor = Visitor {
196-
inner: BTreeMap::new(),
197-
};
198-
source.visit(&mut visitor)?;
199-
Ok(visitor.inner)
200206
}
201207
}
202208

‎src/encode/pattern/mod.rs‎

Lines changed: 62 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,8 @@ use crate::encode::{
144144
#[cfg(feature = "config_parsing")]
145145
use crate::config::{Deserialize, Deserializers};
146146

147+
use self::parser::Formatter;
148+
147149
mod parser;
148150

149151
thread_local!(
@@ -505,87 +507,25 @@ impl<'a> From<Piece<'a>> for Chunk {
505507
"P" | "pid" => no_args(&formatter.args, parameters, FormattedChunk::ProcessId),
506508
"i" | "tid" => no_args(&formatter.args, parameters, FormattedChunk::SystemThreadId),
507509
"t" | "target" => no_args(&formatter.args, parameters, FormattedChunk::Target),
508-
"X" | "mdc" => {
509-
if formatter.args.len() > 2 {
510-
return Chunk::Error("expected at most two arguments".to_owned());
511-
}
512-
513-
let key = match formatter.args.first() {
514-
Some(arg) => {
515-
if let Some(arg) = arg.first() {
516-
match arg {
517-
Piece::Text(key) => key.to_owned(),
518-
Piece::Error(ref e) => return Chunk::Error(e.clone()),
519-
_ => return Chunk::Error("invalid MDC key".to_owned()),
520-
}
521-
} else {
522-
return Chunk::Error("invalid MDC key".to_owned());
523-
}
524-
}
525-
None => return Chunk::Error("missing MDC key".to_owned()),
526-
};
527-
528-
let default = match formatter.args.get(1) {
529-
Some(arg) => {
530-
if let Some(arg) = arg.first() {
531-
match arg {
532-
Piece::Text(key) => key.to_owned(),
533-
Piece::Error(ref e) => return Chunk::Error(e.clone()),
534-
_ => return Chunk::Error("invalid MDC default".to_owned()),
535-
}
536-
} else {
537-
return Chunk::Error("invalid MDC default".to_owned());
538-
}
539-
}
540-
None => "",
541-
};
542-
543-
Chunk::Formatted {
544-
chunk: FormattedChunk::Mdc(key.into(), default.into()),
510+
"X" | "mdc" => match kv_parsing(&formatter) {
511+
Err(e) => Chunk::Error(format!("MDC: {e}")),
512+
Ok((key, default)) => Chunk::Formatted {
513+
chunk: FormattedChunk::Mdc(key, default),
545514
params: parameters,
546-
}
547-
}
515+
},
516+
},
548517
#[cfg(feature = "log_kv")]
549-
"K" | "key_value" => {
550-
if formatter.args.len() > 2 {
551-
return Chunk::Error("expected at most two arguments".to_owned());
552-
}
553-
554-
let key = match formatter.args.first() {
555-
Some(arg) => {
556-
if let Some(arg) = arg.first() {
557-
match arg {
558-
Piece::Text(key) => key.to_owned(),
559-
Piece::Error(ref e) => return Chunk::Error(e.clone()),
560-
_ => return Chunk::Error("invalid log::kv key".to_owned()),
561-
}
562-
} else {
563-
return Chunk::Error("invalid log::kv key".to_owned());
564-
}
565-
}
566-
None => return Chunk::Error("missing log::kv key".to_owned()),
567-
};
568-
569-
let default = match formatter.args.get(1) {
570-
Some(arg) => {
571-
if let Some(arg) = arg.first() {
572-
match arg {
573-
Piece::Text(value) => value.to_owned(),
574-
Piece::Error(ref e) => return Chunk::Error(e.clone()),
575-
_ => return Chunk::Error("invalid log::kv default".to_owned()),
576-
}
577-
} else {
578-
return Chunk::Error("invalid log::kv default".to_owned());
579-
}
580-
}
581-
None => "",
582-
};
583-
584-
Chunk::Formatted {
585-
chunk: FormattedChunk::Kv(key.into(), default.into()),
518+
"K" | "key_value" => match kv_parsing(&formatter) {
519+
Err(e) => Chunk::Error(format!("key_value: {e}")),
520+
Ok((key, default)) => Chunk::Formatted {
521+
chunk: FormattedChunk::Kv(key, default),
586522
params: parameters,
587-
}
588-
}
523+
},
524+
},
525+
#[cfg(not(feature = "log_kv"))]
526+
"K" | "key_value" => Chunk::Error(
527+
"The log_kv feature is required to parse the key_value argument".to_owned(),
528+
),
589529
"" => {
590530
if formatter.args.len() != 1 {
591531
return Chunk::Error("expected exactly one argument".to_owned());
@@ -618,6 +558,43 @@ fn no_args(arg: &[Vec<Piece>], params: Parameters, chunk: FormattedChunk) -> Chu
618558
}
619559
}
620560

561+
fn kv_parsing<'a>(formatter: &'a Formatter) -> Result<(String, String), &'a str> {
562+
if formatter.args.len() > 2 {
563+
return Err("expected at most two arguments");
564+
}
565+
566+
let key = match formatter.args.first() {
567+
Some(arg) => {
568+
if let Some(arg) = arg.first() {
569+
match arg {
570+
Piece::Text(key) => key.to_owned(),
571+
Piece::Error(ref e) => return Err(e),
572+
_ => return Err("invalid key"),
573+
}
574+
} else {
575+
return Err("invalid key");
576+
}
577+
}
578+
None => return Err("missing key"),
579+
};
580+
581+
let default = match formatter.args.get(1) {
582+
Some(arg) => {
583+
if let Some(arg) = arg.first() {
584+
match arg {
585+
Piece::Text(key) => key.to_owned(),
586+
Piece::Error(ref e) => return Err(e),
587+
_ => return Err("invalid default"),
588+
}
589+
} else {
590+
return Err("invalid default");
591+
}
592+
}
593+
None => "",
594+
};
595+
Ok((key.into(), default.into()))
596+
}
597+
621598
#[derive(Clone, Eq, PartialEq, Hash, Debug)]
622599
enum Timezone {
623600
Utc,
@@ -812,7 +789,9 @@ mod tests {
812789
#[cfg(feature = "simple_writer")]
813790
use std::thread;
814791

815-
use super::*;
792+
#[cfg(feature = "log_kv")]
793+
use super::Parser;
794+
use super::{Chunk, PatternEncoder};
816795
#[cfg(feature = "simple_writer")]
817796
use crate::encode::writer::simple::SimpleWriter;
818797
#[cfg(feature = "simple_writer")]
@@ -1141,11 +1120,11 @@ mod tests {
11411120
"expected at most two arguments",
11421121
),
11431122
("[{K({l user_id):<5.5}]", "expected '}'"),
1144-
("[{K({l} user_id):<5.5}]", "invalid log::kv key"),
1145-
("[{K:<5.5}]", "missing log::kv key"),
1123+
("[{K({l} user_id):<5.5}]", "key_value: invalid key"),
1124+
("[{K:<5.5}]", "key_value: missing key"),
11461125
("[{K(user_id)({l):<5.5}]", "expected '}'"),
1147-
("[{K(user_id)({l}):<5.5}]", "invalid log::kv default"),
1148-
("[{K(user_id)():<5.5} {M}]", "invalid log::kv default"),
1126+
("[{K(user_id)({l}):<5.5}]", "key_value: invalid default"),
1127+
("[{K(user_id)():<5.5} {M}]", "key_value: invalid default"),
11491128
];
11501129

11511130
for (pattern, error_msg) in tests {

0 commit comments

Comments
 (0)