Merge pull request #1984 from diffora/am/user-cleanup-caller-context

feat(account-management): cleanup caller context

Author: Artifizer

libs/modkit-db/src/contention.rs

@@ -44,6 +44,16 @@ const MYSQL_DEADLOCK_SQLSTATE: &str = "40001";
 const PG_SERIALIZATION_FAILURE: &str = "40001";
 const PG_DEADLOCK_DETECTED: &str = "40P01";
 
+/// `PostgreSQL` retryable error MESSAGE fragments. sea-orm/sqlx surface the
+/// condition through the error's *message text* on the `Display` path, not the
+/// numeric SQLSTATE — so matching the code alone (`40001` / `40P01`) misses real
+/// serialization failures and deadlocks (e.g. `"could not serialize access due
+/// to concurrent update"` carries no `"40001"` substring). Matching the message
+/// is the reliable signal here. Message text is `lc_messages`-dependent, so the
+/// numeric-code checks above remain as a locale-independent belt-and-suspenders.
+const PG_SERIALIZATION_MSG: &str = "could not serialize access";
+const PG_DEADLOCK_MSG: &str = "deadlock detected";
+
 /// `SQLite` error codes for write contention.
 ///
 /// sqlx surfaces these as `"error returned from database: (code: N) database is locked"`.
