fix(otel): preserve reader-safe queued diagnostics - #593
Closed
seonghobae wants to merge 31 commits into
Closed
Conversation
* 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
ADR 0119 retired "buyer" project-wide the day before this PR's ADR 0122 and tests/test_server_diagnostics.py introduced it again in a non-exempt new ADR and new source identifiers. Rename to "reader-safe" to keep `grep -ri buyer` clean outside the ADR 0119-exempted historical ADRs, CHANGELOG*, and fixture/test domain content. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B5M79L945DMyMs3sg5yJ14
Record classified Ask and post-chat failures inside the API span so structured logs share the same W3C TraceId and SpanId. Inject traceparent on GET as well as POST, export OTLP logs when an endpoint is set, and keep a blank Compose interpolation from wiping orchestrator env_file telemetry.
Keep backend.app.main and the HTTP client import-sorted and wrap the over-length lines in the classified Ask/chat and GET tracing paths so the diagnostics change is ruff-clean on the files it touches.
Assert the activity XADD span id differs from the parent so removing the traced() wrapper cannot pass by inheriting the parent span.
OIDC, Searxng, and CalDAV GETs now set a bounded service.peer.name instead of inheriting contextual-orchestrator. The post session header stays on orchestrator and TEPP only; W3C traceparent still propagates.
Semantic graft: this PR's diagnostics (traced span, classified record_server_failure, generic customer-facing 503s) onto main's evolved surfaces -- the Global Ask route keeps main's newer response shape (lineage_graph / cited_post_images and the 422 content constant), the worker keeps main's docstring plus this PR's spans, http_client keeps main's transport-error wrap alongside this PR's observability plumbing, and the gap baseline unions both audit sections. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015HgzNGsCxqiTaT4YuJEb5J
…d, bump version A peer session independently merged origin/main into this branch while I was mid-merge on the same PR (functionally identical resolution to mine, already pushed) -- rebased onto their tip rather than force-push a redundant duplicate. One genuine gap remained in both resolutions: CHANGELOG.d/2.13.2-otel- server-diagnostics.md existed in this PR from the start but was never promoted into CHANGELOG.md, and pyproject.toml/__init__.py/uv.lock never bumped past main's 2.12.18 -- same version-bump gap already fixed on #480/#482/#434/this-branch's-first-pass this session. Verified: full suite 940 passed, 16 skipped, zero failures.
…agnostics-main Resolves fresh conflicts in CHANGELOG.md (additive entry concatenation), pyproject.toml (version bump), and uv.lock (relock with OTel deps).
Merge conflicts, largest in backend/app/main.py: this PR's import section (predating a since-landed lineageweave.* client-import reorganization) had to be reconstructed against main's current layout rather than resolved line-by-line -- git's diff had fragmented the same origin/main hunk across two non-adjacent conflict markers, producing ~19 duplicate imports on a naive per-block resolution. Rebuilt the whole import section from main's copy plus this PR's one net-new import (lineageweave.observability), then re-verified there were zero duplicate module imports before touching logic. lifespan(): combined this PR's OTel-wrapped, defensive-shutdown structure with main's independently-added third worker (global_ask_worker) that didn't exist when this PR forked. ask_agent(): main's evolution (durable job queue, GET /api/ask/jobs) fully supersedes this PR's synchronous version; kept main's endpoints as-is. The OTel span + classified-failure diagnostics this PR wanted for Global Ask now belong on the actual provider-call site instead -- relocated into global_ask_queue.py's compute_global_ask_answer (traced() span + record_server_failure(), alongside the already-merged operability.py structured logging from #577, which serves a different observability channel and isn't redundant with it). Moved source gathering inside the traced/try block too so a source-assembly failure shares the Ask span instead of escaping unclassified, matching this PR's original intent for that failure mode. Rewrote tests/test_server_diagnostics.py's 4 Global Ask tests to call compute_global_ask_answer directly (the sync main.ask_agent they called no longer exists) and to match the classification scheme compute_global_ask_answer already had in production before this merge (HttpClientError/OSError -> provider_unavailable, KeyError/ValueError -> internal_error) rather than this PR's original one-tuple scheme, since that split is the tested, already-shipped contract elsewhere. post_content_worker.py: kept only the OTel record_server_failure classification for the worker's terminal exception handler, not a _logger.exception logging call for it too -- an already-existing, unconflicted test in this same file asserts the raw exception message must never appear in caplog even in server-side logs, which a bare _logger.exception call would have violated.
…s version) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015HgzNGsCxqiTaT4YuJEb5J
# Conflicts: # docs/product-technical-gap-baseline.md
seonghobae
enabled auto-merge (squash)
August 25, 2026 00:24
|
Warning Review limit reachedNext included review available in 10 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (32)
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 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| except RuntimeError as exc: | ||
| assert str(exc) == "provider failure" | ||
| else: # pragma: no cover | ||
| raise AssertionError("traced must preserve operation failures") |
| ) | ||
| trace.set_tracer_provider(provider) | ||
| _TRACE_PROVIDER = provider | ||
| _CONFIGURED = True |
| global _LOG_HANDLER, _FAILURE_COUNTER | ||
| if _LOG_HANDLER is not None: | ||
| _LOGGER.removeHandler(_LOG_HANDLER) | ||
| _LOG_HANDLER = None |
| "telemetry.provider_shutdown_failed provider=%s", | ||
| provider_name, | ||
| ) | ||
| _TRACE_PROVIDER = None |
| provider_name, | ||
| ) | ||
| _TRACE_PROVIDER = None | ||
| _METER_PROVIDER = None |
| ) | ||
| _TRACE_PROVIDER = None | ||
| _METER_PROVIDER = None | ||
| _LOG_PROVIDER = None |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
376
to
+391
| await republish_queued_post_content_jobs(client, pool) | ||
| last_recovery = now | ||
| last_id = await consume_post_content_stream_once( | ||
| client, | ||
| pool, | ||
| last_id=last_id, | ||
| vision_factory=vision_factory, | ||
| embedding_factory=embedding_factory, | ||
| structure_factory=structure_factory, | ||
| ) | ||
| try: | ||
| last_id = await consume_post_content_stream_once( | ||
| client, | ||
| pool, | ||
| last_id=last_id, | ||
| vision_factory=vision_factory, | ||
| embedding_factory=embedding_factory, | ||
| structure_factory=structure_factory, | ||
| ) | ||
| except (redis.RedisError, OSError) as exc: | ||
| _logger.warning( | ||
| "post-content Valkey poll failed; retrying (error_type=%s)", type(exc).__name__ | ||
| ) | ||
| await asyncio.sleep(_BROKER_RECOVERY_DELAY_SECONDS) |
There was a problem hiding this comment.
📝 Info: Broker retry omits the recovery sweep
The redis/OS retry wraps only consume_post_content_stream_once; the periodic republish_queued_post_content_jobs call above it stays unguarded. This does not crash on a Valkey outage because the sweep's only broker call routes through publish_post_content_event, which swallows redis.RedisError at post_content_queue.py. A Postgres error during the sweep still kills the worker, matching consume's own unguarded DB path.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+164
to
+175
| session_id = current_session_id() | ||
| if session_id: | ||
| request_headers["x-lineageweave-session-id"] = session_id | ||
| with traced( | ||
| "lineageweave.http.post_json", | ||
| { | ||
| "http.request.method": "POST", | ||
| "lineageweave.operation_code": "http_post_json", | ||
| "service.peer.name": service_peer_name, | ||
| }, | ||
| ) as span: | ||
| inject_trace_context(request_headers) |
There was a problem hiding this comment.
📝 Info: Trace/session context survives into to_thread
client.answer runs under asyncio.to_thread, which copies contextvars into the worker thread. The active OTel span and the post-session metadata are therefore visible when post_json builds its child span and reads current_session_id(), so provider spans correctly parent to the API span and carry the session header. This is what the traceparent tests exercise.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+225
to
+264
| def _traced_get_json( | ||
| url: str, | ||
| *, | ||
| headers: dict[str, str] | None, | ||
| timeout: float, | ||
| decoder: Callable[[bytes, str], dict | list], | ||
| span_name: str, | ||
| service_peer_name: str, | ||
| ): | ||
| """GET ``url`` under one HTTP span and inject the active W3C context.""" | ||
| hostname = urlparse(url).hostname or url | ||
| request_headers = dict(headers or {}) | ||
| if service_peer_name in _SESSION_HEADER_PEERS: | ||
| session_id = current_session_id() | ||
| if session_id: | ||
| request_headers["x-lineageweave-session-id"] = session_id | ||
| with traced( | ||
| span_name, | ||
| { | ||
| "http.request.method": "GET", | ||
| "lineageweave.operation_code": "http_get_json", | ||
| "service.peer.name": service_peer_name, | ||
| }, | ||
| ) as span: | ||
| inject_trace_context(request_headers) | ||
| status, raw = _request( | ||
| "GET", url, body=None, headers=request_headers, timeout=timeout | ||
| ) | ||
| if span is not None: | ||
| span.set_attribute("http.response.status_code", status) | ||
| if status >= 400: | ||
| if span is not None: | ||
| span.set_attribute("error.type", str(status)) | ||
| raise HttpClientError(f"HTTP {status} from {hostname}") | ||
| try: | ||
| return decoder(raw, hostname) | ||
| except HttpClientError: | ||
| if span is not None: | ||
| span.set_attribute("error.type", "HttpClientError") | ||
| raise |
There was a problem hiding this comment.
🔍 Session header scoped away from third-party GETs
_traced_get_json adds x-lineageweave-session-id only for peers in _SESSION_HEADER_PEERS, so Searxng/CalDAV/OIDC GETs get traceparent but not the post session id. Note post_json with service_peer_name="tepp" does send the session header, whereas ADR 0122 names only contextual-orchestrator POSTs as carriers; confirm that reaching TEPP is intended.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+113
to
+121
| ## [2.13.2] - 2026-08-24 | ||
|
|
||
| ### Added | ||
|
|
||
| - Reader-safe Global Ask and post-chat failures now carry bounded | ||
| OpenTelemetry metrics, traces, and structured server diagnostics for | ||
| GRC consumption. Failure logs carry the active TraceId and SpanId so | ||
| another agent can join the Error span to the audit record. | ||
|
|
There was a problem hiding this comment.
📝 Info: New changelog entry sits out of version order
The added ## [2.13.2] block lands between ## [2.12.26] and ## [2.12.19], while pyproject.toml remains 2.15.0. The section is only loosely date-ordered, so this is cosmetic, but the placement reads oddly against the surrounding version numbers.
Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
auto-merge was automatically disabled
August 25, 2026 00:34
Pull request was closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #361.
Verification