refactor(middleware): migrate PrometheusMiddleware to ASGIMiddleware - #5006
Open
Kumzy wants to merge 1 commit into
Open
refactor(middleware): migrate PrometheusMiddleware to ASGIMiddleware#5006Kumzy wants to merge 1 commit into
Kumzy wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## mw/rate-limit #5006 +/- ##
=================================================
+ Coverage 67.19% 67.21% +0.02%
=================================================
Files 293 293
Lines 15404 15415 +11
Branches 1750 1751 +1
=================================================
+ Hits 10350 10361 +11
Misses 4900 4900
Partials 154 154 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
PrometheusMiddleware moves off the legacy AbstractMiddleware base onto
ASGIMiddleware, taking its settings as keyword arguments (each defaulted
to mirror the PrometheusConfig field) instead of an app and a config.
Since the middleware is directly constructible, PrometheusConfig is
obsolete: its middleware property is deprecated (removal in 4.0) and the
documented migration path is passing a PrometheusMiddleware instance to
the middleware list directly. The excluded_http_methods check stays
inside handle(), unchanged.
PrometheusConfig.scopes is user-configurable, so like allowed-hosts the
configured scopes are also enforced per connection via
should_bypass_for_scope: mounted ASGI apps stay wrapped regardless and
their connections are filtered by scope type at runtime, keeping e.g.
scopes={'websocket'} effective for connections through mounts. exclude
patterns now match the handler's path template at startup and excluded
handlers bypass the middleware entirely, matching the other migrated
middleware.
Tests and docs examples construct the middleware directly; new tests
cover the prometheus-specific behavior of this migration: mounted apps
stay instrumented, configured scopes hold for connections through
mounts, group_path=False uses the request path, non-default buckets
replace the default boundaries, subclasses work with the new handle
contract, and the deprecated property warns while still returning a
configured instance.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #4009. Stacked PR to avoid conflicts in the changelog and whats-new docs.
Moving
PrometheusMiddlewarefromAbstractMiddlewaretoASGIMiddleware.The constructor takes keyword arguments now instead of an
appand aPrometheusConfigobject, with each argument defaulting to the matching config field.PrometheusConfigitself is unchanged and stays the way to configure this: itsmiddlewareproperty now returns a configuredPrometheusMiddlewareinstance instead of aDefineMiddleware, somiddleware=[config.middleware]keeps working as before. Theexcluded_http_methodscheck stays insidehandle().Same breaking change as the other PRs in the series:
excludepatterns now match the handler's path template at startup instead of the request path (documented in the changelog and whats-new-3).Same subtlety as allowed-hosts:
PrometheusConfig.scopesis user-configurable and mounted ASGI apps receive bothhttpandwebsocketconnections at runtime, so configured scopes are also enforced per connection viashould_bypass_for_scope. Without this, restricting toscopes={"websocket"}would have silently kept instrumenting http connections through ASGI mounts.Added some tests, covering the prometheus-specific behavior of this migration:
ASGIfrom the scopes tuple was undetected before.scopes={"websocket"}). It was failing.group_path=Falseuses the request path instead of the template. TheFalsebranch was not covered before.middleware_classworks with the newhandlecontract, since this PR changes that hook (no moreself._config).bucketsfrom the property was undetected.Two pre-existing quirks found while porting, unchanged here (possible follow-ups):
request.methodraisesKeyErrorfor websocket connections whenexcluded_http_methodsis set, andPrometheusConfig.exclude_unhandled_pathsis a dead field nothing reads.📚 Documentation preview 📚: https://litestar-org.github.io/litestar-docs-preview/5006