@@ -82,7 +92,10 @@ fn is_mysql_deadlock(msg: &str) -> bool {
 }
 
 fn is_pg_contention(msg: &str) -> bool {
-    msg.contains(PG_SERIALIZATION_FAILURE) || msg.contains(PG_DEADLOCK_DETECTED)
+    msg.contains(PG_SERIALIZATION_FAILURE)
+        || msg.contains(PG_DEADLOCK_DETECTED)
+        || msg.contains(PG_SERIALIZATION_MSG)
+        || msg.contains(PG_DEADLOCK_MSG)
 }
 
 fn is_sqlite_busy(msg: &str) -> bool {
@@ -126,6 +139,23 @@ mod tests {
         assert!(is_retryable_contention(DbBackend::Postgres, &err));
     }
 
+    #[test]
+    fn pg_serialization_failure_by_message_detected() {
+        // Real sea-orm/sqlx Display carries the message, not the numeric
+        // SQLSTATE — this is the exact text Postgres emits for 40001 and the
+        // concurrent-DELETE regression that the numeric-only match missed.
+        let err = exec_err(
+            "error returned from database: could not serialize access due to concurrent update",
+        );
+        assert!(is_retryable_contention(DbBackend::Postgres, &err));
+    }
+
+    #[test]
+    fn pg_deadlock_by_message_detected() {
+        let err = exec_err("error returned from database: deadlock detected");
+        assert!(is_retryable_contention(DbBackend::Postgres, &err));
+    }
+
     // ── SQLite BUSY (code 5) ─────────────────────────────────────────
 
     #[test]

modules/system/account-management/account-management/src/domain/metrics.rs

@@ -20,6 +20,22 @@
 //! call-site migration onto the typed ports proceeds family-by-family;
 //! the bridge and the [`emit_*`] helpers are removed in the final
 //! cleanup PR once every call site has moved.
+//!
+//! ## Metric naming
+//!
+//! The `AM_*` constants are the **full, literal Prometheus names** —
+//! the exact series names that appear in Prometheus / `VictoriaMetrics`.
+//! They bake in the suffix that the OTel→Prometheus translation would
+//! otherwise add: counters carry `_total`, and gauges / histograms that
+//! measure a quantity carry the unit word (`_seconds`, `_milliseconds`);
+//! gauges that are bare counts carry no suffix. No `.with_unit()` hint
+//! is set on any instrument. Because the name is already literal and
+//! unit-free at the instrument level, the rendered Prometheus name is
+//! identical whether the collector has `add_metric_suffixes` on or off
+//! (the exporter dedups an existing `_total` and adds no unit suffix
+//! when none is configured). The const VALUES equal the rendered name
+//! under the default `"am"` prefix, so the facade-bridge `match family`
+//! dispatch routes correctly.
 
 use std::sync::Arc;
 use std::sync::LazyLock;
@@ -29,62 +45,62 @@ use modkit_macros::domain_model;
 
 // @cpt-begin:cpt-cf-account-management-dod-errors-observability-metric-catalog:p1:inst-dod-metric-catalog-constants
 /// Dependency-call health: `IdP` / Resource Group / GTS / `AuthZ` outbound calls.
-pub const AM_DEPENDENCY_HEALTH: &str = "am.dependency_health";
+pub const AM_DEPENDENCY_HEALTH: &str = "am_dependency_health_total";
 
 /// Tenant-metadata resolution operations and inheritance policy outcomes.
-pub const AM_METADATA_RESOLUTION: &str = "am.metadata_resolution";
+pub const AM_METADATA_RESOLUTION: &str = "am_metadata_resolution_total";
 
 /// Root-tenant bootstrap lifecycle (phase transitions, IdP-wait timeouts).
-pub const AM_BOOTSTRAP_LIFECYCLE: &str = "am.bootstrap_lifecycle";
+pub const AM_BOOTSTRAP_LIFECYCLE: &str = "am_bootstrap_lifecycle_total";
 
 /// Provisioning reaper / hard-delete / deprovision background job telemetry.
-pub const AM_TENANT_RETENTION: &str = "am.tenant_retention";
+pub const AM_TENANT_RETENTION: &str = "am_tenant_retention_total";
 
 /// Invalid retention-window configuration encountered while evaluating due-ness.
-pub const AM_RETENTION_INVALID_WINDOW: &str = "am.retention.invalid_window";
+pub const AM_RETENTION_INVALID_WINDOW: &str = "am_retention_invalid_window_total";
 
 /// Mode-conversion request transitions and outcomes.
-pub const AM_CONVERSION_LIFECYCLE: &str = "am.conversion_lifecycle";
+pub const AM_CONVERSION_LIFECYCLE: &str = "am_conversion_lifecycle_total";
 
 /// Hierarchy-depth threshold exceedance (warning-band + hard-limit rejects).
-pub const AM_HIERARCHY_DEPTH_EXCEEDANCE: &str = "am.hierarchy_depth_exceedance";
+pub const AM_HIERARCHY_DEPTH_EXCEEDANCE: &str = "am_hierarchy_depth_exceedance_total";
 
 /// Cross-tenant denial counter (security-alert candidate family).
-pub const AM_CROSS_TENANT_DENIAL: &str = "am.cross_tenant_denial";
+pub const AM_CROSS_TENANT_DENIAL: &str = "am_cross_tenant_denial_total";
 
 /// Hierarchy-integrity violation telemetry (one per integrity category).
-pub const AM_HIERARCHY_INTEGRITY_VIOLATIONS: &str = "am.hierarchy_integrity_violations";
+pub const AM_HIERARCHY_INTEGRITY_VIOLATIONS: &str = "am_hierarchy_integrity_violations";
 
 /// Periodic integrity-check job tick outcome (`outcome` ∈ `completed` |
 /// `skipped_in_progress` | `failed`). Distinguishes a clean tick from a
 /// never-ran job, which [`AM_HIERARCHY_INTEGRITY_VIOLATIONS`] alone cannot.
-pub const AM_HIERARCHY_INTEGRITY_RUNS: &str = "am.hierarchy_integrity_runs";
+pub const AM_HIERARCHY_INTEGRITY_RUNS: &str = "am_hierarchy_integrity_runs_total";
 
 /// Periodic auto-repair tick outcome — separate family from
 /// [`AM_HIERARCHY_INTEGRITY_RUNS`] so its fixed-label set is not widened.
-pub const AM_HIERARCHY_INTEGRITY_REPAIR_RUNS: &str = "am.hierarchy_integrity_repair_runs";
+pub const AM_HIERARCHY_INTEGRITY_REPAIR_RUNS: &str = "am_hierarchy_integrity_repair_runs_total";
 
 /// Periodic integrity-check tick wall-clock duration in milliseconds.
 /// The `phase` label disaggregates the check phase (`phase = "check"`)
 /// from the chained auto-repair phase (`phase = "repair"`) so
 /// dashboards can tell a slow check from a slow check + repair.
 /// Drives capacity-planning alerts ("p95 > 60s"), distinct from
 /// [`AM_HIERARCHY_INTEGRITY_RUNS`] which is a tick-outcome counter.
-pub const AM_HIERARCHY_INTEGRITY_DURATION: &str = "am.hierarchy_integrity_duration";
+pub const AM_HIERARCHY_INTEGRITY_DURATION: &str = "am_hierarchy_integrity_duration_milliseconds";
 
 /// Unix-epoch seconds of the last successful integrity-check tick.
 /// Used for a freshness watchdog (alert when `last_success` is older
 /// than twice the configured interval) that the violation gauge
 /// cannot satisfy on its own — a stuck job and a perfectly-clean tree
 /// look identical at the violation-gauge level until this gauge stops
 /// advancing.
-pub const AM_HIERARCHY_INTEGRITY_LAST_SUCCESS: &str = "am.hierarchy_integrity_last_success";
+pub const AM_HIERARCHY_INTEGRITY_LAST_SUCCESS: &str = "am_hierarchy_integrity_last_success_seconds";
 
 /// Unix-epoch seconds of the last failed integrity-check tick — paired
 /// with [`AM_HIERARCHY_INTEGRITY_LAST_SUCCESS`] so operators can tell
 /// "sustained failure" from "never ran" (the success gauge keeps the last
 /// good timestamp indefinitely).
-pub const AM_HIERARCHY_INTEGRITY_LAST_FAILURE: &str = "am.hierarchy_integrity_last_failure";
+pub const AM_HIERARCHY_INTEGRITY_LAST_FAILURE: &str = "am_hierarchy_integrity_last_failure_seconds";
 
 /// Lock-lifecycle event counter for `integrity_check_runs`. Emitted
 /// from [`crate::infra::storage::integrity::lock::release`] when the
@@ -97,7 +113,7 @@ pub const AM_HIERARCHY_INTEGRITY_LAST_FAILURE: &str = "am.hierarchy_integrity_la
 /// scheduler-tick outcome set) so dashboards keyed on
 /// `RUNS{outcome=*}` stay stable; this counter exists for
 /// lock-health alerting.
-pub const AM_INTEGRITY_LOCK_EVENTS: &str = "am.integrity_lock_events";
+pub const AM_INTEGRITY_LOCK_EVENTS: &str = "am_integrity_lock_events_total";
 
 /// Hierarchy-integrity repair telemetry. Emits one gauge sample per
 /// run with `category` ∈ all 10
@@ -107,13 +123,25 @@ pub const AM_INTEGRITY_LOCK_EVENTS: &str = "am.integrity_lock_events";
 /// did not appear). The five derivable categories carry counts only
 /// in `bucket = repaired`; the five operator-triage categories carry
 /// counts only in `bucket = deferred`.
-pub const AM_HIERARCHY_INTEGRITY_REPAIRED: &str = "am.hierarchy_integrity_repaired";
+pub const AM_HIERARCHY_INTEGRITY_REPAIRED: &str = "am_hierarchy_integrity_repaired";
 
 /// SERIALIZABLE-isolation retry telemetry for the AM repo's
 /// `with_serializable_retry` helper.
-pub const AM_SERIALIZABLE_RETRY: &str = "am.serializable_retry";
+pub const AM_SERIALIZABLE_RETRY: &str = "am_serializable_retry_total";
 // @cpt-end:cpt-cf-account-management-dod-errors-observability-metric-catalog:p1:inst-dod-metric-catalog-constants
 
+/// Live tenant inventory gauge: current tenant row count, broken down
+/// by `status` (provisioning | active | suspended | deleted) and
+/// `self_managed` (true | false). A bare-count gauge, so it carries no
+/// unit suffix. Refreshed each reaper tick.
+pub const AM_TENANTS: &str = "am_tenants";
+
+/// Live `tenant_closure` table size gauge: total ancestor-descendant
+/// edge count. A bare-count gauge (no unit suffix), refreshed each
+/// reaper tick alongside [`AM_TENANTS`]. Grows ~O(tenants × depth);
+/// a divergence from that expectation flags closure bloat / stale edges.
+pub const AM_TENANT_CLOSURE_ROWS: &str = "am_tenant_closure_rows";
+
 /// Kinds of metric samples the emitter supports.
 #[domain_model]
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]

