fix(logging): stage Reply SLA telemetry successor on canonical owner #1486 - #1710
seonghobae wants to merge 59 commits into
Conversation
…ror에 exc_info=True 사용)
…ror에 exc_info=True 사용)
…ror에 core.safe_logging.redacted_exception_info(e) 사용)
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Reply SLA scheduler now logs redacted exception details at two failure points. New tests verify removal of secrets, paths, tracebacks, and owner identifiers. Documentation records the telemetry migration and retained fields. ChangesReply SLA exception telemetry
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The scheduler’s exception telemetry change has no identified merge-blocking issue. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review Please review exact |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base='2d6800aea110ac04eeedcd5faa91479f62877408'
head='eee00bda624c58795a8adb17a5b2b734fd20d0a1'
printf '%s\n' '=== exact range and changed files ==='
git diff --stat "$base" "$head"
git diff --name-status "$base" "$head"
printf '%s\n' '=== scheduler diff ==='
git diff --find-renames --find-copies "$base" "$head" -- backend/services/reply_sla_scheduler.py
printf '%s\n' '=== test diff ==='
git diff --find-renames --find-copies "$base" "$head" -- backend/tests/test_reply_sla_scheduler_exception_telemetry.py
printf '%s\n' '=== doctoring diff ==='
git diff --find-renames --find-copies "$base" "$head" -- docs/doctoring/reply-sla-exception-telemetry.md
printf '%s\n' '=== scheduler outline ==='
ast-grep outline backend/services/reply_sla_scheduler.py --items all || true
printf '%s\n' '=== test outline ==='
ast-grep outline backend/tests/test_reply_sla_scheduler_exception_telemetry.py --items all || true
printf '%s\n' '=== redaction helper definitions and uses ==='
rg -n -C 4 --glob '*.py' '\bdef\s+redacted_exception_info\s*\(|\bredacted_exception_info\s*\(' backend core 2>/dev/null || true
printf '%s\n' '=== exception logging in scheduler ==='
rg -n -C 7 --glob '*.py' '(logger|logging)\.[a-z_]+\(' backend/services/reply_sla_scheduler.py
printf '%s\n' '=== scheduler source ==='
cat -n backend/services/reply_sla_scheduler.py
printf '%s\n' '=== telemetry test source ==='
cat -n backend/tests/test_reply_sla_scheduler_exception_telemetry.py
printf '%s\n' '=== helper source candidates ==='
fd -t f -e py . | xargs -r rg -l 'def redacted_exception_info' | xargs -r -n1 sh -c 'echo "=== $0 ==="; cat -n "$0"'Length of output: 38774 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='2d6800aea110ac04eeedcd5faa91479f62877408'
head='eee00bda624c58795a8adb17a5b2b734fd20d0a1'
printf '%s\n' '=== commit topology ==='
git merge-base "$base" "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'base is ancestor of head'
git rev-list --count "$base..$head"
git diff --check "$base" "$head"
git show --no-patch --format='head=%H%nparents=%P%nsubject=%s' "$head"
printf '%s\n' '=== scheduler test candidates ==='
fd -t f -e py . backend/tests | xargs -r rg -n -l 'ReplySlaScheduler|_try_acquire_sweep_lease|_release_sweep_lease|_sweep_configured_owners' || true
printf '%s\n' '=== scheduler test definitions and assertions ==='
fd -t f -e py . backend/tests | xargs -r rg -n -C 5 'async def test_.*(scheduler|sweep|lease|owner)|def test_.*(scheduler|sweep|lease|owner)|ReplySlaScheduler|_try_acquire_sweep_lease|_release_sweep_lease' || true
printf '%s\n' '=== static exception sink inventory in scheduler at head ==='
git show "$head:backend/services/reply_sla_scheduler.py" | rg -n -C 4 'except (Exception|BaseException)|exc_info'
printf '%s\n' '=== direct changes against parent commit ==='
git show --format=fuller --find-renames --find-copies --stat "$head"
printf '%s\n' '=== safe logging formatter tests ==='
cat -n backend/tests/test_safe_logging.py
printf '%s\n' '=== exception boundary formatter tests ==='
cat -n backend/tests/test_exception_logging_boundaries.pyLength of output: 81543
I found no blocking issue in this range.
I did not execute repository code or tests. I did not use parent review or check evidence as evidence for this head. ✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
seonghobae
changed the title
|
Evidence correction: CodeRabbit submitted |
1 participant
Refs #1698. Canonical Reply-SLA/Noema source owner: #1486.
Current authority
22c173bb93ef15c3244477fe32540aeb2bcf11992d6800aea110ac04eeedcd5faa91479f62877408eee00bda624c58795a8adb17a5b2b734fd20d0a1This PR was initially created directly on #1700 after the protected-
develop#1698 sink inventory identifiedbackend/services/reply_sla_scheduler.py. A fresh open-PR ownership sweep then found that #1486 is already the canonical broad Reply-SLA/Noema writer and currently modifies the same scheduler source and tests. The initial three-file claim is therefore not merge authority and must not become a parallel writer.The PR is now retargeted to #1486. The branch has not been force-rebased or destructively rewritten, and no merge commit has been manufactured that would discard either #1486 or #1700 valid deltas. Ordinary ancestry reconciliation is deferred until the prerequisite lineages can be combined without dropping source, tests, migrations, contracts, or evidence.
Valid finding to preserve
The protected-develop scheduler loop still uses ordinary
exc_info=True; Python logging renders the original exception value and traceback for that form. The first implementation attempt also covered the protected-develop per-owner sink, but #1486 has already independently changed that path to a bounded exception-type diagnostic and removed raw owner interpolation. Any eventual successor must start from #1486's live scheduler semantics rather than overwrite them.The valid remaining security requirement is:
exc_info=Truepath with fix(security): bound exception telemetry without raw traceback #1700's canonical bounded telemetry after that contract is available through valid ancestry;config.user_id;Preserved TDD provenance
The initial #1700-based lane has two ordinary commits that remain useful provenance but are not accepted owner integration:
c198326a006137b920fd34394f778ec237e8c3d0— formatter-level RED regressions for the protected-develop scheduler sinks.eee00bda624c58795a8adb17a5b2b734fd20d0a1— initial bounded-telemetry repair against the fix(security): bound exception telemetry without raw traceback #1700-era scheduler source.A focused exact-source harness reproduced secret-bearing exception/credential/path/owner-id disclosure before that repair and bounded output after it. That result is local/source-level evidence only and cannot be transferred to #1486's different current scheduler implementation.
Required ancestry order
Until then, no source delta in this PR is merge authority. Do not temporary-retarget to
develop, copy #1700 helper source, fabricate a two-parent tree that drops parent deltas, use dummy/no-op evidence commits, synthesize statuses, self-approve, force-push, destructively rebase, or weaken gates.