Skip to content

[api] tag limiter decisions and reconnect metrics with the bounded client id - #1058

Merged
the-gigi-pplx merged 1 commit into
mainfrom
gigi/pla-6000-tag-limiter-reconnect-metrics-bounded-client-id
Oct 30, 2026
Merged

the-gigi-pplx merged 1 commit into
mainfrom
gigi/pla-6000-tag-limiter-reconnect-metrics-bounded-client-id

Conversation

@the-gigi-pplx

@the-gigi-pplx the-gigi-pplx commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

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. Today sandbox.api.client_concurrency.decision says only client_scope:client|unspecified, and the reconnect metrics carry only client_class. An allowlisted caller with no class mapping shows up as other, so an observe-mode sample or an enforce cutover can't say which caller the would-rejects or 429s land on.

The tag value is the caller's X-Client-ID when the limiter allowlists it, else unknown. The allowlist is SPACE_API_CLIENT_CONCURRENCY_KNOWN_CLIENTS plus 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 to unknown and 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):
    • New ClientIdTag, carried on SandboxApiMetricIdentity via with_client_id. Its variants are private. A value exists only as ClientIdTag::UNKNOWN, ClientIdTag::OVERFLOW, or through ClientIdTag::allowlisted, whose caller owns the bound.
    • sandbox.reconnect.attempts and sandbox.reconnect.duration_ms gain a client_id tag.
    • client_limit::decision takes the ClientIdTag instead of a ClientScope and emits client_id alongside client_scope. It derives client_scope from the tag, so the two can't disagree.
  • Limiter (crates/libs/api/src/client_concurrency/mod.rs):
    • ClientKey::classify resolves 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.
    • Key-set entries that can't name a client are ignored, and their callers share the unspecified bucket:
      • the reserved keys unknown, overflow and native, in any case;
      • entries that aren't valid Kubernetes label values, checked with the existing LabelValue grammar.
        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.
    • The metric tag derives from the key (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 the Copy metric identity:
      • The static allowlist is interned at construction, so its keys borrow that copy and its lookups take no lock.
      • Override-only IDs are interned when first tagged, capped at 64 per process, because override keys can change at runtime and are never freed. Past the cap, an override-only client keeps its own bucket but reports client_id:overflow with client_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.
    • The limiter attributes every decision, including the disabled, zero-cap, misconfigured and store-error fail-open paths, and returns the tag on ClientConcurrencyDecision::client_id(). While the limiter is off it resolves against the static allowlist only, matching how it already derived client_scope there.
    • Startup also warns when the static allowlist has IDs that differ only in case. They keep separate buckets but share one client_id series after Datadog lowercases it.
  • ECW connect (crates/libs/api/src/routes/e2b/lifecycle.rs): reserves the slot first, then stamps the limiter's client_id onto 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 report unknown.

Validation

At this head:

  • cargo clippy --workspace --all-features --all-targets -- -D warnings is clean.
  • Full workspace through CI's entry points on a Linux build box. run-rust-checks.sh tests-unprivileged passed 10903 tests at the previous head, and the touched crates' suites pass 2109 tests at this one. tests-privileged passed 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: dropping native from 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_runtime pins 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, overflow or native, 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_id added 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 the client_id tag, its allowlist, its unknown cases, and the overflow cap.
  • docs/configuration.md: SPACE_API_CLIENT_CONCURRENCY_KNOWN_CLIENTS notes that it also bounds client_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.

@the-gigi-pplx
the-gigi-pplx force-pushed the gigi/pla-6000-tag-limiter-reconnect-metrics-bounded-client-id branch 7 times, most recently from d05dc44 to b9fc73a Compare October 28, 2026 21:55
@the-gigi-pplx
the-gigi-pplx marked this pull request as ready for review October 28, 2026 22:20
@the-gigi-pplx
the-gigi-pplx force-pushed the gigi/pla-6000-tag-limiter-reconnect-metrics-bounded-client-id branch from b9fc73a to 07b15f1 Compare October 30, 2026 07:13
@the-gigi-pplx

Copy link
Copy Markdown
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:

  • native is reserved alongside unknown and overflow, in any case.
  • Key-set entries that aren't valid Kubernetes label values, whether static entries or override keys, are ignored, and their callers share the unspecified bucket. Static entries are reported in a startup warning. An operator with such an entry renames it before upgrading.
  • The metric tag derives from ClientKey, and interning and the overflow cap are now part of that conversion rather than of bucketing.
  • ClientIdTag's variants are private. Values come from ClientIdTag::UNKNOWN, ClientIdTag::OVERFLOW or ClientIdTag::allowlisted.

Redis bucket keys for valid, non-reserved clients are unchanged. The description is updated to match.

@the-gigi-pplx
the-gigi-pplx force-pushed the gigi/pla-6000-tag-limiter-reconnect-metrics-bounded-client-id branch 2 times, most recently from 9879d37 to 2c8b76f Compare October 30, 2026 07:35
@the-gigi-pplx
the-gigi-pplx force-pushed the gigi/pla-6000-tag-limiter-reconnect-metrics-bounded-client-id branch from 2c8b76f to d955258 Compare October 30, 2026 16:32
@the-gigi-pplx
the-gigi-pplx added this pull request to the merge queue Oct 30, 2026
Merged via the queue into main with commit 2f4b4e5 Oct 30, 2026
23 checks passed
@the-gigi-pplx
the-gigi-pplx deleted the gigi/pla-6000-tag-limiter-reconnect-metrics-bounded-client-id branch October 30, 2026 16:59
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