Fix missing commas in logs

common/logging/src/tracing_logging_layer.rs

@@ -155,13 +155,22 @@ where
                 &mut writer,
             );
         } else {
+            let mut collected_span_fields = Vec::new();
+            if let Some(scope) = ctx.event_scope(event) {
+                for span in scope {
+                    let id = span.id();
+                    let span_fields_map = self.span_fields.lock().unwrap();
+                    if let Some(span_data) = span_fields_map.get(&id) {
+                        collected_span_fields
+                            .push((span_data.name.clone(), span_data.fields.clone()));
+                    }
+                }
+            }
             build_log_text(
                 &visitor,
                 plain_level_str,
                 &timestamp,
-                &ctx,
-                &self.span_fields,
-                event,
+                collected_span_fields,
                 &location,
                 color_level_str,
                 self.log_color,
@@ -326,38 +335,23 @@ fn build_log_json<'a, S>(
 }
 
 #[allow(clippy::too_many_arguments)]
-fn build_log_text<'a, S>(
+fn build_log_text(
     visitor: &LogMessageExtractor,
     plain_level_str: &str,
     timestamp: &str,
-    ctx: &Context<'_, S>,
-    span_fields: &Arc<Mutex<HashMap<Id, SpanData>>>,
-    event: &tracing::Event<'_>,
+    collected_span_fields: Vec<(String, Vec<(String, String)>)>,
     location: &str,
     color_level_str: &str,
     use_color: bool,
     writer: &mut impl Write,
-) where
-    S: Subscriber + for<'lookup> LookupSpan<'lookup>,
-{
+) {
     let bold_start = "\x1b[1m";
     let bold_end = "\x1b[0m";
-    let mut collected_span_fields = Vec::new();
-
-    if let Some(scope) = ctx.event_scope(event) {
-        for span in scope {
-            let id = span.id();
-            let span_fields_map = span_fields.lock().unwrap();
-            if let Some(span_data) = span_fields_map.get(&id) {
-                collected_span_fields.push((span_data.name.clone(), span_data.fields.clone()));
-            }
-        }
-    }
 
     let mut formatted_spans = String::new();
     for (_, fields) in collected_span_fields.iter().rev() {
-        for (i, (field_name, field_value)) in fields.iter().enumerate() {
-            if i > 0 && !visitor.fields.is_empty() {
+        for (field_name, field_value) in fields.iter() {
+            if !formatted_spans.is_empty() {
                 formatted_spans.push_str(", ");
             }
             if use_color {
@@ -455,3 +449,155 @@ fn parse_field(val: &str) -> Value {
     };
     serde_json::from_str(cleaned).unwrap_or(Value::String(cleaned.to_string()))
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::tracing_logging_layer::{build_log_text, LogMessageExtractor};
+    use std::io::Write;
+
+    struct Buffer {
+        data: Vec<u8>,
+    }
+
+    impl Buffer {
+        fn new() -> Self {
+            Buffer { data: Vec::new() }
+        }
+
+        fn into_string(self) -> String {
+            String::from_utf8(self.data).unwrap()
+        }
+    }
+
+    impl Write for Buffer {
+        fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
+            self.data.extend_from_slice(buf);
+            Ok(buf.len())
+        }
+
+        fn flush(&mut self) -> std::io::Result<()> {
+            Ok(())
+        }
+    }
+
+    #[test]
+    fn test_build_log_text_single_log_field() {
+        let log_fields = vec![("field_name".to_string(), "field_value".to_string())];
+        let span_fields = vec![];
+        let expected = "Jan 1 08:00:00.000 INFO  test message                                  field_name: field_value \n";
+        test_build_log_text(log_fields, span_fields, expected);
+    }
+
+    #[test]
+    fn test_build_log_text_multiple_log_fields() {
+        let log_fields = vec![
+            ("field_name1".to_string(), "field_value1".to_string()),
+            ("field_name2".to_string(), "field_value2".to_string()),
+        ];
+        let span_fields = vec![];
+        let expected = "Jan 1 08:00:00.000 INFO  test message                                  field_name1: field_value1, field_name2: field_value2 \n";
+        test_build_log_text(log_fields, span_fields, expected);
+    }
+
+    #[test]
+    fn test_build_log_text_log_field_and_span() {
+        let log_fields = vec![("field_name".to_string(), "field_value".to_string())];
+        let span_fields = vec![(
+            "span_name".to_string(),
+            vec![(
+                "span_field_name".to_string(),
+                "span_field_value".to_string(),
+            )],
+        )];
+        let expected = "Jan 1 08:00:00.000 INFO  test message                                  field_name: field_value, span_field_name: span_field_value\n";
+        test_build_log_text(log_fields, span_fields, expected);
+    }
+
+    #[test]
+    fn test_build_log_text_single_span() {
+        let log_fields = vec![];
+        let span_fields = vec![(
+            "span_name".to_string(),
+            vec![(
+                "span_field_name".to_string(),
+                "span_field_value".to_string(),
+            )],
+        )];
+        let expected = "Jan 1 08:00:00.000 INFO  test message                                 span_field_name: span_field_value\n";
+        test_build_log_text(log_fields, span_fields, expected);
+    }
+
+    #[test]
+    fn test_build_log_text_multiple_spans() {
+        let log_fields = vec![];
+        let span_fields = vec![
+            (
+                "span_name1".to_string(),
+                vec![(
+                    "span_field_name1".to_string(),
+                    "span_field_value1".to_string(),
+                )],
+            ),
+            (
+                "span_name2".to_string(),
+                vec![(
+                    "span_field_name2".to_string(),
+                    "span_field_value2".to_string(),
+                )],
+            ),
+        ];
+        let expected = "Jan 1 08:00:00.000 INFO  test message                                 span_field_name2: span_field_value2, span_field_name1: span_field_value1\n";
+        test_build_log_text(log_fields, span_fields, expected);
+    }
+
+    #[test]
+    fn test_build_log_text_multiple_span_fields() {
+        let log_fields = vec![];
+        let span_fields = vec![(
+            "span_name1".to_string(),
+            vec![
+                (
+                    "span_field_name1-1".to_string(),
+                    "span_field_value1-1".to_string(),
+                ),
+                (
+                    "span_field_name1-2".to_string(),
+                    "span_field_value1-2".to_string(),
+                ),
+            ],
+        )];
+        let expected = "Jan 1 08:00:00.000 INFO  test message                                 span_field_name1-1: span_field_value1-1, span_field_name1-2: span_field_value1-2\n";
+        test_build_log_text(log_fields, span_fields, expected);
+    }
+
+    fn test_build_log_text(
+        log_fields: Vec<(String, String)>,
+        span_fields: Vec<(String, Vec<(String, String)>)>,
+        expected: &str,
+    ) {
+        let visitor = LogMessageExtractor {
+            message: "test message".to_string(),
+            fields: log_fields,
+            is_crit: false,
+        };
+        let plain_level_str = "INFO";
+        let timestamp = "Jan 1 08:00:00.000";
+        let location = "";
+        let color_level_str = "\x1b[32mINFO\x1b[0m";
+        let use_color = false;
+        let mut writer = Buffer::new();
+
+        build_log_text(
+            &visitor,
+            plain_level_str,
+            timestamp,
+            span_fields,
+            location,
+            color_level_str,
+            use_color,
+            &mut writer,
+        );
+
+        assert_eq!(expected, &writer.into_string());
+    }
+}