fix(api): deterministic correlationId fallback in audit factory#86
Open
christopherhouse wants to merge 1 commit into
Open
fix(api): deterministic correlationId fallback in audit factory#86christopherhouse wants to merge 1 commit into
christopherhouse wants to merge 1 commit into
Conversation
Spec 006 / FR-042 — every audit event needs a non-empty correlationId
to link back to OTel traces in App Insights. The factory was reading
Activity.Current?.TraceId.ToHexString() with a string.Empty fallback.
In production with the AspNetCore OTel instrumentation active (auto-
wired by UseAzureMonitor when APPLICATIONINSIGHTS_CONNECTION_STRING is
set), there is always an ambient Activity carrying a real trace id, so
this code path worked. In test mode (no AI connection string → no auto-
instrumentation) Activity.Current can race to null, leaving audit
events with correlationId = '' and tripping the
EndToEndScenariosTests.QuickstartScenarios_AllThreeUserStories_PassInOneRun
test's NotBeNullOrEmpty assertion every few CI runs. The same race
would emit empty correlationIds from any background-job path that
audits without an inbound HTTP request, breaking trace linkage in
production for those events too.
Fix: fall back to Guid.NewGuid().ToString('N') instead of string.Empty
when no ambient Activity is present. Audit events always have a
non-empty correlationId; real cross-system trace linkage continues to
work when an Activity IS present.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
EndToEndScenariosTests.QuickstartScenarios_AllThreeUserStories_PassInOneRunhas been intermittently failing on:Tripped CI on this branch (after #85 merge), and previously on PR #80. Genuine flaky test surfacing a real defect, not a test infrastructure problem.
Root cause
RegistryAuditFactory.cs:38was:In production with the AspNetCore OTel instrumentation auto-wired by
UseAzureMonitor(only kicks in whenAPPLICATIONINSIGHTS_CONNECTION_STRINGis set), the ambientActivityalways carries a real trace id. In tests there's no connection string, so the auto-instrumentation never wires up andActivity.Currentcan race to null in WebApplicationFactory request flows. Same race would hit any background-job path that audits without an inbound HTTP request — and silently break trace linkage in production for those events.Fix
Every audit event now has a non-empty correlationId. When an Activity IS present, real cross-system trace linkage continues to work unchanged.
Test plan
QuickstartScenarios_AllThreeUserStories_PassInOneRunon future CI runsRisk
None — additive fallback, semantics improve (no more empty correlationId), no schema change.
🤖 Generated with Claude Code