From afd463e657931a153843975893f0ae4c8043ea0f Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Fri, 23 Oct 2020 00:52:54 +0200 Subject: [PATCH 1/5] Use `sentry` to log server panics --- .env.sample | 4 ++ Cargo.lock | 176 ++++++++++++++++++++++++++++++++++++++++++++++ Cargo.toml | 1 + src/bin/server.rs | 17 +++++ 4 files changed, 198 insertions(+) diff --git a/.env.sample b/.env.sample index 97fdee89991..098d86f5cf7 100644 --- a/.env.sample +++ b/.env.sample @@ -56,3 +56,7 @@ export GH_CLIENT_SECRET= # export MAILGUN_SMTP_LOGIN= # export MAILGUN_SMTP_PASSWORD= # export MAILGUN_SMTP_SERVER= + +# Credentials for connecting to the Sentry error reporting service. +# export SENTRY_DSN_API= +export SENTRY_ENV_API=local diff --git a/Cargo.lock b/Cargo.lock index c2dba4886fc..d126d8156e1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -179,6 +179,15 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" +[[package]] +name = "bitmaps" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "031043d04099746d8db04daf1fa424b2bc8bd69d92b25962dcde24da39ab64a2" +dependencies = [ + "typenum", +] + [[package]] name = "block-buffer" version = "0.7.3" @@ -306,6 +315,7 @@ dependencies = [ "reqwest", "scheduled-thread-pool", "semver 0.10.0", + "sentry", "serde", "serde_json", "sha2 0.9.1", @@ -598,6 +608,16 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "debugid" +version = "0.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f91cf5a8c2f2097e2a32627123508635d47ce10563d999ec1a95addf08b502ba" +dependencies = [ + "serde", + "uuid", +] + [[package]] name = "derive_deref" version = "1.1.1" @@ -1196,6 +1216,20 @@ dependencies = [ "unicode-normalization", ] +[[package]] +name = "im" +version = "15.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "111c1983f3c5bb72732df25cddacee9b546d08325fb584b5ebd38148be7b0246" +dependencies = [ + "bitmaps", + "rand_core", + "rand_xoshiro", + "sized-chunks", + "typenum", + "version_check", +] + [[package]] name = "indexmap" version = "1.6.0" @@ -2044,6 +2078,15 @@ dependencies = [ "rand_core", ] +[[package]] +name = "rand_xoshiro" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a9fcdd2e881d02f1d9390ae47ad8e5696a9e4be7b547a1da2afbc61973217004" +dependencies = [ + "rand_core", +] + [[package]] name = "redox_syscall" version = "0.1.57" @@ -2221,6 +2264,98 @@ version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" +[[package]] +name = "sentry" +version = "0.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "144e85b28d129f056ef91664fe2b985eade906d2838752c2f61c9f233cd98e4a" +dependencies = [ + "httpdate", + "reqwest", + "sentry-backtrace", + "sentry-contexts", + "sentry-core", + "sentry-failure", + "sentry-panic", +] + +[[package]] +name = "sentry-backtrace" +version = "0.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "92dabd890482f152fb6d261fe2034a193facc2c99c0c571bbf7687c356fcb2e8" +dependencies = [ + "backtrace", + "lazy_static", + "regex", + "sentry-core", +] + +[[package]] +name = "sentry-contexts" +version = "0.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "039ac50d2d740d51c5d376c2e9e93725eea662fa3acdcbcfe1b8b93a3b30c478" +dependencies = [ + "hostname", + "lazy_static", + "libc", + "regex", + "rustc_version", + "sentry-core", + "uname", +] + +[[package]] +name = "sentry-core" +version = "0.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3fe4fe890b12416701f838c702898a9c5e574c333cfbbee9fb7855a14e6490a3" +dependencies = [ + "im", + "lazy_static", + "rand", + "sentry-types", + "serde", + "serde_json", +] + +[[package]] +name = "sentry-failure" +version = "0.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ead35e7019f77a79ed0345b3f3c28427139100f87f318c1c3e2788db2cdea8b7" +dependencies = [ + "failure", + "sentry-backtrace", + "sentry-core", +] + +[[package]] +name = "sentry-panic" +version = "0.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ab8a3ac989339a76efd6155f9d02675ce4b04419cd8083ca58d083c222554147" +dependencies = [ + "sentry-backtrace", + "sentry-core", +] + +[[package]] +name = "sentry-types" +version = "0.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8124f0e9bc1113ecbcc8c3746e0e590943cf23e7d09c70a088c116869bb12e3" +dependencies = [ + "chrono", + "debugid", + "serde", + "serde_json", + "thiserror", + "url", + "uuid", +] + [[package]] name = "serde" version = "1.0.117" @@ -2342,6 +2477,16 @@ version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fa8f3741c7372e75519bd9346068370c9cdaabcc1f9599cbcf2a2719352286b7" +[[package]] +name = "sized-chunks" +version = "0.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ec31ceca5644fa6d444cc77548b88b67f46db6f7c71683b0f9336e671830d2f" +dependencies = [ + "bitmaps", + "typenum", +] + [[package]] name = "slab" version = "0.4.2" @@ -2569,6 +2714,26 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "thiserror" +version = "1.0.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "318234ffa22e0920fe9a40d7b8369b5f649d490980cf7aadcf1eb91594869b42" +dependencies = [ + "thiserror-impl", +] + +[[package]] +name = "thiserror-impl" +version = "1.0.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cae2447b6282786c3493999f40a9be2a6ad20cb8bd268b0a0dbf5a065535c0ab" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "thread_local" version = "1.0.1" @@ -2764,6 +2929,15 @@ version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56dee185309b50d1f11bfedef0fe6d036842e3fb77413abef29f8f8d1c5d4c1c" +[[package]] +name = "uname" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b72f89f0ca32e4db1c04e2a72f5345d59796d4866a1ee0609084569f73683dc8" +dependencies = [ + "libc", +] + [[package]] name = "unchecked-index" version = "0.2.2" @@ -2828,6 +3002,7 @@ dependencies = [ "idna", "matches", "percent-encoding", + "serde", ] [[package]] @@ -2843,6 +3018,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9fde2f6a4bea1d6e007c4ad38c6839fa71cbb63b6dbf5b595aa38dc9b1093c11" dependencies = [ "rand", + "serde", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index b081c1ca41f..26a6a22a464 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -76,6 +76,7 @@ regex = "1.3.9" reqwest = { version = "0.10", features = ["blocking", "gzip", "json"] } scheduled-thread-pool = "0.2.0" semver = { version = "0.10", features = ["diesel", "serde"] } +sentry = "0.20.1" serde = { version = "1.0.0", features = ["derive"] } serde_json = "1.0.0" sha2 = "0.9" diff --git a/src/bin/server.rs b/src/bin/server.rs index 48d75864483..95dede536fd 100644 --- a/src/bin/server.rs +++ b/src/bin/server.rs @@ -2,6 +2,7 @@ use cargo_registry::{boot, App, Env}; use std::{ + borrow::Cow, fs::File, sync::{mpsc::channel, Arc, Mutex}, thread, @@ -12,6 +13,7 @@ use civet::Server as CivetServer; use conduit_hyper::Service; use futures_util::future::FutureExt; use reqwest::blocking::Client; +use sentry::{ClientOptions, IntoDsn}; const CORE_THREADS: usize = 4; @@ -24,6 +26,21 @@ enum Server { use Server::*; fn main() -> Result<(), Box> { + let _sentry = dotenv::var("SENTRY_DSN_API") + .ok() + .into_dsn() + .expect("SENTRY_DSN_API is not a valid Sentry DSN value") + .map(|dsn| { + let mut opts = ClientOptions::from(dsn); + opts.environment = Some( + dotenv::var("SENTRY_ENV_API") + .map(Cow::Owned) + .expect("SENTRY_ENV_API must be set when using SENTRY_DSN_API"), + ); + + sentry::init(opts) + }); + // Initialize logging env_logger::init(); From 5e7b95d1ff32e0b08d5946e2a75ca5aa354e129e Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Mon, 26 Oct 2020 01:00:41 +0100 Subject: [PATCH 2/5] middleware/log: Report errors and slow requests to Sentry --- src/middleware/log_request.rs | 105 +++++++++++++++++++++++++++++++++- 1 file changed, 104 insertions(+), 1 deletion(-) diff --git a/src/middleware/log_request.rs b/src/middleware/log_request.rs index b80dbbad779..8b8341d8a2a 100644 --- a/src/middleware/log_request.rs +++ b/src/middleware/log_request.rs @@ -3,8 +3,11 @@ use super::prelude::*; use crate::util::request_header; -use conduit::{header, RequestExt, StatusCode}; +use conduit::{header, Host, RequestExt, Scheme, StatusCode}; +use sentry::protocol::IpAddress; +use sentry::Level; use std::fmt::{self, Display, Formatter}; +use std::net::IpAddr; use std::time::Instant; const SLOW_REQUEST_THRESHOLD_MS: u64 = 1000; @@ -40,6 +43,8 @@ impl Middleware for LogRequests { } ); + report_to_sentry(req, &res, response_time); + res } } @@ -60,6 +65,104 @@ pub fn add_custom_metadata(req: &mut dyn RequestExt, key: &'static s } } +fn report_to_sentry(req: &dyn RequestExt, res: &AfterResult, response_time: u64) { + let (message, level) = match res { + Err(e) => (e.to_string(), Level::Error), + Ok(_) => { + if response_time <= SLOW_REQUEST_THRESHOLD_MS { + return; + } + + let message = format!("Slow Request: {} {}", req.method(), req.path()); + (message, Level::Info) + } + }; + + let config = |scope: &mut sentry::Scope| { + let method = Some(req.method().as_str().to_owned()); + + let scheme = match req.scheme() { + Scheme::Http => "http", + Scheme::Https => "https", + }; + + let host = match req.host() { + Host::Name(name) => name.to_owned(), + Host::Socket(addr) => addr.to_string(), + }; + + let path = &req.extensions().find::().unwrap().0; + + let url = format!("{}://{}{}", scheme, host, path).parse().ok(); + + { + let ip_addr = match req.headers().get("x-real-ip") { + Some(value) => value + .to_str() + .ok() + .and_then(|str| str.parse::().ok()), + None => Some(req.remote_addr().ip()), + }; + + let user = sentry::User { + ip_address: ip_addr.map(IpAddress::Exact), + ..Default::default() + }; + + scope.set_user(Some(user)); + } + + { + let headers = req + .headers() + .iter() + .map(|(k, v)| (k.to_string(), v.to_str().unwrap_or_default().to_string())) + .collect(); + + let sentry_req = sentry::protocol::Request { + method, + url, + headers, + ..Default::default() + }; + + scope.add_event_processor(Box::new(move |mut event| { + if event.request.is_none() { + event.request = Some(sentry_req.clone()); + } + Some(event) + })); + } + + if let Some(request_id) = req + .headers() + .get("x-request-id") + .and_then(|value| value.to_str().ok()) + { + scope.set_tag("request.id", request_id); + } + + { + let status = res + .as_ref() + .map(|resp| resp.status()) + .unwrap_or(StatusCode::INTERNAL_SERVER_ERROR); + + scope.set_tag("response.status", status.as_str()); + } + + scope.set_extra("Response time [ms]", response_time.into()); + + if let Some(metadata) = req.extensions().find::() { + for (key, value) in &metadata.entries { + scope.set_extra(key, value.to_string().into()); + } + } + }; + + sentry::with_scope(config, || sentry::capture_message(&message, level)); +} + #[cfg(test)] pub(crate) fn get_log_message(req: &dyn RequestExt, key: &'static str) -> String { // Unwrap shouldn't panic as no other code has access to the private struct to remove it From 593e18d51f6d9d9c318aa934d7dc10fd0b34dcea Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Mon, 26 Oct 2020 10:47:17 +0100 Subject: [PATCH 3/5] middleware/log: Replace IP address in `user` with ID from signed cookie --- src/middleware/log_request.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/middleware/log_request.rs b/src/middleware/log_request.rs index 8b8341d8a2a..7bad88eee6b 100644 --- a/src/middleware/log_request.rs +++ b/src/middleware/log_request.rs @@ -2,12 +2,11 @@ //! information that we care about like User-Agent use super::prelude::*; +use crate::middleware::current_user::TrustedUserId; use crate::util::request_header; use conduit::{header, Host, RequestExt, Scheme, StatusCode}; -use sentry::protocol::IpAddress; use sentry::Level; use std::fmt::{self, Display, Formatter}; -use std::net::IpAddr; use std::time::Instant; const SLOW_REQUEST_THRESHOLD_MS: u64 = 1000; @@ -96,16 +95,13 @@ fn report_to_sentry(req: &dyn RequestExt, res: &AfterResult, response_time: u64) let url = format!("{}://{}{}", scheme, host, path).parse().ok(); { - let ip_addr = match req.headers().get("x-real-ip") { - Some(value) => value - .to_str() - .ok() - .and_then(|str| str.parse::().ok()), - None => Some(req.remote_addr().ip()), - }; + let id = req + .extensions() + .find::() + .map(|x| x.0.to_string()); let user = sentry::User { - ip_address: ip_addr.map(IpAddress::Exact), + id, ..Default::default() }; From 7fd8bfaeab0a3f6b887558c9d00bfc17b19f9cde Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Mon, 26 Oct 2020 11:02:54 +0100 Subject: [PATCH 4/5] middleware/log: Filter out sensitive headers --- src/middleware/log_request.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/middleware/log_request.rs b/src/middleware/log_request.rs index 7bad88eee6b..212f1aa7a68 100644 --- a/src/middleware/log_request.rs +++ b/src/middleware/log_request.rs @@ -109,9 +109,12 @@ fn report_to_sentry(req: &dyn RequestExt, res: &AfterResult, response_time: u64) } { + let filtered_headers = vec!["Authorization", "Cookie", "X-Real-Ip"]; + let headers = req .headers() .iter() + .filter(|(k, _v)| !filtered_headers.iter().any(|name| k == name)) .map(|(k, v)| (k.to_string(), v.to_str().unwrap_or_default().to_string())) .collect(); From 29bfde80036fa73a5bb72cc4555da5a330b4f909 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Mon, 26 Oct 2020 13:09:19 +0100 Subject: [PATCH 5/5] middleware/log: Convert `filtered_headers` to `const` --- src/middleware/log_request.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/middleware/log_request.rs b/src/middleware/log_request.rs index 212f1aa7a68..7609cf17244 100644 --- a/src/middleware/log_request.rs +++ b/src/middleware/log_request.rs @@ -11,6 +11,8 @@ use std::time::Instant; const SLOW_REQUEST_THRESHOLD_MS: u64 = 1000; +const FILTERED_HEADERS: [&str; 3] = ["Authorization", "Cookie", "X-Real-Ip"]; + #[derive(Default)] pub(super) struct LogRequests(); @@ -109,12 +111,10 @@ fn report_to_sentry(req: &dyn RequestExt, res: &AfterResult, response_time: u64) } { - let filtered_headers = vec!["Authorization", "Cookie", "X-Real-Ip"]; - let headers = req .headers() .iter() - .filter(|(k, _v)| !filtered_headers.iter().any(|name| k == name)) + .filter(|(k, _v)| !FILTERED_HEADERS.iter().any(|name| k == name)) .map(|(k, v)| (k.to_string(), v.to_str().unwrap_or_default().to_string())) .collect();