Skip to content

Commit

Permalink
refactor(app): Outline route label extractor (#3337)
Browse files Browse the repository at this point in the history
* refactor(app): Hoist label extractor out of retry middleware

this commit promotes the `RetryLabelExtract` type out of the route retry
middleware, and into the metrics submodule.

in preparation for generalizing this component, we rename it to
`RouteLabelExtract`.

Signed-off-by: katelyn martin <[email protected]>

* refactor(app): Outline route label extraction

this addresses the `TODO` comment, moving the `RouteLabelExtract` type
out of the policy route layer function.

this defines a method on `MatchedRoute` which may be called to retrieve
a `RouteLabelExtract`. that type holds references to the
contruction-time information about the route, and can be used to later
inspect an outbound request at call-time, returning a `labels::Route`
set of labels.

this is a helpful piece of reusable glue that is broadly useful in
instrumenting our route-level middleware.

Signed-off-by: katelyn martin <[email protected]>

---------

Signed-off-by: katelyn martin <[email protected]>
  • Loading branch information
cratelyn authored Nov 21, 2024
1 parent 049e01b commit e190dae
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 30 deletions.
13 changes: 2 additions & 11 deletions linkerd/app/outbound/src/http/logical/policy/route.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,18 +124,9 @@ where
.push_on_service(svc::LoadShed::layer())
.push(filters::NewApplyFilters::<Self, _, _>::layer())
.push({
// TODO(kate): extracting route labels like this should ideally live somewhere
// else, like e.g. the `SetExtensions` middleware.
let mk_extract = |rt: &Self| {
let Route {
parent_ref,
route_ref,
..
} = &rt.params;
retry::RetryLabelExtract(parent_ref.clone(), route_ref.clone())
};
let mk = Self::label_extractor;
let metrics = metrics.retry.clone();
retry::NewHttpRetry::layer_via_mk(mk_extract, metrics)
retry::NewHttpRetry::layer_via_mk(mk, metrics)
})
.check_new::<Self>()
.check_new_service::<Self, http::Request<http::BoxBody>>()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Prometheus label types.
use linkerd_app_core::{
dns, errors, metrics::prom::EncodeLabelSetMut, proxy::http, Error as BoxError,
dns, errors, metrics::prom::EncodeLabelSetMut, proxy::http, svc::ExtractParam,
Error as BoxError,
};
use prometheus_client::encoding::*;

Expand Down Expand Up @@ -39,6 +40,9 @@ pub struct GrpcRsp {
pub error: Option<Error>,
}

#[derive(Clone, Debug)]
pub struct RouteLabelExtract(pub ParentRef, pub RouteRef);

#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
pub enum Error {
FailFast,
Expand Down Expand Up @@ -211,6 +215,37 @@ impl EncodeLabelSet for GrpcRsp {
}
}

// === impl MatchedRoute ===

impl<T, M, F, P> super::super::MatchedRoute<T, M, F, P> {
/// Returns a [`RouteLabelExtract`].
///
/// The extractor returned by this function provides a [`ExtractParam<P, T>`] implementation
/// that can be used to acquire the route-level labels corresponding to a given outbound
/// request.
pub(crate) fn label_extractor(&self) -> RouteLabelExtract {
use super::super::Route;
let Route {
parent_ref,
route_ref,
..
} = &self.params;

RouteLabelExtract(parent_ref.clone(), route_ref.clone())
}
}

// === impl RouteLabelExtract ===

impl<B> ExtractParam<Route, http::Request<B>> for RouteLabelExtract {
fn extract_param(&self, t: &http::Request<B>) -> Route {
let Self(parent, route) = self;
let uri = t.uri();

Route::new(parent.clone(), route.clone(), uri)
}
}

// === impl Error ===

impl Error {
Expand Down
24 changes: 6 additions & 18 deletions linkerd/app/outbound/src/http/logical/policy/route/retry.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
use super::{extensions, metrics::labels::Route as RouteLabels};
use crate::{ParentRef, RouteRef};
use super::{
extensions,
metrics::labels::{Route as RouteLabels, RouteLabelExtract},
};
use futures::future::{Either, Ready};
use linkerd_app_core::{
cause_ref, classify,
exp_backoff::ExponentialBackoff,
is_caused_by,
proxy::http::{self, stream_timeouts::ResponseTimeoutError},
svc::{self, http::h2, ExtractParam},
svc::{self, http::h2},
Error, Result,
};
use linkerd_http_retry::{self as retry, peek_trailers::PeekTrailersBody};
Expand All @@ -18,7 +20,7 @@ use tokio::time;
pub struct IsRetry(());

pub type NewHttpRetry<F, N> =
retry::NewHttpRetry<RetryPolicy, RouteLabels, F, RetryLabelExtract, N>;
retry::NewHttpRetry<RetryPolicy, RouteLabels, F, RouteLabelExtract, N>;

#[derive(Clone, Debug)]
pub struct RetryPolicy {
Expand All @@ -31,9 +33,6 @@ pub struct RetryPolicy {
pub backoff: Option<ExponentialBackoff>,
}

#[derive(Clone, Debug)]
pub struct RetryLabelExtract(pub ParentRef, pub RouteRef);

pub type RouteRetryMetrics = retry::MetricFamilies<RouteLabels>;

// === impl RetryPolicy ===
Expand Down Expand Up @@ -163,14 +162,3 @@ impl RetryPolicy {
false
}
}

// === impl RetryLabelExtract ===

impl<B> ExtractParam<RouteLabels, http::Request<B>> for RetryLabelExtract {
fn extract_param(&self, t: &http::Request<B>) -> RouteLabels {
let Self(parent, route) = self;
let uri = t.uri();

RouteLabels::new(parent.clone(), route.clone(), uri)
}
}

0 comments on commit e190dae

Please sign in to comment.