Skip to content

Commit b728ffe

Browse files
authored
feat(app): Add hostname label to route metrics (#3258)
this commit introduces an additional label to HTTP and gRPC route metrics, containing the DNS host of requests. requests destined for an ip address of some kind are not recorded with a hostname metric, as a way to help minimize the impact of time series cardinality. the core of this change is in this field addition: ```diff // /linkerd/app/outbound/src/http/logical/policy/route/metrics/labels.rs -pub struct Route(pub ParentRef, pub RouteRef); +pub struct Route { + parent: ParentRef, + route: RouteRef, + hostname: Option<dns::Name>, +} ``` see this part of the change to our `MkStreamLabel` implementation, used in our metrics tracking request durtion, and counting response status codes: ```diff - fn mk_stream_labeler<B>(&self, _: &::http::Request<B>) -> Option<Self::StreamLabel> { + fn mk_stream_labeler<B>(&self, req: &::http::Request<B>) -> Option<Self::StreamLabel> { let parent = self.params.parent_ref.clone(); let route = self.params.route_ref.clone(); - Some(metrics::LabelHttpRsp::from(metrics::labels::Route::from(( - parent, route, - )))) + Some(metrics::LabelHttpRsp::from(metrics::labels::Route::new( + parent, + route, + req.uri(), + ))) } ``` we now inspect the request, and use the URI to label metrics related to this traffic by hostname. a `http_request_hostnames()` test case is added to exercise this. some todo comments are left, noting where we would ideally like to simplify or generalize the machinery related to `RetryLabelExtract`, the type that bridges the labels needed based on our `NewService<T>` target, and the request type later accepted by the instantiated `Service<T>`. Signed-off-by: katelyn martin <[email protected]>
1 parent 618838e commit b728ffe

File tree

6 files changed

+292
-62
lines changed

6 files changed

+292
-62
lines changed

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

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ pub(crate) mod retry;
1414

1515
pub(crate) use self::backend::{Backend, MatchedBackend};
1616
pub use self::filters::errors;
17-
use self::metrics::labels::Route as RouteLabels;
1817

1918
pub use self::metrics::{GrpcRouteMetrics, HttpRouteMetrics};
2019

@@ -117,16 +116,35 @@ where
117116
.push(MatchedBackend::layer(metrics.backend.clone()))
118117
.lift_new_with_target()
119118
.push(NewDistribute::layer())
119+
.check_new::<Self>()
120+
.check_new_service::<Self, http::Request<http::BoxBody>>()
120121
// The router does not take the backend's availability into
121122
// consideration, so we must eagerly fail requests to prevent
122123
// leaking tasks onto the runtime.
123124
.push_on_service(svc::LoadShed::layer())
124125
.push(filters::NewApplyFilters::<Self, _, _>::layer())
125-
.push(retry::NewHttpRetry::layer(metrics.retry.clone()))
126+
.push({
127+
// TODO(kate): extracting route labels like this should ideally live somewhere
128+
// else, like e.g. the `SetExtensions` middleware.
129+
let mk_extract = |rt: &Self| {
130+
let Route {
131+
parent_ref,
132+
route_ref,
133+
..
134+
} = &rt.params;
135+
retry::RetryLabelExtract(parent_ref.clone(), route_ref.clone())
136+
};
137+
let metrics = metrics.retry.clone();
138+
retry::NewHttpRetry::layer_via_mk(mk_extract, metrics)
139+
})
140+
.check_new::<Self>()
141+
.check_new_service::<Self, http::Request<http::BoxBody>>()
126142
// Set request extensions based on the route configuration
127143
// AND/OR headers
128144
.push(extensions::NewSetExtensions::layer())
129145
.push(metrics::layer(&metrics.requests))
146+
.check_new::<Self>()
147+
.check_new_service::<Self, http::Request<http::BoxBody>>()
130148
// Configure a classifier to use in the endpoint stack.
131149
// TODO(ver) move this into NewSetExtensions?
132150
.push(classify::NewClassify::layer())
@@ -149,15 +167,6 @@ impl<T: Clone, M, F, P> svc::Param<BackendDistribution<T, F>> for MatchedRoute<T
149167
}
150168
}
151169

152-
impl<T: Clone, M, F, P> svc::Param<RouteLabels> for MatchedRoute<T, M, F, P> {
153-
fn param(&self) -> RouteLabels {
154-
RouteLabels(
155-
self.params.parent_ref.clone(),
156-
self.params.route_ref.clone(),
157-
)
158-
}
159-
}
160-
161170
// === impl Http ===
162171

