Skip to content

Commit

Permalink
XXX: 🍞 breadcrumb comments
Browse files Browse the repository at this point in the history
  • Loading branch information
cratelyn committed Oct 1, 2024
1 parent f10cbc8 commit 26975e4
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 1 deletion.
6 changes: 5 additions & 1 deletion linkerd/app/outbound/src/http/logical/policy/route.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ pub(crate) type Grpc<T> = MatchedRoute<
pub(crate) type BackendDistribution<T, F> = distribute::Distribution<Backend<T, F>>;
pub(crate) type NewDistribute<T, F, N> = distribute::NewDistribute<Backend<T, F>, (), N>;

// XXX(kate): this is the router's metrics type. (route- and backend-level labels)
// XXX(kate): this does not need to be generic over R and B the `StreamLabel` is shared in practice
pub type Metrics<R, B> = metrics::RouteMetrics<
<R as http_prom::MkStreamLabel>::StreamLabel,
<B as http_prom::MkStreamLabel>::StreamLabel,
Expand Down Expand Up @@ -91,7 +93,7 @@ where
Self: svc::Param<extensions::Params>,
Self: http_prom::MkStreamLabel,
MatchedBackend<T, M, F>: filters::Apply,
MatchedBackend<T, M, F>: http_prom::MkStreamLabel,
MatchedBackend<T, M, F>: http_prom::MkStreamLabel, // XXX(kate): MkStreamLabel bound
{
/// Builds a route stack that applies policy filters to requests and
/// distributes requests over each route's backends. These [`Concrete`]
Expand Down Expand Up @@ -173,6 +175,7 @@ impl<T> filters::Apply for Http<T> {
}
}

// XXX(kate): an implementation of `MkStreamLabel`.
impl<T> http_prom::MkStreamLabel for Http<T> {
type StreamLabel = metrics::LabelHttpRouteRsp;

Expand Down Expand Up @@ -225,6 +228,7 @@ impl<T> filters::Apply for Grpc<T> {
}
}

// XXX(kate): an implementation of `MkStreamLabel`.
impl<T> http_prom::MkStreamLabel for Grpc<T> {
type StreamLabel = metrics::LabelGrpcRouteRsp;

Expand Down
2 changes: 2 additions & 0 deletions linkerd/app/outbound/src/http/logical/policy/route/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ impl<T> filters::Apply for Http<T> {
}
}

// XXX(kate): an implementation of `MkStreamLabel`.
impl<T> http_prom::MkStreamLabel for Http<T> {
type StreamLabel = route::metrics::LabelHttpRouteBackendRsp;

Expand All @@ -173,6 +174,7 @@ impl<T> filters::Apply for Grpc<T> {
}
}

// XXX(kate): an implementation of `MkStreamLabel`.
impl<T> http_prom::MkStreamLabel for Grpc<T> {
type StreamLabel = route::metrics::LabelGrpcRouteBackendRsp;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use linkerd_http_prom::{
#[cfg(test)]
mod tests;

// XXX(kate): this is the application's view of route backend metrics.
#[derive(Debug)]
pub struct RouteBackendMetrics<L: StreamLabel> {
requests: RequestCountFamilies<labels::RouteBackend>,
Expand All @@ -30,6 +31,7 @@ pub fn layer<T, N>(
> + Clone
where
T: MkStreamLabel,
// XXX(kate): each layer of middleware has to show its `NewService<T>` bounds here.
N: svc::NewService<T>,
NewCountRequests<
ExtractRequestCount,
Expand Down
4 changes: 4 additions & 0 deletions linkerd/app/outbound/src/http/logical/policy/route/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub(super) mod test_util;
#[cfg(test)]
mod tests;

// XXX(kate): this is the application's view of route metrics.
#[derive(Debug)]
pub struct RouteMetrics<R: StreamLabel, B: StreamLabel> {
pub(super) retry: retry::RouteRetryMetrics,
Expand Down Expand Up @@ -76,6 +77,7 @@ where

// === impl RouteMetrics ===

// XXX(kate): buckets for request/response durations for route-level metrics.
impl<R: StreamLabel, B: StreamLabel> RouteMetrics<R, B> {
// There are two histograms for which we need to register metrics: request
// durations, measured on routes, and response durations, measured on
Expand Down Expand Up @@ -168,6 +170,7 @@ impl<P> From<P> for LabelHttpRsp<P> {
}
}

// XXX(kate): stream label implementation for http responses.
impl<P> StreamLabel for LabelHttpRsp<P>
where
P: EncodeLabelSetMut + Clone + Eq + std::fmt::Debug + std::hash::Hash + Send + Sync + 'static,
Expand Down Expand Up @@ -219,6 +222,7 @@ impl<P> From<P> for LabelGrpcRsp<P> {
}
}

// XXX(kate): stream label implementation for grpc responses.
impl<P> StreamLabel for LabelGrpcRsp<P>
where
P: EncodeLabelSetMut + Clone + Eq + std::fmt::Debug + std::hash::Hash + Send + Sync + 'static,
Expand Down
8 changes: 8 additions & 0 deletions linkerd/http/prom/src/record_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ pub use self::{
///
/// This is specifically to support higher-cardinality status counters and
/// lower-cardinality stream duration histograms.
// XXX(kate): this is a trait we would like to remove.
// XXX(kate): consider a blanket implementation of this for `T: Fn(..) -> ..`
pub trait MkStreamLabel {
type StreamLabel: StreamLabel;

Expand Down Expand Up @@ -83,6 +85,9 @@ pub struct NewRecordResponse<L, X, M, N> {
}

/// A Service that can record a request/response durations.
//
// XXX(kate): this middle instruments an inner service `S` with a `L`-typed [`MkStreamLabel`].
// XXX(kate): service impls instrument `S` with a `RequestMetrics` and `ResponseMetrics`.
#[derive(Clone, Debug)]
pub struct RecordResponse<L, M, S> {
inner: S,
Expand All @@ -101,6 +106,9 @@ where
}

/// Notifies the response labeler when the response body is flushed.
//
// XXX(kate): i do not believe this comment is correct. `RequestBody` does that, this type in
// fact updates the metrics when finished.
#[pin_project::pin_project(PinnedDrop)]
struct ResponseBody<L: StreamLabel> {
#[pin]
Expand Down

0 comments on commit 26975e4

Please sign in to comment.