Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

wip(prom): Simplify metrics traits #3242

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

wip(prom): Simplify metrics traits #3242

wants to merge 6 commits into from

Commits on Sep 27, 2024

  1. docs(prom): Document record_response interfaces

    @cratelyn
    cratelyn committed Sep 27, 2024
    Configuration menu
    Copy the full SHA
    cf8a249 View commit details
    Browse the repository at this point in the history

  2. refactor(prom): Remove MkStreamLabel's associated types (#3234) 

    `MkStreamLabel` is, in short, a generic
`(&Request) -> Option<StreamLabel>` 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 <[email protected]>
    @cratelyn
    cratelyn committed Sep 27, 2024
    Configuration menu
    Copy the full SHA
    c024811 View commit details
    Browse the repository at this point in the history

  3. refactor(prom): Make MkStreamLabel and StreamLabel object-safe 

    our north star is making our metrics middleware easier to work with.

we want `StreamLabel` to be boxable. this will let us avoid needing to
thread baroque generics throughout our layer / service code, and instead
rely on dynamic dispatch instead of static `L: StreamLabel` bounds.

in order to move in that direction, we must take a small step back first.

these traits must be object-safe in order to do that. for a trait to be
object safe, per the Rust Reference, we may not use any function-level
generics. accordingly, this moves the `<B>` bounds up to the top-level
traits.

this makes some of our type aliases icky, and introduces new generic
parameters to various types/interfaces. we will be able to walk those
back in subsequent commits, now that these traits can be placed in e.g.
`Box<dyn StreamLabel>` values.

NB: as this is an "Zwischenzug"¹ commit, we don't update the app crates.
this commit will fail to compile if the entire workspace is built.

Signed-off-by: katelyn martin <[email protected]>

1 - https://en.wikipedia.org/wiki/Zwischenzug
    @cratelyn
    cratelyn committed Sep 27, 2024
    Configuration menu
    Copy the full SHA
    0b6dbb0 View commit details
    Browse the repository at this point in the history

  4. refactor(prom): Provide Parts to label traits 

    this commit updates the signature of `mk_stream_labeler()` and
`init_response()` to instead examine the `http::request::Parts` and
`http::response::Parts`, respectively.

this is another step to simplify our label traits.

no implementation of `MkStreamLabel` examines the body of the request.
moreover, none even uses the `req` parameter at all!

the intent of this signature seems reasonable however, we may want to
look at certain requests and opt to ignore them. one could imagine an
`l5d-*` header to refrain from installing telemetry on the response
body, for example.

however, as the last commit showed, we are subject to plenty of pain
related to body generics. let's remove the need for them.
    @cratelyn
    cratelyn committed Sep 27, 2024
    Configuration menu
    Copy the full SHA
    3d701af View commit details
    Browse the repository at this point in the history

  5. refactor(prom): Remove MkStreamLabel and StreamLabel type parameters 

    now that we have switched over to inspect the `Parts` head material of
the requests and responses, we can remove these type parameters, whilst
remaining object-safe.

nice!
    @cratelyn
    cratelyn committed Sep 27, 2024
    Configuration menu
    Copy the full SHA
    c3d9938 View commit details
    Browse the repository at this point in the history

  6. XXX: 🍞 breadcrumb comments

    @cratelyn
    cratelyn committed Sep 27, 2024
    Configuration menu
    Copy the full SHA
    7965d01 View commit details
    Browse the repository at this point in the history