feat(app): structured logs at domain state transitions#228
Merged
Conversation
- 14 money-path handlers now narrate their transitions: daily entry
record/submit/adjust/void, FIFO allocation (confirm), sale void, order
cancel, payment record/void, feed/water usage, purchase, inventory
adjustment, bird movement. Information on success with stable ids
(DailyEntryId, FlockId, SalesOrderId, PaymentId, ...), Warning with the
failure reason on every failed Result via one ResultLogging extension
({Operation} failed: {ErrorCode} — {ErrorDescription}).
- AccountId on every in-request event (spec §10): the tenant middleware
opens a logging scope beside the existing completion-event enrichment,
so handler lines inherit the account without per-call noise.
- Serilog MEL bridge pinned with preserveStaticLogger: the bridge bound
logger categories through the process-global Log.Logger, so any
co-hosted Serilog app's shutdown (Log.CloseAndFlush -> SilentLogger)
permanently silenced every category created afterwards. Third bug of
this static-fallback family (after options.Logger and DiagnosticContext
in #226); found because handler events vanished only in multi-host test
runs.
- New package: Microsoft.Extensions.Logging.Abstractions in Application;
lock files synced.
4 integration tests drive real HTTP flows through the sink tap: submit
carries ids + account scope, confirm carries the order id, insufficient
stock warns with ErrorCode, payment carries order + payment ids. Suite
734 green, integration run twice.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // ...and on every event logged INSIDE the request (#216): handler | ||
| // transition logs inherit it via FromLogContext. Disposed with the | ||
| // request so the AsyncLocal scope can't bleed past it. | ||
| return logger.BeginScope(new Dictionary<string, object> { ["AccountId"] = accountId }); |
- AdjustDailyEntry's UnknownGrade exit was the single failed-Result path across all 14 handlers that bypassed LogFailure (codex + agent; verified by a statement-level scan of every touched handler). - DailyEntryLockSweep now narrates each lock per entry AFTER the save (DailyEntryId, FlockId, EntryDate, AccountId) — the 'lock' transition is a #216 state transition too, and logging before commit would narrate locks that never happened. Its failure path adopts the canonical DailyEntryId/ErrorCode/ErrorDescription shape (was EntryId/Error). - Insufficient-stock test correlates through a GUID-suffixed grade name instead of fixed wording; middleware comment no longer claims FromLogContext serves MEL scopes. Rejected with evidence (details on the PR): pi's outcome-NRE quartet (every transaction-lambda exit sets outcome before returning false; independently traced by the agent reviewer), pi's Set+BeginScope redundancy (completion event bypasses MEL scopes), cavecrew's ErrorDescription-PII (flock/grade names are operator-authored farm data; grep shows no customer fields in any Error.Description) and date-name drift (each date property names its domain concept; ids are the join keys). Suite 734 green, integration run twice.
Owner
Author
Review round (repo policy: codex + 2 Claude agents + pi)Accepted → fixed in f97898b:
Rejected with evidence:
Reviewer verification highlights (agent): every exit in all 14 handlers traced — no double-logging; zero static Suite after fixes: 734 green, integration run twice, build clean. |
| foreach (var entry in due.Where(e => e.Status == DailyEntryStatus.Locked)) | ||
| logger.LogInformation( | ||
| "Daily entry {DailyEntryId} locked for flock {FlockId} on {EntryDate} (account {AccountId})", | ||
| entry.Id, entry.FlockId, entry.Date, accountId); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #216. Third telemetry slice, on top of #226 + #227.
What
DailyEntryId,FlockId,SalesOrderId,PaymentId,FeedUsageId, …) — property names consistent across handlers so one log query works across features.Resultthrough a singleResultLoggingextension:{Operation} failed: {ErrorCode} — {ErrorDescription}—ErrorCodeis the stable query key.AccountIdon every in-request event (tech spec §10): the tenant middleware opens a logging scope beside the feat(api): activate the telemetry pipeline — OTLP export, request logs, TraceId correlation #226 completion-event enrichment, so handler lines inherit the account with zero per-call noise. Scope disposed with the request.Root-cause find: the third static-logger bug
Handler events vanished — but only in multi-host test runs, and only for logger categories created after another host shut down. The Serilog MEL bridge binds categories through the process-global
Log.Logger; any co-hosted Serilog app's dispose flips that toSilentLogger(viaLog.CloseAndFlush), permanently silencing every category created afterwards (probe showedIsEnabled = falseacross all levels). Fixed withpreserveStaticLogger: true— this host's pipeline never binds through nor stomps the static. Same family as #226'soptions.LoggerandDiagnosticContextpinnings; production is single-host so the latent risk was shutdown-window only, but the test suite made it deterministic.Tests
4 integration tests drive real HTTP flows and assert through the DI sink tap: submit carries ids + account scope; confirm carries the order id; insufficient-stock confirm warns with
EggLot.InsufficientStock; payment carries order + payment ids. Asserts correlate by entity id, so the shared sink can't cross-contaminate.Suite: 734 green, integration run twice, build clean. New package:
Microsoft.Extensions.Logging.Abstractions(Application layer), lock files synced.