Skip to content

[api] tag every sandbox API metric with the bounded client id - #1343

Merged
kumarpallav-pplx merged 1 commit into
mainfrom
kumar/api-metrics-client-id
Nov 2, 2026
Merged

kumarpallav-pplx merged 1 commit into
mainfrom
kumar/api-metrics-client-id

Conversation

@kumarpallav-pplx

Copy link
Copy Markdown
Collaborator

Summary

Operators can't break down the sandbox API's operational metrics by caller. Only the limiter decision counter and the connect-handler reconnect metrics carry client_id today (#1058). Route latency and errors, step latency and errors, and the create counter for create, connect, pause, resume, kill, timeout and the other routes are tagged only by op and surface.

After this change, every sandbox.api.* metric built from an API metric identity carries client_id, and so does sandbox.api.created:

  • The sandbox route middleware (native and ECW routers) reads X-Client-ID once. It resolves the header with the limiter's new ClientConcurrencyLimiter::client_id_tag, which checks only the static SPACE_API_CLIENT_CONCURRENCY_KNOWN_CLIENTS allowlist, and stamps the result on the route metric identity. This covers sandbox.api.latency_ms and sandbox.api.request_error.
  • The middleware also wraps the handler in a task-local scope (scope_request_client_id). api_identity_tags uses that scope when an identity has no client ID of its own. Step latency and errors (sandbox.api.step.latency_ms, sandbox.api.error, including the create steps), restore-claim laps, wake-coalescer outcomes, template selection and dormant activation pick up client_id this way, with no edits at the roughly 90 places that build identities.
  • A client ID the handler stamps itself still takes precedence over the scope. Connect keeps its limiter-resolved ID.

ClientIdTag moves to sandbox/api/client_id.rs, next to the new scope. As a result, api.rs shrinks from 1111 to 1094 code lines.

Cardinality

The tag value is always a ClientIdTag: an allowlisted ID, or unknown for anything else. The raw header never becomes a tag. The route middleware skips the Eppo override map, so it adds no per-request flag evaluation and can never produce overflow. Each affected series multiplies by at most the number of allowlisted clients plus one.

Not covered

These report unknown:

  • Work a request spawns onto another task.
  • The gRPC server metrics.
  • Background reconciler, wake-completion and limiter-gauge metrics, which aren't request-scoped.
  • Override-only callers on route metrics. They are still named on the limiter decision metric.

docs/observability.md documents this.

Validation

  • cargo test -p pplx-space-metrics --lib sandbox::api covers four cases:
    • A route metric carries the stamped client ID.
    • Identity metrics and sandbox.api.created inherit the request scope, and report unknown outside it.
    • A stamped ID wins over the scope.
    • The existing route tests still pass.
  • cargo test -p pplx-space-api --lib client_concurrency::tests::client_id_tag checks that only allowlisted headers are named, and that missing, empty and unlisted headers read unknown.
  • cargo clippy --locked -p pplx-space-metrics -p pplx-space-api --all-features --tests -- -D warnings passes.
  • Not validated: the middleware's end-to-end route tests need Postgres. I didn't run them locally, so CI covers them.

Severity and risk

Severity: observability gap. On-call can't tell which caller is driving create, connect or kill latency and error spikes.

Risk: the tag set grows on existing metrics, which is bounded as described above. Datadog queries that don't group by client_id are unaffected. Every sandbox route now runs one header read and one allowlist lookup. Roll back by reverting this PR.

@kumarpallav-pplx
kumarpallav-pplx added this pull request to the merge queue Nov 2, 2026
Merged via the queue into main with commit 0857533 Nov 2, 2026
25 checks passed
@kumarpallav-pplx
kumarpallav-pplx deleted the kumar/api-metrics-client-id branch November 2, 2026 21:55
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