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.

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

refactor(app): Outline route label extractor #3337

Merged
merged 2 commits into from
Nov 21, 2024

Conversation

cratelyn
Copy link
Collaborator

@cratelyn cratelyn commented Nov 7, 2024

this PR, based upon #3308, generalizes the ExtractParam<P, T> type used to retrieve Prometheus labels for the route-level retry middleware. this is a helpful component we'll want in order to build route-level frame counting middleware.

refactor(app): Hoist label extractor out of retry middleware

this commit promotes the RetryLabelExtract type out of the route retry middleware, and into the metrics
submodule.

in preparation for generalizing this component, we rename it to RouteLabelExtract.

refactor(app): Outline route label extraction

this addresses the TODO comment, moving the RouteLabelExtract type out of the policy route layer
function.

this defines a method on MatchedRoute which may be called to retrieve a RouteLabelExtract. that type
holds references to the contruction-time information about the route, and can be used to later inspect an
outbound request at call-time, returning a labels::Route set of labels.

this is a helpful piece of reusable glue that is broadly useful in instrumenting our route-level
middleware.

🔗 related

@cratelyn cratelyn force-pushed the kate/hoist-route-label-extractor branch from bb26e1e to 2d8d0f2 Compare November 7, 2024 06:34
@cratelyn cratelyn self-assigned this Nov 7, 2024
@cratelyn cratelyn marked this pull request as ready for review November 7, 2024 15:50
@cratelyn cratelyn requested a review from a team as a code owner November 7, 2024 15:50
cratelyn added a commit that referenced this pull request Nov 10, 2024
 ### ⛅ overview

this introduces a new tower middleware for Prometheus metrics, used for
instrumenting HTTP and gRPC request bodies, and observing (a) the
number of frames yielded by a body, and (b) the number of bytes included
in body frames.

this builds upon the backend-level metrics added in #3308. this
additionally uses the route label extractor, hoisted out of the retry
middleware's Prometheus telemetry in #3337.

 ### 📝 changes

* a `linkerd_http_prom::body_data::request::NewRecordBodyData::NewRecordBodyData`
  middleware is added, which complements the equivalent
  `linkerd_http_prom::body_data::response` middleware.

* this is added

Signed-off-by: katelyn martin <[email protected]>
@cratelyn cratelyn force-pushed the kate/record-body-data branch from cd2d1b9 to 3b1db24 Compare November 10, 2024 22:33
@cratelyn cratelyn force-pushed the kate/hoist-route-label-extractor branch from 2d8d0f2 to a75492c Compare November 10, 2024 22:37
cratelyn added a commit that referenced this pull request Nov 10, 2024
 ### ⛅ overview

this introduces a new tower middleware for Prometheus metrics, used for
instrumenting HTTP and gRPC request bodies, and observing (a) the
number of frames yielded by a body, and (b) the number of bytes included
in body frames.

this builds upon the backend-level metrics added in #3308. this
additionally uses the route label extractor, hoisted out of the retry
middleware's Prometheus telemetry in #3337.

 ### 📝 changes

* a `linkerd_http_prom::body_data::request::NewRecordBodyData::NewRecordBodyData`
  middleware is added, which complements the equivalent
  `linkerd_http_prom::body_data::response` middleware.

* this is added

Signed-off-by: katelyn martin <[email protected]>
cratelyn added a commit that referenced this pull request Nov 10, 2024
 ### ⛅ overview

this introduces a new tower middleware for Prometheus metrics, used for
instrumenting HTTP and gRPC request bodies, and observing (a) the
number of frames yielded by a body, and (b) the number of bytes included
in body frames.

this builds upon the backend-level metrics added in #3308. this
additionally uses the route label extractor, hoisted out of the retry
middleware's Prometheus telemetry in #3337.

 ### 📝 changes

* a `linkerd_http_prom::body_data::request::NewRecordBodyData::NewRecordBodyData`
  middleware is added, which complements the equivalent
  `linkerd_http_prom::body_data::response` middleware.

* this is added to policy routes' metrics layer.

Signed-off-by: katelyn martin <[email protected]>
cratelyn added a commit that referenced this pull request Nov 10, 2024
 ### ⛅ overview

