Skip to content

Record metrics and charge for HTTP route egress - #5505

Closed
bradleyshep wants to merge 12 commits into
masterfrom
bradley/record-metrics-and-charge-http
Closed

Record metrics and charge for HTTP route egress#5505
bradleyshep wants to merge 12 commits into
masterfrom
bradley/record-metrics-and-charge-http

Conversation

@bradleyshep

Copy link
Copy Markdown
Contributor

Description of Changes

The HTTP /sql endpoint charged for its DB work (rows/bytes scanned, writes) but not for egress, the bytes sent back to the client. WebSocket queries already count these, but the HTTP path skipped it, so the same query was billed over WS and free over HTTP. There were also no metrics on the HTTP routes.

This PR:

  1. Charges egress for /sql, using the BSATN size of the result rows so the price matches WebSocket queries.
  2. Adds a /v1 middleware that records per-route request count, latency, and body sizes. Body sizes count bytes actually transferred through a small Body wrapper rather than trusting the Content-Length header, so chunked and streamed bodies are counted and spoofed headers are not. Non-standard HTTP methods are bucketed into an OTHER label to keep cardinality bounded.

Adds http-body as a direct dependency (already in the tree via axum/hyper).

Not covered here: CORS preflights and unmatched 404s are not counted (they short-circuit before the middleware), and routes other than /sql are not billed egress yet.

API and ABI breaking changes

None. The /sql response is unchanged, only internal metrics were added.

Expected complexity level and risk

  1. The egress change is one counter increment on the existing SQL path. The middleware is router setup plus a ~35 line body wrapper.

Testing

  • Unit test that a SELECT charges bytes_sent_to_clients equal to the rows' BSATN size.
  • Verified locally: a SELECT over HTTP bumps the egress counter by the exact BSATN size and the spacetime_http_* counters by the exact wire bytes (checked against curl's size_download).
  • Verified locally: a bogus method (curl -X BANANA) lands in the OTHER bucket, and a chunked request body (no Content-Length) is counted.
  • Existing SQL smoketests pass.

Comment thread crates/core/src/sql/execute.rs Outdated
Each transport charges for the rows it sends, matching how WebSocket
queries charge at their serialization point. Prevents double counting
if the executor gains other transport callers.

@joshua-spacetime joshua-spacetime left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add some middleware wrapper that would track all http egress? This covers sql, but there are probably other http endpoints that are also not tracked.

Comment on lines +217 to +219
/// Uses the BSATN size for parity with WebSocket queries.
fn sql_egress_bytes(rows: &[ProductValue]) -> u64 {
rows.iter().map(|row| bsatn::to_len(row).unwrap_or(0) as u64).sum()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But http uses json.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured that we'd want to charge the same as the identical query over WS, so the price doesn't depend on transport, if I understand correctly.

@gefjon
gefjon self-requested a review July 29, 2026 14:26
@bradleyshep

Copy link
Copy Markdown
Contributor Author

Could we add some middleware wrapper that would track all http egress? This covers sql, but there are probably other http endpoints that are also not tracked.

Sure. The ticket description called out sql specifically, so that's where I put the charge. The middleware does measure bytes in/out for every /v1 route though, so /call, /logs etc are all visible in the metrics, they're just not charged for yet. Is there guidance on egress pricing here?

@gefjon

gefjon commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

I'm gonna close this in favor of #5611 . We have follow-up tickets for billing, which we'll evaluate once we've tracked the metric for some time.

@gefjon gefjon closed this Aug 7, 2026
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.

3 participants