Skip to content

Commit 9f0a269

Browse files
authored
outbound: add backend and route metadata to errors (#2428)
PRs #2418 and #2419 add per-route and per-backend request timeouts configured by the `OutboundPolicies` API to the `MatchedRoute` and `MatchedBackend` layers in the outbound `ClientPolicy` stack, respectively. This means that — unlike in the `ServiceProfile` stack — two separate request timeouts can be configured in `ClientPolicy` stacks. However, because both the `MatchedRoute` and `MatchedBackend` layers are in the HTTP logical stack, the errors emitted by both timeouts will have a `LogicalError` as their most specific error metadata, meaning that the log messages and `l5d-proxy-error` headers recorded for these timeouts do not indicate whether the timeout that failed the request was the route request timeout or the backend request timeout. In order to ensure this information is recorded and exposed to the user, this branch adds two new error wrapper types, one of which enriches an error with a `RouteRef`'s metadata, and one of which enriches an error with a `BackendRef`'s metadata. The `MatchedRoute` stack now wraps all errors with `RouteRef` metadata, and the `MatchedBackend` stack wraps errors with `BackendRef` metadata. This way, when the route timeout fails a request, the error will include the route metadata, while when the backend request timeout fails a request, the error will include both the route and backend metadata. Adding these new error wrappers also has the additional side benefit of adding this metadata to errors returned by filters, allowing users to distinguish between errors emitted by a filter on a route rule and errors emitted by a per-backend filter. Also, any other errors emitted lower in the stack for requests that are handled by a client policy stack will now also include this metadata, which seems generally useful. Example errors, taken from a proxy unit test: backend request: ``` logical service logical.test.svc.cluster.local:666: route httproute.test.timeout-route: backend service.test.test-svc:666: HTTP response timeout after 1s ``` route request: ``` logical service logical.test.svc.cluster.local:666: route httproute.test.timeout-route: HTTP response timeout after 2s ```
1 parent 966306b commit 9f0a269

File tree

4 files changed

+60
-3
lines changed

4 files changed

+60
-3
lines changed

linkerd/app/outbound/src/http/concrete.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ pub struct DispatcherFailed(Arc<str>);
4242

4343
/// Wraps errors encountered in this module.
4444
#[derive(Debug, thiserror::Error)]
45-
#[error("{} {}.{}: {source}", backend.0.kind(), backend.0.name(), backend.0.namespace())]
45+
#[error("{}: {source}", backend.0)]
4646
pub struct BalanceError {
4747
backend: BackendRef,
4848
#[source]

linkerd/app/outbound/src/http/logical/policy/route.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,15 @@ pub(crate) type Grpc<T> = MatchedRoute<
5050
pub(crate) type BackendDistribution<T, F> = distribute::Distribution<Backend<T, F>>;
5151
pub(crate) type NewDistribute<T, F, N> = distribute::NewDistribute<Backend<T, F>, (), N>;
5252

53+
/// Wraps errors with route metadata.
54+
#[derive(Debug, thiserror::Error)]
55+
#[error("route {}: {source}", route.0)]
56+
struct RouteError {
57+
route: RouteRef,
58+
#[source]
59+
source: Error,
60+
}
61+
5362
// === impl MatchedRoute ===
5463

5564
impl<T, M, F, E> MatchedRoute<T, M, F, E>
@@ -115,6 +124,15 @@ where
115124
// Sets an optional request timeout.
116125
.push(http::NewTimeout::layer())
117126
.push(classify::NewClassify::layer())
127+
.push(svc::NewMapErr::layer_with(|rt: &Self| {
128+
let route = rt.params.route_ref.clone();
129+
move |source| {
130+
Error::from(RouteError {
131+
route: route.clone(),
132+
source,
133+
})
134+
}
135+
}))
118136
.push(svc::ArcNewService::layer())
119137
.into_inner()
120138
})

linkerd/app/outbound/src/http/logical/policy/route/backend.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use super::{super::Concrete, filters};
2-
use crate::RouteRef;
2+
use crate::{BackendRef, RouteRef};
33
use linkerd_app_core::{proxy::http, svc, Error, Result};
44
use linkerd_http_route as http_route;
55
use linkerd_proxy_client_policy as policy;
@@ -30,6 +30,15 @@ pub struct ExtractMetrics {
3030
metrics: RouteBackendMetrics,
3131
}
3232

33+
/// Wraps errors with backend metadata.
34+
#[derive(Debug, thiserror::Error)]
35+
#[error("backend {}: {source}", backend.0)]
36+
struct BackendError {
37+
backend: BackendRef,
38+
#[source]
39+
source: Error,
40+
}
41+
3342
// === impl Backend ===
3443

3544
impl<T: Clone, F> Clone for Backend<T, F> {
@@ -113,6 +122,15 @@ where
113122
.push(count_reqs::NewCountRequests::layer_via(ExtractMetrics {
114123
metrics: metrics.clone(),
115124
}))
125+
.push(svc::NewMapErr::layer_with(|t: &Self| {
126+
let backend = t.params.concrete.backend_ref.clone();
127+
move |source| {
128+
Error::from(BackendError {
129+
backend: backend.clone(),
130+
source,
131+
})
132+
}
133+
}))
116134
.push(svc::ArcNewService::layer())
117135
.into_inner()
118136
})

linkerd/proxy/client-policy/src/lib.rs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
#![forbid(unsafe_code)]
33

44
use once_cell::sync::Lazy;
5-
use std::{borrow::Cow, hash::Hash, net::SocketAddr, num::NonZeroU16, sync::Arc, time};
5+
use std::{borrow::Cow, fmt, hash::Hash, net::SocketAddr, num::NonZeroU16, sync::Arc, time};
66

77
pub mod grpc;
88
pub mod http;
@@ -264,6 +264,27 @@ impl std::hash::Hash for Meta {
264264
}
265265
}
266266

267+
impl fmt::Display for Meta {
268+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
269+
match self {
270+
Self::Default { name } => write!(f, "default.{name}"),
271+
Self::Resource {
272+
kind,
273+
name,
274+
namespace,
275+
port,
276+
..
277+
} => {
278+
write!(f, "{kind}.{namespace}.{name}")?;
279+
if let Some(port) = port {
280+
write!(f, ":{port}")?
281+
}
282+
Ok(())
283+
}
284+
}
285+
}
286+
}
287+
267288
// === impl FailureAccrual ===
268289

269290
impl Default for FailureAccrual {

0 commit comments

Comments
 (0)