diff --git a/export-webhook/declares.json b/export-webhook/declares.json index c94344c..1614a8f 100644 --- a/export-webhook/declares.json +++ b/export-webhook/declares.json @@ -24,8 +24,8 @@ "code": 7071, "slug": "webhook-delivery-non-2xx", "title": "Webhook log delivery returned non-2xx (log dropped)", - "severity": "benign_recurring", - "summary": "A request-log webhook delivery got a non-2xx response from the sink, so that one log line was dropped (deliveries are fire-and-forget and never retried). This can recur per request when a sink is unhealthy, so it is logged at debug.", + "severity": "actionable", + "summary": "A request-log webhook delivery got a non-2xx response from the sink, so that one log line was dropped (deliveries are fire-and-forget and never retried). It is warned on every dropped line, as 1.5.5 warned it: an unhealthy sink is the operator's to fix.", "action": "If logs are being lost, check the webhook sink's health and the delivery counters. `busbar_webhook_logs_dropped_total` tracks the volume.", "since": "1.6.0" }, @@ -33,8 +33,8 @@ "code": 7072, "slug": "webhook-delivery-transport-error", "title": "Webhook log delivery transport error (log dropped)", - "severity": "benign_recurring", - "summary": "A request-log webhook delivery failed with a transport error (connection/timeout/DNS), so that one log line was dropped. Deliveries are fire-and-forget and never retried; this can recur per request when a sink is unreachable, so it is logged at debug.", + "severity": "actionable", + "summary": "A request-log webhook delivery failed with a transport error (connection/timeout/DNS), so that one log line was dropped. Deliveries are fire-and-forget and never retried. It is warned on every dropped line, as 1.5.5 warned it: an unreachable sink is the operator's to fix.", "action": "If logs are being lost, check the webhook sink's reachability and the delivery counters. `busbar_webhook_logs_dropped_total` tracks the volume.", "since": "1.6.0" } diff --git a/export-webhook/src/lib.rs b/export-webhook/src/lib.rs index ff6ab41..adb8e50 100644 --- a/export-webhook/src/lib.rs +++ b/export-webhook/src/lib.rs @@ -15,10 +15,11 @@ //! connection only), and the request rides the SDK's framed `exchange` under the instance's //! `delivery_timeout_secs`. The sink never dials. A delivery is fire-and-forget and never retried: //! a non-2xx answer or a transport failure drops that one line and raises BUSBAR-7071 / -//! BUSBAR-7072 at debug. +//! BUSBAR-7072 at WARN, as 1.5.5 warned them. //! //! **Admission.** The first delivery asks the host for its verdict on the target (the need's -//! admission at bind); a refused target raises BUSBAR-7070 (`…; disabling this webhook exporter`) +//! admission at bind); a refused target raises BUSBAR-7070 (`…; disabling this webhook exporter`, +//! in 1.5.5's words when the refusal is the `https://` scheme rule) //! once and the instance takes no delivery until a reload — its siblings keep delivering. //! //! **Settings** are checked in two phases, as they always were: their SHAPE while the configuration @@ -214,7 +215,9 @@ impl Webhook { let ok = match verdict { Ok(()) => true, Err(e) => { - tracing::error!(diag = %DISABLED, "{e}; disabling this webhook exporter"); + let url = self.settings().map(|s| s.url); + let words = refusal_words(url.as_deref(), &e); + tracing::error!(diag = %DISABLED, "{words}; disabling this webhook exporter"); false } }; @@ -246,18 +249,39 @@ pub fn request(s: &Settings, line: &[u8]) -> Request { } } +/// BUSBAR-7070's words for a refused target. When the host refused it and the target is not an +/// `https://` URL, the refusal is the scheme rule, and it is said in 1.5.5's words byte for byte +/// (v1.5.5 `crates/busbar/src/observability.rs` `validate_webhook_url`; the scheme read without +/// case, as there). Any other refusal is said in the host's words. +pub fn refusal_words(url: Option<&str>, refused: &ConnFailure) -> String { + match (url, refused) { + (Some(u), ConnFailure::Refused(_)) if !is_https(u) => format!( + "observability.request_log_webhook_url must be an https:// URL (got '{}')", + mask_userinfo(u) + ), + _ => refused.to_string(), + } +} + +/// `url` names the `https` scheme, read without case (RFC 3986; 1.5.5's `scheme_is`). +fn is_https(url: &str) -> bool { + url.split_once("://") + .is_some_and(|(scheme, _)| scheme.eq_ignore_ascii_case("https")) +} + /// What became of one line: nothing to say for a 2xx; BUSBAR-7071 for any other status, -/// BUSBAR-7072 for a transport failure — each at debug, the target's userinfo masked. +/// BUSBAR-7072 for a transport failure — each at WARN, as 1.5.5 warned them +/// (`warn_webhook_delivery_failed`), the target's userinfo masked. pub fn report(url: &str, answered: Result) { match answered { Ok(r) if (200..300).contains(&r.status) => {} - Ok(r) => tracing::debug!( + Ok(r) => tracing::warn!( diag = %NON_2XX, webhook_url = ?mask_userinfo(url), status = %r.status, "request-log webhook delivery returned a non-2xx status; this log was dropped" ), - Err(e) => tracing::debug!( + Err(e) => tracing::warn!( diag = %TRANSPORT_ERROR, webhook_url = ?mask_userinfo(url), error_kind = %e, diff --git a/export-webhook/src/tests.rs b/export-webhook/src/tests.rs index d6fae9e..60c40f6 100644 --- a/export-webhook/src/tests.rs +++ b/export-webhook/src/tests.rs @@ -132,6 +132,7 @@ impl tracing::Subscriber for Capture { fn event(&self, event: &tracing::Event<'_>) { let mut v = Fields(Vec::new()); event.record(&mut v); + v.0.push(("level".to_string(), event.metadata().level().to_string())); self.0.lock().unwrap().push(v.0); } fn enter(&self, _: &tracing::span::Id) {} @@ -193,8 +194,9 @@ fn a_delivery_posts_the_line() { assert_eq!(request(&bare, b"{}").timeout_ms, 2000); } -/// A non-2xx answer and a transport failure each drop the line and raise their code at debug, -/// with the fields in the compiled-in site's order, the target's userinfo masked. +/// A non-2xx answer and a transport failure each drop the line and raise their code at WARN (1.5.5's +/// `warn_webhook_delivery_failed`), with the fields in the compiled-in site's order, the target's +/// userinfo masked. #[test] fn a_failed_delivery_raises_its_code_once() { let url = "https://user:pw@siem.example/in"; @@ -206,7 +208,7 @@ fn a_failed_delivery_raises_its_code_once() { let names = |e: &Event| { e.iter() .map(|(n, _)| n.clone()) - .filter(|n| n != "message") + .filter(|n| n != "message" && n != "level") .collect::>() }; assert_eq!(got.len(), 2, "{got:?}"); @@ -220,6 +222,8 @@ fn a_failed_delivery_raises_its_code_once() { assert_eq!(names(&got[1]), ["diag", "webhook_url", "error_kind"]); assert_eq!(field(&got[1], "diag"), "BUSBAR-7072"); assert_eq!(field(&got[1], "error_kind"), "timed out"); + assert_eq!(field(&got[0], "level"), "WARN"); + assert_eq!(field(&got[1], "level"), "WARN"); assert_eq!( field(&got[1], "message"), "request-log webhook delivery failed (transport error); this log was dropped" @@ -235,6 +239,31 @@ fn the_success_boundary_is_200_to_299() { } } +/// A target the host refused is named in 1.5.5's words when the refusal is the `https://` scheme rule +/// (the scheme read without case; userinfo masked as 1.5.5 masked it), and in the host's words +/// otherwise; a host that gave no verdict is named as such. +#[test] +fn a_refused_target_is_named_in_1_5_5_words_when_it_is_the_scheme() { + let refused = ConnFailure::Refused("destination refused".into()); + assert_eq!( + refusal_words(Some("http://127.0.0.1:4000/sidecar"), &refused), + "observability.request_log_webhook_url must be an https:// URL (got 'http://127.0.0.1:4000/sidecar')" + ); + assert_eq!( + refusal_words(Some("HTTP://u:p@siem.example/in"), &refused), + "observability.request_log_webhook_url must be an https:// URL (got 'http://***@siem.example/in')" + ); + assert_eq!( + refusal_words(Some("HTTPS://siem.example/in"), &refused), + "destination refused" + ); + assert_eq!( + refusal_words(Some("http://siem.example/in"), &ConnFailure::Unarmed), + "the instance was handed no connector" + ); + assert_eq!(refusal_words(None, &refused), "destination refused"); +} + /// With no verdict to be had, the instance is disabled once, in BUSBAR-7070's words, and stays so /// until a reload asks again. #[test] @@ -246,6 +275,7 @@ fn an_unadmitted_target_disables_the_instance_once() { }); assert_eq!(got.len(), 1, "{got:?}"); assert_eq!(field(&got[0], "diag"), "BUSBAR-7070"); + assert_eq!(field(&got[0], "level"), "ERROR"); assert_eq!( field(&got[0], "message"), "the instance was handed no connector; disabling this webhook exporter"