fix(otlp): acknowledge an export only once it is stored; count what was refused - #5963
Merged
Merged
Conversation
|
Contributor
Visual diffComparing 56 of 70 comparison(s) flagged (>1% pixel diff).
Folder: 857da28fb768. Full PNGs also attached as a workflow artefact. Generated by visual-diff bot. Pixel diffs >1% flagged; eyeball the table before merging. This check is non-blocking — fail = bot bug, not a code problem. |
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
Owner
Author
Coordinator reviewVerdict: fix before merge. One blocking regression; the rest is solid. Blocking1. One out-of-range item now makes an export fail forever. For traces, it also loses every valid span in that export.
Verified OK
Non-blocking (follow-ups)
|
vivekchand
added a commit
that referenced
this pull request
Sep 14, 2026
…s export Review of #5963: the token columns are DuckDB INTEGER while OTLP carries int64. One span with gen_ai.usage.input_tokens=3e9 failed the one-transaction span batch, so the export was answered 503, every retry failed the same way and the valid spans beside it never landed (main answered 200 and stored them). A log record with the same value counted as a failed write, so every delivery was answered 503. - local_store: _is_data_error tells a value the store cannot hold (DuckDB DataError, ValueError/OverflowError, the driver's cast error) from a store failure (closed connection, I/O). - ingest_spans_batch: a batch that fails on a data error is written again one span at a time, so valid spans land; other errors still raise. with_outcome=True returns {written, rejected} for the OTLP receiver. - put_otlp_batch: such a row counts as records_rejected, not records_failed. - _event_to_row: an events.token_count beyond INTEGER is stored as unknown. It used to fail the ring flush for every event queued beside it, on every retry. - _i helpers: inf/NaN become unknown instead of raising. - The receiver answers 200 with partialSuccess for the refused items and keeps 503 for a store that did not confirm the write. Regression guard: 4 new tests in tests/test_otlp_durable_ack.py fail on the previous head (27a01b1) and pass here (25/25). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jm9d7s4fN55hN3YzQo75o9
vivekchand
force-pushed
the
feat/otlp-durable-ack
branch
from
September 14, 2026 03:11
27a01b1 to
b29491d
Compare
Owner
Author
Review fix pushed (b29491d): an item the store cannot hold no longer poisons its exportAddresses the blocking item in the coordinator review. Your probe, re-run as regression tests on the real route, mappers and DuckDB store (each export sent 3 times):
A second poison path turned up while writing the logs test. How:
Proof: 4 new tests in Still open, listed in the PR body as follow-ups: an event whose text the driver cannot encode would still fail the ring flush, as it does on |
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
This was referenced Sep 14, 2026
Owner
Author
|
Merged Generated by Claude Code |
vivekchand
added a commit
that referenced
this pull request
Sep 14, 2026
…s export Review of #5963: the token columns are DuckDB INTEGER while OTLP carries int64. One span with gen_ai.usage.input_tokens=3e9 failed the one-transaction span batch, so the export was answered 503, every retry failed the same way and the valid spans beside it never landed (main answered 200 and stored them). A log record with the same value counted as a failed write, so every delivery was answered 503. - local_store: _is_data_error tells a value the store cannot hold (DuckDB DataError, ValueError/OverflowError, the driver's cast error) from a store failure (closed connection, I/O). - ingest_spans_batch: a batch that fails on a data error is written again one span at a time, so valid spans land; other errors still raise. with_outcome=True returns {written, rejected} for the OTLP receiver. - put_otlp_batch: such a row counts as records_rejected, not records_failed. - _event_to_row: an events.token_count beyond INTEGER is stored as unknown. It used to fail the ring flush for every event queued beside it, on every retry. - _i helpers: inf/NaN become unknown instead of raising. - The receiver answers 200 with partialSuccess for the refused items and keeps 503 for a store that did not confirm the write. Regression guard: 4 new tests in tests/test_otlp_durable_ack.py fail on the previous head (27a01b1) and pass here (25/25). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jm9d7s4fN55hN3YzQo75o9
vivekchand
force-pushed
the
feat/otlp-durable-ack
branch
from
September 14, 2026 06:22
9ad6a98 to
005698b
Compare
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
vivekchand
added a commit
that referenced
this pull request
Sep 14, 2026
…s export Review of #5963: the token columns are DuckDB INTEGER while OTLP carries int64. One span with gen_ai.usage.input_tokens=3e9 failed the one-transaction span batch, so the export was answered 503, every retry failed the same way and the valid spans beside it never landed (main answered 200 and stored them). A log record with the same value counted as a failed write, so every delivery was answered 503. - local_store: _is_data_error tells a value the store cannot hold (DuckDB DataError, ValueError/OverflowError, the driver's cast error) from a store failure (closed connection, I/O). - ingest_spans_batch: a batch that fails on a data error is written again one span at a time, so valid spans land; other errors still raise. with_outcome=True returns {written, rejected} for the OTLP receiver. - put_otlp_batch: such a row counts as records_rejected, not records_failed. - _event_to_row: an events.token_count beyond INTEGER is stored as unknown. It used to fail the ring flush for every event queued beside it, on every retry. - _i helpers: inf/NaN become unknown instead of raising. - The receiver answers 200 with partialSuccess for the refused items and keeps 503 for a store that did not confirm the write. Regression guard: 4 new tests in tests/test_otlp_durable_ack.py fail on the previous head (27a01b1) and pass here (25/25). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jm9d7s4fN55hN3YzQo75o9
vivekchand
force-pushed
the
feat/otlp-durable-ack
branch
from
September 14, 2026 06:23
005698b to
2cdb11b
Compare
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
1 similar comment
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
Owner
Author
|
awaiting confirmation — non-trivial rebase, needs human review (conflicts in: Generated by Claude Code |
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
This was referenced Sep 14, 2026
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
vivekchand
added a commit
that referenced
this pull request
Sep 14, 2026
…s export Review of #5963: the token columns are DuckDB INTEGER while OTLP carries int64. One span with gen_ai.usage.input_tokens=3e9 failed the one-transaction span batch, so the export was answered 503, every retry failed the same way and the valid spans beside it never landed (main answered 200 and stored them). A log record with the same value counted as a failed write, so every delivery was answered 503. - local_store: _is_data_error tells a value the store cannot hold (DuckDB DataError, ValueError/OverflowError, the driver's cast error) from a store failure (closed connection, I/O). - ingest_spans_batch: a batch that fails on a data error is written again one span at a time, so valid spans land; other errors still raise. with_outcome=True returns {written, rejected} for the OTLP receiver. - put_otlp_batch: such a row counts as records_rejected, not records_failed. - _event_to_row: an events.token_count beyond INTEGER is stored as unknown. It used to fail the ring flush for every event queued beside it, on every retry. - _i helpers: inf/NaN become unknown instead of raising. - The receiver answers 200 with partialSuccess for the refused items and keeps 503 for a store that did not confirm the write. Regression guard: 4 new tests in tests/test_otlp_durable_ack.py fail on the previous head (27a01b1) and pass here (25/25). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jm9d7s4fN55hN3YzQo75o9
vivekchand
force-pushed
the
feat/otlp-durable-ack
branch
from
September 14, 2026 12:32
9081798 to
c57dd2a
Compare
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
vivekchand
force-pushed
the
feat/otlp-durable-ack
branch
from
September 14, 2026 21:08
9a8f3f4 to
21a8933
Compare
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
Owner
Author
|
Generated by Claude Code |
Owner
Author
|
Blocked on required review — skipping (auto-mergeability sweep). @vivekchand please approve when ready. Generated by Claude Code |
vivekchand
force-pushed
the
feat/otlp-durable-ack
branch
from
September 14, 2026 21:43
21a8933 to
857da28
Compare
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
Owner
Author
|
✨ auto-fixed: merged main into branch (was UNSTABLE/BEHIND — CI will re-run) Generated by Claude Code |
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
vivekchand
force-pushed
the
feat/otlp-durable-ack
branch
from
September 15, 2026 03:50
bbb3a02 to
1238cec
Compare
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
1 similar comment
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
…as refused The OTLP receiver answered 200 to every export that decoded. The store write behind it was best effort: while the daemon owning DuckDB was unreachable the proxy returned None, a warning was logged, and the exporter discarded a batch that was never stored. Spans the store refused were acknowledged the same way, a budget pause answered 429 to all intake, retried spans and metric points doubled the live tiles, and a missing token side was stored as 0. - 200 only after the store confirms the write; 503 + Retry-After when it does not; OTLP partialSuccess (JSON or protobuf) for malformed items. - Traces write one ingest_spans_batch per export (allowlisted) so a lost write is visible; put_span returned None either way. - A value the store cannot hold (an int64 token count in an INTEGER column) is refused on its own: ingest_spans_batch retries a batch that fails on a data error one span at a time, put_otlp_batch counts such a row as rejected, and events.token_count beyond INTEGER is stored as unknown. - Budget pause no longer pauses intake; requests during it are counted. - Live tiles count a span once per (trace, span) id and a metric point once per its own time. - Unknown usage stays NULL; rollup reports cost_usd null with records_with_cost / records_with_tokens. - /api/otel-status gains an intake block (counts and categories only). - Generated docs/INGEST.md states ack, retry, re-delivery identity, session fallback, live-view-only data and OTLP auth. Rebased onto main after #5953 (trace spans reach Guard and are scrubbed) merged first. The two changes are combined, not just both kept: - ingest_spans_batch: spans are redacted outside the write lock BEFORE any write, and both the batch write and the one-span-at-a-time retry write those same redacted rows. A span redaction leaves unstorable is refused, never stored unredacted. - _process_otlp_traces: the confirmed-write accounting runs first; wait events and the tool events from spans are written after it. A refused span yields no tool events. - put_otlp_batch returns both counter sets (events_skipped_other_signal plus the records_* / events_* outcome counters); apply_batch_outcome reads any events_skipped_* key as skipped, not as a failed write. - tests/test_otlp_trace_guard_redaction.py gains test_spans_written_one_at_a_time_after_a_data_error_are_still_scrubbed, checked against a mutant that writes unredacted rows on the retry path. - docs/acceptance_criteria.json carries AC-OBS-OIA-001.1-.8 once each; docs/MODULE_MAP.md regenerated. CHANGELOG.md is unchanged from main. REQ-OBS-OIA-001. Refs #5949 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jm9d7s4fN55hN3YzQo75o9
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jm9d7s4fN55hN3YzQo75o9
vivekchand
force-pushed
the
feat/otlp-durable-ack
branch
from
September 15, 2026 07:37
a4741c0 to
bd47c73
Compare
Owner
Author
PR fixer: rebased linear onto main after #5968 and #5962 (bd47c73)The
#6009 (span parity) can rebase onto this once it merges. Generated by Claude Code |
|
…rs.py is_data_error and int32_or_none were defined near line 20,100 of the 21k-line local_store.py, past what Drift Bot reads, so it reported _is_data_error as undefined. A short leaf module holds them now; local_store imports them under the same private names, so callers and tests are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jm9d7s4fN55hN3YzQo75o9
Owner
Author
Drift Bot finding on bd47c73: the helper existed, past what the bot reads (fixed in 7531e2a)Drift Bot reported that Fix in Verified locally:
Generated by Claude Code |
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
This was referenced Sep 15, 2026
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.
Refs #5949
Product record: https://factory.8090.ai/project/b415065f-ab2f-4f53-8864-0c009fd098cb/requirements/3caa08b2-4e39-4044-91f7-113fb2668ae0 (REQ-OBS-OIA-001, written before the code)
Problem
The OTLP receiver answered 200 to every export it could decode, whether or not anything reached the store. The store write was best effort. While the daemon that owns DuckDB was restarting, busy or unreachable, the dashboard's
_ProxyStorereturnedNone, a warning was logged, and the receiver still answered 200. The exporter then discarded a batch that was never stored. A span the store refuses (no id, no name) got the same answer.The issue also names three gaps:
0, and a rollup group where no record carried a cost showed$0.00.The existing suite was hiding the first gap. On
main,tests/test_otlp_compat_port.pypasses while every span write fails. Another file's fixture leaves a stopped store singleton behind, and running the two files together onmainlogslocal_store.put_span failed: Connection Error: Connection already closed!twice and still reports30 passed.What changes (the verified subset this PR claims)
Retry-After: 5when it does notpartialSuccess(rejectedSpans/rejectedLogRecords/rejectedDataPoints); protobuf senders get a protobuf responsepartialSuccessput_spanper span (one proxy hop each;Noneon success and on loss)ingest_spans_batch(spans=[...], with_outcome=True)per export, returning{written, rejected}(added to_DAEMON_METHODS)already_storedNULL; rollup reportscost_usd: nullplusrecords_with_cost/records_with_tokens/api/otel-status→intake: per-signal item and request counts, last success, last failure category, and a statement that no queue is kept. Counts only: no content, no credentialsdocs/INGEST.mdstates the ack rule, each status and whether to retry it, the re-delivery identity per signal, the session fallback when none is sent, live-view-only data, and OTLP authThe logic lives in a new short module,
clawmetry/otlp_intake.py.dashboard.pyonly gathers counts and calls it.Review fix: one item the store cannot hold no longer poisons its export
The coordinator review found a blocking regression. The token columns (
spans.tokens_input/tokens_output/token_count, the same inotlp_records, andevents.token_count) are DuckDBINTEGER, while OTLP carries int64. On the previous head (27a01b1):gen_ai.usage.input_tokens = 3e9failed the one-transaction span batch. The export got 503 and so did every retry, and the valid span beside it never landed (mainanswered 200 and stored it).What changed:
local_store._is_data_errortells a value the store cannot hold (DuckDBDataError,ValueError/OverflowError, the driver's own cast error) from a store failure (closed connection, I/O, anything else).ingest_spans_batch: a batch that fails on a data error is written again one span at a time, so valid spans land and only the bad one is dropped. Any other error still raises. This also protects the daemon's own span writes.with_outcome=Truereturns{"written", "rejected"}for the receiver; without it the return value and theValueErrorfor a missing field are unchanged.put_otlp_batch: such a row counts asrecords_rejected, notrecords_failed._event_to_row: anevents.token_countbeyond INTEGER range is stored as unknown, so it cannot block the ring flush. The rollups are BIGINT and unaffected._ihelpers treat inf / NaN as unknown instead of raising.partialSuccessfor refused items and keeps 503 for a store that did not confirm the write. ThepartialSuccessmessage and the generated contract now name this case.Coordination
#5684. This extends the ingest contract already on
main(clawmetry/ingest_contract.py+scripts/gen_ingest_doc.py) and does not start a second reference. #5684 rewrites that module and_otlp_receive, so whichever PR merges second must carry the other's sections. The constants added here are self-contained (OTLP_RETRY_AFTER_SECONDS,OTLP_ACKNOWLEDGEMENT,OTLP_REDELIVERY_IDENTITY,OTLP_SESSION_IDENTITY,OTLP_LIVE_VIEW_ONLY,OTLP_AUTH,OTLP_INTAKE_STATUS), so carrying them is a copy.#5953 (span Guard + redaction, a dependency of #5949) merged first; this PR is rebased onto it and combines the two: spans are redacted outside the write lock before any write, and the one-span-at-a-time retry after a data error writes those same redacted rows (pinned by
test_spans_written_one_at_a_time_after_a_data_error_are_still_scrubbed). Both PRs touch_process_otlp_tracesand the dictput_otlp_batchreturns:events_skipped_other_signalkey is already summed intoskipped_other_sourcethrough theevents_skipped_*prefix rule.Done: #5953 merged, and this PR was rebased onto it on 2026-09-14 (confirmed-write accounting first, then wait events and tool events from spans; both counter sets returned).
No new HTTP route, so no
cloud_route_policyentry or cloud PR is needed. The diff adds no@*.route(decorator;/api/otel-statusonly gains a key.Mixed versions.
ingest_spans_batch(and itswith_outcomeargument) and the allowlist entry ship in the same release. A dashboard newer than its daemon, mid-upgrade, getsNonefrom the proxy, so trace exports answer 503 until the daemon updates; the sender's retries then land.Verification
tests/test_otlp_durable_ack.pyhas 25 tests and is in the CI "OTLP receiver suite" job. It drives the real route over the real mappers, a real DuckDB store, and the real_ProxyStorewith no daemon behind it, and declares everyAC-OBS-OIA-001.*criterion. It covers:partialSuccessevery time and the valid item stored once;dashboard.py,routes/meta.py,clawmetry/local_store.py,routes/local_query.py), from a saved patch, with no stash. Red: 18 failed, 3 passed (the 3 pin behaviour that already held, or read files that were not reverted). Green: the tree is byte-identical and all 21 then-existing tests pass.tests/test_otlp_*.pyfile: 163 passed. Five tests fail locally in the same way on the untouched previous head, so this change did not cause them. Four are intest_otlp_daemon_free_intake.py, which was green in this PR's CI OTLP job. The fifth istest_ingest_bulk_flush.py::test_pre_chain_redelivery_gets_stamped_in_place(a DuckDB parameter-count error), which is in no CI job.test_genai_cache_semconv.pyandtest_spans_otlp_edge_cases.pypinned the old per-spanput_spancall. The proxy-keyword guard now asserts exactly oneingest_spans_batch(spans=...)call and its allowlist entry.test_otlp_traces_cost.pyposted a span with no ids, which OTLP forbids and the store refuses.test_otlp_json_no_protobuf.py's fixture now resets the store singleton it stops, which fixes the hidden closed-store leak described above.check_ac_coverage.py --check,check_ci_test_coverage.py --check,lint_daemon_allowlist.py,check_py39_annotations.py,gen_ingest_doc.py --checkandgen_module_map.py --checkall exit 0.Remaining (not claimed by this PR)
openclaw.*andgen_ai.client.*metrics stay live-tile only. That is documented, and they are counted aslive_view_only.main; only the out-of-range token count is fixed here.🤖 Generated with Claude Code
https://claude.ai/code/session_01Jm9d7s4fN55hN3YzQo75o9