-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Infinite loop in some circumstances #33
Comments
Thanks for reporting it! Definitely nasty. Setting the parent to tracing-bunyan-formatter/src/storage_layer.rs Line 112 in c506eaf
Switching to using the parent() for an event would lead to undesirable side-effects (see https://docs.rs/tracing/latest/tracing/event/struct.Event.html#method.parent) since it would be None for events that are descendants of the current span.A hack could be to use a span rather than an event there 🤔 |
Oh right, I totally misread what Maybe something like this could work? let current_span = if event.is_root() {
None
} else {
ctx.lookup_current()
}; |
ctrlaltf24
added a commit
to ctrlaltf24/tracing-bunyan-formatter
that referenced
this issue
Jul 14, 2023
While this log message is nice to have, it's not worth having an infinite loop over Alternative solutions: - Consider renaming colliding field Fixes LukeMathWalker#33
ctrlaltf24
added a commit
to ctrlaltf24/tracing-bunyan-formatter
that referenced
this issue
Jul 14, 2023
While this log message is nice to have, it's not worth having an infinite loop over Alternative solutions: - Consider renaming colliding field Fixes LukeMathWalker#33
LukeMathWalker
pushed a commit
that referenced
this issue
Jul 16, 2023
While this log message is nice to have, it's not worth having an infinite loop over Alternative solutions: - Consider renaming colliding field Fixes #33
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I've encountered an infinite loop (leading to a stack overflow and crash) when the following criteria are met:
name
)tracing_bunyan_formatter::formatting_layer
target(Note that it is pretty easy to accidentally fall into that scenario.
1.
can happen if you add#[instrument]
to a function that has a parameter calledname
, and2.
can happen if you setRUST_LOG=debug
to quickly debug something.)The problem is due to the use of
tracing::debug!(...)
to warn when there are conflicting fields (for instance this line): AnEvent
is logged, the code collects all the fields from all the parent spans, finds one that is invalid, callstracing::debug!()
which creates a newEvent
in the current span, which gets dispatched, the code tries to collect all the fields from all the parent spans, and so on...I'm not sure what the proper fix is here (assuming we want to keep the logs). One option I tried is to make the log event generated by
tracing::debug!()
not be a child of the current span, by usingtracing::debug!(parent: None, "...")
, but that didn't work becauseBunyanFormattingLayer::on_event()
doesn't seem to consider the event's parent at all. It always gets the "current span" from the context. I feel like maybe the current span should be retrieved withevent.parent().and_then(|id| ctx.span(id))
instead ofctx.lookup_current()
, which fixes my test case, but I don't know enough to know if that's correct, or what the side effects would be...Sample code to reproduce the problem:
The text was updated successfully, but these errors were encountered: