sfackler / rust-log-panics Goto Github PK

signalapp/libsignal is used in our Android and iOS apps, and we currently use log-panics with backtraces enabled. On a user's phone, however, the library binary has been stripped, and so most of backtrace's symbolication support is wasted. (It can often get the top-level entry point, since that's usually a public symbol, but that's not that useful for debugging a panic.) In a branch with a custom replacement for log-panics that uses Backtrace::new_unresolved instead of Backtrace::new, the final library size was 100KB smaller (about 5% of that binary's total size).

Would you be interested in a feature flag for this mode? Something like

with-backtrace = ["with-backtrace-unresolved"]
with-backtrace-unresolved = ["backtrace"]

Alternately, if you're looking to deprecate or remove the with-backtrace feature in favor of the implicit feature provided by backtrace being an optional dependency, it could be something like this:

default-features = ["backtrace-resolve-symbols"]
backtrace-resolve-symbols = [] # has no effect is 'backtrace' is disabled

with-backtrace = ["backtrace"] # deprecated

Or should we keep our custom implementation?

Hey @sfackler,

I'm thinking about adding syslog panic logging to my project, and I was wondering if you'd be interested in a patch that adds it to this crate behind a feature gate. What do you think?

Honestly, I don't know why the Rust stdlib doesn't do this, because besaid method exists and also can be obtained using take_hook. Nevertheless, it would be cool if this workaround wasn't necessary and I could simply call log_panics::hook(info) instead.

Motivation: I need to do some cleanup+shutdown in my panic hook.

Hi

I am working on project that have HTTP API that is using actix web (with tokio async). I enabled debug=line-tables-only to get line numbers on stacktrace. The goal is to be able to debug panic easily when it happened on prod (log is sent to datadog)

Changes

[profile.release]
lto = true
debug = "line-tables-only"

Before

   0: backtrace::capture::Backtrace::new
   1: log_panics::Config::install_panic_hook::{{closure}}
   2: std::panicking::rust_panic_with_hook
   3: std::panicking::begin_panic::{{closure}}
   4: std::sys_common::backtrace::__rust_end_short_backtrace
   5: std::panicking::begin_panic
   6: <<Redacted>>
   7: <<Redacted>>
   8: <<Redacted>>
   9: actix_web::handler::handler_service::{{closure}}::{{closure}}
  10: <actix_web::resource::Resource<T> as actix_web::service::HttpServiceFactory>::register::{{closure}}::{{closure}}
  11: <actix_cors::middleware::CorsMiddleware<S> as actix_service::Service<actix_web::service::ServiceRequest>>::call::{{closure}}
  12: <actix_service::map_err::MapErrFuture<A,Req,F,E> as core::future::future::Future>::poll
  13: actix_http::h1::dispatcher::InnerDispatcher<T,S,B,X,U>::poll_response
  14: <actix_http::h1::dispatcher::Dispatcher<T,S,B,X,U> as core::future::future::Future>::poll
  15: tokio::runtime::task::raw::poll
  16: tokio::task::local::LocalSet::tick
  17: <tokio::task::local::RunUntil<T> as core::future::future::Future>::poll
  18: std::sys_common::backtrace::__rust_begin_short_backtrace
  19: core::ops::function::FnOnce::call_once{{vtable.shim}}
  20: std::sys::unix::thread::Thread::new::thread_start
  21: __pthread_joiner_wake

However this changes caused the stacktraces to blow up to 260kb, due to a lot of async tokio on actix web layer that happened above (i.e callers of) main.rs. Are there a way to filter out such stacktraces from that layer? i.e before main.rs, ignore.

(Found this crate on the way to prototyping a fatal! macro in dekellum/tao-log#1, which sort of solves the same problem, but from the opposite direction.)

Should the panic handler flush the logger after logging the panic? Maybe most logger implementations don't buffer messages, but if any did, then this would be important to persist the log message before terminating?

If the answer is no, then feel free to close. If the answer is yes, then would a PR be desired?

I'm too lazy to fork and PR right now, sorry ^^

I just think that

        let msg = info.payload().downcast_ref::<&'static str>()
            .or_else(|| {
                info.payload()
                    .downcast_ref::<String>()
                    .map(String::as_str)
            })
            .unwrap_or("Box<Any>");

would be a bit more idiomatic than the current nested match blocks. I haven't tested the code, but I hope you get the idea

With the current version, when I configure panic logging with BacktraceMode::Unresolved, the message doesn't contain function addresses but just <unknown> (tried on linux-gnu, windows-msvc and macos).

In order to include addresses in the backtrace, it's necessary to format the Backtrace object using the alternate debug form ({:#?} instead of {:?}). If I do, the output changes from

   0: <unknown>
   1: <unknown>
   2: <unknown>

to

   0:        0x102dbfde7 - <unknown>
   1:        0x102dbfd82 - <unknown>
   2:        0x102dc488b - <unknown>

I'm using the following crate versions:

• log-panics 2.1.0
• backtrace 0.3.67

Currently when logging it uses the default target, which is the module name log_panics. But in my opinion it would be nicer to change this to panic, since the panic itself really has nothing to do with the module.

2017-08-04T12:32:58.203374+00:00 [ERROR] log_panics: thread 'tests::log_output' panicked at 'oops'
// vs
2017-08-04T12:32:58.203374+00:00 [ERROR] panic: thread 'tests::log_output' panicked at 'oops'

Would a change like that be acceptable?