Skip to content

Commit

Permalink
Send panic info over json when json logging enabled (#954)
Browse files Browse the repository at this point in the history
  • Loading branch information
rukai authored Dec 13, 2022
1 parent 176d12c commit 8db5a4c
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 0 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions shotover-proxy/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ split-iter = "0.1.0"
# Error handling
thiserror = "1.0"
anyhow = "1.0.31"
backtrace = "0.3.66"

# Parsers
cql3-parser = "0.3.1"
Expand Down
1 change: 1 addition & 0 deletions shotover-proxy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,5 @@ mod server;
pub mod sources;
pub mod tcp;
pub mod tls;
pub mod tracing_panic_handler;
pub mod transforms;
10 changes: 10 additions & 0 deletions shotover-proxy/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,16 @@ impl TracingState {
}
};

// When in json mode we need to process panics as events instead of printing directly to stdout.
// This is so that:
// * We dont include invalid json in stdout
// * panics can be received by whatever is processing the json events
//
// We dont do this for LogFormat::Human because the default panic messages are more readable for humans
if let LogFormat::Json = format {
crate::tracing_panic_handler::setup();
}

Ok(TracingState { guard, handle })
}
}
Expand Down
85 changes: 85 additions & 0 deletions shotover-proxy/src/tracing_panic_handler.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
use backtrace::{Backtrace, BacktraceFmt, BacktraceFrame, BytesOrWideString, PrintFmt};
use std::fmt;

pub fn setup() {
std::panic::set_hook(Box::new(|panic| {
let backtrace = BacktraceFormatter(Backtrace::new());
// If the panic has a source location, record it as structured fields.
if let Some(location) = panic.location() {
tracing::error!(
message = %panic,
panic.file = location.file(),
panic.line = location.line(),
panic.column = location.column(),
panic.backtrace = format!("{backtrace}"),
);
} else {
tracing::error!(
message = %panic,
panic.backtrace = format!("{backtrace}"),
);
}
}));
}

/// The std::backtrace::Backtrace formatting is really noisy because it includes all the pre-main and panic handling frames.
/// Internal panics have logic to remove that but that is missing from std::backtrace::Backtrace.
/// https://github.com/rust-lang/rust/issues/105413
///
/// As a workaround we use the backtrace crate and manually perform the required formatting
struct BacktraceFormatter(Backtrace);

// based on https://github.com/rust-lang/backtrace-rs/blob/5be2e8ba9cf6e391c5fa45219fc091b4075eb6be/src/capture.rs#L371
impl fmt::Display for BacktraceFormatter {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// When printing paths we try to strip the cwd if it exists, otherwise
// we just print the path as-is. Note that we also only do this for the
// short format, because if it's full we presumably want to print
// everything.
let cwd = std::env::current_dir();
let mut print_path = move |fmt: &mut fmt::Formatter<'_>, path: BytesOrWideString<'_>| {
let path = path.into_path_buf();
if let Ok(cwd) = &cwd {
if let Ok(suffix) = path.strip_prefix(cwd) {
return fmt::Display::fmt(&suffix.display(), fmt);
}
}
fmt::Display::fmt(&path.display(), fmt)
};

let mut f = BacktraceFmt::new(f, PrintFmt::Short, &mut print_path);
f.add_context()?;
let mut include_frames = false;
for frame in self.0.frames() {
if frame_is_named(
frame,
"std::sys_common::backtrace::__rust_begin_short_backtrace",
) {
break;
}

if include_frames {
f.frame().backtrace_frame(frame)?;
}

if frame_is_named(
frame,
"std::sys_common::backtrace::__rust_end_short_backtrace",
) {
include_frames = true;
}
}
f.finish()
}
}

fn frame_is_named(frame: &BacktraceFrame, name: &str) -> bool {
for symbol in frame.symbols() {
if let Some(full_name) = symbol.name() {
if format!("{}", full_name).starts_with(name) {
return true;
}
}
}
false
}

0 comments on commit 8db5a4c

Please sign in to comment.