163172
impl<T> filters::Apply for Http<T> {
@@ -177,12 +186,14 @@ impl<T> metrics::MkStreamLabel for Http<T> {
177186
type DurationLabels = metrics::labels::Route;
178187
type StreamLabel = metrics::LabelHttpRouteRsp;
179188

180-
fn mk_stream_labeler<B>(&self, _: &::http::Request<B>) -> Option<Self::StreamLabel> {
189+
fn mk_stream_labeler<B>(&self, req: &::http::Request<B>) -> Option<Self::StreamLabel> {
181190
let parent = self.params.parent_ref.clone();
182191
let route = self.params.route_ref.clone();
183-
Some(metrics::LabelHttpRsp::from(metrics::labels::Route::from((
184-
parent, route,
185-
))))
192+
Some(metrics::LabelHttpRsp::from(metrics::labels::Route::new(
193+
parent,
194+
route,
195+
req.uri(),
196+
)))
186197
}
187198
}
188199

@@ -231,12 +242,14 @@ impl<T> metrics::MkStreamLabel for Grpc<T> {
231242
type DurationLabels = metrics::labels::Route;
232243
type StreamLabel = metrics::LabelGrpcRouteRsp;
233244

234-
fn mk_stream_labeler<B>(&self, _: &::http::Request<B>) -> Option<Self::StreamLabel> {
245+
fn mk_stream_labeler<B>(&self, req: &::http::Request<B>) -> Option<Self::StreamLabel> {
235246
let parent = self.params.parent_ref.clone();
236247
let route = self.params.route_ref.clone();
237-
Some(metrics::LabelGrpcRsp::from(metrics::labels::Route::from((
238-
parent, route,
239-
))))
248+
Some(metrics::LabelGrpcRsp::from(metrics::labels::Route::new(
249+
parent,
250+
route,
251+
req.uri(),
252+
)))
240253
}
241254
}
242255

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

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
11
//! Prometheus label types.
2-
use linkerd_app_core::{errors, metrics::prom::EncodeLabelSetMut, proxy::http, Error as BoxError};
2+
use linkerd_app_core::{
3+
dns, errors, metrics::prom::EncodeLabelSetMut, proxy::http, Error as BoxError,
4+
};
35
use prometheus_client::encoding::*;
46

57
use crate::{BackendRef, ParentRef, RouteRef};
68

79
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
8-
pub struct Route(pub ParentRef, pub RouteRef);
10+
pub struct Route {
11+
parent: ParentRef,
12+
route: RouteRef,
13+
hostname: Option<dns::Name>,
14+
}
915

1016
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
1117
pub struct RouteBackend(pub ParentRef, pub RouteRef, pub BackendRef);
@@ -52,17 +58,47 @@ pub enum Error {
5258

5359
// === impl Route ===
5460

55-
impl From<(ParentRef, RouteRef)> for Route {
56-
fn from((parent, route): (ParentRef, RouteRef)) -> Self {
57-
Self(parent, route)
61+
impl Route {
62+
pub fn new(parent: ParentRef, route: RouteRef, uri: &http::uri::Uri) -> Self {
63+
let hostname = uri
64+
.host()
65+
.map(str::as_bytes)
66+
.map(dns::Name::try_from_ascii)
67+
.and_then(Result::ok);
68+
69+
Self {
70+
parent,
71+
route,
72+
hostname,
73+
}
74+
}
75+
76+
#[cfg(test)]
77+
pub(super) fn new_with_name(
78+
parent: ParentRef,
79+
route: RouteRef,
80+
hostname: Option<dns::Name>,
81+
) -> Self {
82+
Self {
83+
parent,
84+
route,
85+
hostname,
86+
}
5887
}
5988
}
6089

6190
impl EncodeLabelSetMut for Route {
6291
fn encode_label_set(&self, enc: &mut LabelSetEncoder<'_>) -> std::fmt::Result {
63-
let Self(parent, route) = self;
92+
let Self {
93+
parent,
94+
route,
95+
hostname,
96+
} = self;
97+
6498
parent.encode_label_set(enc)?;
6599
route.encode_label_set(enc)?;
100+
("hostname", hostname.as_deref()).encode(enc.encode_label())?;
101+
66102
Ok(())
67103
}
68104
}

0 commit comments

Comments
 (0)