Middleware and metric to track per-database HTTP response egress - #5611
Open
gefjon wants to merge 8 commits into
Open
Middleware and metric to track per-database HTTP response egress#5611gefjon wants to merge 8 commits into
gefjon wants to merge 8 commits into
Conversation
This commit adds a new Prometheus metric, `spacetime_http_response_size_bytes_total`, which tracks bytes sent as responses to HTTP requests related to the database. This includes the `sql` and `call` routes, guest-defined HTTP handlers, logs, plus some misc. management routes. Notably, the `subscribe` route, which initiates a long-lived WebSocket connection, is not counted by the new metric, as we already track its egress separately. The new metric is tracked by an Axum middleware.
gefjon
added
the
release-any
joshua-spacetime
left a comment
joshua-spacetime
left a comment
Collaborator
There was a problem hiding this comment.
Looks good. I just have one billing related question.
Also can you open a PR to incorporate this metric into billing? Even if you'd like to collect some data before flipping the switch.
2 tasks
joshua-spacetime
approved these changes
Aug 6, 2026
joshua-spacetime
left a comment
joshua-spacetime
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. I already reviewed and approved #5681. So feel free to include it in this change and merge them both together.
Based on and targeting #5611 . I thought this was a significant enough change to be worth reviewing separately. # Description of Changes The middleware responsible for the metric `spacetime_http_response_size_bytes_total` has to resolve a request's `:name_or_identity` from name to identity in order to compute the correct metrics label, and from identity to referent database in order to avoid allocating metrics labels for non-existent databases in response to ill-formed requests. Prior to this commit, the middleware did said resolution, then discarded the results, and the specific route handlers did the same resolution again. With this commit, the middleware is expanded to be fully responsible for database resolution. It attaches the resolved `Database` record to the request as an extension, and the route handlers read that extension rather than reading and resolving the `:name_or_identity` themselves. A small number of routes have behavior on non-existent databases other than returning 404. These 404s may occur either when a name is not bound to an identity, or when an identity is not bound to a database. (N.b. a name may be bound to an identity without that identity being bound to a database, which is a somewhat silly situation.) Rather than increasing the complexity of the middleware to cope with these behaviors or changing the behavior of these routes by applying the middleware to them, we opt to simply exclude these routes from the middleware, applying it only to routes which want the 404 response behavior. The specific routes exluded from the middleware are: - `PUT /database/:name_or_identity`, which creates a new database if the name is not already in use. - `DELETE /database/:name_or_identity`, which is a no-op on a non-existent database. - `/database/:name_or_identity/identity`, which only resolves a name to an identity, and does not check whether that identity refers to a database. - `GET /database/:name_or_identity/subscribe`, already excluded prior to this commit, whose response egress is tracked by a different metric than the one used by this middleware. All of the newly excluded routes are infrequent operations with small responses, and said responses are not user-controlled, so it's probably fine that they don't get counted by the egress metric. # API and ABI breaking changes N/a # Expected complexity level and risk <!-- How complicated do you think these changes are? Grade on a scale from 1 to 5, where 1 is a trivial change, and 5 is a deep-reaching and complex change. This complexity rating applies not only to the complexity apparent in the diff, but also to its interactions with existing and future code. If you answered more than a 2, explain what is complex about the PR, and what other components it interacts with in potentially concerning ways. --> # Testing - [x] New automated tests that affected routes don't re-resolve names unnecessarily. - [x] Other behavior should be covered by existing tests.
See comments in mcp.rs
gefjon
disabled auto-merge
August 7, 2026 17:17
gefjon
enabled auto-merge
August 7, 2026 17:26
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
2 participants
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.
Description of Changes
This commit adds a new Prometheus metric,
spacetime_http_response_size_bytes_total, which tracks bytes sent as responses to HTTP requests related to the database. This includes thesqlandcallroutes, guest-defined HTTP handlers, logs, plus some misc. management routes. Notably, thesubscriberoute, which initiates a long-lived WebSocket connection, is not counted by the new metric, as we already track its egress separately.The new metric is tracked by an Axum middleware.
API and ABI breaking changes
N/a
Expected complexity level and risk
1 or 2? I don't have a huge amount of confidence any time I touch our Axum stuff, but this is just metrics, and (at least currently) not billed metrics.
Testing