You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
metrics: move the control plane's families to their owners and delete pplx-space-metrics - #1541
api:sandbox::api (with client_id, requested and tests), restore, sandbox_fork, node_group_placement, runtime_upgrade and operations, plus its parts of warm_pool, admin, auth, internal_api, node_drain, node_service_readiness, reconcile and template.
cluster-controller: the reconcilers' families, transient_lifecycle and sandbox::reconciler.
Other owners: cluster-reconciler, internal-api, data-plane-server, ingress-admission, and public-api-http (response).
pplx-space-sandbox-metrics gains admission and publication, each recorded by two services.
Crate removal: the unmoved and legacy entries and the [layers.direction] baseline edge go, ALSO drops ingress-admission, and docs and skill point to the owners (plugin 0.2.52).
The node-selector source guard (scripts/source_guards/node_selector_extras_smoke.py) now skips binaries under crates/tools/, which are never deployed (crates/tools/AGENTS.md); the workload tool reaches the sandbox node
selector through api. Deployed binaries are checked as before, and a new test covers the exemption.
Porting pplx_space_metrics paths
For open PRs that import pplx_space_metrics::… or edit crates/libs/metrics, generated from the code at this PR's head. Crate names drop the pplx_space_ prefix. Inside the owner, use crate::metrics::…; outside it, use pplx_space_<crate>::metrics::…. Short forms: CC = cluster_controller_server::metrics, IA = internal_api_server::metrics, API = api::metrics, SM = sbx_mgr::metrics, NM = node_mgr, SBX = sandbox_metrics. An item not listed moves with its row. Names, tags and values are unchanged.
As #1515, at 0756e6cb: the same 41 environmental failures. cargo arch reports one new advisory, the legacy removal. The dump is identical to main's; literals.py matches exactly (215 removed, 215 added).
Severity and risk
Severity: does not apply (structural). Risk: compile-time only; roll back by reverting. External code that imports a moved path breaks. Downstream: package pplx-space-metrics and every pplx_space_metrics::… path are gone.
Automatic approval is disabled by the configured approval policy for this pull request. Review comments and requests for changes continue normally.
Commit: 0756e6cb5ca9.
No actionable comments were generated in the recent review.
Walkthrough
The pull request removes pplx-space-metrics from the workspace and redirects metric definitions and call sites to service-owned modules and pplx-space-sandbox-metrics. It adds local metrics for API, controller, and reconciliation events. Existing metric arguments and surrounding control flow remain unchanged where described. Renderer discovery now excludes binaries under crates/tools/, and the plugin version changes to 0.2.52.
Possibly related PRs
perplexityai/SPACE#1540: Moves node-side metric families out of pplx-space-metrics and creates pplx-space-sandbox-metrics.
perplexityai/SPACE#1515: Moves Foundation, storage, and shared metric families from pplx-space-metrics.
perplexityai/SPACE#1517: Leaves a dependency on pplx-space-metrics that this pull request removes.
Merge Risk: High · up to 0756e
The metrics migration has no confirmed functional defect, but the renderer guard now exempts tools binaries. Confirm that this narrower check is acceptable before merging.
Security architecture risk: Low · up to 0756e
The inspected changes relocate telemetry without changing admission decisions, credential verification, or operation-state rules. The validation exemption matches the documented tool-only deployment model. No security regression was identified in the examined paths, but coverage of the broad migration and production deployment assumptions remains incomplete.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
inferred — The cross-service impact examined here is shared telemetry vocabulary and emission, not transferred admission or publication authority. A contract regression could affect diagnostics across multiple services, but the inspected base-to-head comparisons do not show broader tenant reachability or privileges.
Trust Boundaries and Controls
observed — Cluster admission policy remains API-owned. The changed decision sink receives the same decision enums and optional token value and emits counters or gauges. The shared module supplies telemetry rather than acquiring Redis, identity, or substrate-control authority.
Pre-merge checks: 6
Check name
Status
Explanation
Title check
The title describes the metrics migration and crate removal. It meets the specified format.
Description Follows The Pr Template
The description contains all four required items: Summary, Validation, Severity and risk, and an AI assistance line.
Bug Fixes Name A Regression Test
The PR moves metrics to owner crates and removes the shared crate. The application changes shown update metric paths, not application defects. The source-guard change is outside the application paths …
Weakened Safeguards Are Justified
The guard excludes tools binaries because they are not deployed; the description and adjacent comment state this reason. The removed metrics crate explains the CI and guidance updates. Moved metric te…
Agent Tooling Runs No New Commands
The plugin manifests change only their versions. The skill and reference changes are prose. No executable agent configuration or inline shell substitution was added.
Public-Safe Content
The title, description, and diff contain no identified prohibited content. Credential references are identifiers, not credential values. The security-related comment in node_group_labeler.rs was alr…
Full details: Bug Fixes Name A Regression Test
Explanation
The PR moves metrics to owner crates and removes the shared crate. The application changes shown update metric paths, not application defects. The source-guard change is outside the application paths covered by this check.
Full details: Weakened Safeguards Are Justified
Explanation
The guard excludes tools binaries because they are not deployed; the description and adjacent comment state this reason. The removed metrics crate explains the CI and guidance updates. Moved metric tests remain enabled in their new owners.
Full details: Public-Safe Content
Explanation
The title, description, and diff contain no identified prohibited content. Credential references are identifiers, not credential values. The security-related comment in node_group_labeler.rs was already present in the base revision.
Recent review info
Automatic approval policy
Commit: 0756e6cb5ca9.
Setting
Effective value
Source
Automatic approvals
Disabled
Override 3
Merge risk
Moderate
Global configuration
Security risk
Not set
Global configuration
Matching overrides (numbered as in the resolved configuration):
Any matching disabled override withholds approval, regardless of risk. Otherwise, the strictest explicitly set limit wins for each score. Limits omitted by all matching overrides inherit the global configuration.
Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 6 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Approving. The metric definitions and callers preserve the same measurements, including names, labels, units and request-scoped client IDs. The shared contracts reuse existing packages, and the moved metric tests passed in CI.
… pplx-space-metrics
Last of three changes that split pplx-space-metrics. The families the
control plane records move, unchanged, into a `metrics` module of the crate
that records them, and the crate is deleted:
- api: `sandbox::api` (with `client_id`, `requested` and their tests),
`sandbox::restore`, `sandbox_fork`, `node_group_placement`,
`runtime_upgrade`, `operations` and the API server's parts of
`warm_pool`, `admin`, `auth`, `internal_api`, `node_drain`,
`node_service_readiness`, `reconcile` and `template`. The two families the
cluster controller records as well, `operations::completed` and
`node_drain::local_snapshot_promotion`, stay here; the controller already
depends on the API crate.
- cluster-controller-server: `cluster_reconciler`, `leader_election`,
`node_drain`, `node_group_labeler`, `node_group_mirror`,
`node_service_readiness`, `snapshot_storage_sweeper`, `tombstone`, the
warm-pool reconciler's `warm_pool`, `sandbox::transient_lifecycle`, the
orphaned-sandbox reconciler's `sandbox::reconciler`, the audit and session
retention counters, the dispatcher's slots and the reconcile queue GC.
- cluster-reconciler (queue lag, stale claims and trigger fan-out),
internal-api-server (`internal_api`, template visibility lookups, template
node binding, desired-state deletion reports), data-plane-server
(`data_plane`), ingress-admission (the waiter rejection counter) and
public-api-http (`response`, which the API server, data-plane-server and
native-files use from there).
- pplx-space-sandbox-metrics gains `admission`, which the API server and
sbx-mgr both record, and `publication`, which internal-api and sbx-mgr both
record.
The `metrics_workload` validation workload now calls the API crate's and
pplx-space-sandbox-metrics's helpers. With the crate gone, ARCHITECTURE.toml
drops its `unmoved` and `legacy` entries and architecture-baseline.toml its
`[layers.direction]` edge to pplx-space-common-core; the isolated-crates
lint's `ALSO` list drops pplx-space-ingress-admission, which reached
common-core only through it; and the docs, skills and comments that named it
point to the owners. The external console password-session families and the
provisioning-recipe family, added to pplx-space-metrics since, move with
their recorders to the API crate's `metrics::auth` and internal-api's
`metrics::internal_api`.
The node-selector smoke guard skips binaries in the tools layer, which are
never deployed, so the workload's new dependency on the API crate does not
make it a renderer; deployed binaries stay covered.
Metric names, tag keys, kinds and vocabulary values are unchanged: the dump
of every emit site (with its enclosing function) and vocabulary arm is
identical to main's, and the string literals on the removed and added lines
of the Rust diff match.
@johnlouie-pplx The stack is rebased onto main 8d62086. You approved #1515 at its previous head, and that approval covers the re-home commit. Since your last approval of this PR, these changes go beyond conflict resolution:
CI list:ALSO in ci/scripts/lib/rust-isolated-crates.sh now holds only postgres-pool and rpc-auth. The other entries are crates the selection rule now picks by itself, and the script rejects an entry the rule already selects.
Description: it now has a generated table mapping each old pplx_space_metrics:: path to its new crate, for open PRs that import the old paths.
At 0756e6cb: the dump of names, tag keys and tag values is identical to main's (3,141 lines). literals.py matches exactly (215 removed, 215 added). tests-unprivileged shows the same 41 environmental failures as main, and cargo arch reports one new advisory, the legacy removal.
@johnlouie-pplx We enqueued the stack on your approvals of #1515 (at 3929a41) and of this PR at its earlier head, without waiting for your re-look on the changes listed above. They are the same kind of move you approved: families main added since are re-homed, plus the #1665 import port and the isolated-crates list. CI is green at all three heads, and the stack merged onto current main leaves no reference to pplx-space-metrics. If anything in that list looks wrong to you, we'll fix it in a follow-up PR.
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
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.
johnlouie-pplx
left a comment
Summary
Last of three; deletes
pplx-space-metrics.sandbox::api(withclient_id,requestedand tests),restore,sandbox_fork,node_group_placement,runtime_upgradeandoperations, plus its parts ofwarm_pool,admin,auth,internal_api,node_drain,node_service_readiness,reconcileandtemplate.transient_lifecycleandsandbox::reconciler.response).pplx-space-sandbox-metricsgainsadmissionandpublication, each recorded by two services.unmovedandlegacyentries and the[layers.direction]baseline edge go,ALSOdrops ingress-admission, and docs and skill point to the owners (plugin 0.2.52).The node-selector source guard (
scripts/source_guards/node_selector_extras_smoke.py) now skips binaries undercrates/tools/, which are never deployed (crates/tools/AGENTS.md); the workload tool reaches the sandbox nodeselector through api. Deployed binaries are checked as before, and a new test covers the exemption.
Porting
pplx_space_metricspathsFor open PRs that import
pplx_space_metrics::…or editcrates/libs/metrics, generated from the code at this PR's head. Crate names drop thepplx_space_prefix. Inside the owner, usecrate::metrics::…; outside it, usepplx_space_<crate>::metrics::…. Short forms: CC =cluster_controller_server::metrics, IA =internal_api_server::metrics, API =api::metrics, SM =sbx_mgr::metrics, NM =node_mgr, SBX =sandbox_metrics. An item not listed moves with its row. Names, tags and values are unchanged.pplx_space_metrics::adminauthbyok,node_manager,node_networkcluster_reconciler,leader_election,node_group_labeler,node_group_mirror,snapshot_storage_sweeper,tombstonecredmgrdata_planedb::poolfeature_flags,object_store,tokio_runtimejobsgatewaygateway::readinessgrpc,payloadinternal_apinode_apinode_disknode_drainnode_service_readinessnode_group_placement,runtime_upgrade,sandbox_fork,sandbox::restoreoperationsreconcileresponseruntime_supervisorsandbox::admissionsandbox::api(withclient_limit,ClientIdTag,*request_client_id)sandbox::api::reconcilersandbox::publicationsandbox::transient_lifecyclesandbox::vm,sandbox_eviction,snapshot,stream_presencesandbox::workersnapshot_retention_sweepertemplatetest_capturetest-capturefeature)volume_managerwarm_poolValidation
As #1515, at
0756e6cb: the same 41 environmental failures.cargo archreports one new advisory, thelegacyremoval. The dump is identical to main's;literals.pymatches exactly (215 removed, 215 added).Severity and risk
Severity: does not apply (structural). Risk: compile-time only; roll back by reverting. External code that imports a moved path breaks. Downstream: package
pplx-space-metricsand everypplx_space_metrics::…path are gone.Stack
AI assistance: Claude Code drafted the code, scripts and this text.