Repository navigation
[api] tag limiter decisions and reconnect metrics with the bounded client id - #1058
Merged
the-gigi-pplx merged 1 commit intoOct 30, 2026
Conversation
the-gigi-pplx
force-pushed
the
gigi/pla-6000-tag-limiter-reconnect-metrics-bounded-client-id
branch
7 times, most recently
from
October 28, 2026 21:55
d05dc44 to
b9fc73a
Compare
the-gigi-pplx
marked this pull request as ready for review
October 28, 2026 22:20
nikhilbirmiwal-pplx
approved these changes
Oct 29, 2026
shehan-pplx
approved these changes
Oct 29, 2026
the-gigi-pplx
force-pushed
the
gigi/pla-6000-tag-limiter-reconnect-metrics-bounded-client-id
branch
from
October 30, 2026 07:13
b9fc73a to
07b15f1
Compare
Collaborator
Author
|
This adjusts the client key to the per-client admission RFC in PR #1072 ("Decision 2: ids are stored, keys are resolved at use"). This PR is where the key and its resolver live. What changed:
Redis bucket keys for valid, non-reserved clients are unchanged. The description is updated to match. |
the-gigi-pplx
force-pushed
the
gigi/pla-6000-tag-limiter-reconnect-metrics-bounded-client-id
branch
2 times, most recently
from
October 30, 2026 07:35
9879d37 to
2c8b76f
Compare
the-gigi-pplx
force-pushed
the
gigi/pla-6000-tag-limiter-reconnect-metrics-bounded-client-id
branch
from
October 30, 2026 16:32
2c8b76f to
d955258
Compare
the-gigi-pplx
deleted the
gigi/pla-6000-tag-limiter-reconnect-metrics-bounded-client-id
branch
October 30, 2026 16:59
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.
Summary
Tags the per-client concurrency limiter's decision metric and the ECW reconnect metrics with a bounded
client_id. That makes over-cap and rejected traffic attributable to a caller. Todaysandbox.api.client_concurrency.decisionsays onlyclient_scope:client|unspecified, and the reconnect metrics carry onlyclient_class. An allowlisted caller with no class mapping shows up asother, so an observe-mode sample or anenforcecutover can't say which caller the would-rejects or 429s land on.The tag value is the caller's
X-Client-IDwhen the limiter allowlists it, elseunknown. The allowlist isSPACE_API_CLIENT_CONCURRENCY_KNOWN_CLIENTSplus the runtime quota-override keys. Both are operator configuration, so the tag's cardinality is bounded by what an operator listed. A caller sending arbitrary or rotating IDs collapses tounknownand can't mint series.What changed
The key rules follow the per-client admission RFC in PR #1072 ("Decision 2: ids are stored, keys are resolved at use"). This PR is where the client key and its resolver live.
pplx-space-metrics(crates/libs/metrics/src/sandbox/api.rs):ClientIdTag, carried onSandboxApiMetricIdentityviawith_client_id. Its variants are private. A value exists only asClientIdTag::UNKNOWN,ClientIdTag::OVERFLOW, or throughClientIdTag::allowlisted, whose caller owns the bound.sandbox.reconnect.attemptsandsandbox.reconnect.duration_msgain aclient_idtag.client_limit::decisiontakes theClientIdTaginstead of aClientScopeand emitsclient_idalongsideclient_scope. It derivesclient_scopefrom the tag, so the two can't disagree.crates/libs/api/src/client_concurrency/mod.rs):ClientKey::classifyresolves a header to a key against the key set: the static allowlist plus the override keys. An ID the key set names gets its own bucket, and everything else shares the unspecified one.unknown,overflowandnative, in any case;LabelValuegrammar.The static list is checked once at construction, with a warning per ignored entry. An override key is checked only once a header matches it. The first such match warns once per key per process, up to 64 keys.
ClientKey::client_id_tag), and interning is a rule of that conversion, not of bucketing. IDs are interned for the process lifetime so the tag can ride in theCopymetric identity:client_id:overflowwithclient_scope:client. Which IDs fit is first-come per process, so replicas can name different override-only clients once the cap fills. Once it's full, the read path refuses new IDs without taking the write lock.ClientConcurrencyDecision::client_id(). While the limiter is off it resolves against the static allowlist only, matching how it already derivedclient_scopethere.client_idseries after Datadog lowercases it.crates/libs/api/src/routes/e2b/lifecycle.rs): reserves the slot first, then stamps the limiter'sclient_idonto the reconnect metric identity. The route rejections and the controller wake path report the same caller the limiter saw. Data-plane reconnects aren't attributed and reportunknown.Validation
At this head:
cargo clippy --workspace --all-features --all-targets -- -D warningsis clean.run-rust-checks.sh tests-unprivilegedpassed 10903 tests at the previous head, and the touched crates' suites pass 2109 tests at this one.tests-privilegedpassed 61 of 67; the 6 failures are in node-runtime and guest-worker crates this PR doesn't touch. They need fs-verity, root-owned path ancestors, or a private cgroup namespace, which the box lacks and CI's privileged lane provides.cargo arch: 0 required findings.Tests cover bucket resolution, not emitted tags:
key_set_entries_that_cant_name_a_client_are_ignored: each reserved key in two cases, plus entries with a space, a leading dash,|,,and a 64-character value, share the unspecified bucket. That holds whether the entry is in the static list or only an override key, and valid entries alongside them still get their own bucket. Three mutants each fail it: droppingnativefrom the reserved keys, keeping a non-label static entry instead of skipping it after the warning, and skipping the check on override keys.override_only_ids_keep_their_bucket_past_the_interning_cap: more override-only IDs than the cap all keep their own bucket and cap. The interned set stops at the cap, and an ID interned before it filled resolves to the same copy again.eppo_override_key_allowlists_a_client_at_runtimepins the resolved ID, not just the bucket kind.Not validated against a live deployment.
Dependencies
Severity and risk
Severity: observability only. Admission and caps are unchanged. The bucket change: a caller whose allowlisted ID is reserved (
unknown,overflowornative, in any case) or isn't a valid Kubernetes label value now shares the unspecified bucket. Operators with such an entry rename it before upgrading.Risk: the main cost is metric series. Each of the three metrics grows by a factor of at most (allowlist size + override keys + 1). The reconnect duration is a distribution, so that's its most expensive series. Deployments that filter metric tags by key need
client_idadded to these three metrics, or the new tag is dropped and the metrics look unchanged. Interned IDs are never freed: the static allowlist plus at most 64 override-only IDs per process. Roll back by reverting. The tag is additive, so existing queries on the other tags keep working.Docs
docs/observability.md: the reconnect-attribution section describes theclient_idtag, its allowlist, itsunknowncases, and theoverflowcap.docs/configuration.md:SPACE_API_CLIENT_CONCURRENCY_KNOWN_CLIENTSnotes that it also boundsclient_id, which IDs are reserved or ignored, and how IDs differing only in case report.AI assistance: Claude Code drafted the code, tests, docs, and this description.