Skip to content
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

Sync internal commits #65

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .bleep
Original file line number Diff line number Diff line change
@@ -1 +1 @@
f123f5e43e9ada31a0e541b917ea674527fd06a3
d2eaddcd278a00f25792bf06f046b39aa321abe3
7 changes: 6 additions & 1 deletion docs/user_guide/panic.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
17 changes: 12 additions & 5 deletions pingora-core/src/protocols/http/v2/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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()
)
}
Expand Down
21 changes: 8 additions & 13 deletions pingora-core/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -62,14 +63,14 @@ pub struct Server {
shutdown_watch: watch::Sender<bool>,
// 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<ServerConf>,
/// the parser command line options
/// The parser command line options
pub options: Option<Opt>,
/// the Sentry DSN
/// The Sentry ClientOptions.
///
/// Panics and other events sentry captures will send to this DSN **only in release mode**
pub sentry: Option<String>,
/// Panics and other events sentry captures will be sent to this DSN **only in release mode**
pub sentry: Option<ClientOptions>,
}

// TODO: delete the pid when exit
Expand Down Expand Up @@ -256,10 +257,7 @@ 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()));

if self.options.as_ref().map_or(false, |o| o.test) {
info!("Server Test passed, exiting");
Expand Down Expand Up @@ -303,10 +301,7 @@ 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()));

let mut runtimes: Vec<Runtime> = Vec::new();

Expand Down
Loading