feat(app/inbound): introduce response duration metrics (#4420)

* feat(app/inbound): introduce response duration metrics

this commit introduces a new middleware layer to the inbound proxy.
this instruments inbound traffic with Prometheus telemetry that records
response body latency, and emits a histogram of response body durations.

the buckets are chosen to mimic the request and response buckets emitted
by the outbound proxy, with their granularity flipped. in other words,
the inbound proxy is more interested in fine-grained request body
metrics than response body metrics, while the outbound proxy is more
interested in fine-grained response body metrics than request body
metrics.

* #4418
* #4419

Signed-off-by: katelyn martin <kate@buoyant.io>

* docs(app): document latency metrics' histogram buckets

this addresses review feedback, and introduces further documentation to
the inbound proxy's response latency metrics.

this commit also further polishes the outbound proxy's language around
these constants, and adds mention to each half of the proxy regarding
the other corresponding pair of metrics, to prevent drift in the future.

to avoid churn, this commit phrases the inbound proxy's comments and
language in terms that expect the forthcoming addition of a request
latency layer to the inbound proxy, though that has not yet been
implemented in this branch at time of writing. a todo comment is left in
place to indicate this.

Signed-off-by: katelyn martin <kate@buoyant.io>

---------

Signed-off-by: katelyn martin <kate@buoyant.io>

Author: cratelyn

linkerd/app/inbound/src/http/router/metrics.rs

@@ -1,19 +1,23 @@
 use crate::InboundMetrics;
 use linkerd_app_core::svc;
 
-pub use self::{count_reqs::*, labels::RouteLabels, req_body::*, rsp_body::*, status::*};
+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(super) fn layer<N>(
     InboundMetrics {
         request_count,
         request_body_data,
         response_body_data,
+        response_duration,
         status_codes,
         ..
     }: &InboundMetrics,
@@ -25,6 +29,11 @@ pub(super) fn layer<N>(
         count_reqs::NewCountRequests::layer_via(extract)
     };
 
+    let response_duration = {
+        let extract = ExtractResponseDurationMetrics(response_duration.clone());
+        NewResponseDuration::layer_via(extract)
+    };
+
     let response_body = {
         let extract = ExtractResponseBodyDataMetrics::new(response_body_data.clone());
         NewRecordResponseBodyData::layer_via(extract)
@@ -41,10 +50,15 @@ pub(super) fn layer<N>(
     };
 
     svc::layer::mk(move |inner| {
-        count.layer(response_body.layer(request_body.layer(status.layer(inner))))
+        count.layer(
+            response_duration.layer(response_body.layer(request_body.layer(status.layer(inner)))),
+        )
     })
 }
 
 /// An `N`-typed service instrumented with metrics middleware.
-type Instrumented<N> =
-    NewCountRequests<NewRecordResponseBodyData<NewRecordRequestBodyData<NewRecordStatusCode<N>>>>;
+type Instrumented<N> = NewCountRequests<
+    NewResponseDuration<
+        NewRecordResponseBodyData<NewRecordRequestBodyData<NewRecordStatusCode<N>>>,
+    >,
+>;

linkerd/app/inbound/src/http/router/metrics/rsp_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 NewResponseDuration<N> =
+    record_response::NewResponseDuration<MkLabelDuration, ExtractResponseDurationMetrics, N>;
+
+pub type ResponseDurationParams =
+    Params<MkLabelDuration, record_response::ResponseMetrics<ResponseDurationLabels>>;
+
+#[derive(Clone, Debug)]
+pub struct ExtractResponseDurationMetrics(pub ResponseDurationFamilies);
+
+#[derive(Clone, Debug)]
+pub struct ResponseDurationFamilies {
+    grpc: record_response::ResponseMetrics<ResponseDurationLabels>,
+    http: record_response::ResponseMetrics<ResponseDurationLabels>,
+}
+
+#[derive(Clone, Debug, Hash, PartialEq, Eq)]
+pub struct ResponseDurationLabels {
+    route: RouteLabels,
+}
+
+pub type MkLabelDuration = MkWithLabels<ResponseDurationLabels>;
+
+// === impl ResponseDurationFamilies ===
+
+impl ResponseDurationFamilies {
+    /// Registers a new [`ResponseDurationFamilies`] with the given registry.
+    pub fn register(
+        reg: &mut prom::Registry,
+        histo: impl Clone + IntoIterator<Item = f64>,
+    ) -> Self {
+        let grpc = {
+            let reg = reg.sub_registry_with_prefix("grpc");
+            record_response::ResponseMetrics::register(reg, histo.clone())
+        };
+
+        let http = {
+            let reg = reg.sub_registry_with_prefix("http");
+            record_response::ResponseMetrics::register(reg, histo)
+        };
+
+        Self { grpc, http }
+    }
+}
+
+// === impl ExtractResponseDurationMetrics ===
+
+impl<T> svc::ExtractParam<ResponseDurationParams, T> for ExtractResponseDurationMetrics
+where
+    T: svc::Param<PermitVariant> + svc::Param<RouteLabels>,
+{
+    fn extract_param(&self, target: &T) -> ResponseDurationParams {
+        let Self(families) = self;
+
+        let labeler = {
+            let route: RouteLabels = target.param();
+            let labels = ResponseDurationLabels { route };
+            MkLabelDuration::new(labels)
+        };
+
+        let metric = {
+            let variant: PermitVariant = target.param();
+            let ResponseDurationFamilies { grpc, http } = families;
+            match variant {
+                PermitVariant::Grpc => grpc,
+                PermitVariant::Http => http,
+            }
+            .clone()
+        };
+
+        ResponseDurationParams { labeler, metric }
+    }
+}
+
+// === impl ResponseDurationLabels ===
+
+impl prom::EncodeLabelSetMut for ResponseDurationLabels {
+    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 ResponseDurationLabels {
+    fn encode(&self, mut encoder: prom::encoding::LabelSetEncoder<'_>) -> std::fmt::Result {
+        self.encode_label_set(&mut encoder)
+    }
+}

linkerd/app/inbound/src/metrics.rs

@@ -12,7 +12,8 @@ pub(crate) mod authz;
 pub(crate) mod error;
 
 use crate::http::router::{
-    RequestBodyFamilies, RequestCountFamilies, ResponseBodyFamilies, StatusCodeFamilies,
+    RequestBodyFamilies, RequestCountFamilies, ResponseBodyFamilies, ResponseDurationFamilies,
+    StatusCodeFamilies,
 };
 pub use linkerd_app_core::metrics::*;
 
@@ -34,6 +35,7 @@ pub struct InboundMetrics {
     pub request_count: RequestCountFamilies,
     pub request_body_data: RequestBodyFamilies,
     pub response_body_data: ResponseBodyFamilies,
+    pub response_duration: ResponseDurationFamilies,
     pub status_codes: StatusCodeFamilies,
 }
 
@@ -47,6 +49,8 @@ impl InboundMetrics {
         let request_count = RequestCountFamilies::register(reg);
         let request_body_data = RequestBodyFamilies::register(reg);
         let response_body_data = ResponseBodyFamilies::register(reg);
+        let response_duration =
+            ResponseDurationFamilies::register(reg, Self::RESPONSE_BUCKETS.iter().copied());
         let status_codes = StatusCodeFamilies::register(reg);
 
         Self {
@@ -60,9 +64,24 @@ impl InboundMetrics {
             request_count,
             request_body_data,
             response_body_data,
+            response_duration,
             status_codes,
         }
     }
+
+    // There are two histograms for which we need to register metrics:
+    //   (1) request durations, which are measured on routes. TODO(kate): forthcoming.
+    //   (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 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.
+    const RESPONSE_BUCKETS: &'static [f64] = &[0.05, 0.5, 1.0, 10.0];
 }
 
 impl legacy::FmtMetrics for InboundMetrics {

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

@@ -142,18 +142,25 @@ where
     B::DurationLabels: LabelSet,
     B::StatusLabels: LabelSet,
 {
-    // There are two histograms for which we need to register metrics: request
-    // durations, measured on routes, and response durations, measured on
-    // route-backends.
+    // There are two histograms for which we need to register metrics:
+    //   (1) request durations, which are measured on routes.
+    //   (2) response durations, which are measured on route-backends.
     //
-    // Response duration is probably the more meaninful metric
-    // operationally--and it includes more backend metadata--so we opt to
-    // preserve higher fidelity for response durations (especially for lower
-    // values).
-    //
-    // We elide several buckets for request durations to be conservative about
-    // the costs of tracking these two largely overlapping histograms
+    // Should these change in the future, be sure to consider the inbound proxy's corresponding
+    // constants measuring request and response latency for *incoming* traffic.
+
+    /// Histogram buckets for request latency.
+    ///
+    /// These buckets for this histogram are coarser than those of [`Self::RESPONSE_BUCKETS`],
+    /// eliding several buckets for short request durations to be conservative about the costs of
+    /// tracking these two histograms' respective time series.
     const REQUEST_BUCKETS: &'static [f64] = &[0.05, 0.5, 1.0, 10.0];
+
+    /// Histogram buckets for response latency.
+    ///
+    /// Because response duration is the more meaningful metric operationally for the outbound
+    /// proxy, and because backend metrics includes additional metadata, we opt to preserve higher
+    /// fidelity for response durations (especially for lower values).
     const RESPONSE_BUCKETS: &'static [f64] = &[0.025, 0.05, 0.1, 0.25, 0.5, 1.0, 10.0];
 }