Skip to content

fix: add reader-safe OTel server diagnostics - #383

Merged
seonghobae merged 38 commits into
ContextualWisdomLab:mainfrom
seonghobae:feat/otel-session-diagnostics-main
Aug 25, 2026
Merged

fix: add reader-safe OTel server diagnostics#383
seonghobae merged 38 commits into
ContextualWisdomLab:mainfrom
seonghobae:feat/otel-session-diagnostics-main

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • bring the existing post-session OpenTelemetry/Valkey tracing contract onto current main
  • classify provider/transport/schema failures separately from unexpected server defects
  • keep Global Ask and post chat 503 responses reader-safe while retaining GRC-consumable telemetry
  • correlate Error spans with structured audit logs via W3C TraceId/SpanId; inject traceparent on GET and POST; export OTLP logs when an endpoint is set

Evidence

  • lineageweave.server.failures uses only bounded operation_code and failure_outcome metric labels
  • structured logs and trace events retain error class, TraceId, SpanId, and a bounded stack trace only for internal_error; exception values, prompts, responses, source bodies, credentials, actor IDs, and tenant IDs are excluded
  • Global Ask source-gather failures are classified as 503 (not uncaught 500)
  • tests/test_observability.py, tests/test_http_client.py, and tests/test_server_diagnostics.py drive the shipped configure/trace/failure-record/HTTP-propagation functions with an in-memory SDK exporter (no mocked TraceIds)

Closes #361
Related to #345; its OTel implementation was cherry-picked onto current main because #345 is merged into a non-main feature branch.

Summary by CodeRabbit

  • 새 기능
    • 선택적으로 OpenTelemetry 기반 메트릭, 추적 및 로그를 활성화할 수 있습니다.
    • 게시물 채팅, 전역 Ask 작업 및 백그라운드 작업의 진단 정보를 상관관계로 확인할 수 있습니다.
    • 민감한 콘텐츠와 자격 증명을 제외한 제한적 관측성을 제공합니다.
  • 버그 수정
    • 공급자 또는 내부 오류 시 안전한 안내와 HTTP 503 응답을 제공합니다.
    • 일시적인 작업 저장소 오류를 자동 재시도합니다.
    • 로그인 후 원래 페이지로 정확히 돌아가며, 인증 전 관리자 패널이 표시되지 않습니다.
  • 문서
    • 관측성 설정, 데이터 보호 및 운영 추적 방법을 문서화했습니다.

)

* feat: trace post processing and valkey sessions

* docs: align orchestrator runtime pin

* docs: codify telemetry boundary

* fix: normalize OTLP trace endpoint

* chore: pin orchestrator telemetry fix

* fix: keep provider details out of ingestion ledger

* fix: propagate W3C trace context to gateway
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

Next included review available in 13 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3b82b600-f1ac-4bac-9336-87d5d639fd52

📥 Commits

Reviewing files that changed from the base of the PR and between b7714c5 and 7ff6b94.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (36)
  • AGENTS.md
  • CHANGELOG.d/2.13.2-otel-server-diagnostics.md
  • CHANGELOG.md
  • backend/app/activity_stream.py
  • backend/app/analysis_run_outbox.py
  • backend/app/analysis_run_start.py
  • backend/app/analysis_run_worker.py
  • backend/app/auth.py
  • backend/app/global_ask_queue.py
  • backend/app/main.py
  • backend/app/post_content_queue.py
  • backend/app/post_content_worker.py
  • backend/tests/test_api.py
  • docker-compose.yml
  • docker/contextual-orchestrator/Dockerfile
  • docs/adr/0083-orchestrator-runtime-commit-pin.md
  • docs/adr/0122-otel-session-observability.md
  • docs/doctoring/OPENTELEMETRY_REFERENCES.md
  • lineageweave/caldav_client.py
  • lineageweave/embedding_client.py
  • lineageweave/http_client.py
  • lineageweave/observability.py
  • lineageweave/relation_verification.py
  • pyproject.toml
  • scripts/estimate_llm_channel_weights.py
  • scripts/seed_demo_data.py
  • scripts/smoke_test_oidc.py
  • tests/test_activity_stream.py
  • tests/test_analysis_run_worker.py
  • tests/test_caldav_client.py
  • tests/test_embedding_client.py
  • tests/test_http_client.py
  • tests/test_observability.py
  • tests/test_post_content_worker.py
  • tests/test_server_diagnostics.py
  • tests/test_tepp_client.py
