diff --git a/linkerd/app/inbound/src/http/router/metrics.rs b/linkerd/app/inbound/src/http/router/metrics.rs index a687864b35..234c1676db 100644 --- a/linkerd/app/inbound/src/http/router/metrics.rs +++ b/linkerd/app/inbound/src/http/router/metrics.rs @@ -1,21 +1,28 @@ +use self::{ + count_reqs::{ExtractRequestCount, NewCountRequests}, + labels::RouteLabels, + req_body::{ExtractRequestBodyDataParams, NewRecordRequestBodyData}, + req_duration::{ExtractRequestDurationMetrics, NewRequestDuration}, + rsp_body::{ExtractResponseBodyDataMetrics, NewRecordResponseBodyData}, + rsp_duration::{ExtractResponseDurationMetrics, NewResponseDuration}, + status::{ExtractStatusCodeParams, NewRecordStatusCode}, +}; use crate::InboundMetrics; use linkerd_app_core::svc; -pub use self::{ - count_reqs::*, labels::RouteLabels, req_body::*, rsp_body::*, rsp_duration::*, status::*, -}; - -mod count_reqs; -mod labels; -mod req_body; -mod rsp_body; -mod rsp_duration; -mod status; +pub mod count_reqs; +pub mod labels; +pub mod req_body; +pub mod req_duration; +pub mod rsp_body; +pub mod rsp_duration; +pub mod status; pub(super) fn layer( InboundMetrics { request_count, request_body_data, + request_duration, response_body_data, response_duration, status_codes, @@ -34,6 +41,11 @@ pub(super) fn layer( NewResponseDuration::layer_via(extract) }; + let request_duration = { + let extract = ExtractRequestDurationMetrics(request_duration.clone()); + NewRequestDuration::layer_via(extract) + }; + let response_body = { let extract = ExtractResponseBodyDataMetrics::new(response_body_data.clone()); NewRecordResponseBodyData::layer_via(extract) @@ -50,15 +62,17 @@ pub(super) fn layer( }; svc::layer::mk(move |inner| { - count.layer( - response_duration.layer(response_body.layer(request_body.layer(status.layer(inner)))), - ) + count.layer(response_duration.layer( + request_duration.layer(response_body.layer(request_body.layer(status.layer(inner)))), + )) }) } /// An `N`-typed service instrumented with metrics middleware. type Instrumented = NewCountRequests< NewResponseDuration< - NewRecordResponseBodyData>>, + NewRequestDuration< + NewRecordResponseBodyData>>, + >, >, >; diff --git a/linkerd/app/inbound/src/http/router/metrics/req_duration.rs b/linkerd/app/inbound/src/http/router/metrics/req_duration.rs new file mode 100644 index 0000000000..e765d18a4d --- /dev/null +++ b/linkerd/app/inbound/src/http/router/metrics/req_duration.rs @@ -0,0 +1,102 @@ +use super::RouteLabels; +use crate::policy::PermitVariant; +use linkerd_app_core::{ + metrics::prom::{self, EncodeLabelSetMut}, + svc, +}; +use linkerd_http_prom::{ + record_response::{self, Params}, + stream_label::with::MkWithLabels, +}; + +pub type NewRequestDuration = + record_response::NewRequestDuration; + +pub type RequestDurationParams = + Params>; + +#[derive(Clone, Debug)] +pub struct ExtractRequestDurationMetrics(pub RequestDurationFamilies); + +#[derive(Clone, Debug)] +pub struct RequestDurationFamilies { + grpc: record_response::RequestMetrics, + http: record_response::RequestMetrics, +} + +#[derive(Clone, Debug, Hash, PartialEq, Eq)] +pub struct RequestDurationLabels { + route: RouteLabels, +} + +pub type MkLabelDuration = MkWithLabels; + +// === impl RequestDurationFamilies === + +impl RequestDurationFamilies { + /// Registers a new [`RequestDurationFamilies`] with the given registry. + pub fn register( + reg: &mut prom::Registry, + histo: impl Clone + IntoIterator, + ) -> Self { + let grpc = { + let reg = reg.sub_registry_with_prefix("grpc"); + record_response::RequestMetrics::register(reg, histo.clone()) + }; + + let http = { + let reg = reg.sub_registry_with_prefix("http"); + record_response::RequestMetrics::register(reg, histo) + }; + + Self { grpc, http } + } +} + +// === impl ExtractRequestDurationMetrics === + +impl svc::ExtractParam for ExtractRequestDurationMetrics +where + T: svc::Param + svc::Param, +{ + fn extract_param(&self, target: &T) -> RequestDurationParams { + let Self(families) = self; + + let labeler = { + let route: RouteLabels = target.param(); + let labels = RequestDurationLabels { route }; + MkLabelDuration::new(labels) + }; + + let metric = { + let variant: PermitVariant = target.param(); + let RequestDurationFamilies { grpc, http } = families; + match variant { + PermitVariant::Grpc => grpc, + PermitVariant::Http => http, + } + .clone() + }; + + RequestDurationParams { labeler, metric } + } +} + +// === impl RequestDurationLabels === + +impl prom::EncodeLabelSetMut for RequestDurationLabels { + fn encode_label_set( + &self, + encoder: &mut prom::encoding::LabelSetEncoder<'_>, + ) -> std::fmt::Result { + let Self { route } = self; + route.encode_label_set(encoder)?; + Ok(()) + } +} + +impl prom::encoding::EncodeLabelSet for RequestDurationLabels { + fn encode(&self, mut encoder: prom::encoding::LabelSetEncoder<'_>) -> std::fmt::Result { + self.encode_label_set(&mut encoder) + } +} diff --git a/linkerd/app/inbound/src/metrics.rs b/linkerd/app/inbound/src/metrics.rs index f0a8a0fdc3..f16bbaaf7b 100644 --- a/linkerd/app/inbound/src/metrics.rs +++ b/linkerd/app/inbound/src/metrics.rs @@ -12,9 +12,11 @@ pub(crate) mod authz; pub(crate) mod error; use crate::http::router::metrics::{ - RequestBodyFamilies, RequestCountFamilies, ResponseBodyFamilies, ResponseDurationFamilies, - StatusCodeFamilies, + count_reqs::RequestCountFamilies, req_body::RequestBodyFamilies, + req_duration::RequestDurationFamilies, rsp_body::ResponseBodyFamilies, + rsp_duration::ResponseDurationFamilies, status::StatusCodeFamilies, }; + pub use linkerd_app_core::metrics::*; /// Holds LEGACY inbound proxy metrics. @@ -34,6 +36,7 @@ pub struct InboundMetrics { pub direct: crate::direct::MetricsFamilies, pub request_count: RequestCountFamilies, pub request_body_data: RequestBodyFamilies, + pub request_duration: RequestDurationFamilies, pub response_body_data: ResponseBodyFamilies, pub response_duration: ResponseDurationFamilies, pub status_codes: StatusCodeFamilies, @@ -48,6 +51,8 @@ impl InboundMetrics { ); let request_count = RequestCountFamilies::register(reg); let request_body_data = RequestBodyFamilies::register(reg); + let request_duration = + RequestDurationFamilies::register(reg, Self::REQUEST_BUCKETS.iter().copied()); let response_body_data = ResponseBodyFamilies::register(reg); let response_duration = ResponseDurationFamilies::register(reg, Self::RESPONSE_BUCKETS.iter().copied()); @@ -63,6 +68,7 @@ impl InboundMetrics { direct, request_count, request_body_data, + request_duration, response_body_data, response_duration, status_codes, @@ -70,17 +76,24 @@ impl InboundMetrics { } // There are two histograms for which we need to register metrics: - // (1) request durations, which are measured on routes. TODO(kate): forthcoming. + // (1) request durations, which are measured on routes. // (2) response durations, which are measured on route-backends. // // Should these change in the future, be sure to consider the outbound proxy's corresponding // constants measuring request and response latency for *outgoing* traffic. + /// Histogram buckets for request latency. + /// + /// Because request duration is the more meaningful metric operationally for the inbound + /// proxy, we opt to preserve higher fidelity for request durations (especially for lower + /// values). + const REQUEST_BUCKETS: &'static [f64] = &[0.025, 0.05, 0.1, 0.25, 0.5, 1.0, 10.0]; + /// Histogram buckets for response latency. /// - /// These buckets for this histogram are coarse, eliding several buckets for short response - /// durations to be conservative about the costs of tracking two histograms' respective time - /// series. + /// These buckets for this histogram are coarser than those of [`Self::REQUEST_BUCKETS`], + /// eliding several buckets for short response durations to be conservative about the costs of + /// tracking two histograms' respective time series. const RESPONSE_BUCKETS: &'static [f64] = &[0.05, 0.5, 1.0, 10.0]; } diff --git a/linkerd/app/inbound/src/policy/http.rs b/linkerd/app/inbound/src/policy/http.rs index 5ddfb81f50..83992cc223 100644 --- a/linkerd/app/inbound/src/policy/http.rs +++ b/linkerd/app/inbound/src/policy/http.rs @@ -1,6 +1,6 @@ use super::{RoutePolicy, Routes}; use crate::{ - http::router::metrics::RouteLabels, + http::router::metrics::labels::RouteLabels, metrics::authz::HttpAuthzMetrics, policy::{AllowPolicy, HttpRoutePermit}, };