this introduces a new tower middleware for Prometheus metrics, used for
instrumenting HTTP and gRPC request bodies, and observing (a) the
number of frames yielded by a body, and (b) the number of bytes included
in body frames.

this builds upon the backend-level metrics added in #3308. this
additionally uses the route label extractor, hoisted out of the retry
middleware's Prometheus telemetry in #3337.

 ### 📝 changes

* a `linkerd_http_prom::body_data::request::NewRecordBodyData::NewRecordBodyData`
  middleware is added, which complements the equivalent
  `linkerd_http_prom::body_data::response` middleware.

* this is added to policy routes' metrics layer.

Signed-off-by: katelyn martin <[email protected]>
@cratelyn cratelyn removed their assignment Nov 10, 2024
@cratelyn
Copy link
Collaborator Author

this pr has been rebased onto main as of release v2.263.1.

@cratelyn
Copy link
Collaborator Author

process note, while this is a pr based on #3308, i'd like to commit it separately. i'm going to refrain from merging this until the underlying pr lands.

@cratelyn cratelyn force-pushed the kate/record-body-data branch from 95e796f to c728e18 Compare November 20, 2024 19:38
@cratelyn cratelyn force-pushed the kate/hoist-route-label-extractor branch from a75492c to a84a157 Compare November 20, 2024 19:40
@cratelyn
Copy link
Collaborator Author

this pr has been rebased onto main as of release v2.666.0.

@cratelyn cratelyn force-pushed the kate/record-body-data branch from c728e18 to 33f5f60 Compare November 20, 2024 19:47
Copy link

codecov bot commented Nov 20, 2024

Codecov Report

Attention: Patch coverage is 81.81818% with 4 lines in your changes missing coverage. Please review.

Please upload report for BASE (kate/record-body-data@33f5f60). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...nd/src/http/logical/policy/route/metrics/labels.rs 72.72% 3 Missing ⚠️
...d/src/http/logical/policy/route/backend/metrics.rs 87.50% 1 Missing ⚠️
Additional details and impacted files
@@                   Coverage Diff                    @@
##             kate/record-body-data    #3337   +/-   ##
========================================================
  Coverage                         ?   67.38%           
========================================================
  Files                            ?      386           
  Lines                            ?    18048           
  Branches                         ?        0           
========================================================
  Hits                             ?    12161           
  Misses                           ?     5887           
  Partials                         ?        0           
Files with missing lines Coverage Δ
...kerd/app/outbound/src/http/logical/policy/route.rs 91.04% <100.00%> (ø)
...pp/outbound/src/http/logical/policy/route/retry.rs 86.20% <ø> (ø)
linkerd/http/prom/src/body_data/response.rs 100.00% <100.00%> (ø)
...d/src/http/logical/policy/route/backend/metrics.rs 94.87% <87.50%> (ø)
...nd/src/http/logical/policy/route/metrics/labels.rs 51.66% <72.72%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33f5f60...a84a157. Read the comment docs.

---- 🚨 Try these New Features:

Base automatically changed from kate/record-body-data to main November 20, 2024 20:45
this commit promotes the `RetryLabelExtract` type out of the route retry
middleware, and into the metrics submodule.

in preparation for generalizing this component, we rename it to
`RouteLabelExtract`.

Signed-off-by: katelyn martin <[email protected]>
this addresses the `TODO` comment, moving the `RouteLabelExtract` type
out of the policy route layer function.

this defines a method on `MatchedRoute` which may be called to retrieve
a `RouteLabelExtract`. that type holds references to the
contruction-time information about the route, and can be used to later
inspect an outbound request at call-time, returning a `labels::Route`
set of labels.

this is a helpful piece of reusable glue that is broadly useful in
instrumenting our route-level middleware.

Signed-off-by: katelyn martin <[email protected]>
@cratelyn cratelyn force-pushed the kate/hoist-route-label-extractor branch from a84a157 to 71cca7f Compare November 21, 2024 14:50
@cratelyn
Copy link
Collaborator Author

rebased on main, now that #3308 has landed.

@cratelyn cratelyn enabled auto-merge (squash) November 21, 2024 14:52
@cratelyn
Copy link
Collaborator Author

set to auto-merge upon ci completing ✔️

@cratelyn cratelyn merged commit e190dae into main Nov 21, 2024
15 checks passed
@cratelyn cratelyn deleted the kate/hoist-route-label-extractor branch November 21, 2024 14:56
cratelyn added a commit that referenced this pull request Nov 21, 2024
 ### ⛅ overview

this introduces a new tower middleware for Prometheus metrics, used for
instrumenting HTTP and gRPC request bodies, and observing (a) the
number of frames yielded by a body, and (b) the number of bytes included
in body frames.

this builds upon the backend-level metrics added in #3308. this
additionally uses the route label extractor, hoisted out of the retry
middleware's Prometheus telemetry in #3337.

 ### 📝 changes

* a `linkerd_http_prom::body_data::request::NewRecordBodyData::NewRecordBodyData`
  middleware is added, which complements the equivalent
  `linkerd_http_prom::body_data::response` middleware.

* this is added to policy routes' metrics layer.

Signed-off-by: katelyn martin <[email protected]>
cratelyn added a commit that referenced this pull request Nov 21, 2024
 ### ⛅ overview

this introduces a new tower middleware for Prometheus metrics, used for
instrumenting HTTP and gRPC request bodies, and observing (a) the
number of frames yielded by a body, (b) the number of bytes included
in body frames, and (c) a distribution of the size of frames yielded.

this builds upon the backend-level metrics added in #3308. this
additionally uses the route label extractor, hoisted out of the retry
middleware's Prometheus telemetry in #3337.

 ### 📝 changes

* a `linkerd_http_prom::body_data::request::NewRecordBodyData::NewRecordBodyData`
  middleware is added, which complements the equivalent
  `linkerd_http_prom::body_data::response` middleware.

* this is added to policy routes' metrics layer.

see prometheus/client_rust#241 and prometheus/client_rust#242, which
track upstream proposals to add accessors to `Histogram` that will allow
us to make test assertions that metrics are working properly. for now,
these are feature gated as also done in #3308.

Signed-off-by: katelyn martin <[email protected]>
cratelyn added a commit that referenced this pull request Nov 21, 2024
 ### ⛅ overview

this introduces a new tower middleware for Prometheus metrics, used for
instrumenting HTTP and gRPC request bodies, and observing (a) the
number of frames yielded by a body, (b) the number of bytes included
in body frames, and (c) a distribution of the size of frames yielded.

this builds upon the backend-level metrics added in #3308. this
additionally uses the route label extractor, hoisted out of the retry
middleware's Prometheus telemetry in #3337.

 ### 📝 changes

* a `linkerd_http_prom::body_data::request::NewRecordBodyData::NewRecordBodyData`
  middleware is added, which complements the equivalent
  `linkerd_http_prom::body_data::response` middleware.

* this is added to policy routes' metrics layer.

see prometheus/client_rust#241 and prometheus/client_rust#242, which
track upstream proposals to add accessors to `Histogram` that will allow
us to make test assertions that metrics are working properly. for now,
these are feature gated as also done in #3308.

Signed-off-by: katelyn martin <[email protected]>
cratelyn added a commit that referenced this pull request Nov 21, 2024
### ⛅ overview

this introduces a new tower middleware for Prometheus metrics, used for
instrumenting HTTP and gRPC request bodies, and observing (a) the
number of frames yielded by a body, (b) the number of bytes included
in body frames, and (c) a distribution of the size of frames yielded.

this builds upon the backend-level metrics added in #3308. this
additionally uses the route label extractor, hoisted out of the retry
middleware's Prometheus telemetry in #3337.

 ### 📝 changes

* a `linkerd_http_prom::body_data::request::NewRecordBodyData::NewRecordBodyData`
  middleware is added, which complements the equivalent
  `linkerd_http_prom::body_data::response` middleware.

* this is added to policy routes' metrics layer.

see prometheus/client_rust#241 and prometheus/client_rust#242, which
track upstream proposals to add accessors to `Histogram` that will allow
us to make test assertions that metrics are working properly. for now,
these are feature gated as also done in #3308.

Signed-off-by: katelyn martin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants