refinement phase 9
This commit is contained in:
hamid
@@ -0,0 +1,110 @@
|
||||
# Refinement Phase 9 — Observability, ops hardening, docs honesty & scale-later — Report (2026-07-13)
|
||||
|
||||
**Track:** backend (observability/docs) + explicit deferrals · **Depends on:** nothing hard ·
|
||||
**Gate:** `dotnet build` **0 new warnings** · `dotnet test` **407 pass** (402 prior + 5 new: audit-retention ×2,
|
||||
ticket-body encryption ×2, liveness ×1; the existing messaging suite now also exercises the encrypted body).
|
||||
|
||||
This phase makes the running platform **diagnosable and honest**, and records the explicitly-deferred scale work so
|
||||
nobody mistakes it for missing MVP scope. No feature behaviour changed; the money/trust rules are untouched.
|
||||
|
||||
## Observability & ops hardening (finish before launch)
|
||||
|
||||
### 9.1 — Tracing added; one metrics stack; `requestId` = trace id
|
||||
- **One metrics stack.** Removed the duplicate **prometheus-net** stack (`UseMetricServer`/`UseHttpMetrics`/
|
||||
`ForwardToPrometheus` + the three `prometheus-net*` packages). **OpenTelemetry is now the only metrics source**,
|
||||
scraped at `/metrics` via `UseOpenTelemetryPrometheusScrapingEndpoint()`. HTTP request metrics now come from the
|
||||
OTel ASP.NET Core instrumentation; the `mediator_meter` request-duration histogram (`MetricsBehaviour`) is now
|
||||
actually exported (added to the meter list — the old prometheus-net stack never captured it).
|
||||
- **Tracing.** Added `WithTracing` (ASP.NET Core + **EF Core** instrumentation) sharing one resource
|
||||
(`service.name = Baya.Web.Api`), so a cross-service money flow (webhook → confirm → ledger) is one trace.
|
||||
- **OTLP export is opt-in.** Traces + metrics export to an OTLP collector **only when `OpenTelemetry:Otlp:Endpoint`
|
||||
is set** — an MVP with Prometheus alone runs unchanged and no exporter spams an absent collector.
|
||||
- **`requestId` already carries the trace id** (`ApiResult.RequestId = Activity.Current.TraceId`) with
|
||||
`Activity.DefaultIdFormat = W3C` — a support ticket maps 1:1 to a trace with no extra wiring.
|
||||
- **New packages** (all cached, no restore risk): `OpenTelemetry.Exporter.OpenTelemetryProtocol` (1.15.3),
|
||||
`OpenTelemetry.Instrumentation.EntityFrameworkCore` (1.15.1-beta.1).
|
||||
|
||||
### 9.2 — Health checks broadened; liveness/readiness split
|
||||
- `/healthz/live` — process only (a dependency-free `self` check), so a dependency outage never restart-loops.
|
||||
- `/healthz/ready` — the app DB, the log DB (**deployed only** — its conn string is a placeholder in Dev/Testing),
|
||||
and a real **object-storage write round-trip** (`ObjectStorageWriteHealthCheck`: put → get → delete a probe blob).
|
||||
- `/HealthCheck` — the aggregate, retained for backward compatibility. The dead `currentUrl` line is gone.
|
||||
- **Redis** is noted as the next readiness check to add *when* it becomes a real dependency (>1 instance); not now.
|
||||
- `Baya.Infrastructure.Monitoring` now references `Baya.Application` (for the `IObjectStorage` probe) — a legitimate
|
||||
Infrastructure→Application edge, noted in the server Project map.
|
||||
|
||||
### 9.3 — Prod log level raised to Information+; no PII; dead ES sink removed
|
||||
- Deployed envs now log **Information+** (was Warning+, which dropped every Information-level audit trail), with
|
||||
framework categories held at Warning so the floor raise doesn't flood the sink.
|
||||
- **No secrets/PII in logs.** `LoggingSmsSender` **no longer logs the OTP code** (a login secret) in any
|
||||
environment — a developer gets it from the Development-only `GET /api/v1/dev/last_otp`. Clinical text / IBANs /
|
||||
phone numbers are already encrypted or masked before any handler logs. The `columnOptions` (previously built but
|
||||
never applied) are now wired to the SQL sink.
|
||||
- **Dead Elasticsearch sink resolved by deletion:** removed the commented ES sink block *and* the unused
|
||||
`Serilog.Sinks.Elasticsearch` package (this also revealed `Serilog.Sinks.File` was only a transitive of the ES
|
||||
package — added it explicitly). Log-table retention is documented as an ops/DBA responsibility (or ship logs to
|
||||
the OTLP collector).
|
||||
|
||||
### 9.4 — Audit-log retention as a scheduled job
|
||||
- New `AuditLogRetentionJob` (`IRecurringJob`, registered like the others) runs a **two-tier** retention sweep over
|
||||
the append-only `ops.AuditLogs`: **financial/verification** entity types (`Refund`, `NurseClawback`,
|
||||
`NursePayout`, `NursePayoutBatch`, `NurseVerification`, `PlatformConfig`, `PartnerCenter`) keep a long legal window
|
||||
(`audit_retention_financial_days`, default **2555** ≈ 7 years); everyday rows a shorter one
|
||||
(`audit_retention_general_days`, default **730** ≈ 2 years). Cadence key `audit_retention_scan_cadence_hours` (24).
|
||||
- `IAuditLogger.PurgeExpiredAsync(...)` does the delete: oldest-first (Id is monotonic with `OccurredAt`), capped at
|
||||
20 000 rows/run so a backlog drains across runs; age compared in memory (SQLite can't translate a `DateTimeOffset`
|
||||
predicate), delete is a single id-keyed `ExecuteDeleteAsync`. Idempotent.
|
||||
- Migration `RefinementPhase9TicketBodyEncryptionAndAuditRetention` seeds the three config keys (ids 24–26).
|
||||
|
||||
### 9.5 — `TicketMessage.Body` encrypted; gRPC reflection gated to Development
|
||||
- **Ticket bodies are the refund/dispute paper trail** (users type phone numbers, addresses, clinical detail) — now
|
||||
**encrypted at rest** through the existing `IFieldEncryptor` converter (wired in `ApplicationDbContext`, like every
|
||||
other PII column). The stored column is widened to `nvarchar(max)` (ciphertext is longer than plaintext); the 4000-
|
||||
char plaintext limit stays a boundary-validation rule (Open/PostMessage validators). Body is never a SQL search/
|
||||
filter predicate (the admin thread read decrypts per row), so losing SQL-searchability is an accepted trade-off.
|
||||
- **gRPC decision — keep the plugin, gate reflection to Development.** The plugin exposes only the User service and
|
||||
the client is HTTP/JSON, but removing it is more invasive than the risk warrants. gRPC **reflection** (which
|
||||
advertises the full schema) is now registered/mapped **only in Development**. The HTTP/2-posture concern is already
|
||||
mitigated (refinement-phase-5 set Kestrel `Http1AndHttp2`), so the plugin shares the mixed-protocol listener (ALPN
|
||||
negotiates h2 for gRPC clients) — no dedicated port needed.
|
||||
|
||||
### 9.6 — Docs made honest
|
||||
- **Mocks-registry:** pruned the 7 **stale duplicate 🔴 rows** (`IDistributedLock`/`INurseSearch`/`IPaymentProvider`/
|
||||
`ISettlementSplitProvider`/`IWebhookVerifier`/`IMoadianClient`/`ILicenseVerificationService`) the detailed rows
|
||||
already correct; the recurring-jobs row is the real in-process scheduler; the `IPaymentCaptureSimulator` row now
|
||||
reflects its 6.4 prod removal (fail-closed in prod; Dev/Testing mock is a test affordance). Added a phase-9 banner.
|
||||
- **REQ tracker:** already honest — refinement-phase-3 marked every delivered/deferred/resolved REQ; this phase ships
|
||||
no new contract, so no REQ status changed. (The pre-phase-3 "all 15 open" state the audit flagged is long fixed.)
|
||||
- **Architecture maps:** `server/CLAUDE.md` updated (observability wiring, the audit-retention cron, ticket-body
|
||||
encryption, the gRPC decision, the new Monitoring→Application edge); `runtime-services.md` updated (OTel
|
||||
consolidation, tracing, health split).
|
||||
|
||||
## Scale & later — explicitly NOT MVP (recorded, not built). Each has a written pull-trigger.
|
||||
|
||||
| # | Deferred item | Where it lives today (the real MVP) | **Pull it when…** |
|
||||
| --- | --- | --- | --- |
|
||||
| 9.7 | **Elasticsearch read backend + outbox feeder** — `ElasticNurseSearch` + the CDC/outbox stream | `SqlNurseSearch` is real & correct; `Search:Backend` fails fast on any non-`sql` value | **SQL search shows strain** (latency/throughput on `nurse_search_index`). Build `ElasticNurseSearch` (same filters/sort/paging) + the outbox feeder off `ISearchIndexMaintainer`; keep SQL as the reconciliation source (`RebuildAsync`). |
|
||||
| 9.8 | **Analytics pipeline** — warehouse/stream sink | `IAnalyticsSink` writes `ops.SystemEvents` fire-and-forget (real, queryable) | **Product needs cross-event analytics** beyond SQL queries. Pipe `SystemEvents` to a warehouse/stream (e.g. Kafka→ClickHouse), keeping fire-and-forget semantics. |
|
||||
| 9.9 | **Holiday-calendar feed** — automated lunar-Hijri drift feed | `IHolidayCalendar` reads the seeded, manually-maintained `ops.IranianHolidays` table (real) | **The manual yearly refresh becomes a burden.** A **yearly ops-checklist item to top up the table is an acceptable MVP alternative** to a feed — the read interface stays. |
|
||||
| 9.10 | **Push/SMS notification channels** — SMS/FCM fan-out | `InAppNotificationDispatcher` writes real in-app `ops.Notifications`; non-InApp channels are dropped by design | **The notification UX demands** out-of-app reach. Fan out to SMS (via the phase-8 `ISmsSender`) and FCM push behind the same `INotificationDispatcher`. |
|
||||
| 9.11 | **Deferred product tables** — `organizations`, `organization_nurses`, `fraud_flags`, `recurring_booking_schedules`, `bnpl_settlement_entries`, availability slots, customer national-ID KYC, geo bulk import | All **verified absent**; each is a **pure additive migration** when product pulls it | **Product pulls the feature.** No structural blocker — additive migration + feature slice; nothing in the current schema needs to change first. |
|
||||
|
||||
**These are decisions, not gaps.** SQL search, in-app notifications, and the manual holiday table are the real,
|
||||
correct MVP; Elasticsearch/analytics/push each has a concrete trigger above and stays out until then.
|
||||
|
||||
## How it was verified
|
||||
- **Build:** `dotnet build Baya.sln` — 0 new warnings (the pre-existing NU1510 + NU1903 transitive-dependency audit
|
||||
warnings are unrelated to this phase).
|
||||
- **Tests:** `dotnet test Baya.sln` — all green. New: `AuditLogRetentionTests` (two-tier purge + idempotency),
|
||||
`TicketMessageEncryptionTests` (encrypted at rest + round-trips on read), `HealthCheckApiTests` (liveness healthy
|
||||
without dependencies).
|
||||
- **Trace/requestId:** a request's `ApiResult.requestId` is `Activity.Current.TraceId` (W3C) — the same id a
|
||||
configured OTLP collector records.
|
||||
- **No PII in logs:** the OTP code is no longer logged in any environment; clinical text/IBANs are encrypted/masked.
|
||||
|
||||
## Follow-ups for later phases
|
||||
- Wire an OTLP collector (Grafana Tempo / Jaeger / OTEL Collector) in the deploy topology and set
|
||||
`OpenTelemetry:Otlp:Endpoint` to turn tracing export on.
|
||||
- When the first `redis` dependency lands (>1 instance), add its readiness check to `/healthz/ready`.
|
||||
- The NU1903 transitive-dependency vulnerability warnings (`Microsoft.OpenApi`, `SQLitePCLRaw`) are a separate
|
||||
dependency-bump task, out of this phase's scope.
|
||||
Reference in New Issue
Block a user