From dbddcdaaea59709d47c5bdc62b5d2b52dd77e5fc Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Thu, 15 Jun 2023 08:13:50 -0700 Subject: [PATCH 1/4] add `fmt::Display` impl for `policy::Meta` --- linkerd/app/outbound/src/http/concrete.rs | 2 +- linkerd/proxy/client-policy/src/lib.rs | 23 ++++++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/linkerd/app/outbound/src/http/concrete.rs b/linkerd/app/outbound/src/http/concrete.rs index 0fbfcca0cd..e690530f6e 100644 --- a/linkerd/app/outbound/src/http/concrete.rs +++ b/linkerd/app/outbound/src/http/concrete.rs @@ -42,7 +42,7 @@ pub struct DispatcherFailed(Arc); /// Wraps errors encountered in this module. #[derive(Debug, thiserror::Error)] -#[error("{} {}.{}: {source}", backend.0.kind(), backend.0.name(), backend.0.namespace())] +#[error("{}: {source}", backend.0)] pub struct BalanceError { backend: BackendRef, #[source] diff --git a/linkerd/proxy/client-policy/src/lib.rs b/linkerd/proxy/client-policy/src/lib.rs index f525a041f0..92d942f241 100644 --- a/linkerd/proxy/client-policy/src/lib.rs +++ b/linkerd/proxy/client-policy/src/lib.rs @@ -2,7 +2,7 @@ #![forbid(unsafe_code)] use once_cell::sync::Lazy; -use std::{borrow::Cow, hash::Hash, net::SocketAddr, num::NonZeroU16, sync::Arc, time}; +use std::{borrow::Cow, fmt, hash::Hash, net::SocketAddr, num::NonZeroU16, sync::Arc, time}; pub mod grpc; pub mod http; @@ -264,6 +264,27 @@ impl std::hash::Hash for Meta { } } +impl fmt::Display for Meta { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Default { name } => write!(f, "default.{name}"), + Self::Resource { + kind, + name, + namespace, + port, + .. + } => { + write!(f, "{kind}.{namespace}.{name}")?; + if let Some(port) = port { + write!(f, ":{port}")? + } + Ok(()) + } + } + } +} + // === impl FailureAccrual === impl Default for FailureAccrual { From 3fb8920378ace999c169c3058a6da6a2bf5cdbf7 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Thu, 15 Jun 2023 08:38:21 -0700 Subject: [PATCH 2/4] wip --- .../outbound/src/http/logical/policy/route.rs | 18 +++++++++++++++++- .../src/http/logical/policy/route/backend.rs | 16 ++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/linkerd/app/outbound/src/http/logical/policy/route.rs b/linkerd/app/outbound/src/http/logical/policy/route.rs index 24224481a8..eb1d9d664d 100644 --- a/linkerd/app/outbound/src/http/logical/policy/route.rs +++ b/linkerd/app/outbound/src/http/logical/policy/route.rs @@ -9,7 +9,7 @@ use std::{fmt::Debug, hash::Hash, sync::Arc}; pub(crate) mod backend; pub(crate) mod filters; -pub(crate) use self::backend::{Backend, MatchedBackend}; +pub(crate) use self::backend::{Backend, BackendError, MatchedBackend}; pub use self::filters::errors; /// A target type that includes a summary of exactly how a request was matched. @@ -50,6 +50,15 @@ pub(crate) type Grpc = MatchedRoute< pub(crate) type BackendDistribution = distribute::Distribution>; pub(crate) type NewDistribute = distribute::NewDistribute, (), N>; +/// Wraps errors encountered in this module. +#[derive(Debug, thiserror::Error)] +#[error("{}: {source}", route.0)] +pub struct RouteError { + route: RouteRef, + #[source] + source: Error, +} + // === impl MatchedRoute === impl MatchedRoute @@ -115,6 +124,13 @@ where // Sets an optional request timeout. .push(http::NewTimeout::layer()) .push(classify::NewClassify::layer()) + .push(svc::NewMapErr::layer_with(|rt: Self| { + let route = rt.params.route_ref.clone(); + move |source| RouteError { + route: route.clone(), + source, + } + })) .push(svc::ArcNewService::layer()) .into_inner() }) diff --git a/linkerd/app/outbound/src/http/logical/policy/route/backend.rs b/linkerd/app/outbound/src/http/logical/policy/route/backend.rs index a0d335c000..2ae8ced4f8 100644 --- a/linkerd/app/outbound/src/http/logical/policy/route/backend.rs +++ b/linkerd/app/outbound/src/http/logical/policy/route/backend.rs @@ -29,6 +29,15 @@ pub struct ExtractMetrics { metrics: RouteBackendMetrics, } +/// Wraps errors encountered in this module. +#[derive(Debug, thiserror::Error)] +#[error("backend {}: {source}", backend.0)] +pub(crate) struct BackendError { + backend: BackendRef, + #[source] + source: Error, +} + // === impl Backend === impl Clone for Backend { @@ -110,6 +119,13 @@ where .push(count_reqs::NewCountRequests::layer_via(ExtractMetrics { metrics: metrics.clone(), })) + .push(svc::NewMapErr::layer_with(|t: Self| { + let backend = t.params.concrete.backend_ref.clone(); + move |source| BackendError { + backend: backend.clone(), + source, + } + })) .push(svc::ArcNewService::layer()) .into_inner() }) From b874347f57ec1fe44f5a09f9d2125e7f17796fd5 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Thu, 15 Jun 2023 09:03:44 -0700 Subject: [PATCH 3/4] outbound: add route and backend metadata to errors --- .../app/outbound/src/http/logical/policy/route.rs | 12 +++++++----- .../src/http/logical/policy/route/backend.rs | 12 +++++++----- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/linkerd/app/outbound/src/http/logical/policy/route.rs b/linkerd/app/outbound/src/http/logical/policy/route.rs index eb1d9d664d..c3578273c4 100644 --- a/linkerd/app/outbound/src/http/logical/policy/route.rs +++ b/linkerd/app/outbound/src/http/logical/policy/route.rs @@ -9,7 +9,7 @@ use std::{fmt::Debug, hash::Hash, sync::Arc}; pub(crate) mod backend; pub(crate) mod filters; -pub(crate) use self::backend::{Backend, BackendError, MatchedBackend}; +pub(crate) use self::backend::{Backend, MatchedBackend}; pub use self::filters::errors; /// A target type that includes a summary of exactly how a request was matched. @@ -124,11 +124,13 @@ where // Sets an optional request timeout. .push(http::NewTimeout::layer()) .push(classify::NewClassify::layer()) - .push(svc::NewMapErr::layer_with(|rt: Self| { + .push(svc::NewMapErr::layer_with(|rt: &Self| { let route = rt.params.route_ref.clone(); - move |source| RouteError { - route: route.clone(), - source, + move |source| { + Error::from(RouteError { + route: route.clone(), + source, + }) } })) .push(svc::ArcNewService::layer()) diff --git a/linkerd/app/outbound/src/http/logical/policy/route/backend.rs b/linkerd/app/outbound/src/http/logical/policy/route/backend.rs index 2ae8ced4f8..68bcb3bce8 100644 --- a/linkerd/app/outbound/src/http/logical/policy/route/backend.rs +++ b/linkerd/app/outbound/src/http/logical/policy/route/backend.rs @@ -1,5 +1,5 @@ use super::{super::Concrete, filters}; -use crate::RouteRef; +use crate::{BackendRef, RouteRef}; use linkerd_app_core::{proxy::http, svc, Error, Result}; use linkerd_http_route as http_route; use linkerd_proxy_client_policy as policy; @@ -119,11 +119,13 @@ where .push(count_reqs::NewCountRequests::layer_via(ExtractMetrics { metrics: metrics.clone(), })) - .push(svc::NewMapErr::layer_with(|t: Self| { + .push(svc::NewMapErr::layer_with(|t: &Self| { let backend = t.params.concrete.backend_ref.clone(); - move |source| BackendError { - backend: backend.clone(), - source, + move |source| { + Error::from(BackendError { + backend: backend.clone(), + source, + }) } })) .push(svc::ArcNewService::layer()) From d7ae54debad3545527fc506650960eab7afbbc7f Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Thu, 15 Jun 2023 09:11:02 -0700 Subject: [PATCH 4/4] comments/misc --- linkerd/app/outbound/src/http/logical/policy/route.rs | 6 +++--- .../app/outbound/src/http/logical/policy/route/backend.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/linkerd/app/outbound/src/http/logical/policy/route.rs b/linkerd/app/outbound/src/http/logical/policy/route.rs index c3578273c4..3ef64224cc 100644 --- a/linkerd/app/outbound/src/http/logical/policy/route.rs +++ b/linkerd/app/outbound/src/http/logical/policy/route.rs @@ -50,10 +50,10 @@ pub(crate) type Grpc = MatchedRoute< pub(crate) type BackendDistribution = distribute::Distribution>; pub(crate) type NewDistribute = distribute::NewDistribute, (), N>; -/// Wraps errors encountered in this module. +/// Wraps errors with route metadata. #[derive(Debug, thiserror::Error)] -#[error("{}: {source}", route.0)] -pub struct RouteError { +#[error("route {}: {source}", route.0)] +struct RouteError { route: RouteRef, #[source] source: Error, diff --git a/linkerd/app/outbound/src/http/logical/policy/route/backend.rs b/linkerd/app/outbound/src/http/logical/policy/route/backend.rs index 68bcb3bce8..5d0fb3b804 100644 --- a/linkerd/app/outbound/src/http/logical/policy/route/backend.rs +++ b/linkerd/app/outbound/src/http/logical/policy/route/backend.rs @@ -29,10 +29,10 @@ pub struct ExtractMetrics { metrics: RouteBackendMetrics, } -/// Wraps errors encountered in this module. +/// Wraps errors with backend metadata. #[derive(Debug, thiserror::Error)] #[error("backend {}: {source}", backend.0)] -pub(crate) struct BackendError { +struct BackendError { backend: BackendRef, #[source] source: Error,