Repository navigation
layers: delete pplx-space-common-svc - #1575
Merged
Merged
Conversation
|
Important Review skippedToo many files! This PR contains 399 files, which is 99 over the limit of 300. To get a review, reduce the PR to 300 files or fewer by splitting it into smaller PRs or changing its base branch.
 |
|
Thanks for opening this pull request! A SPACE maintainer will take a look. CONTRIBUTING.md explains how we review outside contributions. |
ac-pplx
force-pushed
the
ac/delete-common-svc
branch
from
November 5, 2026 23:15
294871f to
2b5ae33
Compare
ac-pplx
force-pushed
the
ac/delete-common-svc
branch
from
November 6, 2026 00:19
2b5ae33 to
870942c
Compare
ac-pplx
marked this pull request as ready for review
November 6, 2026 04:54
shehan-pplx
approved these changes
Nov 6, 2026
shehan-pplx
left a comment
shehan-pplx
left a comment
Member
There was a problem hiding this comment.
Reviewed by Shehan's Review Bot
ac-pplx
force-pushed
the
ac/delete-common-svc
branch
from
November 6, 2026 15:35
870942c to
b9a5efc
Compare
ac-pplx
force-pushed
the
ac/delete-common-svc
branch
from
November 6, 2026 17:59
b9a5efc to
dc970cd
Compare
Moves what is left of common-svc into its owning crates and deletes it: - pplx-space-grpc-server (crates/l40_api/grpc/server, new): the server builder and its timeouts, the health service, the gRPC trace layer and the request-parameter logging (`bootstrap.rs`, `grpc_trace.rs`, `grpc_request_params.rs`, their tests and `tests/grpc_server_timing.rs`). - pplx-space-local-health (crates/l40_api/local/health, new): `local_health`. Both are l40_api: the trace layer names SPACE span fields and the space retry class, and every user is an API crate. - pplx-space-service-host (crates/l40_api/service/host, new): the `init_service*` family, `shutdown_telemetry`, `TelemetryGuard` and `tests/logging_process.rs`, with the `otel` and `datadog` features. It is l40 because it reads the process cluster id from pplx-space-cluster-id. - `SandboxSpanLayer`, whose only user is net-gateway, moves there (`sandbox_span`); `ServiceName`, whose only user is the runtime manager, moves there (`service_name`); the cache disk-admission handle moves into node-mgr as `disk_admission::cache_handle`. - The thirteen unused types (`AuthContext`, the parse-error and bad-request helpers, `HttpsPollingUrl`, `ServiceEndpoint` and the four socket-path newtypes) are deleted. Every dependent imports from the owning crates instead of the re-exports, and the `otel`/`datadog` features that forwarded to common-svc forward to service-host where the crate calls it, and to observability otherwise. The deprecated `datadog-otel` feature is not carried over. node-mgr's dev-dependencies declare the tokio `rt-multi-thread` feature its tests used to get through common-svc. ARCHITECTURE.toml drops common-svc from `unmoved` and `legacy`; the baseline drops its direction, storage_access and crate_names entries. CI drops common-svc's clippy step and its special case in the isolated-crates selection, and lints grpc-server, local-health and service-host alone (ALSO), which covers service-host without its telemetry features. Docs, skills, the Kind pointers, the log-fixture build script and the crate-move map follow. Event targets change for events without an explicit target: `pplx_space_common_svc::bootstrap::grpc_trace` (the `grpc.server` span) becomes `pplx_space_grpc_server::grpc_trace`; `pplx_space_common_svc::local_health` (5 events) becomes `pplx_space_local_health`; `pplx_space_common_svc::disk_admission` (4) becomes `pplx_space_node_mgr::disk_admission::cache_handle`; and `pplx_space_common_svc` (the telemetry shutdown warning) becomes `pplx_space_service_host`.
ac-pplx
force-pushed
the
ac/delete-common-svc
branch
from
November 6, 2026 18:28
dc970cd to
482fc86
Compare
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
Deletes
pplx-space-common-svc, the last of three changes.bootstrap→ newpplx-space-grpc-server(crates/l40_api/grpc/server), exceptSandboxSpanLayer, whichmoves to net-gateway, its only user;
local_health→ newpplx-space-local-health(crates/l40_api/local/health);interprets the space retry class, and every user is an l40 crate;
init_service*family,shutdown_telemetryandTelemetryGuard→ newpplx-space-service-host(
crates/l40_api/service/host, featuresotelanddatadog; l40 because it reads the process cluster ID);ServiceName→pplx_space_node_runtime_manager::service_name, its only user;pplx_space_node_mgr::disk_admission::cache_handle, its only user.AuthContext,DomainParseError,GrpcParseError,invalid_argument,HttpParseError,HttpBadRequest,bad_request,HttpsPollingUrl,ServiceEndpoint,VsockUdsPath,SpacedHostSocketPath,NetGatewayControlSocketPath,BridgeControlSocketPath).otel/datadogfeatures forward to service-host whereit calls it, otherwise to observability; the deprecated
datadog-otelis not carried over. volmgr's and cluster-controller-server'sfeatures say they reach only their own tests (their svc binaries enable service-host's). node-mgr's
dev-dependencies declare tokio
rt-multi-thread, which used to arrive through common-svc.ARCHITECTURE.toml'sunmovedandlegacylists; its remainingdirection,storage_accessand
crate_namesbaseline entries go. CI drops common-svc's clippy step and its isolated-crates special case;grpc-server, local-health and service-host join the isolated-crates
ALSOlist, which covers service-host withoutits telemetry features. Docs, skills, Kind pointers and the log-fixture script (now
-p pplx-space-service-host)follow.
Event targets that change: the
grpc.serverspan (pplx_space_common_svc::bootstrap::grpc_trace→pplx_space_grpc_server::grpc_trace); fivelocal_healthevents (→pplx_space_local_health); fourdisk_admissionevents (→pplx_space_node_mgr::disk_admission::cache_handle); one root event(→
pplx_space_service_host). Events with an explicittarget: "grpc.server"don't change. No metric, span field orwire text changes.
Dependency graphs of the 17 service binaries and sbx-mgr against main: no kept crate gains a feature. The binaries
stop linking packages only common-svc pulled in (sqlx-postgres, redis, rustls-native-certs and similar). The only
runtime-relevant feature losses are
tracing/login 4 binaries andtower/login 14.Porting from
pplx-space-common-svcWhere each item lives after this stack (#1516, #1574 and this PR), for open PRs that still use it:
pplx_space_common_svc::…)init_service,init_service_with_log_context,init_service_subprocess*,shutdown_telemetry; featuresotel,datadogpplx_space_service_host(featuresotel,datadog)TelemetryGuard,LogContext,ObservabilityError,observabilitypplx_space_observabilityshutdown_token*,sigterm_shutdown,stop_tokens,StopTokens,wait_for_sigterm,lifecycle,DEFAULT_SERVICE_MUTEX_TIMEOUT,DEFAULT_DRAIN_TIMEOUT,drain_server,DrainSignalpplx_space_service_lifecycletokio_runtimepplx_space_tokio_runtimegrpc_server_builder*,grpc_health_service,grpc_trace_layer,GRPC_REQUEST_TIMEOUT,GRPC_HTTP2_KEEPALIVE_*,format_grpc_request_params,log_failed_grpc_request_params,record_safe_grpc_request_params,truncate_utf8_for_logpplx_space_grpc_servercurrent_service_name,set_current_service_name,CLIENT_SERVICE_METADATA_KEYpplx_space_common_grpchttp_trace_layer,HttpServerTraceLayer,HttpServerMakeSpan,HttpSpanMetadata,TraceparentResponseLayer,is_probe_request,REDACTED_HTTP_PATH_VALUE,http_listenerpplx_space_http_serverrequest_id_from_headerspplx_space_request_idbind_unix_socket,UnixSocketOptions,SocketGroup,unix_group_exists,apply_unix_socket_permissionspplx_space_unix_socket_bindtypes::UnixSocketPathpplx_space_unix_socket_pathlocal_health,LocalHealth*,local_health_pairpplx_space_local_healthauth,template_lookup(featuresk8s,test-hooks,test-support)pplx_space_rpc_auth(same features);kube_client→pplx_space_kube_clientsql(pools, identity);sql::contentionpplx_space_postgres_pool;pplx_space_postgres_pool::contentionsql::sandbox_residencypplx_space_sandbox_residencysqlstoresmigration-checksumspplx-space-migration-checksumstypes::k8s(space_k8s_node_id,space_node_id)pplx_space_k8s_node_idtypes::reconcile,types::auto_lifecyclepplx_space_state_reconciliation,pplx_space_auto_lifecycle_receiptpeer_authand its re-exports (PeerIdentity,UdsPeerAuthorizer, …)pplx_space_uds_peer_authservice_mutexand its re-exports (ServiceMutex, …)pplx_space_service_mutexrate_limiterand its re-exportspplx_space_grpc_rate_limitercore;space_api_route*;bearer_token;dependency_drain;ecw_credential;federated_identity;page_cache;systemd;ingress_admission;redis,shared_valkey;install_default_rustls_crypto_providerpplx_space_common_core,pplx_space_api_route,pplx_space_bearer_token,pplx_space_dependency_drain,pplx_space_ecw_credential,pplx_space_federated_identity,pplx_space_page_cache,pplx_space_systemd_watchdog,pplx_space_ingress_admission,pplx_space_valkey,pplx_space_rustls_providerSandboxSpanLayer,ServiceName,disk_admissionValidation
--all-targetsand--lib.Rust tests fail only 40 tests that need process permissions the validation host lacks and that fail the same way on
main.
cargo arch: no required findings; the baseline only shrinks.sql!locator comments shift where imports moved lines (statement bodies unchanged): apicontrollers/substrate/kubernetes/tests.rs(129, +2),routes/admin/node_maintenance.rs(6, −2),controllers/substrate/mod.rs(1, +2),controllers/sandbox/spaced.rs(1, +1),controllers/sandbox/mod.rs(1 of 7, −5),
routes/admin/templates/tests/native_build.rs(2 of 24, −2); internal-api-servertests.rs(85, +1),tests/published_state.rs(37, +1),metadata.rs(10, −2),lib.rs(9, −12); credmgrstorage_vend_store.rs(6, +1),
kek_provisioning.rs(3, +2); db-migrateuser_password.rs(3, +1), sqlitelib.rs(2, −2),sandbox-quarantine-store
lib.rs(2, −1).Severity and risk
Severity: does not apply (structural). Risk: a crate built alone missing a feature it used to get through common-svc,
covered by the per-crate builds and the isolated-crates job; downstream code importing a
pplx_space_common_svc::path stops compiling. Roll back by reverting.
Rollout
Code outside this repository that builds against
pplx-space-common-svcneeds the same change this PR makes incrates/l50_app/*/svc:pplx-space-service-hostforinit_service*,shutdown_telemetryandTelemetryGuard,pplx-space-service-lifecyclefor the shutdown token and the service mutex timeout,pplx-space-tokio-runtimedirectly, and
otel/datadogforwarded to service-host. Those crates don't exist before this PR, so that changecan only be made against a revision that includes it.
Stack
AI assistance: Claude Code made the changes and drafted this description.