Repository navigation
Export chat telemetry and honor OTLP environment settings - #80
Conversation
Let the SDK resolve omitted endpoints and headers from its standard environment variables. Keep explicit YAML settings authoritative and avoid a null endpoint crash in log setup. Test real exporter configuration without starting network workers.
Keep telemetry active across engine setup and adapter execution, including EOF, interrupts and failures. Track owned providers so traces, metrics and logs are drained without closing application-owned providers or masking the original error. Exercise piped and multi-turn chat with real batch processors and in-memory exporters.
Provide an environment-only OTLP credential example and explain chat flush behavior and finish reasons. Use a new patch version so the corrected behavior can be published separately from 0.13.0, with matching lock metadata.
reviewer9io
left a comment
There was a problem hiding this comment.
Ready to merge - no findings.
Reviewed chat telemetry lifecycle, OTLP configuration precedence, caller compatibility and release metadata. Passed 103 focused tests, five boundary probes and changed-file Ruff; base controls confirmed regression coverage, while live collectors/models and local release builds were not exercised.
Coverage detail
Coverage: 8 of 9 passes run. Skipped: Prior discussion (Packet contains no previous reviews, issue comments or inline discussions.).
- Intent: Ledgered 20 code/test claims, 5 historical assertions and 11 files; 0 failed claims; checked issue #79 acceptance criteria.
- Correctness and safety: Read all 11 diffs, engine/LLM/config callers; ran 103 focused tests, 5 boundary probes and 14 base cases.
- Reliability: Checked startup/EOF/error cleanup, all owned providers, failure isolation, optional SDK imports and pinned SDK export bounds.
- Cost and resource: Inspected SDK batch queues/timeouts, metric reader cadence and per-turn engine/LLM instrumentation; disabled path remains opt-in.
- Contract and rollout: Text search traced run/chat setup, engine, Anthropic and Bedrock readers; checked YAML/env priority, SDK imports and release metadata.
- Tests: Tested 10 of 10 changed behavior sites; four changed test files run in ci.yml test (Python 3.11/3.12/3.13); checked base failures.
- Maintainability: Reviewed provider ownership list, local tracer binding, metrics return contract, chat extraction and test fixture isolation.
- Guidance compliance: Checked added lines in chat_cmd.py, tracer.py, metrics.py, docs and four test files; changed-file Ruff passed.
|
@coder9io update docs related to this change |
Document OTLP precedence and log forwarding in the published configuration reference, with deployment checks and troubleshooting for chat exports. This addresses the documentation review request without changing runtime behavior.
|
Updated the published configuration, deployment, and troubleshooting guides in 58b4174 with OTLP environment precedence, chat shutdown/export behavior, a Last9 verification example, and log-forwarding diagnostics. This is a documentation-only revision, so no new failing regression test applies. Validation: 53 focused tests passed, Python wheel/sdist built, and the docs site built all 12 pages using temporary writable build caches. Ruff reported 198 findings in unchanged Python files; the full suite and native binary build remain with CI. |
reviewer9io
left a comment
There was a problem hiding this comment.
Ready to merge - no findings. Since e8f2bdc: 0 fixed, 0 new, 0 open.
Verified the three-guide documentation update against the implementation and rechecked the full diff. Passed 21 focused tests and 12 snippet/precedence probes; reused prior tests, base controls and lint after source/lockfile/toolchain identity checks, with live collectors/models and local release/docs builds unexercised.
Coverage detail
Coverage: 9 of 9 passes run.
- Intent: Ledgered 25 code/docs claims, 8 historical assertions and 14 files; 0 failed claims; checked issue #79 acceptance text.
- Correctness and safety: Fresh inspection of all 14 diffs and CLI/engine/LLM/config paths; 21 native tests and 12 actual-snippet probes.
- Prior discussion: Read prior approval and both author/human comments; verified three-file docs delta against requested guide updates.
- Contract and rollout: Text-only reach: run/chat, engine, Anthropic, Bedrock and metric setup caller; checked docs defaults, links and YAML/env priority.
- Tests: Tested 10 of 10 runtime sites with fresh/reused evidence; four changed test files run in ci.yml test (Python 3.11/3.12/3.13).
- Guidance compliance: Scanned all added lines against applicable CLAUDE.md; verified identical lint inputs/toolchain; recorded command dispositions.
What
Why
Chat never initialized telemetry, leaving its existing request, LLM, and tool instrumentation inactive. Hard-coded exporter endpoints also bypassed environment settings, and a null endpoint crashed log setup. The providers already record finish reasons and exceptions; the new chat tests verify those reach the exporter.
Tests
Observed
test_otlp_settings_for_all_signals,test_piped_chat_exports_request_and_children, finish-reason/error and exit-path regressions fail before the changes. In-memory exporters now verify parentage, correlated logs, max_tokens, SDK errors, and one trace per interactive turn, including multi-agent chat. Cleanup tests cover pending metrics and provider shutdown failures.Three commits prepared for the harness (none pushed by this job), each passing focused tests independently via
git rebase --exec: 88, 102, and 103 tests. Final coverage includes telemetry, chat, CLI adapter, configuration, and release-version tests. Wheel/sdist and native binary builds pass; help/version smoke tests pass with 0.13.1 metadata from the built wheel. Changed-file Ruff checks pass. Repository-wide Ruff reports 198 existing findings versus 201 on the base, with no new findings. Full suite and Python-version matrix remain with CI.Fixes #79
Opened by coder9io. Review comments and CI results on this PR are picked up automatically.