From a8111f9a28d2b197c531ca48aaca734d54ca0f7d Mon Sep 17 00:00:00 2001 From: Congyu WANG Date: Wed, 7 Aug 2024 07:36:06 +0000 Subject: [PATCH 1/4] fix h2 request_summary --- also add port number --- also print query with path for h2 just as h1 Includes-commit: 2df4e29e180f5e79bee04aa4b4d83536e9f7f3a4 Includes-commit: 53e1810a27de581f6d5a9606198d9ac443f4a63c Includes-commit: 8102a0900bf30bf740e58c3a541bb86d296eb660 Replicated-from: https://github.com/cloudflare/pingora/pull/345 --- .bleep | 2 +- pingora-core/src/protocols/http/v2/server.rs | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/.bleep b/.bleep index 9c45175b..44a405d4 100644 --- a/.bleep +++ b/.bleep @@ -1 +1 @@ -f123f5e43e9ada31a0e541b917ea674527fd06a3 \ No newline at end of file +352835e0dc76946e3fd5e8c536d5c080d0b1ddb6 \ No newline at end of file diff --git a/pingora-core/src/protocols/http/v2/server.rs b/pingora-core/src/protocols/http/v2/server.rs index 5bd6bc02..447a4e30 100644 --- a/pingora-core/src/protocols/http/v2/server.rs +++ b/pingora-core/src/protocols/http/v2/server.rs @@ -20,6 +20,7 @@ use h2::server; use h2::server::SendResponse; use h2::{RecvStream, SendStream}; use http::header::HeaderName; +use http::uri::PathAndQuery; use http::{header, HeaderMap, Response}; use log::{debug, warn}; use pingora_http::{RequestHeader, ResponseHeader}; @@ -346,13 +347,19 @@ impl HttpSession { /// Return a string `$METHOD $PATH $HOST`. Mostly for logging and debug purpose pub fn request_summary(&self) -> String { format!( - "{} {}, Host: {}", + "{} {}, Host: {}:{}", self.request_header.method, - self.request_header.uri, self.request_header - .headers - .get(header::HOST) - .map(|v| String::from_utf8_lossy(v.as_bytes())) + .uri + .path_and_query() + .map(PathAndQuery::as_str) + .unwrap_or_default(), + self.request_header.uri.host().unwrap_or_default(), + self.req_header() + .uri + .port() + .as_ref() + .map(|port| port.as_str()) .unwrap_or_default() ) } From 5e6a2c67959bb9cbfd960f21c731a865cda0f21c Mon Sep 17 00:00:00 2001 From: Matthew Gumport Date: Mon, 12 Aug 2024 12:39:48 -0700 Subject: [PATCH 2/4] move cargo config to toml --- .bleep | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.bleep b/.bleep index 44a405d4..c40e0dc7 100644 --- a/.bleep +++ b/.bleep @@ -1 +1 @@ -352835e0dc76946e3fd5e8c536d5c080d0b1ddb6 \ No newline at end of file +bb9f706e0012bcb8d6734744c0fa8c4352969450 \ No newline at end of file From fd752da09fd00e5e5ccee33809a3065266e93ffe Mon Sep 17 00:00:00 2001 From: Matthew Gumport Date: Mon, 12 Aug 2024 09:56:11 -0700 Subject: [PATCH 3/4] add support for passing sentry release This changes the Sentry field from the DSN string to the entire ClientOptions struct. This will let us pass more information to the client SDK, including the git-parsed version information. --- .bleep | 2 +- pingora-core/src/server/mod.rs | 29 ++++++++++++++++------------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/.bleep b/.bleep index c40e0dc7..85a7e4b4 100644 --- a/.bleep +++ b/.bleep @@ -1 +1 @@ -bb9f706e0012bcb8d6734744c0fa8c4352969450 \ No newline at end of file +46345c2f7b66cf4460de73ca2e62c819ab588735 \ No newline at end of file diff --git a/pingora-core/src/server/mod.rs b/pingora-core/src/server/mod.rs index 8ebf6fde..5fdecb2e 100644 --- a/pingora-core/src/server/mod.rs +++ b/pingora-core/src/server/mod.rs @@ -22,6 +22,7 @@ use daemon::daemonize; use log::{debug, error, info, warn}; use pingora_runtime::Runtime; use pingora_timeout::fast_timeout; +use sentry::ClientOptions; use std::sync::Arc; use std::thread; use tokio::signal::unix; @@ -62,14 +63,14 @@ pub struct Server { shutdown_watch: watch::Sender, // TODO: we many want to drop this copy to let sender call closed() shutdown_recv: ShutdownWatch, - /// the parsed server configuration + /// The parsed server configuration pub configuration: Arc, - /// the parser command line options + /// The parser command line options pub options: Option, - /// the Sentry DSN + /// The Sentry ClientOptions. /// - /// Panics and other events sentry captures will send to this DSN **only in release mode** - pub sentry: Option, + /// Panics and other events sentry captures will be sent to this DSN **only in release mode** + pub sentry: Option, } // TODO: delete the pid when exit @@ -256,10 +257,11 @@ impl Server { /* only init sentry in release builds */ #[cfg(not(debug_assertions))] - let _guard = match self.sentry.as_ref() { - Some(uri) => Some(sentry::init(uri.as_str())), - None => None, - }; + let _guard = self + .sentry + .as_ref() + .map(|opts| sentry::init(opts.clone())) + .expect("sentry ClientOptions are valid"); if self.options.as_ref().map_or(false, |o| o.test) { info!("Server Test passed, exiting"); @@ -303,10 +305,11 @@ impl Server { /* only init sentry in release builds */ #[cfg(not(debug_assertions))] - let _guard = match self.sentry.as_ref() { - Some(uri) => Some(sentry::init(uri.as_str())), - None => None, - }; + let _guard = self + .sentry + .as_ref() + .map(|opts| sentry::init(opts.clone())) + .expect("sentry ClientOptions are valid"); let mut runtimes: Vec = Vec::new(); From c9d75714e5238c8e7e4a45ea074593c3988fd974 Mon Sep 17 00:00:00 2001 From: Matthew Gumport Date: Tue, 13 Aug 2024 18:21:13 -0700 Subject: [PATCH 4/4] fixup sentry unwrap Don't unconditionally unwrap Sentry, guard it with a default value. --- .bleep | 2 +- docs/user_guide/panic.md | 7 ++++++- pingora-core/src/server/mod.rs | 12 ++---------- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/.bleep b/.bleep index 85a7e4b4..ceecaa73 100644 --- a/.bleep +++ b/.bleep @@ -1 +1 @@ -46345c2f7b66cf4460de73ca2e62c819ab588735 \ No newline at end of file +d2eaddcd278a00f25792bf06f046b39aa321abe3 \ No newline at end of file diff --git a/docs/user_guide/panic.md b/docs/user_guide/panic.md index 58bf19d8..b2e0626e 100644 --- a/docs/user_guide/panic.md +++ b/docs/user_guide/panic.md @@ -4,7 +4,12 @@ Any panic that happens to particular requests does not affect other ongoing requ In order to monitor the panics, Pingora server has built-in Sentry integration. ```rust -my_server.sentry = Some("SENTRY_DSN"); +my_server.sentry = Some( + sentry::ClientOptions{ + dsn: "SENTRY_DSN".into_dsn().unwrap(), + ..Default::default() + } +); ``` Even though a panic is not fatal in Pingora, it is still not the preferred way to handle failures like network timeouts. Panics should be reserved for unexpected logic errors. diff --git a/pingora-core/src/server/mod.rs b/pingora-core/src/server/mod.rs index 5fdecb2e..c3659f2a 100644 --- a/pingora-core/src/server/mod.rs +++ b/pingora-core/src/server/mod.rs @@ -257,11 +257,7 @@ impl Server { /* only init sentry in release builds */ #[cfg(not(debug_assertions))] - let _guard = self - .sentry - .as_ref() - .map(|opts| sentry::init(opts.clone())) - .expect("sentry ClientOptions are valid"); + let _guard = self.sentry.as_ref().map(|opts| sentry::init(opts.clone())); if self.options.as_ref().map_or(false, |o| o.test) { info!("Server Test passed, exiting"); @@ -305,11 +301,7 @@ impl Server { /* only init sentry in release builds */ #[cfg(not(debug_assertions))] - let _guard = self - .sentry - .as_ref() - .map(|opts| sentry::init(opts.clone())) - .expect("sentry ClientOptions are valid"); + let _guard = self.sentry.as_ref().map(|opts| sentry::init(opts.clone())); let mut runtimes: Vec = Vec::new();