modules/system/account-management/account-management/src/domain/ports/metrics.rs

@@ -25,11 +25,14 @@
 //! * **Cardinality discipline.** No label accepts a free `&str`.
 //!   Numeric labels (`threshold` on `hierarchy_depth_exceedance`) are
 //!   typed primitives; the adapter renders them at emit time.
-//! * **Metric names** keep the dot-separated form
-//!   (`am.dependency_health`, `am.bootstrap_lifecycle`) from
-//!   [`crate::domain::metrics`]. Unit hints are attached on the
-//!   adapter via `.with_unit("ms")`/`.with_unit("s")`, not embedded
-//!   in the metric name.
+//! * **Metric names** are the full, literal Prometheus names
+//!   (`am_dependency_health_total`, `am_bootstrap_lifecycle_total`)
+//!   from [`crate::domain::metrics`], with the OTel→Prometheus suffix
+//!   baked in (counters `_total`; quantity gauges / histograms carry
+//!   the unit word, e.g. `_seconds` / `_milliseconds`). No
+//!   `.with_unit()` hint is set on the adapter, so the rendered name is
+//!   identical regardless of the collector's `add_metric_suffixes`
+//!   setting.
 
 use account_management_sdk::idp::{IdpDeprovisionFailure, IdpProvisionFailure};
 use account_management_sdk::idp_user::IdpUserOperationFailure;

