fix(observability): mark failed operation spans as Error - #106
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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughOpenTelemetry 운영 계약을 갱신했습니다. 오류 발생 시 span에 ChangesOpenTelemetry 오류 상태 처리
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Provider
participant Observability
participant SpanContext
Provider->>Observability: 관찰된 작업 실행
Observability->>SpanContext: span context 진입
SpanContext-->>Observability: 진입 결과 반환
Provider-->>Observability: 결과 또는 원래 예외 반환
Observability->>SpanContext: 성공적으로 진입한 context만 종료
🚥 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.
|
RCA
Protected-main
OpenTelemetryBatchAPIClientdeliberately disables automatic exception recording and automatic status-on-exception so provider/caller exception messages, stack traces, bodies, identifiers, credentials, prompts, and dynamic exception names are not handed to telemetry. Propagated failures nevertheless need one bounded semantic error signal. This branch therefore marks failed spansErrorwith no description while keeping success status unset.A later lifecycle audit found a separate fail-open telemetry defect in the same owned surface:
_run_observed()swallowed a span context's__enter__()failure but retained the context object and later invoked__exit__(None, None, None)anyway. Calling__exit__on a context manager that never entered violates its lifecycle contract and can create secondary telemetry side effects after otherwise successful or failed provider work.The narrow remedy tracks whether context entry actually completed. Provider work still proceeds when telemetry entry fails, but cleanup is attempted only for a successfully entered context. The entry-success flag is separate from the returned span object, because a valid context manager may enter successfully while returning
None.Test-first proof and follow-up hardening
910ce25e2f0bfcad11e0ae175a1c9ef8e9279d4aestablished the missing failure-span Error status. CI31386926658failed exactly that expectation on Python 3.10/3.12/3.14.0b967937d2bcd4a491c83d1c9e7cd560383119f3added a context whose__enter__()raises and records subsequent exit calls. CI31412639286failed on Python 3.10/3.12/3.14 and coverage:test_failed_span_entry_is_not_exited_after_provider_successobserved one invalid__exit__call instead of zero.1a5f1bc6bd8668258e1281407b35864459a6d8c7implemented explicit entry-success tracking. Unit tests became green, while the exact 100% coverage gate correctly exposed an unproved provider-failure/failed-entry branch and an unreachable defensive guard.74562a779d9c453bc53698c6d4bb4155afd877d1added the provider-failure regression, proving telemetry entry failure neither causes exit nor replaces the exact provider exception.d15cbff533189cb6cf0d2827220469573fb27d54removes the now-unreachableNoneguard from_close_span_context; only proven-entered contexts reach that helper.docs/doctoring/opentelemetry-operations.mdretains the privacy/security trade-off and current OpenTelemetry primary sources in APA 7 form.Current exact state
d15cbff533189cb6cf0d2827220469573fb27d54.main:bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10.31413270341: completed / success. Python 3.10, 3.12 and 3.14 unit jobs, exact 100% coverage/docstrings/lint/package, lock freshness, Compose validation, and both container builds succeeded.31413268868: completed / success.31413270335: completed / success.The PR workflow is still governed by protected-main pre-#88 checkout semantics, so these are strong staged integration checks rather than final contributor-source-head acceptance.
Integration boundary
Keep Draft while repository exact-source governance #88 and its read-only central prerequisite remain outside protected main. Do not weaken or bypass that gate merely because staged PR checks are green. Once the exact-source contract is protected, regenerate required source-head CI/security/package/provenance/review evidence on the unchanged final head and merge only if live policy is genuinely satisfied.
This slice does not change provider transport, database state, checkpoint telemetry, credentials, schema, release authority, scheduler authority, or the active canonical documentation branch.
Summary by CodeRabbit
문서
버그 수정
테스트