📝 Walkthrough

Walkthrough

OpenTelemetry 관측성 경계와 선택적 OTLP 내보내기를 추가했습니다. HTTP와 Valkey 작업에 trace context를 연결했습니다. 서버 실패를 분류하고 진단 데이터와 일반화된 503 응답을 기록합니다. OIDC 로그인 반환 URL 처리도 변경했습니다.

Changes

OpenTelemetry 관측성과 서버 진단

Layer / File(s) Summary
관측성 계약과 런타임 설정
AGENTS.md, docs/adr/0122-otel-session-observability.md, docs/doctoring/OPENTELEMETRY_REFERENCES.md, docs/product-technical-gap-baseline.md, pyproject.toml, docker-compose.yml, docker/contextual-orchestrator/Dockerfile, docs/adr/0083-orchestrator-runtime-commit-pin.md, CHANGELOG.d/*
OpenTelemetry 데이터 경계, 세션 상관관계, OTLP 설정, 런타임 의존성, 서비스 환경 변수와 orchestrator 커밋 핀을 정의하고 구성합니다.
텔레메트리 런타임과 HTTP 전파
lineageweave/observability.py, lineageweave/http_client.py, backend/app/analysis_run_start.py, tests/test_observability.py, tests/test_http_client.py, tests/test_tepp_client.py
선택적 provider 설정, 제한된 속성 필터링, W3C trace context 주입, 실패 기록과 HTTP 서비스 피어 추적을 구현하고 검증합니다.
서버 장애 진단과 수명주기
backend/app/main.py, tests/test_server_diagnostics.py
애플리케이션 초기화와 종료에 텔레메트리 정리를 연결합니다. 게시물 채팅과 Global Ask의 공급자 오류 및 내부 오류를 분류하고 일반화된 503으로 변환합니다.
Valkey 작업 계측과 워커 복구
backend/app/activity_stream.py, backend/app/analysis_run_outbox.py, backend/app/analysis_run_worker.py, backend/app/post_content_queue.py, backend/app/post_content_worker.py, tests/test_activity_stream.py, tests/test_analysis_run_worker.py, tests/test_post_content_worker.py
Activity stream, analysis outbox와 post content의 XADD, XREAD, XREVRANGE 작업을 추적합니다. 유휴 폴링, 배치 처리, 오류 전파와 broker 재시도를 검증합니다.

OIDC 로그인 반환 URL

Layer / File(s) Summary
로그인 반환 URL 처리
frontend/src/App.tsx
로그인 리다이렉트 전에 반환 URL을 계산하고 저장한 뒤 auth.signinRedirect의 상태에 전달합니다. 비인증 화면의 AdminPanel 렌더링을 제거합니다.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: 🟠 High · up to 81f72

The PR adds reader-safe 503 handling and telemetry, but current paths can still misclassify local defects as provider outages and let source-gathering failures bypass the intended safe response and diagnostics. The change is not ready to merge until these production behavior issues and the required integration coverage are addressed or explicitly accepted.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning 로그인 반환 URL 저장과 비인증 화면의 AdminPanel 제거는 연결된 서버 진단 요구사항과 직접 관련이 없습니다. frontend/src/App.tsx 변경을 별도 PR로 분리하거나 연결된 이슈에서 해당 범위를 명시하세요.
Docstring Coverage ⚠️ Warning Docstring coverage is 51.97% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 127 functions across 18 files. (5 skipped: 5 unsupported.) Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed 일반화된 503 응답, 오류 분류, 구조화된 진단, 민감 정보 제외, 예외 연결 및 관련 테스트 요구사항을 충족합니다.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 reader-safe OTel 서버 진단 추가라는 주요 변경 사항을 명확하고 간결하게 요약합니다.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Reviewed exact current head c9e95e73; no additional patch is needed.

The worker redaction finding is already fixed at this head: _UNEXPECTED_FAILURE_DETAIL exists and unexpected failures persist only the fixed buyer-safe detail while the exception remains in server telemetry. The TEPP span-label finding is also fixed: post_json accepts service_peer_name and the TEPP transport passes tepp.

Exact-head validation:

  • post-content worker, HTTP client, and analysis-run-start tests: 28 passed
  • CodeGraph and diff checks: passed

Please rerun the required Checks and obtain protected formal approval for c9e95e73 before merge.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@claude Please review exact current head c9e95e7. Confirm buyer-safe post-content failure detail redaction and the distinct TEPP telemetry peer label through post_json service_peer_name. Publish an exact-head review; do not modify, self-approve, or merge.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Revalidated the remote Agent head and fixed the remaining contract edge: _bounded_session_id now uses str.isprintable() so the documented printable boundary also rejects DEL/control characters, and tests cover trimming, control rejection, length bounds, non-string input, and unknown telemetry keys.

Exact current head: b0c9bc64
Validation: focused observability/worker/server/TEPP tests 24 passed; activity, outbox, worker, HTTP, queue, normalization, persistence, and server tests 79 passed; compileall and diff checks passed.

@claude Please review exact current head b0c9bc6. Do not modify, self-approve, or merge.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Validation update for exact head 7200049: local observability, server diagnostics, HTTP transport, activity stream, analysis outbox/worker, and post-content worker tests pass (40 passed). Hosted security and product checks observed for this head are successful where completed; formal independent approval is still absent, so no merge was attempted.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@claude Please review exact current head 7200049 for actionable correctness, security, privacy, and operational regression findings. Do not approve or merge on my behalf.

@seonghobae
seonghobae enabled auto-merge August 21, 2026 15:25
devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head validation for 7a79cfa: restored the validated OIDC return-url helper in the diagnostics branch after the latest frontend-only commit had removed it. This keeps OpenTelemetry server diagnostics while preserving safe redirect validation and storage. Local frontend lint, 140 tests, TypeScript build, Vite build, backend diagnostics/observability/HTTP/worker targeted tests 43 passed, and diff check passed. Hosted Checks are recalculating; formal independent approval is absent, so no merge was attempted.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@claude Please review exact current head 7a79cfa for OIDC redirect safety together with server diagnostics privacy, failure classification, and operational regressions. Report actionable findings only; do not modify, self-approve, or merge.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head correction for e614946: the validated OIDC return-url helper was restored on the fork PR branch, preserving redirect validation and storage alongside server diagnostics. Local frontend lint, 140 tests, TypeScript build, Vite build, backend diagnostics/observability/HTTP/worker targeted tests 43 passed, and diff check passed. Hosted Checks are recalculating; formal independent approval is absent, so no merge was attempted.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@claude Please review exact current head e614946 for OIDC redirect safety together with server diagnostics privacy, failure classification, and operational regressions. Report actionable findings only; do not modify, self-approve, or merge.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae
seonghobae enabled auto-merge (squash) August 25, 2026 00:34
devin-ai-integration[bot]

This comment was marked as resolved.

@github-actions
github-actions Bot disabled auto-merge August 25, 2026 00:40
@seonghobae
seonghobae enabled auto-merge (squash) August 25, 2026 00:45
@opencode-agent
opencode-agent Bot disabled auto-merge August 25, 2026 01:06
The branch's diagnostic spans around the Valkey xread path now wrap
main's fail-closed per-run AnalysisRunCreateError refusal handling, so a
refused run is logged and skipped without ending the worker while the
batch keeps its traced span. Also drops conflict markers that an earlier
branch-side merge committed and unifies on _worker_logger.
@seonghobae
seonghobae enabled auto-merge August 25, 2026 01:55
devin-ai-integration[bot]

This comment was marked as resolved.

@github-actions
github-actions Bot disabled auto-merge August 25, 2026 02:01
@seonghobae
seonghobae enabled auto-merge August 25, 2026 02:27
@opencode-agent
opencode-agent Bot disabled auto-merge August 25, 2026 02:27
# Conflicts:
#	backend/app/main.py
#	lineageweave/http_client.py
#	tests/test_http_client.py
#	tests/test_post_content_worker.py
@seonghobae
seonghobae enabled auto-merge (squash) August 25, 2026 02:49
devin-ai-integration[bot]

This comment was marked as resolved.

@opencode-agent
opencode-agent Bot disabled auto-merge August 25, 2026 03:25
@seonghobae
seonghobae enabled auto-merge (squash) August 25, 2026 04:15
devin-ai-integration[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

Open in Devin Review

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Worker unavailable message still names ORCHESTRATOR_ env vars to the reader*

When the chat client is unavailable at processing time, the job settles with detail "...set ORCHESTRATOR_BASE_URL / ORCHESTRATOR_API_KEY", which read_ask_job returns verbatim to the reader. The PR made the /api/ask and post-chat endpoint messages reader-safe but left this worker-side _SafeJobError unchanged, so this path still exposes config-hint text. Pre-existing and outside the diff.

(Refers to this code)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread backend/app/main.py
Comment on lines +2843 to +2844
"Post chat is temporarily unavailable. "
"Saved evidence is still available.",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Post-chat message change leaves a test asserting the old text

The malformed/transport-failure branch now raises the 503 detail "Post chat is temporarily unavailable. Saved evidence is still available.", but test_post_chat_malformed_provider_reply_is_unavailable still asserts the old "no complete evidence object" text. The test raises TypeError, which this branch rewrites to the new message, so the assertion fails against a live stack. The sibling evaluate-endpoint test was updated for its message change; this one was not.

Prompt for agents
The post-chat endpoint's 503 detail was changed to "Post chat is temporarily unavailable. Saved evidence is still available." but backend/tests/test_api.py:3759 in test_post_chat_malformed_provider_reply_is_unavailable still asserts the old detail substring "no complete evidence object". Update that assertion to match the new reader-safe message (and consider asserting the raw exception text "provider message content is not a string" does not appear), consistent with how test_evaluate_is_unavailable_without_orchestrator was updated.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@seonghobae
seonghobae merged commit e837f00 into ContextualWisdomLab:main Aug 25, 2026
23 of 24 checks passed
seonghobae added a commit that referenced this pull request Aug 25, 2026
…484)

* feat: name Allen interval relations on Event Lineage edges (v2.15.0)

Persist interval_relation_code on post_lineage_edge after reconstruct
chooses a parent. Ticket-aware windows (created day through earliest
open due date) make the A-100 pricing follow-up Contain the revised
quote and Overlap the delivery question. Click Contains to open the
revised quote.

ADR 0122. Independent of leftover-map #481 and SKOS chips #482.

* fix: restore PR 481 CI contracts

* docs: remove duplicate Storybook inventory row

* fix: normalize interval dates to UTC

* fix: ground Event Lineage interval evidence

* fix: fail closed on missing interval evidence

* docs: align seeded interval claims with point evidence

* test: prove activity feed requires post read

* fix: resolve migration/ADR numbering collisions, drop dead i18n key

- Renumber migrations/0105_post_lineage_interval_relation.sql to 0140:
  0105 collides with the already-open PR #387's
  migrations/0105_post_lineage_edge_signal.sql on the same
  post_lineage_edge table. Update migrate.sh's whitelist,
  tests/test_schema.py, tests/test_migration_replay.py,
  backend/tests/test_api.py, and scripts/seed_demo_data.py to match.

- Renumber docs/adr/0122-event-lineage-interval-relation.md to 0160:
  0122 collides with the already-open PR #383's
  docs/adr/0122-otel-session-observability.md. Update every in-tree
  "ADR 0122" cross-reference (AGENTS.md, ARCHITECTURE.md, CLAUDE.md,
  CHANGELOG.d entry, the migration's SQL comment, and the seed script
  docstring) to ADR 0160, and note the renumber + collision in the
  ADR itself (see ContextualWisdomLab/.github#1249).

- Drop the unused "Click Contains to open the revised quote." i18n
  key from all four locale dictionaries and the i18n completeness
  test's required-keys list -- no component ever rendered it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B5M79L945DMyMs3sg5yJ14

* docs: reserve Event Lineage ADR 0161

* test(lineage): restore dropped persist_lineage_edges coverage

The prior merge of origin/main into this branch (daa9dd7) silently
dropped test_persist_requires_observed_points_before_replacing_edges
and its supporting imports (pytest, persist_lineage_edges,
reconstruct_group_key, records_from_source_posts, Edge) while
resolving the interleaved conflict in tests/test_lineage_ingestion.py.
That test predates this PR's own branch and is not present on
origin/main; restore it alongside the Allen-interval tests it was
merged next to.

* fix(lineage): compose interval evidence onto current main

* test(lineage): remove arbitrary reconstruction weights

* fix(migration): defer interval relation FK validation

---------

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: opencode-agent[bot] <219766164+opencode-agent[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Operability] Preserve server diagnostics behind generic orchestrator 503 responses

1 participant