modules/system/account-management/account-management/src/domain/system_actor.rs

@@ -50,10 +50,12 @@
 //!   the provisioning-reaper batch on rows stuck in retry.
 //! * [`for_retention_sweep`] — `deprovision_tenant` from the
 //!   hard-delete retention sweep on rows past their retention window.
-//! * [`for_user_cleanup`] — RG membership cleanup on `delete_user`
-//!   after the `IdP` `deprovision_user` succeeds.
 //! * [`for_user_groups_cascade`] — RG cascade-cleanup hook fired
 //!   when a tenant is hard-deleted.
+//!
+//! `delete_user`'s RG membership cleanup used to mint a `for_user_cleanup`
+//! system actor here; VHP-190 moved it to run under the caller's context
+//! (it is a caller-initiated flow), so no system actor is needed for it.
 
 use modkit_security::SecurityContext;
 use uuid::Uuid;
@@ -147,20 +149,6 @@ pub(crate) fn for_retention_sweep(tenant_id: Uuid) -> SecurityContext {
     build_inner(Some(tenant_id))
 }
 
-/// `delete_user` — RG membership cleanup after the `IdP`
-/// `deprovision_user` round trip succeeds. `tenant_id` is the
-/// owning tenant of the user being removed.
-#[must_use]
-pub(crate) fn for_user_cleanup(tenant_id: Uuid) -> SecurityContext {
-    tracing::info!(
-        target: "am.system_actor",
-        site = "user_cleanup",
-        tenant_id = %tenant_id,
-        "am system actor constructed",
-    );
-    build_inner(Some(tenant_id))
-}
-
 /// User-groups cascade-cleanup hook — fired when a tenant is
 /// hard-deleted and AM must walk its user-group memberships in RG.
 /// `tenant_id` is the tenant being deleted.
