[Backend] Telemetry-fidelity log-call family: S6667 (83) + S2139 (187) need per-site triage with a rubric, not bulk treatment #639
Assignees
spikerj (Joseph Spiker)
Clear assignees
spikerj (Joseph Spiker)
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: spikerj/spikersoft-issues#639
Reference in New Issue
Block a user
From the SonarQube triage loop (2026-07-17). Two related rule families concern exception logging fidelity — 270 findings total. Sampled sites show a genuine mix, so neither bulk-fix nor bulk-FP is correct:
Verified sample verdicts:
TracingBehavior.cs:97— cancellation path is DELIBERATELY quiet (in-code comment: exception-tagged spans were flooding Seq's Error stream / Jaeger failed-span rates) and the exception is rethrown right after. FP for both rules; the pattern is documented in-code.EbookArtifactMirror.cs:131— OperationCanceledException catch logging without the exception: idiomatic (cancellations carry no useful stack). FP-lean.ResidentArtPipeStageExecutor.cs:198— JsonException swallowed, only the raw line logged; passing the exception would add parse-position diagnostics. Real, trivial fix.QuizController.cs:254— expected business rejection (#507) logging only ex.Message. Borderline; passing ex preserves type fidelity at zero cost.Proposed rubric for the batch (one focused PR per project or two): pass the caught exception to the logger UNLESS (a) it is rethrown immediately (S2139's own guidance — then don't log at error level at all, or keep the deliberate-quiet pattern), (b) it's a cancellation-path where quietness is deliberate (comment it), or (c) an in-code comment documents deliberate message-only logging for an expected business condition. Sites failing the rubric get the exception added; sites passing it get FP'd in SonarQube with the reason.
Distribution (S6667): Business 11, CodeExecution 10, Api 9, GameServer.Initializer 6, ArtPipe 5, UploadCoordinator 5, rest ≤3. Best worked AFTER the fresh #632 baseline lands so counts/lines are true to master.
Distribution data for planning (still against the stale 07-14 analysis): S2139 = 187 findings across 66 files — a long tail, not a hotspot; densest is GamePersistenceService (13), the Mongo-primary/Redis-cache dual-write path where log-then-rethrow is plausibly deliberate crash-with-context. Attempted spot-verification hit line drift (the flagged coordinates no longer match current sources — these files changed since 07-14), which is the final confirmation that this batch must wait for the post-#632 rescan before any per-site work. No action taken on drifted findings.
Batch 1 done against the fresh 2026-07-17 baseline: 47 of 81 S6667 findings were the canonical cancellation-quietness idiom (rubric clause b) — every site's actual catch clause was programmatically verified against source before transitioning (catch(OperationCanceledException) + Information-level shutdown log; one site even carries an in-code 'this is normal when stopping' comment). All 47 FP'd in SonarQube with the rubric reasoning. Remaining 34 S6667s need individual reads — they include the genuine fix candidates (swallowed JsonException etc.). S2139 (187→ re-baselined count TBD) still queued behind those.
S6667 half fully resolved: PR #348 merged (16/16 checks green) — the six genuinely-lossy sites now pass the exception (3× artpipe JsonException, 2× Redis-admin broad catches, PingCommandHandler's message-as-template bug). Family ledger: 81 findings → 75 FP'd with per-site reasons, 6 fixed. S2139 half awaits PR #349 (CI queued): 188 → 177 FP'd across five verified shapes, 11 double-report fixes in the PR. Ticket closes when #349 lands.
S2139 half resolved: PR #349 merged (19/19 checks green) — the 11 handler-level double-reports removed on the proven TracingBehavior evidence. Final family ledger across both rules (270 findings): S6667 81 → 75 FP'd / 6 fixed (PR #348, merged); S2139 188 → 177 FP'd across five verified shapes / 11 fixed (PR #349, merged). Every FP carries its per-site reason in SonarQube; the fixed findings auto-close on the next master scan. Closing.