Skip to content

Commit

Permalink
feat(app): Add hostname label to route metrics (#3258)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
cratelyn authored Oct 15, 2024
1 parent 618838e commit b728ffe
Show file tree
Hide file tree
Showing 6 changed files with 292 additions and 62 deletions.
51 changes: 32 additions & 19 deletions linkerd/app/outbound/src/http/logical/policy/route.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ pub(crate) mod retry;

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

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

Expand Down Expand Up @@ -117,16 +116,35 @@ where
.push(MatchedBackend::layer(metrics.backend.clone()))
.lift_new_with_target()
.push(NewDistribute::layer())
.check_new::<Self>()
.check_new_service::<Self, http::Request<http::BoxBody>>()
// The router does not take the backend's availability into
// consideration, so we must eagerly fail requests to prevent
// leaking tasks onto the runtime.
.push_on_service(svc::LoadShed::layer())
.push(filters::NewApplyFilters::<Self, _, _>::layer())
.push(retry::NewHttpRetry::layer(metrics.retry.clone()))
.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 metrics = metrics.retry.clone();
retry::NewHttpRetry::layer_via_mk(mk_extract, metrics)
})
.check_new::<Self>()
.check_new_service::<Self, http::Request<http::BoxBody>>()
// Set request extensions based on the route configuration
// AND/OR headers
.push(extensions::NewSetExtensions::layer())
.push(metrics::layer(&metrics.requests))
.check_new::<Self>()
.check_new_service::<Self, http::Request<http::BoxBody>>()
// Configure a classifier to use in the endpoint stack.
// TODO(ver) move this into NewSetExtensions?
.push(classify::NewClassify::layer())
Expand All @@ -149,15 +167,6 @@ impl<T: Clone, M, F, P> svc::Param<BackendDistribution<T, F>> for MatchedRoute<T
}
}

impl<T: Clone, M, F, P> svc::Param<RouteLabels> for MatchedRoute<T, M, F, P> {
fn param(&self) -> RouteLabels {
RouteLabels(
self.params.parent_ref.clone(),
self.params.route_ref.clone(),
)
}
}

// === impl Http ===

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

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(),
)))
}
}

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

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::LabelGrpcRsp::from(metrics::labels::Route::from((
parent, route,
))))
Some(metrics::LabelGrpcRsp::from(metrics::labels::Route::new(
parent,
route,
req.uri(),
)))
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
//! Prometheus label types.
use linkerd_app_core::{errors, metrics::prom::EncodeLabelSetMut, proxy::http, Error as BoxError};
use linkerd_app_core::{
dns, errors, metrics::prom::EncodeLabelSetMut, proxy::http, Error as BoxError,
};
use prometheus_client::encoding::*;

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

#[derive(Clone, Debug, Hash, PartialEq, Eq)]
pub struct Route(pub ParentRef, pub RouteRef);
pub struct Route {
parent: ParentRef,
route: RouteRef,
hostname: Option<dns::Name>,
}

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

// === impl Route ===

impl From<(ParentRef, RouteRef)> for Route {
fn from((parent, route): (ParentRef, RouteRef)) -> Self {
Self(parent, route)
impl Route {
pub fn new(parent: ParentRef, route: RouteRef, uri: &http::uri::Uri) -> Self {
let hostname = uri
.host()
.map(str::as_bytes)
.map(dns::Name::try_from_ascii)
.and_then(Result::ok);

Self {
parent,
route,
hostname,
}
}

#[cfg(test)]
pub(super) fn new_with_name(
parent: ParentRef,
route: RouteRef,
hostname: Option<dns::Name>,
) -> Self {
Self {
parent,
route,
hostname,
}
}
}

impl EncodeLabelSetMut for Route {
fn encode_label_set(&self, enc: &mut LabelSetEncoder<'_>) -> std::fmt::Result {
let Self(parent, route) = self;
let Self {
parent,
route,
hostname,
} = self;

parent.encode_label_set(enc)?;
route.encode_label_set(enc)?;
("hostname", hostname.as_deref()).encode(enc.encode_label())?;

Ok(())
}
}
Expand Down
Loading

0 comments on commit b728ffe

Please sign in to comment.