fix(etl): keep connector lifecycle diagnostics out of logs - #176
fix(etl): keep connector lifecycle diagnostics out of logs#176seonghobae wants to merge 5 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough대상 커넥터의 정리 및 종료 실패 로그에서 예외 객체 출력을 제거했습니다. 예외 전파와 suppressed 예외 처리는 유지했습니다. 로그 기밀성, 수명주기 동작, 문서 정책을 검증하는 테스트와 문서를 추가했습니다. Changes대상 커넥터 진단 기밀성
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
✅ Action performedReview finished.
|
Purpose
Fix #175 test-first. Protected
develop@622e5e6c3d534f230c390f10e3832efadfc01825serialized third-party target-connector exception objects to ordinary logs when open cleanup or shutdown close failed. Provider exceptions can contain endpoints, tokens, request fragments, storage paths, account/workspace identifiers, or other deployment-sensitive diagnostics.This remains a direct-
develop, bounded Draft. It changes the target-connector lifecycle logging boundary, focused regression tests, and source-local security doctoring only. Nothing here isimplemented_on_developbefore protected integration.Exact current identity
develop@622e5e6c3d534f230c390f10e3832efadfc01825;fix/target-connector-safe-logs-622e5e6;6f9feda67deb8c8c8d9373ce863fd0190051537b;5f2c92004d8ede691fe866055450fb3a3ff77833;f1461b03a70752f56e9113ef5ea02c5f41f7fcce;cc20453800e596f7982dc3984764aeb765733c73;3bc42f17b1a21850a646bebebf0001516f54cbdb;88268208321d1bc1c1df1f727c47abc1c4c03f63;Every check/review from a predecessor head or base is historical and does not transfer.
RCA
TargetConnectorDispatchercalledlog.warn(..., cleanupFailure)andlog.error(..., exception).TDD / remediation evidence
RED 1 — provider diagnostics reached ordinary logs
Fail-first
6f9feda67deb8c8c8d9373ce863fd0190051537badded the lifecycle-log assertions before production changed. Hosted CI reached the intended ETL logging boundary and proved that close/open-cleanup failures exported credential-bearing provider diagnostics and exception stack traces. The tests also proved the original failed-open exception retained cleanup causality through suppressed state and that shutdown remained best-effort.GREEN 1 — bounded lifecycle classifications
The smallest production correction removes exception-object transport from the two ordinary log calls while preserving connector ID, lifecycle classification, suppressed-exception causality, cleanup ordering, close continuation, open-state bookkeeping, and exception propagation. No regex masking or logging-framework weakening is introduced.
RED 2 — missing source-backed doctoring
Predecessor
f1461b03a70752f56e9113ef5ea02c5f41f7fcceadded a permanent documentation contract requiringdocs/doctoring/target-connector-diagnostic-confidentiality.mdwith CWE-532, OWASP Logging Cheat Sheet, suppressed-exception, connector-ID, connection-string, purpose-bound-observability and APA 7 evidence. CI reached that exact contract after the behavior tests were green and failed because the doctoring artifact was absent.GREEN 2 — exact current head
cc20453800e596f7982dc3984764aeb765733c73added the source-local doctoring. Its first hosted CI run isolated one literal documentation-contract mismatch: the prose rendered the OWASP title with Markdown betweenOWASPandLogging Cheat Sheet, so the required canonical name did not occur contiguously. Production behavior and the logging regressions remained green.Exact current head
3bc42f17b1a21850a646bebebf0001516f54cbdbchanges only that wording to the canonical literalOWASP Logging Cheat Sheet. The documentation now records data minimization / purpose-bound observability rather than blanket masking, retained causal suppressed exceptions, finite connector-ID metadata, avoidance of regex-only masking, and APA 7 references to CWE-532 and current OWASP logging guidance.Current workflow evidence
All exposed pull-request workflow aggregates for exact current source head
3bc42f17b1a21850a646bebebf0001516f54cbdbare terminal-success:31350291551: success;31350291554: success;31350291535: success;31350291550: success;31350291537: success.This is not merge authorization. The protected pull-request workflows execute GitHub's synthetic merge preview rather than accepted literal-source execution, the shared protected JaCoCo control remains vacuous until #162/#164 or equivalent integration, and the hard scanner-completeness gap tracked by #196 remains a separate acceptance property. A green aggregate cannot change the revision or dependency graph actually examined.
Scope / shared prerequisites
This PR does not alter connector registry/scaffold decisions in #153/#156/#158/#163/#203, CDC diagnostics #170–#174, database schema, connector payloads, target capability architecture #202, or exception propagation contracts. Shared #160/#164, literal-source controls, scanner completeness, and review governance remain separate controls.
Merge boundary
Keep Draft. Merge only after the unchanged exact source head has accepted literal-source deterministic/security evidence, complete same-revision vulnerability/dependency evidence, non-vacuous owned-production coverage, zero valid unresolved review findings, every required repository/security gate, and qualifying independent non-author approval where governance requires it. No predecessor-head, synthetic-merge, status-only, incomplete-scanner, skipped-required, or other-PR evidence transfers.
Summary by CodeRabbit