Skip to content

fix(observability): mark failed operation spans as Error - #106

Merged
seonghobae merged 12 commits into
mainfrom
fix/operation-span-error-status
Aug 11, 2026
Merged

seonghobae merged 12 commits into
mainfrom
fix/operation-span-error-status

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

RCA

Protected-main OpenTelemetryBatchAPIClient deliberately 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 spans Error with 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

  • RED 910ce25e2f0bfcad11e0ae175a1c9ef8e9279d4a established the missing failure-span Error status. CI 31386926658 failed exactly that expectation on Python 3.10/3.12/3.14.
  • The status implementation is lazy, optional, description-free and fail-open; status-construction or mutation failures cannot mask the application exception.
  • RED 0b967937d2bcd4a491c83d1c9e7cd560383119f3 added a context whose __enter__() raises and records subsequent exit calls. CI 31412639286 failed on Python 3.10/3.12/3.14 and coverage: test_failed_span_entry_is_not_exited_after_provider_success observed one invalid __exit__ call instead of zero.
  • 1a5f1bc6bd8668258e1281407b35864459a6d8c7 implemented 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.
  • 74562a779d9c453bc53698c6d4bb4155afd877d1 added the provider-failure regression, proving telemetry entry failure neither causes exit nor replaces the exact provider exception.
  • Current d15cbff533189cb6cf0d2827220469573fb27d54 removes the now-unreachable None guard from _close_span_context; only proven-entered contexts reach that helper.
  • docs/doctoring/opentelemetry-operations.md retains the privacy/security trade-off and current OpenTelemetry primary sources in APA 7 form.

Current exact state

  • Source head: d15cbff533189cb6cf0d2827220469573fb27d54.
  • Independently resolved protected main: bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10.
  • GitHub reports Draft and mergeable.
  • CI 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.
  • Security Scan 31413268868: completed / success.
  • SAST Semgrep 31413270335: completed / success.
  • Formal reviews: none. Unresolved inline review threads: zero at the latest refetch.

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

  • 문서

    • OpenTelemetry 운영 문서를 독립 실행, 의존성 주입, 취소 전파 및 예외 처리 방식에 맞게 보완했습니다.
    • 오류 신호 계약, 개인정보 보호, 카디널리티 및 동시성 제한을 구체화했습니다.
  • 버그 수정

    • 작업 실패 시 span에 오류 상태를 기록하되 민감한 예외 정보는 노출하지 않습니다.
    • 텔레메트리 처리 실패가 원래 작업의 예외를 가리지 않도록 개선했습니다.
    • span 컨텍스트 진입에 실패한 경우 잘못된 종료 처리를 방지했습니다.
  • 테스트

    • 성공·실패 작업과 텔레메트리 API 오류 상황에 대한 회귀 테스트를 추가했습니다.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7ffb9ab5-3cc1-4197-81dd-469fc79e902d

📥 Commits

Reviewing files that changed from the base of the PR and between aea5a9d and ad78220.

📒 Files selected for processing (2)
  • pg_llm_batch/observability.py
  • tests/test_opentelemetry_lifecycle_safety.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • pg_llm_batch/observability.py

📝 Walkthrough

Walkthrough

OpenTelemetry 운영 계약을 갱신했습니다. 오류 발생 시 span에 Status(ERROR)를 설정하고, 성공 시 상태를 변경하지 않습니다. 상태 API 또는 span 진입 실패가 원래 provider 결과와 예외를 변경하지 않는지 회귀 테스트로 검증합니다.

Changes

OpenTelemetry 오류 상태 처리

Layer / File(s) Summary
운영 계약 및 검증 기준
docs/doctoring/opentelemetry-operations.md
SDK 소유권, 의존성 주입, 오류 상태와 유형, 민감 정보 제외, 중첩 작업 억제 및 품질 검증 기준을 문서화했습니다.
Span 상태 및 컨텍스트 수명 구현
pg_llm_batch/observability.py
Status(ERROR) 지연 로더를 추가했습니다. span 진입 성공 여부를 추적하고, 성공적으로 진입한 context만 종료합니다. telemetry 오류는 원래 작업 예외를 대체하지 않습니다.
오류 상태 및 수명 회귀 테스트
tests/test_opentelemetry_span_status.py, tests/test_opentelemetry_lifecycle_safety.py
실패 작업의 오류 상태와 비민감 속성, 성공 작업의 상태 미변경, 상태 API 실패, 선택적 API 부재 및 span 진입 실패를 검증합니다.

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만 종료
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 실패한 작업의 span을 Error 상태로 표시하는 주요 변경 사항을 정확하고 간결하게 설명합니다.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/operation-span-error-status

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@seonghobae
seonghobae marked this pull request as ready for review August 11, 2026 22:16
@seonghobae
seonghobae merged commit 2028352 into main Aug 11, 2026
34 checks passed
@seonghobae
seonghobae deleted the fix/operation-span-error-status branch August 11, 2026 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant