diff --git a/.claude/skills/README.md b/.claude/skills/README.md index 038a17d..7abc9e4 100644 --- a/.claude/skills/README.md +++ b/.claude/skills/README.md @@ -78,6 +78,8 @@ index plus the relevant [Architecture](../../docs/architecture/) doc. | [add-permission](add-permission/SKILL.md) | Registering a permission key with the closed action set and scope (Platform / Tenant / Organization). | | [add-feature-key](add-feature-key/SKILL.md) | Adding a `FeatureKey` / `LimitKey` to the typed registry and wiring entitlement-projection reads. | | [wire-dapr-pubsub](wire-dapr-pubsub/SKILL.md) | Declaring a Dapr pub/sub topic with the `learnstack.{module}.{aggregate}` convention and `InProcessEventBus` dev fallback. | +| [wire-cross-cutting-foundation](wire-cross-cutting-foundation/SKILL.md) | One-time foundation wiring per backend host — `IExceptionHandler`, 8-step MediatR pipeline, Serilog + OTel, `TenantContextSpanProcessor`, `IErrorTrackingProvider`, `IProviderResilience`. Phase 02a deliverable per [ADR-0032](../../docs/decisions/0032-exception-handling-logging-and-observability.md). | +| [add-provider-adapter](add-provider-adapter/SKILL.md) | Adding an external-integration adapter (LiveKit / Stripe / Iyzico / Meilisearch / SeaweedFS / Keycloak / …) with port interface, SDK-exception → `ProviderException` translation, and Polly v8 `ResiliencePipeline` via `IProviderResilience`. | ### Backend — guard rules diff --git a/.claude/skills/add-mediatr-handler/SKILL.md b/.claude/skills/add-mediatr-handler/SKILL.md index 0c9a72f..7dae527 100644 --- a/.claude/skills/add-mediatr-handler/SKILL.md +++ b/.claude/skills/add-mediatr-handler/SKILL.md @@ -15,8 +15,11 @@ description: > ## Purpose Write a command/query handler that participates correctly in the LearnStack MediatR -pipeline: validators → audit → idempotency → handler → outbox commit. The pipeline -is shared, so the handler stays focused on its own business logic. +pipeline: `Validation → Logging → Audit → TenantContext → Authorization → +Transaction → OutboxFlush → Handler`. The pipeline is shared (per +[ADR-0032 § Sub-decision 2](../../../docs/decisions/0032-exception-handling-logging-and-observability.md) +and [Standards 02 § Pipeline Behaviors](../../../docs/standards/02-backend-coding.md)), +so the handler stays focused on its own business logic. ## When to use @@ -246,10 +249,24 @@ public sealed class EnrollmentsController(ISender mediator) : ControllerBase ## Common pitfalls - **Throwing for expected failures.** Use `Result.Fail(...)`. Exceptions are for - *unexpected* paths (DB unavailable). The pipeline maps `Result.Fail` to RFC 7807 - Problem Details automatically. + *unexpected* paths (DB unavailable, infrastructure faults, programmer error). + `Result.Fail` values are converted to RFC 7807 Problem Details at the + **controller/API boundary** when the endpoint calls + `Result.ToActionResult()` (Step 7 above) — the pipeline itself just + propagates the `Result` unchanged; the explicit `.ToActionResult()` call is + where the mapping happens. Per + [ADR-0032 § Sub-decision 4](../../../docs/decisions/0032-exception-handling-logging-and-observability.md), + `DomainException` is reserved for **bugs** — "expected business-rule + violation" means `Result.Fail(business_rule_violation, …)`, not a throw. The + Roslyn analyzer `LearnStackException-DomainExceptionThrow` flags violations. +- **Throwing `FluentValidation.ValidationException` from a validator.** The + pipeline `ValidationBehavior` already produces + `Result.Fail(validation_failed, errors)`. A throw from the validator is a + bug; FluentValidation runs in collect-mode by default and pipeline behavior + never raises a validation exception. - **Calling `IAuditStore` directly.** The `AuditLogBehavior` does this for you. A - direct call writes a duplicate row. + direct call writes a duplicate row. The architecture test + `Modules_Do_Not_Write_AuditLog_Directly` enforces it. - **Two transactions for write + outbox.** The outbox row must be in the **same** `SaveChangesAsync` as the aggregate. Otherwise the system can publish without committing (or commit without publishing). @@ -260,3 +277,11 @@ public sealed class EnrollmentsController(ISender mediator) : ControllerBase rejected at runtime because the policy is unknown. - **Using raw `Guid` in the command.** Loses type safety; the architecture test `Commands_Use_StronglyTypedIds` rejects it. +- **Logging `ILogger.LogError(ex, ...)` then rethrowing.** The L1 + `IExceptionHandler` already logs + records the OTel span error + captures + to `IErrorTrackingProvider` per + [ADR-0032 § Sub-decision 7](../../../docs/decisions/0032-exception-handling-logging-and-observability.md). + Re-logging at the handler doubles the entry. +- **Per-call `Activity.Current?.SetTag("tenant.id", ...)`.** The + `TenantContextSpanProcessor` enriches every span automatically. Per-call + tagging is duplication and a maintenance burden. diff --git a/.claude/skills/add-provider-adapter/SKILL.md b/.claude/skills/add-provider-adapter/SKILL.md new file mode 100644 index 0000000..54c794f --- /dev/null +++ b/.claude/skills/add-provider-adapter/SKILL.md @@ -0,0 +1,362 @@ +--- +name: add-provider-adapter +description: > + Add a provider adapter (LiveKit / Stripe / Iyzico / Meilisearch / SeaweedFS + / Keycloak / external HTTP service) in `LearnStack.Infrastructure.` with + the canonical pattern: port interface in `SharedKernel`, adapter in + `Infrastructure` doing only SDK-exception → `ProviderException` translation, + and the `IProviderResilience` decorator carrying retry + circuit + breaker + timeout + bulkhead from `appsettings.Resilience::`. + USE FOR: every new external integration that LearnStack reaches over the + network. DO NOT USE FOR: the Dapr building-block ports (`IEventBus`, + `ICacheService`, `ISecretProvider`) — those are wired by + [wire-cross-cutting-foundation](../wire-cross-cutting-foundation/SKILL.md) + and their resilience is handled by Dapr's runtime, not Polly; the four + Hub HTTPS endpoints (`IEntitlementProvider`, `IUsageReporter`, + `IHubTenantSync`) which have their own contractual mTLS + HMAC wrapper + per [ADR-0019](../../../docs/decisions/0019-learnstack-hub.md); or adapters + for in-process pure libraries (e.g. a JSON serializer) — no resilience + needed. +--- + +# Adding a provider adapter + +## Purpose + +Every external integration (LiveKit room creation, Stripe charge, +Meilisearch search, SeaweedFS object PUT, Keycloak admin call, …) goes +through the same shape: + +```text +Application code + ↓ (port interface in SharedKernel) +ResilientProviderAdapter ← Polly v8 ResiliencePipeline + ↓ +LiveKitClient (adapter in Infrastructure) ← SDK exception → ProviderException + ↓ +LiveKit .NET SDK + ↓ +upstream +``` + +The point: the application sees one port; resilience is centralised in the +decorator; exception translation is the adapter's only job. This skill walks +the canonical wiring per +[ADR-0032 § Sub-decision 5](../../../docs/decisions/0032-exception-handling-logging-and-observability.md). + +## When to use + +- New external integration (a new payment processor, a new email provider, a + new SMS provider, a new search index, a new media SDK). +- Replacing an existing adapter with a different upstream while keeping the + port interface unchanged (e.g. Stripe → Iyzico for a tenant). +- Splitting one adapter into two with different resilience policies (e.g. + one critical-path Stripe call vs. one fire-and-forget Stripe webhook reply). + +## When not to use + +- Wiring `IEventBus` / `ICacheService` / `ISecretProvider` — those are Dapr + building blocks, handled by + [wire-cross-cutting-foundation](../wire-cross-cutting-foundation/SKILL.md). + Dapr's runtime provides retry + DLQ + circuit-breaker semantics already. +- The four Hub HTTPS endpoints (`IEntitlementProvider`, `IUsageReporter`, + `IHubTenantSync`) — those use the dedicated mTLS + signed JWT + HMAC + wrapper per [ADR-0019](../../../docs/decisions/0019-learnstack-hub.md). +- Pure in-process integrations (a JSON converter, a hash function) — no + network call, no resilience needed. +- Trivial single-method clients that wrap a static method — write a static + helper instead. + +## Inputs + +| Input | Required | Description | +|-------|----------|-------------| +| Port name | Yes | `liveclass`, `payment`, `storage`, `search`, `notifications`, … . Used as the `appsettings.Resilience::` section key. | +| Port interface | Yes | `ILiveClassProvider`, `IPaymentProvider`, etc. Lives in `LearnStack.SharedKernel.Abstractions.`. | +| Adapter class | Yes | Concrete implementation in `LearnStack.Infrastructure.`. | +| SDK package | Yes | The upstream client SDK. | +| `ProviderException` subclass | Yes | `LiveClassProviderException`, `PaymentProviderException`, `StorageProviderException`, etc. Subclass of `LearnStackException`. | + +## Workflow + +### Step 1: Declare the port interface in `SharedKernel` + +In `LearnStack.SharedKernel.Abstractions./IProvider.cs`: + +```csharp +public interface ILiveClassProvider +{ + Task CreateRoomAsync( + CreateRoomCommand command, CancellationToken ct = default); + + Task IssueTokenAsync( + LiveRoomTokenCommand command, CancellationToken ct = default); + + Task EndRoomAsync(LiveRoomId roomId, CancellationToken ct = default); +} +``` + +Rules: + +- Strongly-typed parameters and return values; no `string roomId` in the + contract. +- Async + `CancellationToken` everywhere. +- No SDK types in the contract — the adapter translates SDK shapes to + LearnStack domain types. +- Return `Result` only at the application-layer boundary; provider ports + return raw values and throw `ProviderException` on failure. The application + handler catches at the boundary and converts to `Result.Fail` when the + failure is expected (e.g. "room name already taken" → 409). + +### Step 2: Add the `ProviderException` subclass + +In `LearnStack.SharedKernel.Exceptions/`: + +```csharp +public sealed class LiveClassProviderException : ProviderException +{ + public LiveClassProviderException( + string code, + string message, + Exception? innerException = null, + bool isClientError = false) + : base(code, message, innerException, isClientError) + { + } +} +``` + +Rules: + +- `isClientError` is `true` for 4xx upstream (the provider rejected the + request because of bad input) and `false` for 5xx (the provider's infra + failed). The L1 `IExceptionHandler` uses this flag to decide whether to + Sentry-capture. See + [09-error-handling.md § Sentry vs OpenTelemetry — Error Capture Boundary](../../../docs/standards/09-error-handling.md). +- Codes follow the pattern `provider.` (e.g. + `provider.room_full`, `provider.unauthenticated`, `provider.unavailable`). + +### Step 3: Write the adapter — translation only + +In `LearnStack.Infrastructure.LiveClassroom.LiveKit/LiveKitClient.cs`: + +```csharp +internal sealed class LiveKitClient( + LiveKitClientOptions options, + ILogger logger) : ILiveClassProvider +{ + private readonly LiveKit.RoomServiceClient _sdk = new( + options.WsUrl, options.ApiKey, options.ApiSecret); + + public async Task CreateRoomAsync( + CreateRoomCommand cmd, CancellationToken ct) + { + try + { + var room = await _sdk.CreateRoom(/* SDK call */, ct); + return MapToDomain(room); + } + catch (LiveKit.RoomAlreadyExistsException ex) + { + throw new LiveClassProviderException( + "provider.room_already_exists", ex.Message, ex, isClientError: true); + } + catch (LiveKit.QuotaExceededException ex) + { + throw new LiveClassProviderException( + "provider.quota_exceeded", ex.Message, ex, isClientError: true); + } + // .NET 5+ exposes `HttpRequestException.StatusCode` as `HttpStatusCode?`. + // `null` = transport failure (DNS, connection refused, timeout) which is + // an infra fault → isClientError: false. 5xx upstream → isClientError: false. + // 4xx upstream is handled by the SDK-specific catches above; if a raw 4xx + // reaches this clause it falls through to the catch-all below. + catch (HttpRequestException ex) + when (ex.StatusCode is null || (int)ex.StatusCode >= 500) + { + throw new LiveClassProviderException( + "provider.unavailable", "Live-class provider unavailable.", + ex, isClientError: false); + } + catch (Exception ex) + { + throw new LiveClassProviderException( + "provider.unknown", "Unexpected live-class provider failure.", + ex, isClientError: false); + } + } + + // ... other methods follow the same pattern +} +``` + +Rules: + +- **No** retry, **no** circuit breaker, **no** timeout in the adapter — that + is the decorator's job. +- The adapter is `internal sealed` — the composition root sees only the port + interface. +- Every public method is wrapped in a `try / catch` whose only purpose is + exception translation. +- Provider SDK exception types (`LiveKit.RoomAlreadyExistsException`, + `Stripe.StripeException`, `Meilisearch.MeilisearchApiError`, …) **never** + leave the adapter's namespace. The architecture test + `Adapters_Wrap_Provider_Exceptions` enforces it. + +### Step 4: Wire resilience in the composition root + +In `LearnStack.Infrastructure//ServiceCollectionExtensions.cs`: + +```csharp +public static IServiceCollection AddLiveClassroomProvider( + this IServiceCollection services, IConfiguration config) +{ + services.Configure(config.GetSection("LiveKit")); + + services.AddProviderResilience("liveclass"); + + return services; +} +``` + +The `AddProviderResilience` extension (registered by +[wire-cross-cutting-foundation](../wire-cross-cutting-foundation/SKILL.md)) +does three things: + +1. Registers `TImpl` as the base implementation. +2. Builds an `IProviderResilience` carrying the Polly v8 + `ResiliencePipeline` from `appsettings.Resilience::`. +3. Decorates `TPort` with `ResilientProviderAdapter` so every call + goes through the pipeline. + +### Step 5: Author the resilience configuration + +Add to `appsettings.json`: + +```jsonc +{ + "Resilience": { + "liveclass": { + "retry": { + "maxAttempts": 3, + "delaySeconds": 1, + "useJitter": true + }, + "circuitBreaker": { + "failureRatio": 0.5, + "samplingDurationSeconds": 30, + "minimumThroughput": 10, + "breakDurationSeconds": 30 + }, + "timeout": { "totalSeconds": 10 }, + "bulkhead": { "maxConcurrency": 50 } + } + } +} +``` + +Tune per port based on the upstream's known characteristics. Document the +chosen values in the adapter's README (under +`backend/src/LearnStack.Infrastructure./README.md`) so reviewers know +*why* `maxAttempts: 3` and not 5. + +### Step 6: Map provider 4xx vs 5xx correctly + +The decorator only retries on `IsClientError == false` provider exceptions +and on `InfrastructureException`. A `LiveClassProviderException` with +`isClientError: true` skips retry — retrying "room already exists" is wrong. +Verify the mapping table for every translated SDK exception: + +| Upstream signal | `ProviderException.IsClientError` | Sentry capture | Decorator retries | +|---|---|---|---| +| 4xx response | `true` | No | No | +| 5xx response | `false` | Yes | Yes | +| Timeout | `false` | Yes | Yes | +| DNS / connection refused | `false` | Yes | Yes | +| Auth failure (401 / 403 from upstream) | `true` (provider thinks our creds are bad — that's our config bug) | Yes (config bug) | No | +| Rate-limit (429) | `true` | No | Yes, but with a longer backoff — special-case if needed | + +### Step 7: Adapter tests + +In `LearnStack.Tests.Integration/Providers/`: + +```csharp +[Fact] +public async Task CreateRoomAsync_translates_RoomAlreadyExists_to_4xx_provider_exception() +{ + // Arrange — SDK throws RoomAlreadyExistsException + // Act + var act = async () => await sut.CreateRoomAsync(cmd, ct); + // Assert + var ex = await act.Should().ThrowAsync(); + ex.Which.Code.Should().Be("provider.room_already_exists"); + ex.Which.IsClientError.Should().BeTrue(); +} + +[Fact] +public async Task ResilientProviderAdapter_retries_on_5xx_until_circuit_opens() +{ + // Arrange — wrap a flaky in-memory adapter + // Act — fire enough requests to open the breaker + // Assert — subsequent calls return BrokenCircuitException wrapped in ProviderException +} +``` + +### Step 8: Module spec entry + +In `docs/modules//providers.md`, add the adapter: + +```markdown +## LiveKit (liveclass) + +- Port: `ILiveClassProvider` +- Adapter: `LearnStack.Infrastructure.LiveClassroom.LiveKit.LiveKitClient` +- Resilience section: `Resilience:liveclass:` +- Exception subclass: `LiveClassProviderException` +- ADR: [ADR-0005](../../decisions/0005-live-classroom-media-stack.md) +``` + +## Validation + +- `dotnet build` succeeds. +- Architecture test `Adapters_Wrap_Provider_Exceptions` passes — no SDK + exception types leak from the adapter's namespace. +- Integration test confirms SDK 4xx → `ProviderException(isClientError: + true)`, SDK 5xx → `ProviderException(isClientError: false)`. +- A deliberately-flaky test adapter triggers the circuit breaker after the + configured threshold; subsequent calls fail fast with + `BrokenCircuitException`. +- An `appsettings.Resilience::` block exists with all four policy + sections (retry, circuit breaker, timeout, bulkhead). + +## Common pitfalls + +- **Adding retry / timeout in the adapter.** The decorator handles those. + Adapter-level retry double-counts attempts and breaks the circuit-breaker + accounting. +- **Letting the SDK exception escape.** A `LiveKit.LiveKitException` + reaching the application layer means the architecture test fires and the + rest of the system can't decide whether to Sentry-capture (no + `IsClientError` flag). +- **Setting `isClientError: false` on 4xx.** Forces a retry on + invalid-input failures (the upstream will reject again and again until + the circuit opens) and floods Sentry with "client mistake" events. +- **Forgetting the `Resilience::` configuration block.** The + decorator falls back to no-policy mode and silently masks failures during + development — they only surface under load. +- **Reusing a single adapter for two ports with different resilience + needs.** Split them. Each port has its own decorator instance and its + own configuration section. +- **Calling the adapter directly from a module** (bypassing the port + interface). The decorator is registered on the port; calling the + concrete adapter skips resilience entirely. + +## References + +- [ADR-0032 § Sub-decision 5](../../../docs/decisions/0032-exception-handling-logging-and-observability.md) +- [09-error-handling.md § Provider Failures](../../../docs/standards/09-error-handling.md) +- [09-error-handling.md § Sentry vs OpenTelemetry — Error Capture Boundary](../../../docs/standards/09-error-handling.md) +- [20-infrastructure-stack.md](../../../docs/standards/20-infrastructure-stack.md) +- [33-cross-cutting-concerns.md § 10. Provider Resilience Pattern](../../../docs/architecture/33-cross-cutting-concerns.md) +- [wire-cross-cutting-foundation](../wire-cross-cutting-foundation/SKILL.md) +- Polly v8 — diff --git a/.claude/skills/wire-cross-cutting-foundation/SKILL.md b/.claude/skills/wire-cross-cutting-foundation/SKILL.md new file mode 100644 index 0000000..f3da7cf --- /dev/null +++ b/.claude/skills/wire-cross-cutting-foundation/SKILL.md @@ -0,0 +1,353 @@ +--- +name: wire-cross-cutting-foundation +description: > + Wire the LearnStack cross-cutting foundation in a backend host + (`LearnStack.Api`, worker, background-service host) — `IExceptionHandler`, + 8-step MediatR pipeline, `Result.ToActionResult()`, Serilog + OTel, + `TenantContextSpanProcessor`, `IErrorTrackingProvider`, + `IProviderResilience`, Roslyn analyzer for `DomainException`. USE + FOR: standing up the foundation in Phase 02a (one-time wiring per host + process), or restoring it after a composition-root refactor. DO NOT USE + FOR: adding a new provider adapter ([add-provider-adapter](../add-provider-adapter/SKILL.md)), + adding a single MediatR handler ([add-mediatr-handler](../add-mediatr-handler/SKILL.md)), + or touching observability backends in Phase 11 (different scope — + dashboard / alert / Sentry SaaS config). +--- + +# Wiring the cross-cutting foundation + +## Purpose + +Bring up the LearnStack error-handling + logging + observability foundation +in a backend host. The contract is bound by +[ADR-0032](../../../docs/decisions/0032-exception-handling-logging-and-observability.md). +This skill walks the canonical wiring step by step so the eight binding +sub-decisions land in the right composition-root order and the architecture +tests pass. + +## When to use + +- Phase 02a — first time the `LearnStack.Api` (or worker host) is stood up; + every piece below has to land in one consistent pass. +- After a composition-root refactor that touched DI registration order; this + skill is the checklist that verifies the pipeline still matches ADR-0032. +- Standing up a new host process (a future dedicated worker, a future + background-service binary) that needs the same cross-cutting plumbing. + +## When not to use + +- Adding a single MediatR handler — use + [add-mediatr-handler](../add-mediatr-handler/SKILL.md). The pipeline is + already wired; new handlers participate automatically. +- Adding a new provider adapter — use + [add-provider-adapter](../add-provider-adapter/SKILL.md). That skill + handles the resilience decorator and `ProviderException` translation for + a single adapter. +- Deploying / configuring an OTel Collector or a Sentry SaaS project — that + is Phase 11 ops work, not application-code wiring. +- Adjusting a Resilience policy for a single port — edit + `appsettings.Resilience::` and review the test; no need to re-walk + the foundation. + +## Inputs + +| Input | Required | Description | +|-------|----------|-------------| +| Host project | Yes | `LearnStack.Api` for the main API, or the worker host name. | +| `DeploymentMode` | Yes | Determines `IErrorTrackingProvider` + OTLP exporter target. | +| Sentry DSN source | Conditional | If `DeploymentMode ∈ { SaaS, Dedicated, SelfHostedOnline }` and Sentry is enabled, the DSN comes from `ISecretProvider`. | +| OTel Collector endpoint | Yes (non-Dev) | OTLP gRPC endpoint; falls back to a local file exporter in `SelfHostedAirGapped`. | +| Module list | Yes | The set of modules the host loads — each module's `IModule.RegisterServices` must be called after the foundation registers. | + +## Workflow + +### Step 1: Read the binding contract + +Open +[ADR-0032](../../../docs/decisions/0032-exception-handling-logging-and-observability.md) +and +[33-cross-cutting-concerns.md](../../../docs/architecture/33-cross-cutting-concerns.md). +You should be able to recite, before you write a line of code: + +- The eight pipeline behaviors and their order + (`Validation → Logging → Audit → TenantContext → Authorization → Transaction → OutboxFlush → Handler`). +- The Sentry-vs-OTel boundary (`ShouldCapture(ex)` table). +- The Serilog + OTLP wiring rule (no `AddOpenTelemetry().WithLogging()` alongside). +- The composition-root branching for `IErrorTrackingProvider`. + +### Step 2: Add the foundation NuGet packages + +Add these to `Directory.Packages.props`: + +```xml + + + + + + + + + +``` + +Architecture test `Modules_Do_Not_Reference_Sentry_SDK_Directly` enforces +the `Sentry.AspNetCore` reference being restricted to +`LearnStack.Infrastructure.ErrorTracking`. + +### Step 3: Wire Serilog (logger primary) + +```csharp +builder.Host.UseSerilog((ctx, services, cfg) => cfg + .ReadFrom.Configuration(ctx.Configuration) + .Enrich.WithCorrelationContext(services) // tenant / org / user / module / correlation_id + .Enrich.With() // strips tokens, passwords, PII + .WriteTo.Console(new RenderedCompactJsonFormatter()) + .WriteTo.OpenTelemetry(o => + { + o.Endpoint = ctx.Configuration["Telemetry:OtlpEndpoint"]; + o.Protocol = OtlpProtocol.Grpc; + })); +``` + +**Do not** also register `AddOpenTelemetry().WithLogging()` — +[ADR-0032 § Sub-decision 8](../../../docs/decisions/0032-exception-handling-logging-and-observability.md) +forbids it. + +### Step 4: Register `ITenantContextAccessor` (singleton, AsyncLocal-backed) + +Per [ADR-0032 § Sub-decision 10](../../../docs/decisions/0032-exception-handling-logging-and-observability.md), +OTel processors are singletons — they cannot inject the request-scoped +`ITenantContext` directly. Register the singleton accessor *before* the OTel +pipeline so `TenantContextSpanProcessor` can resolve it: + +```csharp +services.AddSingleton(); +``` + +`TenantContextAccessor` carries an `AsyncLocal` field. The +following sites populate it at scope start: `TenantResolverMiddleware` +(HTTP), `HubCorrelationMiddleware` (`/api/internal/*`), Hangfire +`JobActivator` (background jobs), outbox / inbox handler scope (integration +events). Modules never write to the accessor. + +### Step 5: Wire OpenTelemetry tracing + metrics + +```csharp +services + .AddOpenTelemetry() + .ConfigureResource(r => r.AddService( + serviceName: "learnstack-api", + serviceVersion: GitSha.Current)) + .WithTracing(t => t + .AddAspNetCoreInstrumentation() + .AddHttpClientInstrumentation() + .AddEntityFrameworkCoreInstrumentation() + .AddSource("LearnStack.*") // every module's manual ActivitySource + .AddProcessor() // singleton; reads ITenantContextAccessor + .AddOtlpExporter(o => + { + o.Endpoint = new Uri(builder.Configuration["Telemetry:OtlpEndpoint"]!); + o.Protocol = OtlpExportProtocol.Grpc; + })) + .WithMetrics(m => m + .AddAspNetCoreInstrumentation() + .AddHttpClientInstrumentation() + .AddMeter("learnstack.*") + .AddOtlpExporter(o => + { + o.Endpoint = new Uri(builder.Configuration["Telemetry:OtlpEndpoint"]!); + o.Protocol = OtlpExportProtocol.Grpc; + })); +``` + +For `DeploymentMode.SelfHostedAirGapped`, swap `AddOtlpExporter` for the +file exporter pointing at `/var/learnstack/otel/`. + +### Step 6: Register `IErrorTrackingProvider` + +In `LearnStack.Infrastructure.ErrorTracking` add the three implementations +and the composition-root extension: + +```csharp +public static IServiceCollection AddErrorTracking( + this IServiceCollection services, DeploymentMode mode, IConfiguration config) +{ + services.AddSingleton(sp => mode switch + { + DeploymentMode.Development => new NoOpErrorTracker(), + DeploymentMode.SaaS => CreateSentry(sp, config), + DeploymentMode.Dedicated => CreateSentry(sp, config), + DeploymentMode.SelfHostedOnline => CreateSentryOrNoOp(sp, config), + DeploymentMode.SelfHostedAirGapped => new LocalFileErrorTracker( + config["ErrorTracking:LocalFile:Directory"]!), + _ => throw new System.Diagnostics.UnreachableException($"DeploymentMode {mode}") + }); + return services; +} +``` + +Modules never reference `Sentry.SentrySdk`. The architecture test +`Modules_Do_Not_Reference_Sentry_SDK_Directly` enforces it. + +### Step 7: Register the L1 `IExceptionHandler` + +```csharp +services.AddProblemDetails(); +services.AddExceptionHandler(); +// in pipeline: +app.UseExceptionHandler(); +``` + +`LearnStackExceptionHandler` (in `LearnStack.Api`) builds the Problem +Details body, calls `Activity.Current.RecordException + SetStatus(Error)`, +and dispatches to `IErrorTrackingProvider.CaptureAsync` only when +`ShouldCapture(ex)` returns true (see +[09-error-handling.md § Sentry vs OpenTelemetry — Error Capture Boundary](../../../docs/standards/09-error-handling.md)). + +### Step 8: Register the MediatR pipeline (8 behaviors in order) + +```csharp +services.AddMediatR(cfg => +{ + cfg.RegisterServicesFromAssemblyContaining(); + + // Order matters — outermost first, innermost last. + // Architecture test MediatR_Pipeline_Order_Matches_Canonical_Sequence enforces this. + cfg.AddBehavior(typeof(IPipelineBehavior<,>), typeof(ValidationBehavior<,>)); + cfg.AddBehavior(typeof(IPipelineBehavior<,>), typeof(LoggingBehavior<,>)); + cfg.AddBehavior(typeof(IPipelineBehavior<,>), typeof(AuditLogBehavior<,>)); + cfg.AddBehavior(typeof(IPipelineBehavior<,>), typeof(TenantContextBehavior<,>)); + cfg.AddBehavior(typeof(IPipelineBehavior<,>), typeof(AuthorizationBehavior<,>)); + cfg.AddBehavior(typeof(IPipelineBehavior<,>), typeof(TransactionBehavior<,>)); + cfg.AddBehavior(typeof(IPipelineBehavior<,>), typeof(OutboxFlushBehavior<,>)); +}); +``` + +Do **not** add an `ExceptionHandlingBehavior`. The `AuditLogBehavior` catches +handler exceptions, writes the failure audit, and rethrows via +`ExceptionDispatchInfo`; the L1 `IExceptionHandler` is the final catch site. + +### Step 9: Register the `IProviderResilience` extension + +In `LearnStack.Infrastructure.Resilience`: + +```csharp +public static IServiceCollection AddProviderResilience( + this IServiceCollection services, string portName) + where TPort : class + where TImpl : class, TPort +{ + services.AddSingleton(); + services.AddSingleton>(sp => + new ProviderResilience( + portName, + sp.GetRequiredService() + .GetSection($"Resilience:{portName}"))); + services.Decorate>(); + return services; +} +``` + +The composition root calls this extension once per provider port (see +[add-provider-adapter](../add-provider-adapter/SKILL.md) for the per-adapter +work). + +### Step 10: Wire `Result.ToActionResult()` extension + +In `LearnStack.Api.Common`: + +```csharp +public static class ResultExtensions +{ + public static IActionResult ToActionResult(this Result result) + => result.IsSuccess + ? new OkObjectResult(result.Value) + : new ObjectResult(ProblemDetailsFactory.For(result.Error!)) + { + StatusCode = HttpStatusMap.For(result.Error!.Code) + }; +} +``` + +Controllers stay thin: + +```csharp +[HttpPost] +public async Task Create(CreateCourseCommand cmd, CancellationToken ct) + => (await _mediator.Send(cmd, ct)).ToActionResult(); +``` + +No action filter, no `ResultUnwrapBehavior`. Explicit beats magic. + +### Step 11: Add the Roslyn analyzer + +The `LearnStackException-DomainExceptionThrow` analyzer lives in +`backend/analyzers/` and ships as a NuGet package referenced by +`Domain` + `Application` projects via +``. Severity: +Warning in Phase 02a, escalates to Error after Phase 03 exit. + +### Step 12: Register module services last + +Each module's `IModule` registration runs **after** the foundation is in +place, so behaviors and instrumentation are already wired before +module-specific code lights up: + +```csharp +services + .AddCrossCuttingFoundation(builder.Configuration, deploymentMode) + .AddModuleAudit() + .AddModuleTenancy() + .AddModuleCustomization() + // ... more modules + .AddModuleApi(); // controllers wire last +``` + +## Validation + +- `dotnet build` succeeds for `LearnStack.Api`. +- Architecture tests pass: + - `IExceptionHandler_Registered_AtStartup` + - `MediatR_Pipeline_Order_Matches_Canonical_Sequence` + - `ValidationBehavior_DoesNotThrow_ValidationException` + - `OTel_Pipeline_Includes_TenantContextSpanProcessor` + - `Logging_Goes_Through_Microsoft_Extensions_Logging` + - `Modules_Do_Not_Reference_Sentry_SDK_Directly` + - `TenantContextSpanProcessor_DoesNotThrow_When_Context_Missing` +- Manual smoke: hit a known-bad endpoint, confirm Problem Details body + carries the correct `code`, `correlationId`, and HTTP status; confirm the + OTel span shows `SetStatus(Error)`; confirm Sentry receives the event in + `Development` only as `NoOp` (no actual dispatch). +- Integration smoke: trigger a `Result.Fail(validation_failed)`, confirm + the OTel span is `SetStatus(Ok)` and Sentry receives nothing. + +## Common pitfalls + +- **Adding `ExceptionHandlingBehavior` "just in case".** Forbidden by + ADR-0032; `AuditLogBehavior` + L1 cover every path. +- **Registering both Serilog OTLP sink and OpenTelemetry `LoggerProvider`.** + Duplicates every log line. The pipeline expects Serilog only. +- **Skipping `TenantContextSpanProcessor`.** Auto-instrumentation spans + show up with no `tenant.id` and Tempo searches become useless. +- **Reading `DeploymentMode` from inside a module.** Forbidden — the + composition root selects implementations once. Architecture test + `Modules_Do_Not_Reference_DeploymentMode` catches it. +- **Capturing every exception to Sentry, including `OperationCanceled` + and 4xx provider responses.** Sentry noise. The `ShouldCapture` switch is + binding. +- **Hand-rolling retry / circuit breaker inside an adapter.** Resilience + policies live in the `IProviderResilience` decorator, not in + adapter code. + +## References + +- [ADR-0032 Exception Handling, Logging, and Observability Architecture](../../../docs/decisions/0032-exception-handling-logging-and-observability.md) +- [33-cross-cutting-concerns.md](../../../docs/architecture/33-cross-cutting-concerns.md) +- [09-error-handling.md](../../../docs/standards/09-error-handling.md) +- [10-observability.md](../../../docs/standards/10-observability.md) +- [02-backend-coding.md § Pipeline Behaviors](../../../docs/standards/02-backend-coding.md) +- [20-infrastructure-stack.md § Composition Root and Deployment Mode](../../../docs/standards/20-infrastructure-stack.md) +- [Phase 02a Roadmap](../../../docs/roadmap/phase-02a-kernel-tenancy.md) +- [add-provider-adapter](../add-provider-adapter/SKILL.md) +- [add-mediatr-handler](../add-mediatr-handler/SKILL.md) diff --git a/CLAUDE.md b/CLAUDE.md index 2020284..9deae9f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -171,6 +171,34 @@ rules: more space, expand the existing doc rather than splintering. - Mention a feature as "deferred to a later phase" without naming the phase that owns it. +- Throw `DomainException` for expected business-rule violations — use + `Result.Fail(business_rule_violation, ...)`. + `DomainException` is reserved for programmer errors / aggregate invariant + bugs ([ADR-0032 § Sub-decision 4](docs/decisions/0032-exception-handling-logging-and-observability.md)). + The Roslyn analyzer `LearnStackException-DomainExceptionThrow` flags + violations; full catalogue entry in + [docs/standards/21-architecture-tests-catalogue.md](docs/standards/21-architecture-tests-catalogue.md). +- Throw `FluentValidation.ValidationException` from `ValidationBehavior` — + the behavior returns `Result.Fail(validation_failed)` and never throws. +- Reference `Sentry.SentrySdk` directly from any module assembly — error + capture goes through `IErrorTrackingProvider`; the L1 `IExceptionHandler` + is the only sanctioned caller in application code. +- Add an `ExceptionHandlingBehavior` to the MediatR pipeline — + `AuditLogBehavior` (catches handler exceptions, audits, rethrows via + `ExceptionDispatchInfo`) plus the L1 `IExceptionHandler` cover every + exception path. +- Register the OpenTelemetry `LoggerProvider` + (`AddOpenTelemetry().WithLogging()`) alongside Serilog. Logs flow through + Serilog → OTLP sink only; double-export would duplicate every line. +- Import `Serilog.ILogger` from a module assembly — modules use + `Microsoft.Extensions.Logging.ILogger`; Serilog is the implementation + wired once at the composition root. +- Import a provider SDK exception type outside the adapter's + `LearnStack.Infrastructure.` namespace — adapters translate SDK + exceptions into `ProviderException` subclasses at the boundary. +- Tag a span with `tenant.id` / `organization.id` / `user.id` / + `correlation.id` from module code — the `TenantContextSpanProcessor` + enriches every span centrally. ## Where to look when stuck diff --git a/docs/architecture/33-cross-cutting-concerns.md b/docs/architecture/33-cross-cutting-concerns.md new file mode 100644 index 0000000..8f92b43 --- /dev/null +++ b/docs/architecture/33-cross-cutting-concerns.md @@ -0,0 +1,442 @@ +# Cross-Cutting Concerns — Errors, Logs, Traces, Metrics + +**Derives from:** [ADR-0032](../decisions/0032-exception-handling-logging-and-observability.md), +[ADR-0014](../decisions/0014-adopt-dapr.md), [ADR-0016](../decisions/0016-audit-log-subsystem.md), +[ADR-0020](../decisions/0020-triple-deployment-hybrid-license.md). For the +day-to-day rules read [09-error-handling.md](../standards/09-error-handling.md), +[10-observability.md](../standards/10-observability.md), and +[02-backend-coding.md § Pipeline Behaviors](../standards/02-backend-coding.md). +This document explains the **shape** behind those rules so a reader can +reason about new cases without re-deriving the decisions. + +LearnStack's cross-cutting layer answers three orthogonal questions: + +1. **What went wrong, and what should the caller see?** — error handling. +2. **What happened, and can we replay it later?** — logging + audit. +3. **How did it perform, and where did time go?** — tracing + metrics. + +The same primitive (`correlation_id` / W3C `traceparent`) threads through all +three so a single request can be reconstructed across HTTP → MediatR → DB → +audit → outbox → Dapr → consumer. + +## 1. Two-Track Failure Model + +```mermaid +flowchart LR + bug[Bug / infra failure] --> exc[Exception thrown] + expected[Expected outcome] --> result["Result<T> returned"] + exc --> wrap[Caught at AuditLogBehavior; rethrown via ExceptionDispatchInfo] + wrap --> handler[IExceptionHandler L1: Problem Details] + result --> mapper["result.ToActionResult(): Problem Details"] + handler --> client[Client] + mapper --> client +``` + +The rule: + +- **Exceptions** are for things that should not happen — bugs, infrastructure + faults, contract violations, programmer errors. +- **`Result`** is for things that *can* happen and the caller needs to + decide what to do — validation failed, not found, forbidden, business-rule + violation. + +Both paths converge on **RFC 7807 Problem Details** at the API boundary, with +a stable `Error.Code` (one of the 13 listed in +[09-error-handling.md § Result Type](../standards/09-error-handling.md)), an +HTTP status mapped from the code, and a `correlationId` field. + +The exception hierarchy +([09-error-handling.md § Hierarchy](../standards/09-error-handling.md)): + +- `LearnStackException` — base. Constructors take a structured `Error` plus + the underlying cause. +- `DomainException` — programmer error. The Roslyn analyzer + `LearnStackException-DomainExceptionThrow` flags every `throw new + DomainException` outside aggregate invariant checks; the architecture test + `Domain_Methods_Do_Not_Throw_For_Expected_Cases` walks + `Result`-returning methods to confirm. +- `InfrastructureException` — transient DB / Valkey / SeaweedFS fault. +- `ProviderException` — upstream provider error; `IsClientError` flag splits + 4xx (provider's user mistake; do not Sentry) from 5xx (provider's infra + fault; Sentry). +- `TenantContextMissingException` — request reached the pipeline without a + resolved tenant. + +For "case branch the type system can't prove impossible" use the BCL +`System.Diagnostics.UnreachableException` (.NET 7+) directly — it is **not** +a `LearnStackException` subclass. The L1 handler treats it the same as any +unhandled exception (Sentry-captured, 500 Problem Details). Per +[09-error-handling.md § Hierarchy](../standards/09-error-handling.md). + +## 2. MediatR Pipeline Order + +[ADR-0032 § Sub-decision 2](../decisions/0032-exception-handling-logging-and-observability.md) +binds the order. Reading bottom-most as innermost: + +```text +Request + ▼ +[1] ValidationBehavior ← FluentValidation; returns Result.Fail(validation_failed) + ▼ +[2] LoggingBehavior ← ILogger.BeginScope(tenant.id, organization.id, user.id, + module, correlation.id); + Activity.StartActivity(name); + latency histogram start + ▼ +[3] AuditLogBehavior ← try { handler outcome } catch { + audit FAILED entry; ExceptionDispatchInfo.Throw(); + } + ▼ +[4] TenantContextBehavior ← assert ITenantContext.IsResolved; + set RLS GUCs via DbConnectionInterceptor + ▼ +[5] AuthorizationBehavior ← IAuthorizationService.AuthorizeAsync; + Result.Fail(forbidden) on deny + ▼ +[6] TransactionBehavior ← DbContext.Database.BeginTransactionAsync(); + commit on success-Result; rollback on fail-Result or exception + ▼ +[7] OutboxFlushBehavior ← enrol IOutbox messages in current tx; + they ship via DaprEventBus on commit + ▼ +[8] Handler ← domain logic; returns Result +``` + +Why this order: + +- **Validation outermost.** Invalid input must never reach DB / audit / tenant + resolution. Cheap to reject, expensive to roll back. +- **Logging before audit.** The structured-log scope (8 correlation fields) + must exist *before* audit's catch-rethrow runs so audit entries inherit the + trace context. If audit's snapshot fails too, the operator wants the failure + reported with the same `correlation_id` as the original request. +- **Audit wraps everything inside it.** Per + [ADR-0016](../decisions/0016-audit-log-subsystem.md), `AuditLogBehavior` + catches handler exceptions, writes a failure-class audit entry, and + rethrows via `ExceptionDispatchInfo` to preserve the original stack. No + separate `ExceptionHandlingBehavior` is introduced — that responsibility + already lives here, and the L1 `IExceptionHandler` is the final catch site. +- **Tenant context just inside audit.** Audit needs `actor`, `tenant_id`, + `organization_id` to build a row; those must be resolved before the audit + snapshot runs. The behavior validates the context (asserts the middleware + populated it) and sets PostgreSQL session variables via the + `DbConnectionInterceptor` so RLS policies see the right values. +- **Authorization after tenant.** A permission decision usually keys on + `(tenant_id, user_id, resource)` — those must be ambient first. +- **Transaction after authorization.** No transaction is opened for a + forbidden request. +- **Outbox flush inside the transaction.** Per + [15-event-and-outbox.md](15-event-and-outbox.md), outbox rows write in the + same transaction as the originating domain change; the behavior is the + enrollment seam. +- **Handler at the innermost layer.** Pure domain + application logic; everything + ambient (tenant, logger, transaction, audit context) is set up before it + runs. + +The MediatR pipeline does **not** contain an `ExceptionHandlingBehavior`. Two +catch sites are sufficient: + +- **Inside `AuditLogBehavior`** — to emit a failure audit row before + rethrowing. +- **At the L1 `IExceptionHandler`** — to translate to Problem Details and + capture to error tracker. + +## 3. Exception Flow Through the System + +```mermaid +sequenceDiagram + participant Client + participant APISIX + participant API as ASP.NET API + participant L1 as IExceptionHandler + participant Pipeline as MediatR pipeline + participant Audit as AuditLogBehavior + participant Handler + participant Adapter as Provider adapter + participant Tracker as IErrorTrackingProvider + + Client->>APISIX: HTTP request (with traceparent or none) + APISIX->>API: forward + API->>Pipeline: Send(command) + Pipeline->>Audit: enter + Audit->>Handler: invoke + Handler->>Adapter: e.g. provider call + Adapter--xHandler: ProviderException(5xx, retryable=true) + Note over Handler: handler throws + Handler--xAudit: ProviderException + Audit->>Audit: write audit entry (outcome=Failed) + Audit--xPipeline: ExceptionDispatchInfo.Throw() + Pipeline--xAPI: rethrown + API->>L1: IExceptionHandler.TryHandleAsync(ex) + L1->>Tracker: CaptureAsync(ex, captured-context) + L1->>L1: Activity.RecordException + SetStatus(Error) + L1-->>Client: 503 ProblemDetails { code: dependency_unavailable, correlationId: ... } +``` + +And the happy-path-with-Result.Fail flow: + +```mermaid +sequenceDiagram + participant Client + participant API as ASP.NET API + participant Pipeline as MediatR pipeline + participant Audit as AuditLogBehavior + participant Handler + + Client->>API: POST /v1/courses { invalid title } + API->>Pipeline: Send(command) + Pipeline->>Pipeline: ValidationBehavior → Result.Fail(validation_failed, errors) + Pipeline-->>API: Result.Fail + API->>API: result.ToActionResult() → ProblemDetails(400) + API-->>Client: 400 ProblemDetails { code: validation_failed, errors: {...} } + Note over Audit: AuditLogBehavior never ran — Validation is outside it +``` + +Validation failures don't reach `AuditLogBehavior` because they short-circuit +upstream. This is deliberate — the audit log is for "something *happened* to +business data" not "the user typed a bad email address". The +`Validation`-class operations remain visible through metrics +(`learnstack_http_request_total{status=400}`) and request logs. + +## 4. Sentry vs OpenTelemetry — Error Capture Boundary + +Both backends receive *some* signal on every failure, but for different +audiences: + +```text +OTel: every failure SetStatus(Error) on its span. + Operators see latency + error rate; long-tail debug via Tempo. + +Sentry: only "something is wrong with our code / our infra". + Sentry release tags pinpoint the commit; breadcrumbs reconstruct the + last actions; alerting noise stays low. + +→ Result.Fail(expected outcome) = OTel span SetStatus(Ok); no Sentry. +→ Exception = OTel RecordException + Sentry capture. +→ ProviderException 4xx = OTel SetStatus(Error); no Sentry. +→ ProviderException 5xx = OTel RecordException + Sentry capture. +→ OperationCanceled = OTel span left Unset (no RecordException); no Sentry. +``` + +The `IErrorTrackingProvider.CaptureAsync` boundary lives in the L1 +`IExceptionHandler` (for unhandled exceptions) and in adapter `try/catch` +blocks (for provider failures that the handler caught and translated into +`Result.Fail`). Modules never reference `Sentry.SentrySdk` directly. + +## 5. Logging Stack + +```mermaid +flowchart LR + module["Module code: ILogger<T>"] --> melib["Microsoft.Extensions.Logging"] + melib --> serilog["Serilog (implementation)"] + serilog --> enrichers["Enrichers: + - CorrelationContext (8 fields) + - RedactSensitiveFields"] + enrichers --> formatter["RenderedCompactJsonFormatter"] + formatter --> console[Console stdout] + enrichers --> otlpsink["Serilog.Sinks.OpenTelemetry → OTLP"] + otlpsink --> collector[OTel Collector] + collector --> loki[Loki / Elastic] +``` + +Every module uses `ILogger`; the Serilog implementation is wired once at +the host composition root. No module references `Serilog.ILogger` directly. + +The 8 correlation fields +([10-observability.md § Correlation](../standards/10-observability.md)) ride +on Serilog's `LogContext` via `Enrich.WithCorrelationContext()`. The pattern +is "scope inherits" — the `LoggingBehavior` opens the scope once at the start +of the request; nested calls (handlers, EF interceptor logs, Dapr-emitted +logs through the auto-instrumentation) inherit it without per-call work. + +Redaction (passwords, tokens, full PII payloads) happens in an enricher +**before** the formatter so neither console nor OTLP sink ever sees the +plaintext. + +## 6. Tracing Stack + +```mermaid +flowchart LR + app["LearnStack hosts (API, workers)"] --> sdk["OpenTelemetry SDK"] + sdk --> auto["Auto-instrumentation: + AspNetCore, HttpClient, EFCore, + Hangfire, Valkey via Dapr"] + sdk --> manual["Manual ActivitySource: + 'learnstack.<module>.<op>'"] + auto --> processor["TenantContextSpanProcessor + (enriches every span with + tenant.id / org.id / user.id / + correlation.id / module)"] + manual --> processor + processor --> otlp["OTLP exporter"] + otlp --> collector[OTel Collector] + collector --> tempo[Tempo / Jaeger] +``` + +`TenantContextSpanProcessor` is the seam that lets auto-instrumentation +library spans (HttpClient, EF Core, Valkey via Dapr, SeaweedFS S3 SDK, +LiveKit) carry tenant attributes without per-call enrichment. The processor +is a singleton (OTel SDK design); it reads from the singleton +`ITenantContextAccessor` (`AsyncLocal`-backed), which the +tenant middleware populates at request start. The same accessor is set by +the Hangfire `JobActivator` (background jobs), the outbox / inbox handler +scope (integration events), and `HubCorrelationMiddleware` +(`/api/internal/*`). Every downstream span — even spans created by libraries +the LearnStack codebase has no knowledge of — picks up the right tags +because the SDK runs the processor's `OnStart` hook synchronously inside the +caller's async context. + +Sampling +([10-observability.md § Tracing](../standards/10-observability.md)) is +tail-based at the collector, not head-based at the SDK. 100% of error traces, +10% of non-error traces in production, 100% in staging. + +## 7. Metrics Stack + +Metrics use the standard OTel `Meter` API. The 15+ required metrics in +[10-observability.md § Metrics](../standards/10-observability.md) are emitted +by: + +- The `LoggingBehavior` (request duration histogram, request count counter, + outcome label from `Result.Code` or exception type). +- Module-specific code via injected `IMeterFactory.Create("learnstack")`. +- Adapter code via the resilience pipeline's built-in telemetry hooks + (`learnstack_provider_request_duration_seconds`, + `learnstack_provider_request_total{provider=...,outcome=...}`). + +No high-cardinality labels (no `user_id`, no `tenant_id` directly — that goes +on spans, not metrics; metrics use `tenant_tier` if a per-tenant axis is +unavoidable). + +## 8. Composition Root Branching for Deployment Mode + +[ADR-0020](../decisions/0020-triple-deployment-hybrid-license.md) + +[Standards 20 § Composition Root and Deployment Mode](../standards/20-infrastructure-stack.md) +table extends with two new rows from this architecture: + +| Concern | `Development` | `SaaS` | `Dedicated` | `SelfHostedOnline` | `SelfHostedAirGapped` | +|---|---|---|---|---|---| +| Error tracking | `NoOpErrorTracker` | `SentryErrorTracker` | `SentryErrorTracker` | `SentryErrorTracker` (optional) | `LocalFileErrorTracker` | +| OTLP exporter target | local OTel Collector (dev compose) | central Collector | central Collector | customer-managed Collector | local file `/var/learnstack/otel/` | + +Air-gapped Self-Hosted is the load-bearing case here: every backend (Sentry, +the central Collector, possibly even DNS) is unreachable. The +`LocalFileErrorTracker` writes structured-JSON error records to a configured +directory; an operator's runbook explains how to ship those off-network later +if the customer ever wants them. The OTLP exporter can be configured to +write to a file sink instead of a network endpoint via the standard OTel +file-exporter. + +Modules **never** branch on `DeploymentMode`. The composition root selects +the adapter at startup; the architecture test +`Modules_Do_Not_Reference_DeploymentMode` enforces the rule. + +## 9. Correlation Propagation Across Async Boundaries + +```mermaid +flowchart TD + http["HTTP request
traceparent header"] --> mw["Tenant + Correlation middleware
(starts Activity, populates ITenantContext)"] + mw --> pipeline["MediatR pipeline
LoggingBehavior opens ILogger scope"] + pipeline --> handler[Handler] + handler --> outbox["IOutbox.EnqueueAsync
(row carries tenant_id, correlation_id, ...)"] + handler --> hangfire["Hangfire enqueue
(payload carries tenant_id, correlation_id)"] + outbox --> processor["OutboxProcessor batch dispatch"] + processor --> dapr["DaprEventBus → Kafka"] + dapr --> consumer["Consumer pod
(restores ITenantContext from envelope,
sets Activity.Parent from traceparent)"] + hangfire --> activator["Hangfire JobActivator
(restores ITenantContext from payload,
sets Activity.Parent from correlation_id)"] + consumer --> consumerPipeline["Same MediatR pipeline"] + activator --> jobHandler["Job handler"] +``` + +The primitive is **W3C `traceparent`** end to end. Every cross-boundary write +includes it. The receiving side resumes the trace by setting +`Activity.ParentId = traceparent`, so Tempo sees one continuous trace from +the original HTTP request to the eventual outbox-dispatched consumer or +Hangfire job execution. + +### Hub HTTPS contract surface + +The four `/api/internal/*` endpoints ([ADR-0019](../decisions/0019-learnstack-hub.md)) +do not have a JWT tenant claim; their tenant context is read from the +request envelope's `tenantId` field after HMAC verification. The +`HubCorrelationMiddleware` accepts the inbound `traceparent` and starts an +`Activity` linked to it, so a Hub-side trace continues into LearnStack +seamlessly. Outbound calls (`POST /api/v1/internal/license/verify`, `POST +/api/v1/usage/report`) inject the current `traceparent` so the Hub-side +trace continues in the other direction. + +## 10. Provider Resilience Pattern + +```mermaid +flowchart LR + app[Application code] --> port["ILiveClassProvider (port)"] + port --> decorator["ResilientProviderAdapter<ILiveClassProvider>
(Polly v8 ResiliencePipeline:
retry, circuit breaker, timeout, bulkhead)"] + decorator --> adapter["LiveKitClient (adapter)
SDK exception → ProviderException"] + adapter --> sdk[LiveKit .NET SDK] + sdk --> upstream[LiveKit server] +``` + +Per [ADR-0032 § Sub-decision 5](../decisions/0032-exception-handling-logging-and-observability.md): + +- The application sees only the port interface (`ILiveClassProvider`, + `IPaymentProvider`, `IStorageProvider`, `ISearchProvider`, etc.). +- A decorator wraps the adapter with a Polly v8 `ResiliencePipeline` built + from `appsettings.Resilience::` configuration. +- The adapter is the only place that imports the provider SDK. Its job is + exception translation — every SDK exception is mapped to the appropriate + `ProviderException` subclass with the `IsClientError` flag set + appropriately (4xx → true, 5xx → false). +- The `[add-provider-adapter](../../.claude/skills/add-provider-adapter/SKILL.md)` + skill walks the canonical wiring; new adapters follow it without + freelancing. + +The architecture test `Adapters_Wrap_Provider_Exceptions` asserts SDK +exception types (`LiveKit.NET.LiveKitException`, `Stripe.StripeException`, +`Meilisearch.MeilisearchApiError`, …) never escape the +`LearnStack.Infrastructure.` namespace. + +## 11. Frontend Surface + +Two integration points +([09-error-handling.md § Frontend Error Handling](../standards/09-error-handling.md)): + +- **Problem Details mapper.** The SDK turns Problem Details bodies into the + `AppError` discriminated union; UI code switches on `code`. The shape is + generated from the same OpenAPI spec so frontend and backend never drift. +- **Recovery surfaces.** App Router segment `error.tsx` handles + segment-scoped failures; the root `app/global-error.tsx` is the last + resort. Both display the `correlationId` from the most recent Problem + Details response so a support handoff is one-step. Frontend Sentry attaches + the same `correlation_id` as a tag. + +## 12. Phase Ownership + +| Concern | Phase | Notes | +|---|---|---| +| `Result` + `Error` shape | Phase 01 (scaffolded) | Code already exists in `LearnStack.SharedKernel` | +| `LearnStackException` hierarchy | Phase 02a | Day-1 foundation | +| MediatR pipeline (8 behaviors) | Phase 02a | Order frozen by ADR-0032 | +| `LearnStackExceptionHandler : IExceptionHandler` | Phase 02a | L1 catch site | +| Serilog + OTel SDK wiring | Phase 02a | Hosts wire it once | +| `TenantContextSpanProcessor` | Phase 02a | OTel span enrichment | +| `IErrorTrackingProvider` socket | Phase 02a | Three implementations registered per `DeploymentMode` | +| `IProviderResilience` + decorator | Phase 02a | Foundation for every adapter | +| Roslyn analyzer for `DomainException` | Phase 02a | Compile-time enforcement of "bug only" | +| Outbox / Hangfire correlation propagation | Phase 02b | Row schema + activator | +| Hub HTTPS correlation middleware | Phase 02b / 02c | Cross-repo | +| OTel Collector + Tempo + Loki + Prometheus deployment | Phase 11 | Production-side backends | +| Sentry SaaS / Self-Hosted config | Phase 11 | Per-deployment-mode wiring | +| 8 first-set dashboards + 10 alerts | Phase 11 | Grafana provisioning | + +## References + +- [ADR-0032 Exception Handling, Logging, and Observability Architecture](../decisions/0032-exception-handling-logging-and-observability.md) +- [ADR-0014 Adopt Dapr](../decisions/0014-adopt-dapr.md) +- [ADR-0016 Audit Log Subsystem](../decisions/0016-audit-log-subsystem.md) +- [ADR-0020 Triple Deployment + Hybrid License](../decisions/0020-triple-deployment-hybrid-license.md) +- [09-error-handling.md](../standards/09-error-handling.md) +- [10-observability.md](../standards/10-observability.md) +- [02-backend-coding.md § Pipeline Behaviors](../standards/02-backend-coding.md) +- [20-infrastructure-stack.md](../standards/20-infrastructure-stack.md) +- [31-audit-subsystem.md](31-audit-subsystem.md) +- [15-event-and-outbox.md](15-event-and-outbox.md) diff --git a/docs/decisions/0032-exception-handling-logging-and-observability.md b/docs/decisions/0032-exception-handling-logging-and-observability.md new file mode 100644 index 0000000..ffc62c2 --- /dev/null +++ b/docs/decisions/0032-exception-handling-logging-and-observability.md @@ -0,0 +1,681 @@ +# ADR 0032: Exception Handling, Logging, and Observability Architecture + +## Status + +Accepted + +**Date:** 2026-05-20 +**Deciders:** @platform + +## Decision Drivers + +- **Standards 09 ↔ Standards 02 ↔ ADR-0016 are out of step.** Standards 02 § + Pipeline Behaviors lists Logging → Validation → … without `AuditLogBehavior`; + ADR-0016 § Pipeline behavior order has Validation → Logging → AuditLog → … as + a binding order with `try/catch + ExceptionDispatchInfo` rethrow. Standards 09 + refers to a "global exception middleware" without saying which .NET pattern. + Pre-implementation is the window to close the gap; after Phase 02a lands code, + the cost of changing it goes up sharply. +- **Foundation Day-1 commitment.** Per CLAUDE.md, observability + audit + infrastructure ship in Phase 02a, not in Phase 11. Without a binding contract + here, every module added in Phase 03-10 freelances its own error / log / + trace pattern, and the audit-coverage matrix becomes aspirational. +- **Two-track failure model is already a project rule** ([Standards 09](../standards/09-error-handling.md)): + exceptions for unexpected (bug / infra), `Result` for expected outcomes. + The remaining decisions are about *how the rails are laid down* so that the + rule is mechanically enforced. +- **Triple deployment.** SaaS / Dedicated / Self-Hosted online / Self-Hosted + air-gapped (per [ADR-0020](0020-triple-deployment-hybrid-license.md)) all run + the same binary. The same instrumentation must produce useful signals where + network egress to Sentry / SaaS observability backends is available **and** + silently degrade where it is not. +- **Provider-adapter parity.** Every external integration (LiveKit, Stripe, + Iyzico, Meilisearch, SeaweedFS, Keycloak, Hub) is reached through a + `LearnStack.Infrastructure.` adapter + ([20-infrastructure-stack.md](../standards/20-infrastructure-stack.md)). The + resilience + exception-wrap pattern must be the same across all of them so + reviewers don't need to re-learn it per adapter. +- **Hub HTTPS contract is closed at four endpoints.** Inbound `/api/internal/*` + calls do not carry a tenant JWT; their correlation must come from + `traceparent` + the request envelope, not from `ITenantContext`. +- **Pre-implementation status.** Only `Result` and `Error` records exist as + code (`backend/src/LearnStack.SharedKernel/Results/`). Everything below is a + contract, not a code change. + +## Considered Options + +### 1. **Option A — Single binding ADR codifying the entire cross-cutting contract (chosen)** + +One ADR pins: pipeline order, `IExceptionHandler` as L1, validation returning +`Result.Fail` (no throw), `DomainException` reserved for bugs, Polly v8 +`ResiliencePipeline` as the provider-resilience primitive, an +`IErrorTrackingProvider` abstraction over Sentry, the Serilog + OpenTelemetry +bridge, and `traceparent` as the correlation primitive across HTTP / outbox / +Hangfire / Hub. + +**Pros:** + +- Closes Standards 02 ↔ ADR-0016 gap in a single place; standards then *cite* + this ADR instead of redefining the contract. +- Mechanical enforcement via architecture tests becomes possible because every + rule has one canonical reference. +- Future architectural tweaks (e.g. switching error tracker, adding a 5th + pipeline behavior) land as Amendments here, not scattered edits. + +**Cons:** + +- Larger than a typical ADR (10+ binding sub-decisions). Mitigated by treating + the Implementation Notes section as the source of truth for each sub-decision + and keeping the Decision section short. +- Combines decisions of different blast radius (pipeline order is project-wide; + Sentry-vs-OTel split is observability-only). Mitigated by per-section + structure so an Amendment can address one slice without rewriting the rest. + +### 2. **Option B — Multiple smaller ADRs (one per gap) (rejected)** + +Split into: ADR-X "Pipeline order canonicalisation", ADR-Y "Error tracking +provider abstraction", ADR-Z "Provider resilience pattern", etc. + +**Pros:** + +- Each ADR is small and focused. +- Easier to amend a single concern. + +**Cons:** + +- These decisions are coupled: the pipeline order assumes a specific exception + flow; the Sentry-vs-OTel boundary assumes the pipeline order; the provider + resilience pattern feeds the Sentry-vs-OTel split. Splitting them produces 3-5 + ADRs that must always be read together. +- Reviewers must traverse the chain to confirm consistency; gaps reopen as the + set grows. +- Standards 02 / 09 / 10 each cite five ADRs instead of one — citation noise + with no decoupling benefit. + +### 3. **Option C — Amend ADR-0016 in place (rejected)** + +Extend ADR-0016 ("Audit Log Subsystem") with the rest of the pipeline-behavior +decisions: exception handling, logging, observability glue. + +**Pros:** + +- One Accepted ADR carries all pipeline rules. + +**Cons:** + +- Violates the project rule "an Accepted ADR's Decision section is immutable; + write a new ADR that supersedes it" (CLAUDE.md). ADR-0016 is Accepted and its + Decision section already covers audit; pipeline-order canonicalisation is + related but distinct scope. +- The Amendment block on ADR-0016 would balloon to cover concerns far from + "audit subsystem", muddying the ADR's topic. + +### 4. **Option D — Defer to Phase 02a "to be decided in code review" (rejected)** + +Make none of these decisions binding; leave Standards 09 / 10 as-is and resolve +ambiguity as it comes up during Phase 02a implementation. + +**Pros:** + +- Zero up-front cost. + +**Cons:** + +- Pre-implementation is precisely the cheapest moment to make these calls. +- Phase 02a deliverables include `AuditLogBehavior` plus architecture tests + asserting the pipeline order; without a binding contract those tests can't be + written. +- Phase 03-10 modules pick their own conventions; pipeline order rot starts + before the architecture tests catch it. + +## Decision + +LearnStack adopts **Option A**: a single binding ADR fixing the cross-cutting +contract for error handling, logging, and observability across the backend +runtime and the per-module Application layer. The contract has thirteen +binding sub-decisions; standards documents (02, 09, 10) are updated to cite +this ADR instead of redefining the rules. + +### Sub-decisions (each binding) + +1. **L1 exception handler is `IExceptionHandler` (.NET 8+).** Every host + (`LearnStack.Api`, worker, background-service) registers + `LearnStackExceptionHandler : IExceptionHandler` via + `services.AddExceptionHandler()` + + `app.UseExceptionHandler()`. The handler maps unhandled exceptions to RFC + 7807 Problem Details, attaches `correlation_id`, records the OTel span + error, and dispatches to `IErrorTrackingProvider` (sub-decision 9). The + older `app.UseExceptionHandler(lambda)` and `app.Use(ctx, next)` patterns + are not used in new code. + +2. **MediatR pipeline order is the canonical eight-step list below.** Standards + 02 § Pipeline Behaviors and ADR-0016 § Pipeline behavior order are aligned + to it: + + ```text + Request + → ValidationBehavior (FluentValidation; returns Result.Fail on invalid) + → LoggingBehavior (ILogger.BeginScope + Activity + correlation tags) + → AuditLogBehavior (handler wrap; try/catch + audit + ExceptionDispatchInfo) + → TenantContextBehavior (assert resolved; set RLS GUC) + → AuthorizationBehavior (permission check; Result.Fail(forbidden)) + → TransactionBehavior (UnitOfWork begin / commit / rollback) + → OutboxFlushBehavior (publish enrolled events on commit) + → Handler + ``` + + The order is bottom-most = innermost. Validation runs first because invalid + input must never reach DB / audit / business code. Audit wraps everything + from `TenantContextBehavior` inward so it sees both `Result.Fail` outcomes + and exception failures with the same try/catch pattern (per ADR-0016). + `ExceptionHandlingBehavior` is **not** introduced — `AuditLogBehavior` + already catches handler exceptions, audits the failure entry, and rethrows + via `ExceptionDispatchInfo`; the L1 `IExceptionHandler` is the final catch + site. Adding a separate behavior would duplicate that responsibility. + +3. **`ValidationBehavior` returns `Result.Fail(validation_failed)`; it does + not throw.** FluentValidation results are aggregated and lifted into the + `Error.Details` dictionary. The pipeline never raises a + `FluentValidation.ValidationException`. This keeps "exception ≠ control + flow" as a single rule and removes the need for catch logic in any + downstream behavior. The behavior is implemented with a generic constraint + `where TResponse : IResultBase` so it can construct the correct `Result` + shape via the static `Result.FailFor(error)` factory. + +4. **`DomainException` is reserved for programmer errors (bugs).** Expected + business-rule violations return `Result.Fail(business_rule_violation, …)` + from the domain method. The Roslyn analyzer + `LearnStackException-DomainExceptionThrow` flags every `throw new + DomainException` in `Domain` / `Application` projects as a Warning by + default and as an Error after the codebase reaches the green-bar threshold + (Phase 03 exit). Architecture test + `Domain_Methods_Do_Not_Throw_For_Expected_Cases` complements the analyzer + by walking the `Result`-returning methods and asserting that the + corresponding analyzer report is empty for the module. + + *"Unreachable case branch"* uses the BCL `System.Diagnostics.UnreachableException` + (.NET 7+) directly — it is **not** a `LearnStackException` subclass and + must not be modelled as one. The L1 `IExceptionHandler` treats it the + same as any other unhandled exception (Sentry-captured, 500 Problem + Details). Per [09-error-handling.md § Hierarchy](../standards/09-error-handling.md) + and [33-cross-cutting-concerns.md § Two-Track Failure Model](../architecture/33-cross-cutting-concerns.md). + +5. **Provider-adapter resilience uses Polly v8 `ResiliencePipeline` via + `IProviderResilience`.** The composition root wires every + tenant-facing third-party adapter (`LiveKitClient`, + `StripePaymentClient`, `IyzicoPaymentClient`, `MeilisearchClient`, + `SeaweedFSStorageClient`, …) with a pipeline carrying retry (exp backoff + + jitter), circuit breaker, timeout, and bulkhead policies declared in + `appsettings.{env}.json` under the `Resilience::` section. + Adapters' only exception-related job is translating provider SDK + exceptions into the appropriate `ProviderException` subclass + (`LiveClassProviderException`, `PaymentProviderException`, + `StorageProviderException`, …). The + `[add-provider-adapter](../../.claude/skills/add-provider-adapter/SKILL.md)` + skill walks the canonical wiring. Architecture test + `Adapters_Wrap_Provider_Exceptions` asserts the SDK exception types never + leave `LearnStack.Infrastructure.` namespaces. **Hub HTTP + clients (`IEntitlementProvider`, `IUsageReporter`, `IHubTenantSync`) are + excluded from this rule** — they have an additional mTLS + signed JWT + + HMAC wrapper per [ADR-0019](0019-learnstack-hub.md) and their resilience + policy lives inside that wrapper, defined by Phase 02c when the Hub + adapter itself lands. Re-introducing them into the standard + `IProviderResilience` table would split their resilience + configuration across two files; the Hub-specific wrapper owns the policy + end-to-end. + +6. **Controller-to-Result mapping uses an explicit extension method.** The + sanctioned shape: + + ```csharp + [HttpPost("courses")] + public async Task Create(CreateCourseCommand command, CancellationToken ct) + => (await _mediator.Send(command, ct)).ToActionResult(); + ``` + + `ResultExtensions.ToActionResult()` lives in `LearnStack.Api.Common`; it + matches on `Error.Code` and produces the Problem Details body. No action + filter, no MediatR `ResultUnwrapBehavior`, no implicit conversion. The + explicit pattern keeps the diff under review honest and the debug + experience straightforward. + +7. **Sentry-versus-OpenTelemetry error capture is partitioned, not + duplicated.** Every failure tags its OTel span; **Sentry capture is + reserved for "something is wrong with our code or our infrastructure"**. + The pattern in short: + + - **Capture to `IErrorTrackingProvider`**: unhandled `Exception`, + `LearnStackException` subclasses at L1, `ProviderException` with + `IsClientError == false` (5xx upstream). + - **OTel span only (no Sentry)**: `ProviderException` with + `IsClientError == true` (4xx upstream), every `Result.Fail(...)` + outcome, `OperationCanceledException`. + + The full eight-row partition table — including the per-row `Activity` + status mapping and rationale — is authoritative in + [09-error-handling.md § Sentry vs OpenTelemetry — Error Capture Boundary](../standards/09-error-handling.md); + keeping a second copy here would drift. The L1 handler's + `ShouldCapture(Exception ex)` switch implements the rule. + +8. **Serilog is the primary logger; logs reach OTel via the OTLP sink.** The + composition root wires: + + ```csharp + builder.Host.UseSerilog((ctx, services, cfg) => cfg + .ReadFrom.Configuration(ctx.Configuration) + .Enrich.WithCorrelationContext(services) // tenant / org / user / module / correlation_id + .Enrich.With() // strips tokens, passwords, PII + .WriteTo.Console(new RenderedCompactJsonFormatter()) + .WriteTo.OpenTelemetry(o => // Serilog.Sinks.OpenTelemetry → OTLP + { + o.Endpoint = otelEndpoint; + o.Protocol = OtlpProtocol.Grpc; + })); + ``` + + `Microsoft.Extensions.Logging` is the seam every module logs through; + Serilog is the implementation. The OTel logger provider is **not** also + registered (`AddOpenTelemetry().WithLogging()` is skipped); double-export + would duplicate every log line. Phase 02a's architecture test + `Logging_Goes_Through_Microsoft_Extensions_Logging` asserts no module + references `Serilog.ILogger` directly — modules only see `ILogger`. + +9. **`IErrorTrackingProvider` socket abstracts Sentry.** Composition root + branches on `DeploymentMode`: + + | `DeploymentMode` | Implementation | Notes | + |---|---|---| + | `Development` | `NoOpErrorTracker` | No external egress | + | `SaaS` | `SentryErrorTracker` | DSN from `ISecretProvider` | + | `Dedicated` | `SentryErrorTracker` | Per-tenant DSN allowed via Hub config | + | `SelfHostedOnline` | `SentryErrorTracker` (optional; `NoOpErrorTracker` if DSN absent) | Customer chooses | + | `SelfHostedAirGapped` | `LocalFileErrorTracker` (writes JSON to `/var/learnstack/errors/`) | No outbound network | + + `IErrorTrackingProvider.CaptureAsync(LearnStackException, CapturedContext)` + is the only sanctioned entry point. The architecture test + `Modules_Do_Not_Reference_Sentry_SDK_Directly` enforces it. + +10. **Telemetry signals carry `tenant.id`, `organization.id`, `user.id`, + `module`, `correlation_id` automatically.** A `TenantContextSpanProcessor : + BaseProcessor` is registered once at the composition root: + + ```csharp + services + .AddOpenTelemetry() + .WithTracing(t => t + .AddAspNetCoreInstrumentation() + .AddHttpClientInstrumentation() + .AddEntityFrameworkCoreInstrumentation() + .AddProcessor() // enrich every span + .AddOtlpExporter()); + ``` + + Because OTel processors are singletons by SDK design, the processor reads + tenant context from **`ITenantContextAccessor`** — a singleton, + `AsyncLocal`-backed accessor (analogous to `IHttpContextAccessor`) — + **not** from the request-scoped `ITenantContext` directly. The scoped + `ITenantContext` (handler-facing) and the singleton + `ITenantContextAccessor` (cross-cutting-infrastructure-facing) are two + contracts populated together: the `TenantResolverMiddleware` (HTTP), the + Hangfire `JobActivator` (background jobs), and the + integration-event-handler scope (outbox consumers) each set the accessor's + `AsyncLocal` value at scope start so any singleton (OTel processor, + Serilog enricher) can read the current tenant without a scope-validation + failure. Phase 02a ships both contracts together. Auto-instrumentation + libraries (EF Core, HttpClient, Valkey via Dapr, …) need no per-call + enrichment. + +11. **Hub HTTPS contract surface propagates correlation via `traceparent` + + request envelope.** The four `/api/internal/*` endpoints + ([Standards 20 § Hub HTTPS Contract Surface](../standards/20-infrastructure-stack.md)) + accept inbound `traceparent` headers; `HubCorrelationMiddleware` + instantiates the corresponding `Activity` so the inbound call continues + the upstream trace. Tenant context is **not** inferred from a JWT (there + is none) — it is read from the JSON envelope's `tenantId` field and + asserted against the HMAC-signed body. Outbound calls + LearnStack → Hub (`POST /api/v1/internal/license/verify`, `POST + /api/v1/usage/report`) inject the current `traceparent` so the Hub-side + trace continues seamlessly. + +12. **Outbox + Hangfire correlation propagation is contractual.** Every + `outbox_messages` row carries `tenant_id`, `organization_id?`, + `correlation_id`, `event_id`, `occurred_at`, `type` (already specified in + Phase 02b deliverables). The `correlation_id` column stores the **full + W3C `traceparent` header string** + (`00-<32-hex trace-id>-<16-hex parent-span-id>-<2-hex flags>`), not a + bare UUID — so the consumer rehydrates the trace deterministically: + `ActivityContext.TryParse(row.CorrelationId, traceState: null, out var parentCtx)`, + then `_activitySource.StartActivity(name, kind, parentCtx)`. Every + integration-event handler also restores `ITenantContext` from the + envelope before the inner pipeline runs. Every Hangfire job payload + includes `tenant_id` and `correlation_id` (same `traceparent` format); + the job activator restores the ambient context before handler + invocation. The architecture tests + `Hangfire_Job_Payloads_Include_TenantId` and + `Outbox_Row_Carries_Correlation_Context` are added in Phase 02b. + +13. **Frontend Sentry attaches `correlation_id` from the last server + response.** The Next.js app surfaces a recovery page when an unhandled + error reaches a root or segment error boundary; the page displays the + `correlation_id` returned in the most recent Problem Details body so a + support handoff is one-step. Frontend Sentry runs in `Production` and + staging modes; in `Development` it is off. + +## Context + +The codebase is pre-implementation (Phase 01 packets 1-6 shipped; only +`Result` and `Error` exist as live code). Standards 09 and Standards 10 +already lay down the *what* (two-track model, three signals, RFC 7807, +13 error codes, 15+ required metrics). Three implementation specifics were +left open or contradictory between documents: + +- **Pipeline order** disagreed between Standards 02 § Pipeline Behaviors and + ADR-0016 § Pipeline behavior order. ADR-0016 is Accepted; Standards 02 was + the one that drifted. +- **Where exceptions are caught** was unclear: Standards 09 referenced a + "global exception middleware" without naming a .NET pattern. ADR-0016 + expected `AuditLogBehavior` to be the catch + audit + rethrow point; + Standards 02 omitted that behavior entirely. +- **Sentry-versus-OTel split** for error capture, **Serilog-or-OTel** for + log emission, **how `tenant.id` reaches every span**, **how + `IErrorTrackingProvider` reacts to `DeploymentMode`**, and **how + correlation propagates through Hub HTTPS / outbox / Hangfire** were not + anywhere. + +Pre-implementation is the cheap moment to close those gaps. Once Phase 02a +ships an opinionated pipeline, retrofitting changes pulls the audit +infrastructure, the architecture tests, and every module's handler signature +in tow. + +ADR-0016 stays authoritative for the audit subsystem; this ADR cites it and +adopts its pipeline order verbatim. ADR-0014 stays authoritative for Dapr; +this ADR plugs Dapr's emitted traces / metrics into the OTel pipeline without +introducing new building blocks. ADR-0020 stays authoritative for +`DeploymentMode`; this ADR adds `IErrorTrackingProvider` to the composition +root's adapter table in [Standards 20 § Composition Root and Deployment Mode](../standards/20-infrastructure-stack.md). + +## Consequences + +### Positive + +- **Standards 02 ↔ ADR-0016 ↔ Standards 09 ↔ Standards 10 collapse to one + source of truth.** Each standard now cites this ADR for the cross-cutting + rules instead of restating them. +- **Architecture tests become writable.** `MediatR_Pipeline_Order_Matches_Canonical_Sequence`, + `Domain_Methods_Do_Not_Throw_For_Expected_Cases`, + `Adapters_Wrap_Provider_Exceptions`, + `Modules_Do_Not_Reference_Sentry_SDK_Directly`, + `Logging_Goes_Through_Microsoft_Extensions_Logging` all enforce specific + binding rules; without this ADR they would be opinion. +- **Air-gapped Self-Hosted works without Sentry egress.** + `LocalFileErrorTracker` keeps the contract intact while obeying ADR-0020's + no-network constraint. +- **Provider adapters become uniform.** Each adapter does exception + translation only; resilience policy is one declarative `appsettings` + section per port. +- **Phase 02a `AuditLogBehavior` is the only failure-catch site below L1.** + Reviewers don't have to ask "where else might the exception be caught?". +- **One mental model for new contributors.** "Exceptions = bugs / infra = + Sentry. `Result.Fail` = refused = no Sentry. Provider failure = wrapped at + the adapter boundary. Audit sees both via `AuditLogBehavior`." + +### Negative + +- **Polly v8 dependency** lands now as part of the foundation. Net new + package but the canonical choice — `Microsoft.Extensions.Resilience` + builds on top of it. +- **`IErrorTrackingProvider` is one more adapter** in the composition-root + branching table. Mitigated by the same pattern used for `IEventBus` / + `ICacheService` / `ISecretProvider` / `IEntitlementProvider`. +- **`TenantContextSpanProcessor` couples OTel pipeline to + `ITenantContext`.** If `ITenantContext` is uninitialised (very early + middleware path, malformed request), the processor must no-op rather than + throw. Implementation detail enforced by Phase 02a unit test + `TenantContextSpanProcessor_DoesNotThrow_When_Context_Missing`. +- **Roslyn analyzer for `DomainException` throws** is real work and lives in + `backend/analyzers/`. Without it, the "DomainException = bug" discipline + relies on reviewer vigilance. + +### Neutral + +- The choice of Sentry as the error backend is unchanged; this ADR only adds + the abstraction. Switching to another error backend (Honeybadger, Rollbar, + self-hosted GlitchTip) is now a composition-root swap. +- `traceparent` is a W3C standard already mandated by Standards 10 § Tracing; + this ADR makes it the binding correlation primitive across HTTP, outbox, + Hangfire, and Hub. + +## Implementation Notes + +### Phase 02a deliverables that flow from this ADR + +| Item | Owner | Architecture test | +|---|---|---| +| `LearnStackExceptionHandler : IExceptionHandler` | `LearnStack.Api` | `IExceptionHandler_Registered_AtStartup` | +| MediatR pipeline order (8 behaviors) | `LearnStack.Application` | `MediatR_Pipeline_Order_Matches_Canonical_Sequence` | +| `ValidationBehavior` returns `Result.Fail` | `LearnStack.Application` | `ValidationBehavior_DoesNotThrow_ValidationException` | +| `AuditLogBehavior` catches + audits + rethrows via `ExceptionDispatchInfo` | `LearnStack.Infrastructure.Audit` | (covered by ADR-0016's `AuditLogBehavior_NeverBlocks_BusinessWrites`) | +| `TenantContextSpanProcessor` registered on OTel tracing pipeline | `LearnStack.Infrastructure.Observability` | `OTel_Pipeline_Includes_TenantContextSpanProcessor` | +| Serilog + OTLP sink wired; no `AddOpenTelemetry().WithLogging()` | `LearnStack.Api`, worker hosts | `Logging_Goes_Through_Microsoft_Extensions_Logging` | +| `IErrorTrackingProvider` interface + 3 implementations | `LearnStack.SharedKernel`, `LearnStack.Infrastructure.ErrorTracking` | `Modules_Do_Not_Reference_Sentry_SDK_Directly` | +| `Result.ToActionResult()` extension | `LearnStack.Api.Common` | n/a (lint rule) | +| Roslyn analyzer flagging `throw new DomainException` outside `Domain` invariants | `backend/analyzers/` | `Domain_Methods_Do_Not_Throw_For_Expected_Cases` (uses the analyzer) | + +### Phase 02b deliverables that flow from this ADR + +| Item | Owner | Architecture test | +|---|---|---| +| Outbox row schema columns: `tenant_id`, `organization_id?`, `correlation_id`, `event_id`, `occurred_at`, `type` | `LearnStack.Infrastructure.Outbox` | `Outbox_Row_Carries_Correlation_Context` | +| Hangfire job payloads include `tenant_id` + `correlation_id` | `LearnStack.Infrastructure.BackgroundJobs` | `Hangfire_Job_Payloads_Include_TenantId` | +| Integration-event handler scope restores `ITenantContext` from envelope | `LearnStack.Infrastructure.Outbox` | `Integration_Event_Handler_Restores_Tenant_Context` | +| `HubCorrelationMiddleware` for `/api/internal/*` | `LearnStack.Api` | (covered by `Standards 20 § Hub HTTPS Contract Surface` audit) | + +### `IProviderResilience` shape + +```csharp +public interface IProviderResilience where TPort : class +{ + ResiliencePipeline Pipeline { get; } + string PortName { get; } // "liveclass", "payment", "storage", ... +} + +// Composition-root extension (lives in LearnStack.Infrastructure) +public static IServiceCollection AddProviderResilience( + this IServiceCollection services, + string portName) + where TPort : class + where TImpl : class, TPort +{ + services.AddSingleton(); // base adapter + services.AddSingleton>(sp => + new ProviderResilience( + portName, + sp.GetRequiredService().GetSection($"Resilience:{portName}"))); + services.Decorate>(); + return services; +} +``` + +The decorator reads `Resilience::` from configuration and builds a +`ResiliencePipeline` with retry + circuit breaker + timeout + bulkhead. The +configuration shape is fixed in [Standards 09 § Provider Failures](../standards/09-error-handling.md). + +### `LearnStackExceptionHandler` shape + +```csharp +internal sealed class LearnStackExceptionHandler( + IErrorTrackingProvider errorTracker, + ILogger logger, + IProblemDetailsFactory problemDetailsFactory) : IExceptionHandler +{ + public async ValueTask TryHandleAsync( + HttpContext context, Exception ex, CancellationToken ct) + { + var problem = problemDetailsFactory.For(ex, context); + var captured = ShouldCapture(ex); + if (captured) + await errorTracker.CaptureAsync(ex, CapturedContext.From(context), ct); + + // OperationCanceledException → leave the span Unset and skip + // RecordException. RecordException tags the span with `exception.*` + // attributes which most OTel exporters render as Error in the trace + // UI; that would contradict Sub-decision 7's "no Sentry, no Error + // span" rule for client disconnects. (`ActivityStatusCode` is the + // 3-value OTel enum — `Unset | Ok | Error` — so a cancelled request + // simply stays at the default Unset, the same state the SDK assigns + // to any request that runs to completion without an error.) + if (ex is not OperationCanceledException) + { + Activity.Current?.RecordException(ex); + Activity.Current?.SetStatus(ActivityStatusCode.Error, ex.GetType().Name); + } + + context.Response.StatusCode = problem.Status ?? 500; + await context.Response.WriteAsJsonAsync(problem, ct); + return true; + } + + private static bool ShouldCapture(Exception ex) => ex switch + { + OperationCanceledException => false, + ProviderException pex when pex.IsClientError => false, // 4xx upstream + _ => true, // 5xx upstream, bug, infra + }; +} +``` + +### `TenantContextSpanProcessor` shape + +```csharp +internal sealed class TenantContextSpanProcessor(ITenantContextAccessor accessor) + : BaseProcessor +{ + public override void OnStart(Activity activity) + { + var context = accessor.Current; + if (context is null) return; // outside any resolved scope; do not throw + + if (context.IsResolved) + { + activity.SetTag("tenant.id", context.TenantId); + if (context.OrganizationId is { } orgId) + activity.SetTag("organization.id", orgId); + if (context.UserId is { } userId) + activity.SetTag("user.id", userId); + } + + if (context.CorrelationId is { } correlationId) + activity.SetTag("correlation.id", correlationId); + if (context.ModuleName is { } moduleName) + activity.SetTag("module", moduleName); + } +} +``` + +`BaseProcessor` is a singleton; injecting the **request-scoped** +`ITenantContext` directly would fail at startup with "Cannot consume scoped +service `ITenantContext` from singleton". The singleton accessor +`ITenantContextAccessor` solves the lifetime mismatch: + +```csharp +public interface ITenantContextAccessor +{ + ITenantContext? Current { get; set; } // AsyncLocal-backed +} + +internal sealed class TenantContextAccessor : ITenantContextAccessor +{ + private static readonly AsyncLocal _current = new(); + public ITenantContext? Current + { + get => _current.Value; + set => _current.Value = value; + } +} +``` + +Population pattern, set at scope start: + +| Host | Where `accessor.Current` is set | +|------|---------------------------------| +| `LearnStack.Api` HTTP request | `TenantResolverMiddleware` reads JWT + host, builds `ITenantContext`, assigns to accessor | +| Hangfire job | `JobActivator` reads `tenant_id` + `correlation_id` from payload, builds `ITenantContext`, assigns to accessor | +| Integration-event handler | Outbox / inbox handler scope reads envelope, builds `ITenantContext`, assigns to accessor | +| `/api/internal/*` | `HubCorrelationMiddleware` reads HMAC-verified envelope, builds `ITenantContext`, assigns to accessor | + +Phase 02a unit test +`TenantContextSpanProcessor_DoesNotThrow_When_Context_Missing` asserts +`OnStart` is safe to call before any scope has populated the accessor (the +SDK creates and disposes warm-up `Activity` instances during startup). + +### Configuration shape (`appsettings.json`) + +```jsonc +{ + "Resilience": { + "liveclass": { + "retry": { "maxAttempts": 3, "delaySeconds": 1, "useJitter": true }, + "circuitBreaker": { "failureRatio": 0.5, "samplingDurationSeconds": 30, "minimumThroughput": 10, "breakDurationSeconds": 30 }, + "timeout": { "totalSeconds": 10 } + }, + "payment": { "...": "..." }, + "storage": { "...": "..." }, + "search": { "...": "..." } + // Hub HTTP clients have their own resilience inside the mTLS + signed-JWT + // + HMAC wrapper per ADR-0019; not configured here. See Sub-decision 5. + }, + + "ErrorTracking": { + "Provider": "Sentry", // matched against DeploymentMode in composition root + "Sentry": { "Dsn": "from-vault", "Environment": "saas-prod" }, + "LocalFile": { "Directory": "/var/learnstack/errors/" } + }, + + "Telemetry": { + "OtlpEndpoint": "http://otel-collector:4317", + "Service": { "Name": "learnstack-api", "Version": "git-sha" } + } +} +``` + +## References + +- [ADR-0002 Initial Architecture](0002-initial-architecture.md) — the + observability stack column. +- [ADR-0006 Events and Outbox](0006-events-and-outbox.md) — outbox row + schema; this ADR adds `correlation_id` propagation to integration-event + handlers. +- [ADR-0010 Cross-Module Communication](0010-cross-module-communication.md) + — the four sanctioned mechanisms; this ADR is observability-side and adds + none. +- [ADR-0014 Adopt Dapr](0014-adopt-dapr.md) — Dapr emits OTel traces + + metrics; this ADR plugs them into the Collector pipeline. +- [ADR-0016 Audit Log Subsystem](0016-audit-log-subsystem.md) — pipeline + order originates here; this ADR adopts it unchanged. +- [ADR-0017 Tenant + Organization Hierarchy](0017-tenant-organization-hierarchy.md) + — `tenant.id` / `organization.id` span attributes match this hierarchy. +- [ADR-0019 LearnStack Hub](0019-learnstack-hub.md) — Hub HTTPS contract + surface; this ADR specifies the correlation propagation across it. +- [ADR-0020 Triple Deployment + Hybrid License](0020-triple-deployment-hybrid-license.md) + — `DeploymentMode` table; this ADR adds `IErrorTrackingProvider` row. +- [02-backend-coding.md § Pipeline Behaviors](../standards/02-backend-coding.md) + — order list cites this ADR. +- [21-architecture-tests-catalogue.md](../standards/21-architecture-tests-catalogue.md) + — single source of truth for every architecture-test / analyzer + identifier this ADR introduces. Other docs cite the catalogue entry by + anchor link so renames touch one place. +- [09-error-handling.md](../standards/09-error-handling.md) — implementation + patterns for L1 / `Result` / Validation / provider resilience cite this + ADR. +- [10-observability.md](../standards/10-observability.md) — Sentry / OTel + split, Serilog bridge, `tenant.id` span propagation cite this ADR. +- [20-infrastructure-stack.md § Composition Root and Deployment Mode](../standards/20-infrastructure-stack.md) + — `IErrorTrackingProvider` row. +- [33-cross-cutting-concerns.md](../architecture/33-cross-cutting-concerns.md) + — conceptual deep dive and diagrams. +- [Phase 02a Roadmap](../roadmap/phase-02a-kernel-tenancy.md) — deliverables. +- [Phase 02b Roadmap](../roadmap/phase-02b-events-auth.md) — outbox / + Hangfire correlation deliverables. +- W3C Trace Context — +- Polly v8 documentation — +- OpenTelemetry .NET — diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 113f3f0..2a6f068 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -40,6 +40,7 @@ Accepted ADRs are not rewritten. A new decision is a new ADR, possibly supersedi | 0029 | [Object Storage — SeaweedFS](0029-object-storage-seaweedfs.md) | Self-hosted SeaweedFS behind the existing `IStorageProvider` S3 contract; partially supersedes ADR-0002's MinIO row | | 0030 | [Redis-compatible Store — Valkey](0030-redis-compatible-store-valkey.md) | Valkey (Linux Foundation, BSD-3-Clause) for the cache + Dapr state-store backend; RESP-protocol drop-in; partially supersedes ADR-0002's Redis row | | 0031 | [PostgreSQL — Start on 18.x](0031-postgresql-major-version.md) | Pin primary RDBMS major version to PostgreSQL 18; native `gen_uuid_v7()` + async I/O + longest LTS runway; partially supersedes ADR-0002's PostgreSQL row | +| 0032 | [Exception Handling, Logging, and Observability](0032-exception-handling-logging-and-observability.md) | `IExceptionHandler` + 8-step MediatR pipeline + `Result.Fail`-only validation + `DomainException`-is-bug discipline + `IProviderResilience` (Polly v8) + Sentry vs OTel error capture boundary + Serilog primary + `TenantContextSpanProcessor` + `IErrorTrackingProvider` deployment-mode branching | ## Superseded ADRs diff --git a/docs/glossary.md b/docs/glossary.md index 048f500..21d8500 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -241,6 +241,23 @@ This glossary defines LearnStack-specific terms. When a term is ambiguous across | **Problem Details** | RFC 7807 JSON error envelope (`type`, `title`, `status`, `code`, `detail`, `instance`, `correlationId`) used by every LearnStack error response. | | **Hub Contract Surface** | The closed set of four endpoints between LearnStack core and the Hub: `POST /api/internal/tenants`, `PUT /api/internal/tenants/{id}/entitlements`, `POST /api/v1/internal/license/verify`, `POST /api/v1/usage/report`. mTLS + signed JWT + HMAC. Adding a fifth requires a new ADR. | +## Cross-Cutting Concerns + +| Term | Definition | +|------|------------| +| **`Result`** | The sealed record `(bool IsSuccess, T? Value, Error? Error)` in `LearnStack.SharedKernel.Results`. Application + Domain layer methods return `Result` for **expected** outcomes (validation failed, not found, forbidden, business-rule violation). Exceptions are reserved for **unexpected** bugs / infrastructure faults. See [09-error-handling.md § Two-Track Model](standards/09-error-handling.md) and [ADR-0032](decisions/0032-exception-handling-logging-and-observability.md). | +| **`Error`** | The sealed record `(string Code, string Message, IReadOnlyDictionary? Details)` that travels inside `Result.Fail(error)`. `Code` is a stable English identifier from the 13-code catalogue ([09-error-handling.md § Result Type](standards/09-error-handling.md)); `Message` is localisable; `Details` carries field-level errors. | +| **Pipeline Behavior** | A MediatR pipeline behavior — one of the eight canonical steps wrapping every command / query: `Validation → Logging → Audit → TenantContext → Authorization → Transaction → OutboxFlush → Handler`. The order is binding per [ADR-0032 § Sub-decision 2](decisions/0032-exception-handling-logging-and-observability.md); the architecture test `MediatR_Pipeline_Order_Matches_Canonical_Sequence` enforces it. | +| **`IExceptionHandler` (LearnStack L1)** | The `.NET 8+` exception-handler interface; `LearnStackExceptionHandler : IExceptionHandler` is the final catch site for every unhandled exception. Maps to Problem Details, attaches `correlationId`, records the OTel span error, calls `IErrorTrackingProvider.CaptureAsync` when `ShouldCapture(ex)` is true. See [ADR-0032 § Sub-decision 1](decisions/0032-exception-handling-logging-and-observability.md). | +| **Correlation ID** | The W3C `traceparent` value (or a derived UUID for fallback) that threads through every signal — logs, traces, audit rows, outbox rows, Hangfire payloads, Problem Details bodies — bound to a single user request or background operation. Same as `Activity.Current.TraceId` for HTTP requests; reconstructed from the outbox row / Hangfire payload / event envelope at consumer side. See [10-observability.md § Correlation](standards/10-observability.md). | +| **Telemetry Signal** | One of the three OpenTelemetry signals — logs, traces, metrics. LearnStack emits all three through Microsoft.Extensions.Logging (Serilog implementation) + OTel SDK; errors additionally flow to `IErrorTrackingProvider`. | +| **`IErrorTrackingProvider`** | The composition-root abstraction over the error backend. Implementations: `NoOpErrorTracker` (Development / SelfHostedOnline-without-DSN), `SentryErrorTracker` (SaaS / Dedicated / SelfHostedOnline-with-DSN), `LocalFileErrorTracker` (SelfHostedAirGapped). Modules never import `Sentry.SentrySdk`; the architecture test `Modules_Do_Not_Reference_Sentry_SDK_Directly` enforces it. See [ADR-0032 § Sub-decision 9](decisions/0032-exception-handling-logging-and-observability.md). | +| **`IProviderResilience`** | The decorator wrapping every provider adapter with a Polly v8 `ResiliencePipeline` (retry + circuit breaker + timeout + bulkhead). The adapter does only SDK exception → `ProviderException` translation; resilience is centralised. Configured per port in `appsettings.Resilience::`. See [ADR-0032 § Sub-decision 5](decisions/0032-exception-handling-logging-and-observability.md). | +| **`ITenantContextAccessor`** | The singleton, `AsyncLocal`-backed accessor that cross-cutting infrastructure (`TenantContextSpanProcessor`, Serilog enricher, Sentry enricher) reads to enrich telemetry without inheriting the request-scoped DI lifetime. Populated at scope start by `TenantResolverMiddleware` (HTTP), `HubCorrelationMiddleware` (`/api/internal/*`), Hangfire `JobActivator` (background jobs), and the outbox / inbox handler scope. Modules never write to it. See [ADR-0032 § Sub-decision 10](decisions/0032-exception-handling-logging-and-observability.md). | +| **`TenantContextSpanProcessor`** | The `BaseProcessor` registered once at the OTel tracing pipeline; its `OnStart` hook reads from `ITenantContextAccessor` and enriches every span with `tenant.id`, `organization.id`, `user.id`, `module`, `correlation.id` — including spans produced by auto-instrumentation libraries (EF Core, HttpClient, Valkey via Dapr, SeaweedFS S3 SDK, LiveKit). See [ADR-0032 § Sub-decision 10](decisions/0032-exception-handling-logging-and-observability.md). | +| **`ProviderException.IsClientError`** | Boolean flag set by adapters when translating upstream 4xx (`true`) or 5xx (`false`) responses. The L1 `IExceptionHandler` reads it to decide whether to Sentry-capture (only 5xx — provider's infra fault) or log-only (4xx — provider's user-error). | +| **L1 / L2 / L3 (cache)** | "L1 cache" is the per-pod in-process `IMemoryCache`; "L2 cache" is the cross-pod Valkey state via Dapr. Both layers are managed through `ICacheService`; do not confuse with error-handling layers. See [20-infrastructure-stack.md § Cache layer cheat sheet](standards/20-infrastructure-stack.md). | + ## Foundation Infrastructure | Term | Definition | diff --git a/docs/roadmap/phase-02a-kernel-tenancy.md b/docs/roadmap/phase-02a-kernel-tenancy.md index 8374300..faf1c50 100644 --- a/docs/roadmap/phase-02a-kernel-tenancy.md +++ b/docs/roadmap/phase-02a-kernel-tenancy.md @@ -26,6 +26,7 @@ They are codified in: - [ADR-0018 Tenant-Driven Customization Model](../decisions/0018-tenant-driven-customization-model.md) - [ADR-0020 Triple Deployment + Hybrid License](../decisions/0020-triple-deployment-hybrid-license.md) - [ADR-0021 Feature-Based Entitlement](../decisions/0021-feature-based-entitlement.md) +- [ADR-0032 Exception Handling, Logging, and Observability Architecture](../decisions/0032-exception-handling-logging-and-observability.md) ## Scope @@ -120,6 +121,67 @@ Per [ADR-0016](../decisions/0016-audit-log-subsystem.md): - MUST-class coverage is enabled for every command and security event the modules declare; modules added in later phases extend the catalog, not the infrastructure. +### Cross-cutting Concerns (Day 1) + +Per [ADR-0032](../decisions/0032-exception-handling-logging-and-observability.md): + +- **L1 exception handler.** `LearnStackExceptionHandler : IExceptionHandler` + ships in `LearnStack.Api`; registered with + `services.AddExceptionHandler() + + services.AddProblemDetails()`. +- **MediatR pipeline behaviors (eight-step canonical order).** + `ValidationBehavior` (returns `Result.Fail(validation_failed)` — never + throws), `LoggingBehavior` (opens the 8-field `ILogger` scope + manual + `Activity` + latency histogram), `AuditLogBehavior` (per ADR-0016 — wraps + inner pipeline with try/catch + audit-fail entry + ExceptionDispatchInfo + rethrow), `TenantContextBehavior` (asserts resolved + sets RLS GUCs), + `AuthorizationBehavior` (returns `Result.Fail(forbidden)` on deny), + `TransactionBehavior` (UoW), `OutboxFlushBehavior` (enrols outbox writes + in current tx). +- **`Result.ToActionResult()` extension** lives in + `LearnStack.Api.Common`; every controller endpoint uses it explicitly. No + action filter, no `ResultUnwrapBehavior`. +- **Roslyn analyzer `LearnStackException-DomainExceptionThrow`** under + `backend/analyzers/` flags every `throw new DomainException(...)` outside + aggregate invariant guards. Warning in Phase 02a, escalates to Error after + Phase 03 exit. +- **`IProviderResilience` decorator** with Polly v8 + `ResiliencePipeline` (retry + circuit breaker + timeout + bulkhead) lives + in `LearnStack.Infrastructure.Resilience`. Configuration shape: + `appsettings.Resilience::`. Every adapter is wired through the + `AddProviderResilience(string portName)` composition-root + extension. The + [add-provider-adapter](../../.claude/skills/add-provider-adapter/SKILL.md) + skill walks the canonical wiring. +- **Serilog primary logger + OTLP sink.** Hosts wire + `builder.Host.UseSerilog(...)` with `WriteTo.Console(...)` + + `WriteTo.OpenTelemetry(...)`. The OTel `LoggerProvider` + (`AddOpenTelemetry().WithLogging()`) is **not** registered alongside. + Modules log through `ILogger` only. +- **OpenTelemetry SDK** wired with `AddAspNetCoreInstrumentation` + + `AddHttpClientInstrumentation` + `AddEntityFrameworkCoreInstrumentation` + + `AddProcessor` + `AddOtlpExporter`. Manual + `ActivitySource` named per module (`learnstack.`) for use-case + spans. +- **`ITenantContextAccessor`** (singleton, `AsyncLocal`-backed) + lives in `LearnStack.SharedKernel` alongside the request-scoped + `ITenantContext`. The scoped interface is what handlers and services + inject; the singleton accessor is what cross-cutting infrastructure + (OTel processor, Serilog enricher, Sentry enricher) reads. The accessor + is populated at scope start by `TenantResolverMiddleware` (HTTP), + `HubCorrelationMiddleware` (`/api/internal/*`), Hangfire `JobActivator` + (background jobs), and the outbox / inbox handler scope (integration + events). Modules never write to the accessor. +- **`TenantContextSpanProcessor : BaseProcessor`** lives in + `LearnStack.Infrastructure.Observability`; reads from + `ITenantContextAccessor.Current` in its `OnStart` hook and enriches every + span with `tenant.id`, `organization.id`, `user.id`, `module`, + `correlation.id`. +- **`IErrorTrackingProvider` socket.** Three implementations land: + `NoOpErrorTracker`, `SentryErrorTracker`, `LocalFileErrorTracker`. + Composition root branches on `DeploymentMode`. DSN comes from + `ISecretProvider`. Modules never reference `Sentry.SentrySdk`. + ### Tenant Customization Foundation (Day 1) Per [ADR-0018](../decisions/0018-tenant-driven-customization-model.md): @@ -229,6 +291,28 @@ The architecture test project starts going green during this phase. Phase 02a co [20-infrastructure-stack.md § Composition Root and Deployment Mode](../standards/20-infrastructure-stack.md). - `Core_Modules_HaveNo_DomainSpecific_Names`, `No_Source_Folder_Named_Verticals`. +- `IExceptionHandler_Registered_AtStartup` — every host registers + `LearnStackExceptionHandler`. +- `MediatR_Pipeline_Order_Matches_Canonical_Sequence` — DI registration produces the eight-step + pipeline in the canonical order. +- `ValidationBehavior_DoesNotThrow_ValidationException` — runtime assertion + via an integration test that triggers a validation failure. +- `Domain_Methods_Do_Not_Throw_For_Expected_Cases` — uses the + `LearnStackException-DomainExceptionThrow` Roslyn analyzer report. +- `Adapters_Wrap_Provider_Exceptions` — provider SDK exception types do not + leave `LearnStack.Infrastructure.` namespaces. +- `Modules_Do_Not_Reference_Sentry_SDK_Directly`. +- `Logging_Goes_Through_Microsoft_Extensions_Logging` — modules import + `Microsoft.Extensions.Logging.ILogger`, not `Serilog.ILogger`. +- `OTel_Pipeline_Includes_TenantContextSpanProcessor`. +- `TenantContextSpanProcessor_DoesNotThrow_When_Context_Missing` — unit + test guard. + +Every identifier in the cross-cutting list above is described — assertion, +type, source ADR / standard — in +[21-architecture-tests-catalogue.md § Cross-cutting: error handling, logging, observability](../standards/21-architecture-tests-catalogue.md). +The catalogue is the canonical reference; rename or relocation lands there +first. The event/outbox-specific tests (serialisable records, job payloads with `TenantId`) land in Phase 02b. @@ -246,6 +330,14 @@ land in Phase 02b. - `LearnStack.Modules.Audit` aggregates + `LearnStack.Infrastructure.Audit` pipeline + partitioned `audit_log` table + retention job. - `platform_entitlement_cache` and `platform_host_to_tenant` tables + read paths. +- Cross-cutting foundation (per + [ADR-0032](../decisions/0032-exception-handling-logging-and-observability.md)): + `LearnStackExceptionHandler`, 8-step MediatR pipeline, `Result.ToActionResult` + extension, `IProviderResilience` decorator, Serilog + OTLP sink, + `TenantContextSpanProcessor`, `IErrorTrackingProvider` with three + implementations, Roslyn analyzer for `DomainException`. The + [wire-cross-cutting-foundation](../../.claude/skills/wire-cross-cutting-foundation/SKILL.md) + skill walks the canonical wiring. - Database conventions implemented and enforced. - API conventions wired (versioning, Problem Details, cursor pagination, idempotency, ETag). diff --git a/docs/roadmap/phase-02b-events-auth.md b/docs/roadmap/phase-02b-events-auth.md index 2130447..e3fc277 100644 --- a/docs/roadmap/phase-02b-events-auth.md +++ b/docs/roadmap/phase-02b-events-auth.md @@ -24,6 +24,9 @@ Decisions made in this phase: (Amendment 1: outbox dispatch target is Dapr pub/sub) - [ADR-0014 Adopt Dapr](../decisions/0014-adopt-dapr.md) (sidecar wired in 02a; the dispatch path lands here) +- [ADR-0032 Exception Handling, Logging, and Observability Architecture](../decisions/0032-exception-handling-logging-and-observability.md) + (outbox / Hangfire correlation propagation; Hub HTTPS surface correlation + middleware) ## Scope @@ -36,7 +39,17 @@ Decisions made in this phase: - Retry with exponential backoff (1s, 5s, 30s, 5min, 1h); dead-letter after max attempts (5). Dead-letter visible via the `OutboxStatusEndpoints` admin API. - Outbox row attaches `tenant_id`, `organization_id?`, `correlation_id`, `event_id`, - `occurred_at`, versioned `type` to every event. + `occurred_at`, versioned `type` to every event (per + [ADR-0032 § Sub-decision 12](../decisions/0032-exception-handling-logging-and-observability.md)). + The `correlation_id` column is the **full W3C `traceparent` header string** + (`00-<32-hex trace-id>-<16-hex parent-span-id>-<2-hex flags>`), not a + bare UUID — so the consumer can rehydrate the trace with one call: + `ActivityContext.TryParse(row.CorrelationId, traceState: null, out var parentCtx)` + then `_activitySource.StartActivity(name, kind, parentCtx)`. Storing the + full string keeps the deserialisation deterministic and avoids any + "how do we map a UUID back to a trace-id?" ambiguity. Consumer handler + also restores `ITenantContext` from the envelope before the inner + pipeline runs, so end-to-end trace + tenant context stay continuous. - Versioned integration event types in `.Application.Contracts`, inheriting `IntegrationEventBase`. - Per-module **`inbox_messages`** table + `IInboxGuard`. Every @@ -115,6 +128,18 @@ In addition to Phase 02a's rules, this phase adds: guard before processing. - `OutboxProcessor_NeverBlocks_OnSingleMessageFailure` — integration test asserts one poisoned message doesn't prevent others in the batch from processing. +- `Outbox_Row_Carries_Correlation_Context` — every persisted outbox row has + non-null `tenant_id` and `correlation_id` per + [ADR-0032 § Sub-decision 12](../decisions/0032-exception-handling-logging-and-observability.md). +- `Hangfire_Job_Payloads_Include_TenantId` — enqueue-time guard rejects + payloads missing `tenant_id` or `correlation_id`. +- `Integration_Event_Handler_Restores_Tenant_Context` — handler scope has + `ITenantContext.IsResolved == true` before the inner pipeline runs. + +The three identifiers above are catalogued (assertion, type, source) in +[21-architecture-tests-catalogue.md § Cross-cutting: error handling, logging, observability](../standards/21-architecture-tests-catalogue.md); +the catalogue is the canonical reference, this list is a Phase 02b +shipping checklist. ## Deliverables diff --git a/docs/standards/02-backend-coding.md b/docs/standards/02-backend-coding.md index 3d0b396..1e90361 100644 --- a/docs/standards/02-backend-coding.md +++ b/docs/standards/02-backend-coding.md @@ -143,15 +143,60 @@ Rules: ## Pipeline Behaviors -Standard MediatR pipeline (in order): - -1. Logging / tracing / correlation propagation. -2. Validation (FluentValidation). -3. Tenant context check. -4. Authorization. -5. Transaction. -6. Outbox flush. -7. Handler. +Standard MediatR pipeline (in order; outermost first, innermost last). Bound by +[ADR-0032 § Sub-decision 2](../decisions/0032-exception-handling-logging-and-observability.md) +and consistent with [ADR-0016 § Pipeline behavior order](../decisions/0016-audit-log-subsystem.md): + +1. **`ValidationBehavior`** — FluentValidation. Invalid input → returns + `Result.Fail(validation_failed, errors)`; never throws + `ValidationException`. Short-circuits the request before any DB / audit / + business code runs. +2. **`LoggingBehavior`** — Opens the `ILogger.BeginScope` carrying the eight + correlation fields ([10-observability.md § Correlation](10-observability.md)), + starts the manual `.` `Activity`, and measures handler + latency for the histogram metric. +3. **`AuditLogBehavior`** — Per [ADR-0016](../decisions/0016-audit-log-subsystem.md), + wraps the inner pipeline with `try / catch`. On exception, writes a + failure-class audit entry and rethrows via `ExceptionDispatchInfo` to + preserve the original stack. On success, reads `IAuditStateCapture` and + writes the success entry. Failure of `IAuditStore` itself is logged but + never blocks the business operation. +4. **`TenantContextBehavior`** — Asserts `ITenantContext.IsResolved` (the + `TenantResolverMiddleware` populated it from the inbound HTTP request, + the Hangfire `JobActivator` populated it from the job payload, or the + integration-event handler scope populated it from the event envelope); + sets the `app.tenant_id` and `app.organization_id` PostgreSQL session + variables via the `DbConnectionInterceptor` so RLS sees the right values. +5. **`AuthorizationBehavior`** — `IAuthorizationService.AuthorizeAsync` + against the command's resource. Denial returns + `Result.Fail(forbidden)`; no exception. +6. **`TransactionBehavior`** — Opens a `DbContext.Database` transaction (UoW). + Commits on a success-`Result`; rolls back on a fail-`Result` or any + exception that bubbles through. No transaction for forbidden or + validation-failed requests because those short-circuit upstream. +7. **`OutboxFlushBehavior`** — Per + [15-event-and-outbox.md](../architecture/15-event-and-outbox.md), enrols + `IOutbox` messages in the current transaction; they ship via + `DaprEventBus` on commit. +8. **Handler** — domain logic; returns `Result`. **No** `throw new + DomainException` for expected business-rule violations — use + `Result.Fail(business_rule_violation, ...)`. The + `LearnStackException-DomainExceptionThrow` Roslyn analyzer + ([ADR-0032 § Sub-decision 4](../decisions/0032-exception-handling-logging-and-observability.md)) + flags violations. + +The pipeline does **not** include a separate `ExceptionHandlingBehavior`. +`AuditLogBehavior`'s catch-and-rethrow + the L1 `IExceptionHandler` +([ADR-0032 § Sub-decision 1](../decisions/0032-exception-handling-logging-and-observability.md)) +together cover every exception path; a third behavior would duplicate the +responsibility. + +Architecture test +[`MediatR_Pipeline_Order_Matches_Canonical_Sequence`](21-architecture-tests-catalogue.md#mediatr_pipeline_order_matches_canonical_sequence) +asserts the DI registration order at startup; the test fails the build if +any behavior is missing, reordered, or duplicated. The catalogue entry in +[21-architecture-tests-catalogue.md](21-architecture-tests-catalogue.md) is +the canonical reference for this identifier. ## Time diff --git a/docs/standards/09-error-handling.md b/docs/standards/09-error-handling.md index 54243bb..0a8e3d8 100644 --- a/docs/standards/09-error-handling.md +++ b/docs/standards/09-error-handling.md @@ -1,20 +1,24 @@ # 09 — Error Handling Standards **Status:** Active -**Derives from:** [ADR 0002 — Initial Architecture](../decisions/0002-initial-architecture.md) (Problem Details + Result\ baseline), [04-api-design.md](04-api-design.md) § Error Responses. +**Derives from:** [ADR 0002 — Initial Architecture](../decisions/0002-initial-architecture.md) (Problem Details + Result\ baseline), [ADR 0032 — Exception Handling, Logging, and Observability Architecture](../decisions/0032-exception-handling-logging-and-observability.md) (implementation patterns), [04-api-design.md](04-api-design.md) § Error Responses. How LearnStack represents, propagates, surfaces, and recovers from failures. +The conceptual deep dive and diagrams live in +[33-cross-cutting-concerns.md](../architecture/33-cross-cutting-concerns.md). +This standard contains the day-to-day rules. + ## Two-Track Model ```mermaid flowchart LR bug[Bug / infra failure] --> exc[Exception thrown] expected[Expected outcome] --> result["Result returned"] - exc --> middleware[Global exception middleware] - result --> handler[Handler maps to ProblemDetails] - middleware --> problemDetails[Problem Details response] - handler --> problemDetails + exc --> l1[L1 IExceptionHandler] + result --> mapper["result.ToActionResult()"] + l1 --> problemDetails[Problem Details response] + mapper --> problemDetails problemDetails --> client[Client] ``` @@ -69,10 +73,15 @@ LearnStackException (base) │ ├── LiveClassProviderException │ ├── StorageProviderException │ └── ... -├── TenantContextMissingException (no tenant resolved where one is required) -└── UnreachableException (case branches that should be impossible) +└── TenantContextMissingException (no tenant resolved where one is required) ``` +For "case branches that should be impossible" use the BCL +`System.Diagnostics.UnreachableException` (.NET 7+) directly — adding a +custom subclass collides with the BCL name and forces every use site to +disambiguate with `using` aliases. The L1 `IExceptionHandler` treats it the +same as any unhandled exception (Sentry-captured, 500 Problem Details). + Rules: - Throw `LearnStackException` subclasses, never raw `Exception`. - Constructors take a structured `Error` plus the underlying cause. @@ -89,6 +98,58 @@ Rules: | `DomainException` | No | | `TenantContextMissingException` | No | +## L1 Exception Handler + +The first-line catch site is `LearnStackExceptionHandler : IExceptionHandler` +(.NET 8+) per +[ADR-0032 § Sub-decision 1](../decisions/0032-exception-handling-logging-and-observability.md). +Every backend host (`LearnStack.Api`, workers, background-service hosts) +registers it the same way: + +```csharp +services.AddExceptionHandler(); +services.AddProblemDetails(); +// in pipeline: +app.UseExceptionHandler(); +``` + +Responsibilities of the handler: + +- Map every `LearnStackException` subclass to its standard `Error.Code` and + HTTP status (the table under § Result Type). +- Build the RFC 7807 Problem Details body with `correlationId` set from + `Activity.Current.TraceId`. +- Call `Activity.Current.RecordException(ex) + SetStatus(Error, ...)` so + Tempo sees the failure. +- Dispatch to `IErrorTrackingProvider.CaptureAsync` **only when** + `ShouldCapture(ex)` returns true (see § Sentry vs OpenTelemetry boundary). + +The older `app.UseExceptionHandler(lambda)` and `app.Use((ctx, next) => { try +{...} catch {...} })` patterns are not used in new code. There is no +`ExceptionHandlingBehavior` inside the MediatR pipeline — +`AuditLogBehavior`'s catch + rethrow + the L1 handler are the only two +catch sites below the framework. + +## Sentry vs OpenTelemetry — Error Capture Boundary + +Per +[ADR-0032 § Sub-decision 7](../decisions/0032-exception-handling-logging-and-observability.md), +the two backends receive complementary signals: + +| Failure | OTel span | `IErrorTrackingProvider` | Rationale | +|---------|-----------|--------------------------|-----------| +| Unhandled `Exception` at L1 | `RecordException` + `SetStatus(Error)` | **Capture** | Bug or infra; high-signal | +| `LearnStackException` subclass at L1 | `RecordException` + `SetStatus(Error)` | **Capture** | Leaked from a failing layer | +| `ProviderException` with `IsClientError == false` (5xx upstream) | `RecordException` + `SetStatus(Error)` | **Capture** | Upstream infra failure | +| `ProviderException` with `IsClientError == true` (4xx upstream) | `SetStatus(Error)` only | **Skip** | Provider's user-error; not our bug | +| `Result.Fail(validation_failed / forbidden / not_found / ...)` | `SetStatus(Ok)` (runtime completed; HTTP response is the appropriate 4xx Problem Details) | **Skip** | Expected outcome; metric counter only | +| `Result.Fail(business_rule_violation)` | `SetStatus(Ok)` | **Skip** | Expected outcome; metric counter only | +| `OperationCanceledException` (client disconnect) | leave span `Unset`; **no** `RecordException` | **Skip** | Noise; not actionable. (`ActivityStatusCode` is `Unset / Ok / Error` — Unset is the right default for "we didn't finish but it wasn't a failure".) | + +The boundary is the L1 handler's `ShouldCapture(Exception)` switch. Modules +never reference `Sentry.SentrySdk` directly — the architecture test +`Modules_Do_Not_Reference_Sentry_SDK_Directly` enforces it. + ## API Surface All API errors are **RFC 7807 Problem Details**: @@ -123,6 +184,61 @@ Rules: - Always include all failures, not just the first one. - Field names match the request shape (`camelCase`). - Messages are localizable; the API returns the locale-appropriate message based on the request's `Accept-Language` or tenant default. +- **`ValidationBehavior` returns `Result.Fail(validation_failed, errors)` — + it does NOT throw `FluentValidation.ValidationException`.** Per + [ADR-0032 § Sub-decision 3](../decisions/0032-exception-handling-logging-and-observability.md), + the pipeline never raises a validation exception. The behavior aggregates + `ValidationResult` failures into the `Error.Details` dictionary and + short-circuits the request. The behavior's generic constraint is + `where TResponse : IResultBase`; the static factory + `Result.FailFor(error)` constructs the correct shape. + +## Domain Exceptions + +Per +[ADR-0032 § Sub-decision 4](../decisions/0032-exception-handling-logging-and-observability.md), +`DomainException` is reserved for **programmer errors** (bugs): + +- Aggregate invariant violations that signal a programming mistake (e.g. a + domain method was called in an impossible order, an aggregate's invariant + was bypassed). +- Anything where "raising this exception means we have a bug to fix". + +**Expected business-rule violations** return +`Result.Fail(business_rule_violation, ...)` from the domain method — they are +not exceptions. Examples that **must** be `Result.Fail`, not throws: + +- "Course capacity reached." +- "Tenant plan limit exceeded." +- "Cannot enrol learner: enrolment closed." +- "Cannot publish course: missing required lesson." + +Enforcement: + +- The Roslyn analyzer `LearnStackException-DomainExceptionThrow` flags every + `throw new DomainException(...)` outside aggregate invariant guards as a + Warning (Phase 02a) and as an Error after Phase 03 exit. +- Architecture test `Domain_Methods_Do_Not_Throw_For_Expected_Cases` walks + `Result`-returning methods and asserts the analyzer's report is empty. + +## Controller Mapping — `Result` → `IActionResult` + +Per +[ADR-0032 § Sub-decision 6](../decisions/0032-exception-handling-logging-and-observability.md), +the sanctioned shape is an explicit extension method: + +```csharp +[HttpPost("courses")] +public async Task Create( + CreateCourseCommand command, CancellationToken ct) + => (await _mediator.Send(command, ct)).ToActionResult(); +``` + +`ResultExtensions.ToActionResult()` lives in `LearnStack.Api.Common`. It +matches on `Error.Code` and emits the Problem Details body with the right +HTTP status (per the table in § Result Type). There is no action filter, no +MediatR `ResultUnwrapBehavior`, no implicit conversion — the explicit pattern +keeps the diff honest and the debug experience straightforward. ## Frontend Error Handling @@ -161,10 +277,64 @@ The SDK maps Problem Details payloads to `AppError`; UI code switches on `code`. ## Provider Failures - Wrap every provider call with `ProviderException` mapping at the adapter boundary. -- Translate provider-specific status codes to our normalized codes. +- Translate provider-specific status codes to our normalized codes; set + `ProviderException.IsClientError` based on the upstream status (`true` for + 4xx, `false` for 5xx). The L1 handler uses this flag to decide whether to + Sentry-capture (5xx) or not (4xx). - Don't leak provider names to end users (`detail: "Recording could not be started. Please try again."` not `"LiveKit returned 503"`). - Capture provider raw response to logs (with redaction) for debugging. +### Provider Resilience — Polly v8 ResiliencePipeline + +Per +[ADR-0032 § Sub-decision 5](../decisions/0032-exception-handling-logging-and-observability.md), +every provider adapter is wrapped with a Polly v8 `ResiliencePipeline` via +the `IProviderResilience` decorator pattern. The composition root +wires every adapter: + +```csharp +services.AddProviderResilience("liveclass"); +services.AddProviderResilience("payment"); +services.AddProviderResilience("storage"); +// ... +``` + +The decorator reads `Resilience::` from `appsettings.{env}.json` +and builds a pipeline with: + +- **Retry** — exponential backoff with jitter; only retries + `ProviderException` with `IsClientError == false` and `InfrastructureException` + (transient). +- **Circuit breaker** — opens on `failureRatio` over `samplingDuration`; + shields the upstream from sustained pressure. +- **Timeout** — bounds the longest single attempt. +- **Bulkhead** — caps concurrent in-flight calls per upstream. + +The adapter's only exception-related job is **SDK → ProviderException +translation**. The decorator is the only place retry / circuit breaker / +timeout live. The +[add-provider-adapter](../../.claude/skills/add-provider-adapter/SKILL.md) +skill walks the canonical shape for every new adapter. + +Configuration shape (excerpt): + +```jsonc +{ + "Resilience": { + "liveclass": { + "retry": { "maxAttempts": 3, "delaySeconds": 1, "useJitter": true }, + "circuitBreaker": { "failureRatio": 0.5, "samplingDurationSeconds": 30, "minimumThroughput": 10, "breakDurationSeconds": 30 }, + "timeout": { "totalSeconds": 10 } + } + } +} +``` + +Architecture test `Adapters_Wrap_Provider_Exceptions` asserts that SDK +exception types (`LiveKit.NET.LiveKitException`, `Stripe.StripeException`, +`Meilisearch.MeilisearchApiError`, …) never leave the +`LearnStack.Infrastructure.` namespaces. + ## Background Jobs - Jobs retry on `InfrastructureException` and `ProviderException` (5xx). @@ -195,3 +365,22 @@ The SDK maps Problem Details payloads to `AppError`; UI code switches on `code`. - Including stack traces or query text in Problem Details. - `Result` with `IsSuccess = true` but `Value = null` (use a Maybe / Option or throw at boundary). - Localizing error codes (codes are stable English identifiers; only `title` and `detail` are localized). +- Throwing `DomainException` for expected business-rule violations — use + `Result.Fail(business_rule_violation, ...)` instead. The Roslyn analyzer + flags violations. +- Throwing `FluentValidation.ValidationException` from the + `ValidationBehavior`. The behavior returns `Result.Fail(validation_failed)`. +- Importing `Sentry.SentrySdk` from any module assembly. Capture happens + centrally via `IErrorTrackingProvider`; the L1 `IExceptionHandler` is the + only sanctioned caller in application code. +- Adding an `ExceptionHandlingBehavior` to the MediatR pipeline. The + `AuditLogBehavior` + L1 `IExceptionHandler` cover the two needed catch + sites; a third behavior would duplicate the responsibility. +- Importing a provider SDK exception type (`LiveKit.NET.LiveKitException`, + `Stripe.StripeException`, …) outside the adapter's + `LearnStack.Infrastructure.` namespace. + +The architecture tests and Roslyn analyzers that enforce the rules above +are listed in +[21-architecture-tests-catalogue.md § Cross-cutting: error handling, logging, observability](21-architecture-tests-catalogue.md); +that catalogue is the canonical reference for every identifier. diff --git a/docs/standards/10-observability.md b/docs/standards/10-observability.md index 8289cd7..db5bc8a 100644 --- a/docs/standards/10-observability.md +++ b/docs/standards/10-observability.md @@ -1,20 +1,44 @@ # 10 — Observability Standards **Status:** Active -**Derives from:** [ADR 0002 — Initial Architecture](../decisions/0002-initial-architecture.md), [ADR 0006 — Events and Outbox](../decisions/0006-events-and-outbox.md) (outbox + tenant-context propagation across async boundaries). +**Derives from:** [ADR 0002 — Initial Architecture](../decisions/0002-initial-architecture.md), [ADR 0006 — Events and Outbox](../decisions/0006-events-and-outbox.md) (outbox + tenant-context propagation across async boundaries), [ADR 0032 — Exception Handling, Logging, and Observability Architecture](../decisions/0032-exception-handling-logging-and-observability.md) (Sentry / OTel boundary, Serilog wiring, span attribute propagation, deployment-mode branching). Three signals — logs, traces, metrics — bound by a single correlation id. Everything we ship is observable from day one. +Conceptual deep dive (logging pipeline, tracing pipeline, span enrichment +seam, correlation across async boundaries) lives in +[33-cross-cutting-concerns.md](../architecture/33-cross-cutting-concerns.md). +This standard contains the day-to-day rules. + ## Stack -- **OpenTelemetry** SDK for traces + metrics + (eventually) logs. -- **Serilog** for structured logs in .NET, exported via OTLP to the collector. +- **OpenTelemetry** SDK for traces + metrics. +- **Serilog** as the primary logger; modules log via `ILogger` + (`Microsoft.Extensions.Logging`), the Serilog implementation is wired once + at the composition root. Logs flow Serilog → OTLP sink → OTel Collector → + log backend. The OTel `LoggerProvider` (`AddOpenTelemetry().WithLogging()`) + is **not** registered alongside; double-export would duplicate every line. + Per [ADR-0032 § Sub-decision 8](../decisions/0032-exception-handling-logging-and-observability.md). - **OTel Collector** as the ingestion layer; forwards to backends. - **Backends:** - Traces → Tempo / Grafana Cloud Tempo / Jaeger. - Metrics → Prometheus / Grafana Mimir. - Logs → Loki / Elastic / equivalent. - - Errors → Sentry. + - Errors → Sentry — accessed exclusively through `IErrorTrackingProvider` + (per + [ADR-0032 § Sub-decision 9](../decisions/0032-exception-handling-logging-and-observability.md)); + composition root selects the implementation by `DeploymentMode`: + + | `DeploymentMode` | `IErrorTrackingProvider` | + |---|---| + | `Development` | `NoOpErrorTracker` | + | `SaaS` | `SentryErrorTracker` (DSN via `ISecretProvider`) | + | `Dedicated` | `SentryErrorTracker` (per-tenant DSN allowed via Hub config) | + | `SelfHostedOnline` | `SentryErrorTracker` (optional; `NoOpErrorTracker` if DSN absent) | + | `SelfHostedAirGapped` | `LocalFileErrorTracker` (writes JSON to `/var/learnstack/errors/`) | + + Modules never import `Sentry.SentrySdk` directly — the architecture test + `Modules_Do_Not_Reference_Sentry_SDK_Directly` enforces it. ## Correlation @@ -26,6 +50,7 @@ Every request, job, event handler carries: | `span_id` | OTel | Per operation | | `correlation_id` | Per request | Stable across retries; equals trace id at request boundary | | `tenant_id` | Resolved tenant | Always present where applicable | +| `organization_id` | Resolved organization | Present where the resource is `[OrganizationScoped]` (per [ADR-0017](../decisions/0017-tenant-organization-hierarchy.md)); nullable for tenant-wide resources | | `user_id` | Authenticated user | Present where applicable | | `module` | Logical module | `education`, `classroom`, etc. | | `request_path` | HTTP route template | Not the raw URL | @@ -35,10 +60,25 @@ These propagate into: - HTTP server middleware - HTTP clients (typed clients via `IHttpClientFactory`) - MediatR pipeline behaviors -- Background jobs (Hangfire activator + filter) -- Outbox dispatcher +- Background jobs (Hangfire activator + filter) — payload carries `tenant_id` + + `correlation_id`; activator restores `ITenantContext` before invocation +- Outbox dispatcher — row schema carries `tenant_id`, `organization_id?`, + `correlation_id`, `event_id`, `occurred_at`, `type`; consumer handler + restores `ITenantContext` from the envelope and starts an `Activity` with + `traceparent` set to the row's `correlation_id` - Integration event handlers - Provider SDK calls (where the SDK exposes a hook) +- **Hub HTTPS contract surface (`/api/internal/*`)** — `HubCorrelationMiddleware` + respects inbound `traceparent`; tenant context is read from the request + envelope's `tenantId` field after HMAC verification. Outbound calls + (LearnStack → Hub `POST /api/v1/internal/license/verify`, `POST + /api/v1/usage/report`) inject the current `traceparent`. Per + [ADR-0032 § Sub-decision 11](../decisions/0032-exception-handling-logging-and-observability.md). + +The propagation primitive is **W3C `traceparent`** end to end. Every +cross-boundary write (outbox row, Hangfire payload, Hub envelope) carries it; +the receiving side resumes the trace by setting `Activity.ParentId = +traceparent`. ## Logging @@ -108,10 +148,23 @@ Manual spans: ### Span Attributes -Required: -- `tenant.id`, `user.id`, `module`, `correlation_id`. - -Common: +Required (auto-enriched by `TenantContextSpanProcessor`): +- `tenant.id`, `organization.id`, `user.id`, `module`, `correlation.id`. + +The `TenantContextSpanProcessor : BaseProcessor` is registered +once at the composition root (per +[ADR-0032 § Sub-decision 10](../decisions/0032-exception-handling-logging-and-observability.md)); +its `OnStart` hook reads from the singleton `ITenantContextAccessor` +(AsyncLocal-backed) and tags every span — including spans produced by +auto-instrumentation libraries (EF Core, HttpClient, Valkey via Dapr, +SeaweedFS S3 SDK, LiveKit) — without per-call enrichment. The accessor is +populated at scope start by `TenantResolverMiddleware` (HTTP), +`HubCorrelationMiddleware` (`/api/internal/*`), Hangfire `JobActivator` +(background jobs), and the outbox / inbox handler scope (integration +events). Modules never call `Activity.Current?.SetTag("tenant.id", ...)` +themselves. + +Common (set by the respective auto-instrumentation library): - `http.method`, `http.route`, `http.status_code`. - `db.system`, `db.operation`, `db.table`. - `messaging.system`, `messaging.destination`, `messaging.message_id`. @@ -122,6 +175,19 @@ Forbidden attributes: - Request bodies. - Tokens or secrets. +### Error span semantics + +Per +[ADR-0032 § Sub-decision 7](../decisions/0032-exception-handling-logging-and-observability.md) +(see also +[09-error-handling.md § Sentry vs OpenTelemetry — Error Capture Boundary](09-error-handling.md)), +the L1 `IExceptionHandler` calls `Activity.Current.RecordException` and +`SetStatus(Error, ...)` on every unhandled exception. `Result.Fail` is **not** +an error span — the HTTP response is still a structured outcome (the Problem +Details with the correct 4xx status), so `SetStatus(Ok)` is the right value. +Putting `SetStatus(Error)` on every refused request would make the trace +backend treat business rejections as system failures. + ### Sampling - Tail-based sampling at the collector. @@ -162,10 +228,19 @@ In addition to system metrics, business KPIs: ## Errors -- All `Error`+ events flow to Sentry with full context (trace id, tenant id, user id, request path). -- Sentry events tagged with `tenant_id` to allow per-tenant inspection. -- PII redaction applied before Sentry receives the event. +- All `Error`+ events flow to `IErrorTrackingProvider` (Sentry in + SaaS / Dedicated / SelfHostedOnline; `LocalFileErrorTracker` in + air-gapped; `NoOpErrorTracker` in Development) with full context (trace id, + tenant id, organization id, user id, request path). +- Provider events are tagged with `tenant_id` (and `organization_id` where + applicable) to allow per-tenant inspection. +- PII redaction applied before the provider receives the event. - Sentry release tags match the deployed git sha. +- The capture boundary — which exceptions go to the provider and which only + to OTel — is in + [09-error-handling.md § Sentry vs OpenTelemetry — Error Capture Boundary](09-error-handling.md). +- Modules never reference `Sentry.SentrySdk` directly. Architecture test + `Modules_Do_Not_Reference_Sentry_SDK_Directly` enforces it. ## Frontend Observability @@ -219,3 +294,17 @@ Alerts route via PagerDuty / Opsgenie; warn-level alerts go to Slack. - Adding cardinality-explosion labels (e.g. user id as a metric label). - Custom log formatters that bypass redaction. - Backend metric names without the `learnstack_` prefix. +- Registering the OpenTelemetry `LoggerProvider` + (`AddOpenTelemetry().WithLogging()`) alongside the Serilog OTLP sink. + Logs go through Serilog only; the OTel logger seam stays unused. +- Importing `Serilog.ILogger` from any module assembly. Modules use + `ILogger` from `Microsoft.Extensions.Logging`. Architecture test + `Logging_Goes_Through_Microsoft_Extensions_Logging` enforces this. +- Per-call `Activity.Current?.SetTag("tenant.id", ...)` enrichment from + module code. The `TenantContextSpanProcessor` does this centrally. +- Importing `Sentry.SentrySdk` from any module assembly. Use + `IErrorTrackingProvider`. + +The architecture tests that enforce the rules above are listed in +[21-architecture-tests-catalogue.md § Cross-cutting: error handling, logging, observability](21-architecture-tests-catalogue.md); +that catalogue is the canonical reference for every identifier. diff --git a/docs/standards/12-infrastructure.md b/docs/standards/12-infrastructure.md index efa5173..c002d21 100644 --- a/docs/standards/12-infrastructure.md +++ b/docs/standards/12-infrastructure.md @@ -54,7 +54,7 @@ adapter table. Shipped in Phase 01 packets 1-6 (`infra/compose/dev.yml`): -``` +```text postgres # PostgreSQL 18.x per ADR-0031 valkey # Linux-Foundation BSD-3 fork of Redis 7.2.4 per ADR-0030 seaweedfs # single dev binary: master + volume + filer + S3 gateway per ADR-0029 @@ -73,7 +73,7 @@ apisix # gateway in file-driven standalone (data_plane) mode Deferred — added by a later phase, not in the Phase 01 stack: -``` +```text livekit-egress # Phase 08c (recording / consent / cost model) otel-collector # Phase 11 (Production hardening — observability stack) ``` diff --git a/docs/standards/20-infrastructure-stack.md b/docs/standards/20-infrastructure-stack.md index 3975515..e9bd254 100644 --- a/docs/standards/20-infrastructure-stack.md +++ b/docs/standards/20-infrastructure-stack.md @@ -57,6 +57,8 @@ Rules: | Entitlement | `NullEntitlementProvider` | `HubEntitlementProvider` | `HubEntitlementProvider` | `HubEntitlementProvider` (phone-home) | `SignedLicenseKeyEntitlementProvider` | | Host → tenant | Config / single tenant | Hub-mirrored projection | Hub-mirrored projection | Hub-mirrored projection | Config / `.lic` claim | | Phone-home | n/a | enabled | enabled | enabled (daily, 30-day grace) | disabled | +| Error tracking ([ADR-0032](../decisions/0032-exception-handling-logging-and-observability.md)) | `NoOpErrorTracker` | `SentryErrorTracker` | `SentryErrorTracker` | `SentryErrorTracker` (optional; `NoOp` if no DSN) | `LocalFileErrorTracker` | +| OTLP exporter target ([ADR-0032](../decisions/0032-exception-handling-logging-and-observability.md)) | local OTel Collector (dev compose) | central Collector | central Collector | customer-managed Collector | local file `/var/learnstack/otel/` | ## Dapr Building Blocks @@ -253,6 +255,9 @@ Full deep dive: [15-event-and-outbox.md](../architecture/15-event-and-outbox.md) - Direct `IConnectionMultiplexer` / `IDistributedCache` injection. - Direct `KafkaProducer` / `ConsumerBuilder` / Confluent.Kafka usage. - Direct `VaultClient` / Vault HTTP API calls. +- Direct `Sentry.SentrySdk` usage — capture happens via + `IErrorTrackingProvider` per + [ADR-0032](../decisions/0032-exception-handling-logging-and-observability.md). - Reading `DeploymentMode` from inside a module. - Calling Hub endpoints from anywhere except the dedicated `IEntitlementProvider` / `IUsageReporter` / `IHubTenantSync` adapters. @@ -269,8 +274,10 @@ Full deep dive: [15-event-and-outbox.md](../architecture/15-event-and-outbox.md) - [ADR-0019 LearnStack Hub](../decisions/0019-learnstack-hub.md) - [ADR-0020 Triple Deployment + Hybrid License](../decisions/0020-triple-deployment-hybrid-license.md) - [ADR-0021 Feature-Based Entitlement](../decisions/0021-feature-based-entitlement.md) +- [ADR-0032 Exception Handling, Logging, and Observability Architecture](../decisions/0032-exception-handling-logging-and-observability.md) - [29-dapr-integration.md](../architecture/29-dapr-integration.md) - [30-api-gateway.md](../architecture/30-api-gateway.md) +- [33-cross-cutting-concerns.md](../architecture/33-cross-cutting-concerns.md) - [24-learnstack-hub.md](../architecture/24-learnstack-hub.md) - [25-deployment-models.md](../architecture/25-deployment-models.md) - [12-infrastructure.md](12-infrastructure.md) — operational rules (CI/CD, DB ops, diff --git a/docs/standards/21-architecture-tests-catalogue.md b/docs/standards/21-architecture-tests-catalogue.md new file mode 100644 index 0000000..039c3c8 --- /dev/null +++ b/docs/standards/21-architecture-tests-catalogue.md @@ -0,0 +1,264 @@ +# 21 — Architecture Tests + Analyzers Catalogue + +**Status:** Active +**Derives from:** [ADR-0032 Exception Handling, Logging, and Observability Architecture](../decisions/0032-exception-handling-logging-and-observability.md) +(ships the first batch of catalogue entries). The catalogue grows as +subsequent ADRs and phases land their tests; per-test ownership stays with +the originating ADR / standard. + +The single source of truth for the **identifier**, the **assertion**, the +**source ADR / standard**, and the **scope** of every non-skippable rule +LearnStack enforces at build time — whether the rule lives in the +`LearnStack.Tests.Architecture` assembly (xUnit / NetArchTest) or in a +compile-time Roslyn analyzer under `backend/analyzers/`. + +## Why a catalogue + +Identifier names propagate across ADRs, standards, roadmap deliverables, +glossary entries, and SKILL.md files. A rename or relocation forces an edit +to every cross-link site. Centralising the registry keeps **one** name +canonical; other documents cite the catalogue entry by anchor link +(`21-architecture-tests-catalogue.md#`) so the next rename touches +exactly one line. + +The catalogue is **not** a substitute for the originating ADR / standard — +the rule still lives there. The catalogue only owns the **name**, the +**short assertion**, and the **pointer back**. + +## How to add an entry + +When a new test or analyzer lands: + +1. Pick a name. Convention: `Subject_Constraint` + (e.g. `Modules_Do_Not_Reference_DeploymentMode`). Don't bake an ADR + number into the identifier (architecture tests are read by humans years + after the ADR is superseded; the test name should age well). Cite the + ADR in the test's `[Description]` / `[FactDescription]` attribute, not + in the type name. +2. Add a row to the right section table below. +3. Cite the catalogue entry from the originating doc: + `[name](../standards/21-architecture-tests-catalogue.md#name-lowercased-with-dashes)`. + +When a test is renamed: + +1. Edit the catalogue row first. +2. `git grep` the old name across `docs/`, `.claude/`, `CLAUDE.md` and + replace; the count should be small because everything points back here. +3. Update the test code last. + +When a test is retired: + +1. Move the row to the "Retired" section at the bottom with a one-line note + on why and which commit. +2. Leave the anchor in place so old links don't 404; the row's body says + "retired — see " or "obsolete — replaced by …". + +## Naming convention + +| Convention | Example | +|---|---| +| Architecture test class / fact | `Subject_Constraint`: `Modules_Do_Not_Reference_DeploymentMode`, `Every_TenantOwned_Command_HasAuditCoverage`, `MediatR_Pipeline_Order_Matches_Canonical_Sequence` | +| Roslyn analyzer ID | `LearnStackException-`: `LearnStackException-DomainExceptionThrow` | + +The two namespaces are disjoint by prefix — a Roslyn analyzer's diagnostic +ID never collides with an architecture test fact name. + +## Catalogue + +### Cross-cutting: error handling, logging, observability + +Source: [ADR-0032](../decisions/0032-exception-handling-logging-and-observability.md). +Ships in [Phase 02a](../roadmap/phase-02a-kernel-tenancy.md) (unless noted +otherwise). + +#### `IExceptionHandler_Registered_AtStartup` + +- **Asserts:** every backend host registers a single + `IExceptionHandler` implementation (`LearnStackExceptionHandler`); the + legacy `app.UseExceptionHandler(lambda)` and inline `app.Use((ctx, next) + => {...})` patterns are absent. +- **Source:** ADR-0032 § Sub-decision 1. +- **Type:** xUnit + service-collection inspection. +- **Phase:** 02a. + +#### `MediatR_Pipeline_Order_Matches_Canonical_Sequence` + +- **Asserts:** the MediatR DI registration order at startup is exactly + `Validation → Logging → AuditLog → TenantContext → Authorization → + Transaction → OutboxFlush → Handler`. No `ExceptionHandlingBehavior` is + registered; no extra behaviors are inserted between the eight canonical + steps. +- **Source:** ADR-0032 § Sub-decision 2; + [02-backend-coding.md § Pipeline Behaviors](02-backend-coding.md). +- **Type:** xUnit + reflection over `IServiceCollection`. +- **Phase:** 02a. + +#### `ValidationBehavior_DoesNotThrow_ValidationException` + +- **Asserts:** triggering a validation failure end-to-end through the + MediatR pipeline produces a `Result.Fail(validation_failed, errors)` + outcome; a `FluentValidation.ValidationException` never escapes the + behavior into the handler scope or up to L1. +- **Source:** ADR-0032 § Sub-decision 3. +- **Type:** integration test (Testcontainers). +- **Phase:** 02a. + +#### `Domain_Methods_Do_Not_Throw_For_Expected_Cases` + +- **Asserts:** the Roslyn analyzer `LearnStackException-DomainExceptionThrow` + produces zero Warnings inside `Domain` + `Application` projects of every + module. Walks `Result`-returning methods and asserts the analyzer + report is empty for the module. +- **Source:** ADR-0032 § Sub-decision 4; + [09-error-handling.md § Domain Exceptions](09-error-handling.md). +- **Type:** xUnit + Roslyn analyzer report inspection. +- **Phase:** 02a (Warning); escalates to Error after Phase 03 exit. + +#### `LearnStackException-DomainExceptionThrow` (Roslyn analyzer) + +- **Diagnostic ID prefix:** `LearnStackException-DomainExceptionThrow`. +- **Asserts:** every `throw new DomainException(...)` outside aggregate + invariant guards is flagged. The analyzer ships in + `backend/analyzers/LearnStack.Analyzers` and is referenced by + `Domain` + `Application` projects via ``. +- **Severity:** Warning in Phase 02a; flipped to Error after the Phase 03 + exit gate is green across all modules. +- **Source:** ADR-0032 § Sub-decision 4; + [09-error-handling.md § Domain Exceptions](09-error-handling.md). +- **Phase:** 02a. + +#### `Adapters_Wrap_Provider_Exceptions` + +- **Asserts:** provider SDK exception types (`LiveKit.NET.LiveKitException`, + `Stripe.StripeException`, `Meilisearch.MeilisearchApiError`, + `SeaweedFS.S3Exception`, …) appear only inside + `LearnStack.Infrastructure.` namespaces. They never escape into + `Application`, `Domain`, or another adapter's namespace. +- **Source:** ADR-0032 § Sub-decision 5; + [09-error-handling.md § Provider Failures](09-error-handling.md). +- **Type:** xUnit + NetArchTest. +- **Phase:** 02a. + +#### `Modules_Do_Not_Reference_Sentry_SDK_Directly` + +- **Asserts:** no module assembly (`LearnStack.Modules.*.{Domain,Application,Infrastructure}`) + has a transitive dependency on `Sentry.*` packages. Only + `LearnStack.Infrastructure.ErrorTracking` may reference the Sentry SDK. +- **Source:** ADR-0032 § Sub-decision 9; + [09-error-handling.md § L1 Exception Handler](09-error-handling.md); + [20-infrastructure-stack.md § Forbidden](20-infrastructure-stack.md). +- **Type:** xUnit + assembly-dependency walk. +- **Phase:** 02a. + +#### `Logging_Goes_Through_Microsoft_Extensions_Logging` + +- **Asserts:** no module assembly imports `Serilog.ILogger` or + `Serilog.Log.*`. Module code logs through + `Microsoft.Extensions.Logging.ILogger` (injected); Serilog is the + implementation wired once at the composition root. +- **Source:** ADR-0032 § Sub-decision 8; + [10-observability.md § Stack](10-observability.md). +- **Type:** xUnit + NetArchTest. +- **Phase:** 02a. + +#### `OTel_Pipeline_Includes_TenantContextSpanProcessor` + +- **Asserts:** the registered OpenTelemetry tracing pipeline includes the + `TenantContextSpanProcessor`. Fails if a future composition-root edit + removes the processor. +- **Source:** ADR-0032 § Sub-decision 10. +- **Type:** xUnit + service-collection inspection of + `IOptions`. +- **Phase:** 02a. + +#### `TenantContextSpanProcessor_DoesNotThrow_When_Context_Missing` + +- **Asserts:** `TenantContextSpanProcessor.OnStart(activity)` does not + throw when `ITenantContextAccessor.Current` is `null` (warm-up + `Activity` instances created during startup, background tasks before + any scope populated the accessor). +- **Source:** ADR-0032 § Sub-decision 10. +- **Type:** xUnit unit test. +- **Phase:** 02a. + +#### `Outbox_Row_Carries_Correlation_Context` + +- **Asserts:** every persisted `outbox_messages` row has non-null + `tenant_id` and `correlation_id` columns. Integration test that writes + through `IOutbox.EnqueueAsync` and inspects the row. +- **Source:** ADR-0032 § Sub-decision 12; + [ADR-0006](../decisions/0006-events-and-outbox.md) Amendment 1. +- **Type:** integration test (Testcontainers). +- **Phase:** 02b. + +#### `Hangfire_Job_Payloads_Include_TenantId` + +- **Asserts:** Hangfire enqueue rejects job payloads missing `tenant_id` + or `correlation_id`. Per the `JobActivator` contract the enqueue path + fails at submission, not at activation, so the failure mode is loud. +- **Source:** ADR-0032 § Sub-decision 12; Phase 02b deliverable. +- **Type:** xUnit + Hangfire enqueue interceptor test. +- **Phase:** 02b. + +#### `Integration_Event_Handler_Restores_Tenant_Context` + +- **Asserts:** when an outbox consumer dispatches an integration event, + the inner handler scope has `ITenantContext.IsResolved == true` before + business code runs. Verifies the envelope-to-context restoration. +- **Source:** ADR-0032 § Sub-decision 12; Phase 02b deliverable. +- **Type:** integration test (Testcontainers + Dapr sidecar). +- **Phase:** 02b. + +### Earlier ADRs (to be backfilled) + +Existing architecture tests already cited from other ADRs / standards live +in their respective docs. They will be migrated into this catalogue in +follow-up PRs as their text is touched (no rewrite-for-rewrite churn); +until then the originating doc remains the single reference. Known +identifiers awaiting migration: + +- ADR-0003 / ADR-0017 — tenant + organization isolation: + `Every_TenantOwned_Command_HasAuditCoverage`, + `Every_OrgScoped_Entity_HasOrgIdAndFilter`. +- ADR-0014 — Dapr building blocks: + `Dapr_SDK_Types_NotImportedOutsideInfrastructure`, + `Modules_DoNotReference_DaprPackage`, + `ICacheService_Is_OnlyCacheAbstraction`, + `Dapr_PubSub_TopicNames_FollowConvention`. +- ADR-0016 — audit subsystem: + `AuditEntry_Inherits_Entity_Not_AuditableEntity`, + `AuditEntry_Is_AppendOnly`, + `AuditLogBehavior_NeverBlocks_BusinessWrites`, + `Modules_Do_Not_Write_AuditLog_Directly`, + `OperationType_Enum_Matches_Catalog`. +- ADR-0018 — domain-specific names forbidden: + `Core_Modules_HaveNo_DomainSpecific_Names`, + `No_Source_Folder_Named_Verticals`. +- ADR-0019 — Hub HTTPS contract: + `LearnStack_Modules_DoNotReference_Hub`. +- ADR-0020 — entitlement providers: + `IEntitlementProvider_Implementations_Are_Three`, + `NullEntitlementProvider_NotRegistered_OutsideDevelopment`, + `LicenseKey_Validation_Is_Pinned_RSA2048`. +- Standards 20 — composition-root + direct-injection bans: + `Modules_Do_Not_Reference_DeploymentMode`, + `Modules_Do_Not_Inject_Valkey_Directly`, + `Modules_Do_Not_Read_Entitlement_Cache_Directly`. + +The next PR that edits any of these source documents folds the +corresponding row in here. + +### Retired + +(none yet) + +## References + +- [ADR-0032 Exception Handling, Logging, and Observability Architecture](../decisions/0032-exception-handling-logging-and-observability.md) +- [02-backend-coding.md § Pipeline Behaviors](02-backend-coding.md) +- [09-error-handling.md](09-error-handling.md) +- [10-observability.md](10-observability.md) +- [20-infrastructure-stack.md](20-infrastructure-stack.md) +- [Phase 02a Roadmap § Architecture Tests](../roadmap/phase-02a-kernel-tenancy.md) +- [Phase 02b Roadmap § Architecture Tests](../roadmap/phase-02b-events-auth.md) +- [add-architecture-test skill](../../.claude/skills/add-architecture-test/SKILL.md) diff --git a/docs/standards/README.md b/docs/standards/README.md index 3a6b7b9..c72bf97 100644 --- a/docs/standards/README.md +++ b/docs/standards/README.md @@ -34,6 +34,7 @@ This directory contains the engineering rules that apply across the LearnStack c | 18 | [Audit Coverage Standards](18-audit-coverage.md) | Which operations must be audited; payload contract; retention; per-module classification matrix. | | 19 | [Permissions Standards](19-permissions.md) | `{module}.{resource}.{action}` naming, closed action set, registry pattern, matrix template, built-in roles. | | 20 | [Infrastructure Stack Standards](20-infrastructure-stack.md) | Dapr building blocks (`IEventBus`, `ICacheService`, `ISecretProvider`), APISIX gateway, Hub HTTPS contract surface, entitlement projection, outbox/inbox usage. | +| 21 | [Architecture Tests + Analyzers Catalogue](21-architecture-tests-catalogue.md) | Single source of truth for the identifier, assertion, and source ADR / standard of every non-skippable architecture test or Roslyn analyzer. Cross-link target so renames touch one place. | ## Status of Each Standard diff --git a/infra/coturn/turnserver.conf b/infra/coturn/turnserver.conf index cdeeeda..48e9f92 100644 --- a/infra/coturn/turnserver.conf +++ b/infra/coturn/turnserver.conf @@ -3,6 +3,18 @@ # clients behind symmetric NATs. Production uses a dedicated Coturn cluster # with TLS certs, ephemeral credentials, and dedicated relay IP ranges; this # file is dev-only. +# +# DEFERRED to Phase 08c (live classroom productionisation): +# - No `external-ip` is set here. Any developer testing real-NAT relay paths +# must run Coturn against the workstation's LAN IP — temporarily add +# `external-ip=` for that session. The production config will inject +# `external-ip` from the deployment manifest, not from this static file. +# - `rtc.turn_servers` is NOT wired in `infra/livekit/livekit.yaml`. Phase 08c +# adds the LiveKit-side block that points at the standalone Coturn cluster. +# - Per-session credentials. Today this file uses a static `user=devuser:devsecret` +# pair so a developer can poke at the TURN port with `turnutils_uclient` without +# bouncing through LiveKit. Phase 08c flips to `use-auth-secret` and routes +# credential minting through `ILiveClassProvider` (per-session, short-TTL). listening-port=3478 tls-listening-port=5349 diff --git a/infra/dapr/README.md b/infra/dapr/README.md index 7c4bdc6..81506da 100644 --- a/infra/dapr/README.md +++ b/infra/dapr/README.md @@ -7,9 +7,25 @@ non-goals. | Building block | Backend (dev) | Component file | Application interface | |----------------|---------------|----------------|-----------------------| -| Pub/Sub | Kafka (`kafka:9092`) | `components/pubsub-kafka.yaml` | `IEventBus` (Phase 02b) | -| State store | Valkey (`valkey:6379`, RESP protocol) | `components/statestore-redis.yaml` | `ICacheService` (Phase 02a) | -| Secret store | Vault (`http://vault:8200`, dev mode) | `components/secretstore-vault.yaml` | `ISecretProvider` (Phase 02a) | +| Pub/Sub | Kafka (`kafka:9092`) | `components/pubsub-kafka.yaml` | `IEventBus` | +| State store | Valkey (`valkey:6379`, RESP protocol) | `components/statestore-redis.yaml` | `ICacheService` | +| Secret store | Vault (`http://vault:8200`, dev mode) | `components/secretstore-vault.yaml` | `ISecretProvider` | + +Phase ownership (per [phase-02a](../../docs/roadmap/phase-02a-kernel-tenancy.md) +§ Shared Kernel and § Dapr Building Blocks, and +[phase-02b](../../docs/roadmap/phase-02b-events-auth.md)): + +- **Phase 02a** declares all three interfaces in `LearnStack.SharedKernel` + with default in-process implementations (`InProcessEventBus`, + `InMemoryCacheService`, `EnvironmentSecretProvider`) **and** ships the + Dapr-backed implementations (`DaprEventBus`, `DaprCacheService`, + `DaprSecretProvider`) in `LearnStack.Infrastructure`. The composition + root picks between in-process and Dapr-backed per `DeploymentMode`. +- **Phase 02b** wires the `OutboxProcessor`, which is the only sanctioned + caller of `IEventBus.PublishAsync` per ADR-0010 Amendment 1. Modules + therefore *consume* `ICacheService` and `ISecretProvider` from 02a but + do not *write* to `IEventBus` directly until the outbox path exists in + 02b. Service invocation, workflow, bindings, **actors**, configuration, and distributed lock are **not adopted**; if a future need appears the gate is a @@ -20,22 +36,28 @@ new ADR. The state store's `actorStateStore` flag is therefore pinned to Dev compose runs one sidecar bound to the `learnstack-api` app id: -```text -┌──────────────────────────┐ ┌──────────────────────────────────────────┐ -│ dotnet run │ │ daprd │ -│ → host:5080 │ │ ./daprd -app-id learnstack-api \ │ -│ │ │ -app-port 5080 \ │ -│ │◄──┤ -app-channel-address \ │ -│ │ │ host.docker.internal \ │ -│ │ │ -dapr-http-port 3500 \ │ -│ │ │ -dapr-grpc-port 50001 \ │ -│ │ │ -placement-host-address \ │ -│ │ │ dapr-placement:50005 \ │ -│ │ │ -resources-path /components \ │ -│ │ │ -config /config/dapr-config.yaml -└──────────────────────────┘ └──────────────────────────────────────────┘ +```mermaid +graph LR + app[".NET API
dotnet run → host:5080
(workstation, outside compose network)"] + daprd["daprd (compose service: dapr-sidecar-api)
-app-id learnstack-api
-app-port 5080
-app-channel-address host.docker.internal
-dapr-http-port 3500
-dapr-grpc-port 50001
-placement-host-address dapr-placement:50005
-resources-path /components
-config /config/dapr-config.yaml"] + daprd -- "inbound subscription delivery
via host.docker.internal:5080" --> app + app -- "outbound publish / get-secret / get-state
via localhost:3500 (HTTP) / :50001 (gRPC)" --> daprd ``` +Text fallback for non-Mermaid renderers: + +- **App** — `dotnet run` on the developer's workstation, listening on + `host:5080` (outside the compose network). +- **daprd** — compose service `dapr-sidecar-api`, run with flags: + `-app-id learnstack-api`, `-app-port 5080`, + `-app-channel-address host.docker.internal`, `-dapr-http-port 3500`, + `-dapr-grpc-port 50001`, `-placement-host-address dapr-placement:50005`, + `-resources-path /components`, `-config /config/dapr-config.yaml`. +- **daprd → app** — inbound subscription deliveries reach the workstation + via `host.docker.internal:5080`. +- **app → daprd** — outbound publish / state / secret calls hit + `localhost:3500` (HTTP) or `localhost:50001` (gRPC). + The .NET host runs **outside the container network** during active dev (developers `dotnet run` from their workstation). `-app-channel-address host.docker.internal` is what makes inbound subscription deliveries reach @@ -61,7 +83,8 @@ public interface ISecretProvider { Task GetSecretAsync(string key, Cance `DaprEventBus`, `DaprCacheService`, `DaprSecretProvider` are the **only** Dapr-aware types in the codebase; they live in `LearnStack.Infrastructure` -and ship in Phase 02b. Architecture tests +and ship in Phase 02a (composition-root selected per `DeploymentMode`). +Architecture tests `Dapr_SDK_Types_NotImportedOutsideInfrastructure`, `Modules_DoNotReference_DaprPackage`, and `ICacheService_Is_OnlyCacheAbstraction` keep this honest. @@ -94,7 +117,9 @@ both places together. ## What does NOT live here - The `IEventBus` / `ICacheService` / `ISecretProvider` implementations — - Phase 02b (`LearnStack.Infrastructure`). + the **interfaces + Dapr-backed adapters** both ship in **Phase 02a** + (see § Phase ownership above); only the *outbox dispatch path* that + becomes the sanctioned caller of `IEventBus.PublishAsync` is Phase 02b. - Outbox dispatcher (`OutboxProcessor` polling + dispatch) — Phase 02b. - Per-module `inbox_messages` table + `IInboxGuard` — Phase 02b. - Production Vault setup (HA mode, auto-unseal, AppRole policies) — Phase 11.