Skip to content

Commit

Permalink
resolve stack overflow issue in logger
Browse files Browse the repository at this point in the history
  • Loading branch information
Chethan-rao committed Dec 23, 2024
1 parent 7bea130 commit afbc9f8
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 63 deletions.
77 changes: 31 additions & 46 deletions src/logger/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ const TIME: &str = "time";
pub static IMPLICIT_KEYS: Lazy<rustc_hash::FxHashSet<&str>> = Lazy::new(|| {
let mut set = rustc_hash::FxHashSet::default();

set.insert(MESSAGE);
set.insert(HOSTNAME);
set.insert(PID);
set.insert(LEVEL);
Expand Down Expand Up @@ -118,11 +117,23 @@ where
pub fn new_with_implicit_entries(
service: &str,
dst_writer: W,
default_fields: HashMap<String, Value>,
mut default_fields: HashMap<String, Value>,
) -> Self {
let pid = std::process::id();
let hostname = gethostname::gethostname().to_string_lossy().into_owned();
let service = service.to_string();
default_fields.retain(|key, value| {
if !IMPLICIT_KEYS.contains(key.as_str()) {
true
} else {
tracing::warn!(
?key,
?value,
"Attempting to log a reserved entry. It won't be added to the logs"
);
false
}
});

Self {
dst_writer,
Expand All @@ -139,16 +150,14 @@ where
map_serializer: &mut impl SerializeMap<Error = serde_json::Error>,
metadata: &Metadata<'_>,
span: Option<&SpanRef<'_, S>>,
storage: Option<&Storage<'_>>,
storage: &Storage<'_>,
name: &str,
message: &str,
) -> Result<(), std::io::Error>
where
S: Subscriber + for<'a> LookupSpan<'a>,
{
let is_extra = |s: &str| !IMPLICIT_KEYS.contains(s);

map_serializer.serialize_entry(MESSAGE, &message)?;
map_serializer.serialize_entry(HOSTNAME, &self.hostname)?;
map_serializer.serialize_entry(PID, &self.pid)?;
map_serializer.serialize_entry(LEVEL, &format_args!("{}", metadata.level()))?;
Expand All @@ -165,20 +174,15 @@ where

// Write down implicit default entries.
for (key, value) in self.default_fields.iter() {
if !IMPLICIT_KEYS.contains(key.as_str()) {
map_serializer.serialize_entry(key, value)?;
} else {
tracing::warn!("{} is a reserved field. Skipping it.", key);
}
map_serializer.serialize_entry(key, value)?;
}

let mut explicit_entries_set: HashSet<&str> = HashSet::default();
// Write down explicit event's entries.
if let Some(storage) = storage {
for (key, value) in storage.values.iter() {
map_serializer.serialize_entry(key, value)?;
explicit_entries_set.insert(key);
}

for (key, value) in storage.values.iter() {
map_serializer.serialize_entry(key, value)?;
explicit_entries_set.insert(key);
}

// Write down entries from the span, if it exists.
Expand Down Expand Up @@ -225,14 +229,15 @@ where
let mut serializer = serde_json::Serializer::new(&mut buffer);
let mut map_serializer = serializer.serialize_map(None)?;
let message = Self::span_message(span, ty);
let mut storage = Storage::default();
storage.record_value(MESSAGE, message.into());

self.common_serialize(
&mut map_serializer,
span.metadata(),
Some(span),
None,
&storage,
span.name(),
&message,
)?;

map_serializer.end()?;
Expand All @@ -256,16 +261,9 @@ where
event.record(&mut storage);

let name = span.map_or("?", SpanRef::name);
let message = Self::event_message(span, event, &storage);
Self::event_message(span, event, &mut storage);

self.common_serialize(
&mut map_serializer,
event.metadata(),
*span,
Some(&storage),
name,
&message,
)?;
self.common_serialize(&mut map_serializer, event.metadata(), *span, &storage, name)?;

map_serializer.end()?;
Ok(buffer)
Expand All @@ -291,32 +289,19 @@ where
fn event_message<S>(
span: &Option<&SpanRef<'_, S>>,
event: &Event<'_>,
storage: &Storage<'_>,
) -> String
where
storage: &mut Storage<'_>,
) where
S: Subscriber + for<'a> LookupSpan<'a>,
{
// Get value of kept "message" or "target" if does not exist.
let mut message = storage
let message = storage
.values
.get("message")
.and_then(|v| match v {
Value::String(s) => Some(s.as_str()),
_ => None,
})
.unwrap_or_else(|| event.metadata().target())
.to_owned();
.entry(MESSAGE)
.or_insert_with(|| event.metadata().target().into());

// Prepend the span name to the message if span exists.
if let Some(span) = span {
message = format!(
"{} {}",
Self::span_message(span, RecordType::Event),
message,
);
if let (Some(span), Value::String(a)) = (span, message) {
*a = format!("{} {}", Self::span_message(span, RecordType::Event), a,);
}

message
}
}

Expand Down
33 changes: 17 additions & 16 deletions src/logger/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,19 @@ pub struct Storage<'a> {
pub values: HashMap<&'a str, serde_json::Value>,
}

impl Storage<'_> {
impl<'a> Storage<'a> {
/// Default constructor.
pub fn new() -> Self {
Self::default()
}

pub fn record_value(&mut self, key: &'a str, value: serde_json::Value) {
if super::formatter::IMPLICIT_KEYS.contains(key) {
tracing::warn!(value =? value, "{} is a reserved entry. Skipping it.", key);
} else {
self.values.insert(key, value);
}
}
}

/// Default constructor.
Expand All @@ -43,32 +51,27 @@ impl Default for Storage<'_> {
impl Visit for Storage<'_> {
/// A i64.
fn record_i64(&mut self, field: &Field, value: i64) {
self.values
.insert(field.name(), serde_json::Value::from(value));
self.record_value(field.name(), serde_json::Value::from(value));
}

/// A u64.
fn record_u64(&mut self, field: &Field, value: u64) {
self.values
.insert(field.name(), serde_json::Value::from(value));
self.record_value(field.name(), serde_json::Value::from(value));
}

/// A 64-bit floating point.
fn record_f64(&mut self, field: &Field, value: f64) {
self.values
.insert(field.name(), serde_json::Value::from(value));
self.record_value(field.name(), serde_json::Value::from(value));
}

/// A boolean.
fn record_bool(&mut self, field: &Field, value: bool) {
self.values
.insert(field.name(), serde_json::Value::from(value));
self.record_value(field.name(), serde_json::Value::from(value));
}

/// A string.
fn record_str(&mut self, field: &Field, value: &str) {
self.values
.insert(field.name(), serde_json::Value::from(value));
self.record_value(field.name(), serde_json::Value::from(value));
}

/// Otherwise.
Expand All @@ -77,12 +80,10 @@ impl Visit for Storage<'_> {
// Skip fields which are already handled
name if name.starts_with("log.") => (),
name if name.starts_with("r#") => {
self.values
.insert(&name[2..], serde_json::Value::from(format!("{value:?}")));
self.record_value(&name[2..], serde_json::Value::from(format!("{value:?}")));
}
name => {
self.values
.insert(name, serde_json::Value::from(format!("{value:?}")));
self.record_value(name, serde_json::Value::from(format!("{value:?}")));
}
};
}
Expand Down Expand Up @@ -148,7 +149,7 @@ impl<S: Subscriber + for<'a> tracing_subscriber::registry::LookupSpan<'a>> Layer
.expect("No visitor in extensions");

if let Ok(elapsed) = serde_json::to_value(elapsed_milliseconds) {
visitor.values.insert("elapsed_milliseconds", elapsed);
visitor.record_value("elapsed_milliseconds", elapsed);
}
}
}
2 changes: 1 addition & 1 deletion src/storage/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ impl LockerInterface for Storage {
Ok(locker) => Ok(locker),
};

output.map_err(From::from).map(|inner| inner.into())
output.map_err(From::from).map(From::from)
}

async fn find_by_hash_id_merchant_id_customer_id(
Expand Down

0 comments on commit afbc9f8

Please sign in to comment.