From ced00b642c3b3f7e1e6a7b03c83bce4851df4518 Mon Sep 17 00:00:00 2001 From: katelyn martin Date: Wed, 25 Sep 2024 00:00:00 +0000 Subject: [PATCH] refactor(prom): Remove `MkStreamLabel`'s associated types (#3234) `MkStreamLabel` is, in short, a generic `(&Request) -> Option` function. we use it to inspect a request, and potentially provide the caller with an object that can provide relevant labels. the `StreamLabel` interface includes associated types for the labels used for metrics related to request/response duration, and counting status codes. we do not however, actually need to separately define these associated types in the `MkStreamLabel` contract. instead, we can return a generic `StreamLabel` of some sort, and leave the responsibility of the (admittedly baroque) associated type access to our type aliases like `RecordResponseDuration` and `RecordRequestDuration`. this change has a pleasant knock-on effect of leaving a number of the labels submodule's type aliases unused. this commit accordingly removes aliases like `HttpRouteRsp`, `GrpcRouteRsp`, `HttpRouteBackendRsp`, and `GrpcRouteBackendRsp`. this is a small initial step towards simplifying code that must interact with the `MkStreamLabel` interface. Signed-off-by: katelyn martin --- .../outbound/src/http/logical/policy/route.rs | 4 ---- .../src/http/logical/policy/route/backend.rs | 4 ---- .../logical/policy/route/metrics/labels.rs | 8 ------- linkerd/http/prom/src/record_response.rs | 22 +------------------ .../http/prom/src/record_response/request.rs | 12 +++++++--- .../http/prom/src/record_response/response.rs | 12 +++++++--- 6 files changed, 19 insertions(+), 43 deletions(-) diff --git a/linkerd/app/outbound/src/http/logical/policy/route.rs b/linkerd/app/outbound/src/http/logical/policy/route.rs index 46d2c698c3..a52640a91a 100644 --- a/linkerd/app/outbound/src/http/logical/policy/route.rs +++ b/linkerd/app/outbound/src/http/logical/policy/route.rs @@ -173,8 +173,6 @@ impl filters::Apply for Http { } impl metrics::MkStreamLabel for Http { - type StatusLabels = metrics::labels::HttpRouteRsp; - type DurationLabels = metrics::labels::Route; type StreamLabel = metrics::LabelHttpRouteRsp; fn mk_stream_labeler(&self, _: &::http::Request) -> Option { @@ -227,8 +225,6 @@ impl filters::Apply for Grpc { } impl metrics::MkStreamLabel for Grpc { - type StatusLabels = metrics::labels::GrpcRouteRsp; - type DurationLabels = metrics::labels::Route; type StreamLabel = metrics::LabelGrpcRouteRsp; fn mk_stream_labeler(&self, _: &::http::Request) -> Option { diff --git a/linkerd/app/outbound/src/http/logical/policy/route/backend.rs b/linkerd/app/outbound/src/http/logical/policy/route/backend.rs index 9005830546..298ce6d3dc 100644 --- a/linkerd/app/outbound/src/http/logical/policy/route/backend.rs +++ b/linkerd/app/outbound/src/http/logical/policy/route/backend.rs @@ -148,8 +148,6 @@ impl filters::Apply for Http { } impl metrics::MkStreamLabel for Http { - type StatusLabels = metrics::labels::HttpRouteBackendRsp; - type DurationLabels = metrics::labels::RouteBackend; type StreamLabel = metrics::LabelHttpRouteBackendRsp; fn mk_stream_labeler(&self, _: &::http::Request) -> Option { @@ -176,8 +174,6 @@ impl filters::Apply for Grpc { } impl metrics::MkStreamLabel for Grpc { - type StatusLabels = metrics::labels::GrpcRouteBackendRsp; - type DurationLabels = metrics::labels::RouteBackend; type StreamLabel = metrics::LabelGrpcRouteBackendRsp; fn mk_stream_labeler(&self, _: &::http::Request) -> Option { diff --git a/linkerd/app/outbound/src/http/logical/policy/route/metrics/labels.rs b/linkerd/app/outbound/src/http/logical/policy/route/metrics/labels.rs index b852c8b02d..bc6d549398 100644 --- a/linkerd/app/outbound/src/http/logical/policy/route/metrics/labels.rs +++ b/linkerd/app/outbound/src/http/logical/policy/route/metrics/labels.rs @@ -13,14 +13,6 @@ pub struct RouteBackend(pub ParentRef, pub RouteRef, pub BackendRef); #[derive(Clone, Debug, Hash, PartialEq, Eq)] pub struct Rsp(pub P, pub L); -pub type RouteRsp = Rsp; -pub type HttpRouteRsp = RouteRsp; -pub type GrpcRouteRsp = RouteRsp; - -pub type RouteBackendRsp = Rsp; -pub type HttpRouteBackendRsp = RouteBackendRsp; -pub type GrpcRouteBackendRsp = RouteBackendRsp; - #[derive(Clone, Debug, Hash, PartialEq, Eq)] pub struct HttpRsp { pub status: Option, diff --git a/linkerd/http/prom/src/record_response.rs b/linkerd/http/prom/src/record_response.rs index 6765fe943a..db9e628a4d 100644 --- a/linkerd/http/prom/src/record_response.rs +++ b/linkerd/http/prom/src/record_response.rs @@ -32,27 +32,7 @@ pub use self::{ /// This is specifically to support higher-cardinality status counters and /// lower-cardinality stream duration histograms. pub trait MkStreamLabel { - type DurationLabels: EncodeLabelSet - + Clone - + Eq - + std::fmt::Debug - + std::hash::Hash - + Send - + Sync - + 'static; - type StatusLabels: EncodeLabelSet - + Clone - + Eq - + std::fmt::Debug - + std::hash::Hash - + Send - + Sync - + 'static; - - type StreamLabel: StreamLabel< - DurationLabels = Self::DurationLabels, - StatusLabels = Self::StatusLabels, - >; + type StreamLabel: StreamLabel; /// Returns None when the request should not be recorded. fn mk_stream_labeler(&self, req: &http::Request) -> Option; diff --git a/linkerd/http/prom/src/record_response/request.rs b/linkerd/http/prom/src/record_response/request.rs index 6daa6b2cfb..9f0c4891ba 100644 --- a/linkerd/http/prom/src/record_response/request.rs +++ b/linkerd/http/prom/src/record_response/request.rs @@ -13,7 +13,7 @@ use std::{ }; use tokio::{sync::oneshot, time}; -use super::{DurationFamily, MkDurationHistogram, MkStreamLabel}; +use super::{DurationFamily, MkDurationHistogram, MkStreamLabel, StreamLabel}; /// Metrics type that tracks completed requests. #[derive(Debug)] @@ -25,13 +25,19 @@ pub struct RequestMetrics { pub type NewRequestDuration = super::NewRecordResponse< L, X, - RequestMetrics<::DurationLabels, ::StatusLabels>, + RequestMetrics< + <::StreamLabel as StreamLabel>::DurationLabels, + <::StreamLabel as StreamLabel>::StatusLabels, + >, N, >; pub type RecordRequestDuration = super::RecordResponse< L, - RequestMetrics<::DurationLabels, ::StatusLabels>, + RequestMetrics< + <::StreamLabel as StreamLabel>::DurationLabels, + <::StreamLabel as StreamLabel>::StatusLabels, + >, S, >; diff --git a/linkerd/http/prom/src/record_response/response.rs b/linkerd/http/prom/src/record_response/response.rs index dacaa73242..870f72a987 100644 --- a/linkerd/http/prom/src/record_response/response.rs +++ b/linkerd/http/prom/src/record_response/response.rs @@ -14,7 +14,7 @@ use std::{ }; use tokio::{sync::oneshot, time}; -use super::{DurationFamily, MkDurationHistogram, MkStreamLabel}; +use super::{DurationFamily, MkDurationHistogram, MkStreamLabel, StreamLabel}; /// Metrics type that tracks completed responses. #[derive(Debug)] @@ -26,13 +26,19 @@ pub struct ResponseMetrics { pub type NewResponseDuration = super::NewRecordResponse< L, X, - ResponseMetrics<::DurationLabels, ::StatusLabels>, + ResponseMetrics< + <::StreamLabel as StreamLabel>::DurationLabels, + <::StreamLabel as StreamLabel>::StatusLabels, + >, N, >; pub type RecordResponseDuration = super::RecordResponse< L, - ResponseMetrics<::DurationLabels, ::StatusLabels>, + ResponseMetrics< + <::StreamLabel as StreamLabel>::DurationLabels, + <::StreamLabel as StreamLabel>::StatusLabels, + >, S, >;