Skip to content

Commit 178df7a

Browse files
authored
Fix duplicate fields being logged when the field exists in both the span and the event (#8183)
Closes #7995. Fix duplicate fields being logged when the field exists in both the span and the event. Prefer event fields when this happens. ``` Sep 15 08:13:46.339 WARN State cache missed state_root: 0xc34826ff7794de63a553832b7aff13572d1c716b9e03d5ef7b29649adf98abe2, block_root: 0xf16d3f5b4cc6ec876b7faeccd9f2d4102dc56ed32e828754b62601637910ec1f, state_root: 0xc34826ff7794de63a553832b7aff13572d1c716b9e03d5ef7b29649adf98abe2, block_root: 0xf16d3f5b4cc6ec876b7faeccd9f2d4102dc56ed32e828754b62601637910ec1f ``` becomes ``` Sep 15 08:13:46.339 WARN State cache missed state_root: 0xc34826ff7794de63a553832b7aff13572d1c716b9e03d5ef7b29649adf98abe2, block_root: 0xf16d3f5b4cc6ec876b7faeccd9f2d4102dc56ed32e828754b62601637910ec1f ``` Co-Authored-By: Jimmy Chen <[email protected]>
1 parent 0c9fdea commit 178df7a

File tree

1 file changed

+63
-51
lines changed

1 file changed

+63
-51
lines changed

common/logging/src/tracing_logging_layer.rs

Lines changed: 63 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::utils::is_ascii_control;
2+
use std::collections::HashSet;
23

34
use chrono::prelude::*;
45
use serde_json::{Map, Value};
@@ -261,6 +262,12 @@ fn build_log_json(
261262
let module_field = format!("{}:{}", module_path, line_number);
262263
log_map.insert("module".to_string(), Value::String(module_field));
263264

265+
// Avoid adding duplicate fields; prefer event fields when duplicates exist.
266+
for (key, val) in span_fields {
267+
let parsed_span_val = parse_field(val);
268+
log_map.insert(key.clone(), parsed_span_val);
269+
}
270+
264271
for (key, val) in visitor.fields.clone().into_iter() {
265272
let cleaned_value = if val.starts_with('\"') && val.ends_with('\"') && val.len() >= 2 {
266273
&val[1..val.len() - 1]
@@ -272,11 +279,6 @@ fn build_log_json(
272279
log_map.insert(key, parsed_val);
273280
}
274281

275-
for (key, val) in span_fields {
276-
let parsed_span_val = parse_field(val);
277-
log_map.insert(key.clone(), parsed_span_val);
278-
}
279-
280282
let json_obj = Value::Object(log_map);
281283
let output = format!("{}\n", json_obj);
282284

@@ -299,23 +301,6 @@ fn build_log_text(
299301
let bold_start = "\x1b[1m";
300302
let bold_end = "\x1b[0m";
301303

302-
let mut formatted_spans = String::new();
303-
for (i, (field_name, field_value)) in span_fields.iter().rev().enumerate() {
304-
if use_color {
305-
formatted_spans.push_str(&format!(
306-
"{}{}{}: {}",
307-
bold_start, field_name, bold_end, field_value
308-
));
309-
} else {
310-
formatted_spans.push_str(&format!("{}: {}", field_name, field_value));
311-
}
312-
313-
// Check if this is not the last span.
314-
if i != span_fields.len() - 1 {
315-
formatted_spans.push_str(", ");
316-
}
317-
}
318-
319304
let pad = if plain_level_str.len() < ALIGNED_LEVEL_WIDTH {
320305
" "
321306
} else {
@@ -351,24 +336,26 @@ fn build_log_text(
351336
message_content.clone()
352337
};
353338

354-
let mut formatted_fields = String::new();
355-
for (i, (field_name, field_value)) in visitor.fields.iter().enumerate() {
356-
if i > 0 {
357-
formatted_fields.push_str(", ");
358-
}
359-
if use_color {
360-
formatted_fields.push_str(&format!(
361-
"{}{}{}: {}",
362-
bold_start, field_name, bold_end, field_value
363-
));
364-
} else {
365-
formatted_fields.push_str(&format!("{}: {}", field_name, field_value));
366-
}
367-
// Check if this is the last field and that we are also adding spans.
368-
if i == visitor.fields.len() - 1 && !span_fields.is_empty() {
369-
formatted_fields.push(',');
370-
}
371-
}
339+
// Avoid adding duplicate fields; prefer event fields when duplicates exist.
340+
let mut added_field_names = HashSet::new();
341+
let formatted_fields = visitor
342+
.fields
343+
.iter()
344+
.chain(span_fields.iter())
345+
.filter_map(|(field_name, field_value)| {
346+
if added_field_names.insert(field_name) {
347+
let formatted_field = if use_color {
348+
format!("{}{}{}: {}", bold_start, field_name, bold_end, field_value)
349+
} else {
350+
format!("{}: {}", field_name, field_value)
351+
};
352+
Some(formatted_field)
353+
} else {
354+
None
355+
}
356+
})
357+
.collect::<Vec<_>>()
358+
.join(", ");
372359

373360
let full_message = if !formatted_fields.is_empty() {
374361
format!("{} {}", padded_message, formatted_fields)
@@ -377,15 +364,12 @@ fn build_log_text(
377364
};
378365

379366
let message = if !location.is_empty() {
380-
format!(
381-
"{} {} {} {} {}\n",
382-
timestamp, level_str, location, full_message, formatted_spans
383-
)
384-
} else {
385367
format!(
386368
"{} {} {} {}\n",
387-
timestamp, level_str, full_message, formatted_spans
369+
timestamp, level_str, location, full_message
388370
)
371+
} else {
372+
format!("{} {} {}\n", timestamp, level_str, full_message)
389373
};
390374

391375
if let Err(e) = writer.write_all(message.as_bytes()) {
@@ -436,7 +420,7 @@ mod tests {
436420
fn test_build_log_text_single_log_field() {
437421
let log_fields = vec![("field_name".to_string(), "field_value".to_string())];
438422
let span_fields = vec![];
439-
let expected = "Jan 1 08:00:00.000 INFO test message field_name: field_value \n";
423+
let expected = "Jan 1 08:00:00.000 INFO test message field_name: field_value\n";
440424
test_build_log_text(log_fields, span_fields, expected);
441425
}
442426

@@ -447,7 +431,7 @@ mod tests {
447431
("field_name2".to_string(), "field_value2".to_string()),
448432
];
449433
let span_fields = vec![];
450-
let expected = "Jan 1 08:00:00.000 INFO test message field_name1: field_value1, field_name2: field_value2 \n";
434+
let expected = "Jan 1 08:00:00.000 INFO test message field_name1: field_value1, field_name2: field_value2\n";
451435
test_build_log_text(log_fields, span_fields, expected);
452436
}
453437

@@ -469,7 +453,7 @@ mod tests {
469453
"span_field_name".to_string(),
470454
"span_field_value".to_string(),
471455
)];
472-
let expected = "Jan 1 08:00:00.000 INFO test message span_field_name: span_field_value\n";
456+
let expected = "Jan 1 08:00:00.000 INFO test message span_field_name: span_field_value\n";
473457
test_build_log_text(log_fields, span_fields, expected);
474458
}
475459

@@ -486,7 +470,7 @@ mod tests {
486470
"span_field_value2".to_string(),
487471
),
488472
];
489-
let expected = "Jan 1 08:00:00.000 INFO test message span_field_name2: span_field_value2, span_field_name1: span_field_value1\n";
473+
let expected = "Jan 1 08:00:00.000 INFO test message span_field_name1: span_field_value1, span_field_name2: span_field_value2\n";
490474
test_build_log_text(log_fields, span_fields, expected);
491475
}
492476

@@ -503,7 +487,35 @@ mod tests {
503487
"span_field_value1-2".to_string(),
504488
),
505489
];
506-
let expected = "Jan 1 08:00:00.000 INFO test message span_field_name1-2: span_field_value1-2, span_field_name1-1: span_field_value1-1\n";
490+
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";
491+
test_build_log_text(log_fields, span_fields, expected);
492+
}
493+
494+
#[test]
495+
fn test_build_log_text_no_duplicate_log_span_fields() {
496+
let log_fields = vec![
497+
("field_name_1".to_string(), "field_value_1".to_string()),
498+
("field_name_2".to_string(), "field_value_2".to_string()),
499+
];
500+
let span_fields = vec![
501+
("field_name_1".to_string(), "field_value_1".to_string()),
502+
("field_name_3".to_string(), "field_value_3".to_string()),
503+
];
504+
let expected = "Jan 1 08:00:00.000 INFO test message field_name_1: field_value_1, field_name_2: field_value_2, field_name_3: field_value_3\n";
505+
test_build_log_text(log_fields, span_fields, expected);
506+
}
507+
508+
#[test]
509+
fn test_build_log_text_duplicate_fields_prefer_log_fields() {
510+
let log_fields = vec![
511+
("field_name_1".to_string(), "field_value_1_log".to_string()),
512+
("field_name_2".to_string(), "field_value_2".to_string()),
513+
];
514+
let span_fields = vec![
515+
("field_name_1".to_string(), "field_value_1_span".to_string()),
516+
("field_name_3".to_string(), "field_value_3".to_string()),
517+
];
518+
let expected = "Jan 1 08:00:00.000 INFO test message field_name_1: field_value_1_log, field_name_2: field_value_2, field_name_3: field_value_3\n";
507519
test_build_log_text(log_fields, span_fields, expected);
508520
}
509521

0 commit comments

Comments
 (0)