@@ -205,7 +193,6 @@ mod tests {
             ("bootstrap", for_bootstrap(tenant)),
             ("provisioning_reaper", for_provisioning_reaper(tenant)),
             ("retention_sweep", for_retention_sweep(tenant)),
-            ("user_cleanup", for_user_cleanup(tenant)),
             ("user_groups_cascade", for_user_groups_cascade(tenant)),
         ] {
             assert_eq!(

modules/system/account-management/account-management/src/domain/tenant/repo.rs

@@ -367,6 +367,25 @@ pub trait TenantRepo: Send + Sync {
         filter: ChildCountFilter,
     ) -> Result<u64, DomainError>;
 
+    /// Count live tenants grouped by `(status, self_managed)` for the
+    /// `am_tenants` inventory gauge. Visibility is bounded by `scope`
+    /// (the periodic refresher passes `AccessScope::allow_all` for a
+    /// platform-wide count). Returns one entry per
+    /// `(TenantStatus, self_managed)` combination — including
+    /// zero-count combos — so the emitted gauge series stay stable
+    /// tick-to-tick.
+    async fn count_tenants_by_status(
+        &self,
+        scope: &AccessScope,
+    ) -> Result<Vec<(TenantStatus, bool, u64)>, DomainError>;
+
+    /// Count rows in `tenant_closure` (ancestor-descendant edges) for the
+    /// `am_tenant_closure_rows` size gauge. The closure grows
+    /// ~O(tenants × depth); tracking its row count surfaces closure bloat
+    /// or integrity drift early (e.g. orphaned / stale edges that the
+    /// integrity checker hasn't swept yet).
+    async fn count_closure_rows(&self, scope: &AccessScope) -> Result<u64, DomainError>;
+
     /// Flip the tenant from its current SDK-visible state to
     /// `Deleted`, stamp `deleted_at = now` (which also starts the
     /// retention timer — eligibility for hard-delete becomes

modules/system/account-management/account-management/src/domain/tenant/service/mod.rs

@@ -878,7 +878,7 @@ impl<R: TenantRepo> TenantService<R> {
                                 AM_TENANT_RETENTION,
                                 MetricKind::Counter,
                                 &[
-                                    ("job", "saga_compensation"),
+                                    ("retention_job", "saga_compensation"),
                                     ("outcome", "compensate_failed"),
                                 ],
                             );
@@ -924,7 +924,7 @@ impl<R: TenantRepo> TenantService<R> {
                                 AM_TENANT_RETENTION,
                                 MetricKind::Counter,
                                 &[
-                                    ("job", "saga_compensation"),
+                                    ("retention_job", "saga_compensation"),
                                     ("outcome", "compensate_failed"),
                                 ],
                             );
@@ -1276,7 +1276,7 @@ impl<R: TenantRepo> TenantService<R> {
                         AM_TENANT_RETENTION,
                         MetricKind::Counter,
                         &[
-                            ("job", "saga_compensation"),
+                            ("retention_job", "saga_compensation"),
                             ("outcome", "unsupported_required"),
                         ],
                     );
@@ -1303,7 +1303,10 @@ impl<R: TenantRepo> TenantService<R> {
                 emit_metric(
                     AM_TENANT_RETENTION,
                     MetricKind::Counter,
-                    &[("job", "saga_compensation"), ("outcome", "idp_unconfirmed")],
+                    &[
+                        ("retention_job", "saga_compensation"),
+                        ("outcome", "idp_unconfirmed"),
+                    ],
                 );
                 false
             }
@@ -1332,7 +1335,7 @@ impl<R: TenantRepo> TenantService<R> {
                 AM_TENANT_RETENTION,
                 MetricKind::Counter,
                 &[
-                    ("job", "saga_compensation"),
+                    ("retention_job", "saga_compensation"),
                     ("outcome", "compensate_failed"),
                 ],
             );