Skip to content

feat(reader): FR4 per-stream completeness flags (Phase 0)#59

Open
ashish-jabble wants to merge 13 commits into
mainfrom
feat/phase0-completeness-flags
Open

feat(reader): FR4 per-stream completeness flags (Phase 0)#59
ashish-jabble wants to merge 13 commits into
mainfrom
feat/phase0-completeness-flags

Conversation

@ashish-jabble

@ashish-jabble ashish-jabble commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Phase 0 of #58 — the first independently-shippable slice. FR4 only: completeness flags. No persistence work.

What

Every /logs/{sys,api,audit} read now reports, per stream, whether its rows came from the durable store or a bounded ring:

GET /logs/sys   → { "rows": [...], "completeness": { "sys": "ring" } }
GET /logs/api   → { "rows": [...], "completeness": { "api": "ring" } }
GET /logs/audit → { "rows": [...], "total": , "completeness": { "audit": "store" } }

With persistence still off (the default), audit is served from the store and api/sys from rings — so the values are static today. The point of this PR is not the value; it is locking the response envelope now so consumers can start reading completeness, and so Phase 1 (FR1) only has to flip a value, not change the contract.

Design notes

  • One-key map, not a flat scalar. Single-stream endpoints emit {"sys":"ring"} etc. — the same shape FR2's /logs/correlate will merge, so that endpoint becomes a trivial assembly later.
  • One resolver seam. persist_streams (Python, a router() kwarg) / persistedStreams (Go, an Engine field seeded to {audit} in Init) is the single place Phase 1 repopulates from real config. Handlers do not change again.
  • Additive only. Existing keys (rows, total, limit, offset, next_after) are untouched; the not-initialised error paths also gain the flag so consumers parse one uniform shape on every response. Satisfies the issue's "behavior unchanged when new config absent" criterion.
  • Python + Go parity (FR5).

Deferred (intentionally)

Truncation honesty (ring-at-capacity / limit-hit signaling) is not in this PR — kept to store-vs-ring per the phased plan. A ring genuinely cannot tell you whether matching older rows were evicted, only that loss is possible; that nuance belongs with Phase 1.

Tests

python/tests/test_completeness.py and go/completeness_test.go cover: store-vs-ring defaults, the persist_streams override flipping api/sys to store, additive-key preservation, and error-path flags.

Note on local verification: I could not run either suite on my machine — both packages load the native libfasten_core and only a Linux .so is present (macOS needs a .dylib), with no Rust toolchain to rebuild it; the existing suites fail the same way locally. As far as the environment allowed: go vet . passes (compiles the package including the new test), py_compile passes, and import fasten + router construction run cleanly under a stubbed core.

Refs #58

Every /logs read now reports, per stream, whether its rows came from the
durable store or a bounded ring. This locks the response envelope so
consumers can adopt `completeness` now; Phase 1 (FR1) only flips the value
once api/sys can persist — no further contract change.

- Single-stream endpoints emit a one-key map ({"sys":"ring"}, …) matching
  the shape FR2's /correlate will merge, not a flat scalar.
- Resolver seam: persist_streams (Python) / persistedStreams (Go), seeded
  to {audit}, is the single point Phase 1 repopulates from config.
- Additive only: existing keys (rows/total/limit/offset/next_after) and the
  not-initialised error paths are unchanged, just gain the flag.
- Python + Go parity (FR5). Tests cover store-vs-ring defaults, the
  persist_streams override, additive keys, and error-path flags.

Refs #58
@ashish-jabble ashish-jabble force-pushed the feat/phase0-completeness-flags branch from 2f33713 to 81997f2 Compare June 25, 2026 09:14
- Reframe completeness comments/docstrings from row provenance ("rows came
  from" / "served from" / "matching the reader's data paths") to the stream's
  configured durability class. The flag is computed from config, not from the
  query, and is emitted on zero-row error responses — "backed by" is the
  honest description; the `error` key remains the signal that a read failed.
- Drop phase labels describing already-implemented behavior; keep only the
  forward note that Phase 1 wires persistedStreams from config.
- Go parity tests: error/uninitialised paths carry the flag, the nil-map
  streamSource fallback, and request_id filtering still narrowing rows
  alongside completeness.
- Python: request_id-coexists-with-completeness, and an empty persist set
  making even audit report "ring" (no audit special-case in _source).
@ashish-jabble

Copy link
Copy Markdown
Collaborator Author

BEFORE (main) — no honesty signal

GET /logs/sys   → {"rows":[{"level":"error","event":"db.timeout","request_id":"req-demo-123",...}]}
GET /logs/api   → {"rows":[ {GET /v1/health 200 ...}, {POST /v1/checkout 502 ...} ]}
GET /logs/audit → {"rows":[...],"total":1,"limit":100,"offset":0}

No way to know whether sys/api are the full history or a truncated ring window.

AFTER (feat/phase0-completeness-flags) — every read declares its source

GET /logs/sys   → {"rows":[...same row...],     "completeness":{"sys":"ring"}}
GET /logs/api   → {"rows":[...same 2 rows...],  "completeness":{"api":"ring"}}
GET /logs/audit → {"rows":[...],"total":1,"limit":100,"offset":0,"completeness":{"audit":"store"}}

The diff

Endpoint BEFORE AFTER
/logs/sys (no field) "completeness":{"sys":"ring"}
/logs/api (no field) "completeness":{"api":"ring"}
/logs/audit (no field) "completeness":{"audit":"store"}
existing keys (rows, total, limit, offset) present unchanged (additive)

Persist the api and sys streams to a per-stream SQLite store so they gain
queryable history beyond the bounded ring. Opt-in and default-off — without
a store, both streams stay ring-only (backward compatible).

- StreamStore (Python store/stream.py, Go stream_store.go): table per stream,
  full row as JSON payload + indexed columns (request_id, timestamp, level,
  service_id, event, method, path, status). Returns rows newest-first,
  byte-identical to what was pushed, with the same filter semantics as that
  language's ring so store and ring reads agree.
- Write-through transport: the ring stays the hot write path; the store is a
  write-through sink. Reads for a persisted stream come from the store
  (durable depth); unpersisted streams read the ring.
- Wiring reuses the completeness seam: when a stream store is configured it
  joins persisted_streams / persistedStreams, so the reader flag flips
  ring -> store with no handler change. Python resolves via FASTEN_API_DSN /
  FASTEN_SYSLOG_DSN or init(api_store=, syslog_store=); Go via
  Config.APIStore / Config.SyslogStore (caller owns the *sql.DB).
- Go QueryAPI/QuerySyslog now return an error, surfaced as HTTP 500 like the
  audit reader; SQLite-only in v1 (Postgres stream store is a later phase).

Tests: store roundtrip/filter, depth-beyond-ring, completeness flip to store,
ring-only fallback, write-through sync — Python (executed, green) and Go
(vet/build clean; executed in CI).

Refs #58
Add a single reader endpoint that returns every stream for one request_id,
so a consumer holding an id gets the whole operation in one call instead of
stitching three. Fans out to the existing per-stream query paths (audit
store + sys/api rings-or-stores) and assembles them — no new query
semantics.

Response: { request_id, audit[], api[], sys[], counts{}, completeness{} }.
completeness reports each stream's durability class (store vs ring) so the
consumer knows whether a stream could be silently truncated; once api/sys
persist (FR1) the historical-investigation case recovers rows past the ring
window, which the tests pin.

Python: GET /correlate, request_id required (422 if missing). Go:
GET /correlate, request_id required (400 if missing), query errors surfaced
as 500 like the audit reader. Tests cover assembly, request_id isolation,
empty/unknown id, missing-param, and the persisted depth-beyond-ring case —
Python executed green; Go vet/build clean, executed in CI.

Refs #58
Guarantee that every sys/api stream row carries a non-empty request_id, so
/logs/correlate is a contract rather than best-effort: today a context-less
row (boot, init, background work, lost-context async) has no request_id and
silently drops out of any correlated view.

- Sentinel namespaces (boot/sched/bg/lib/orphan) with mint + classify helpers:
  Python context.mint_sentinel/request_id_kind/is_sentinel; Go
  MintSentinel/RequestIDKind/IsSentinel. The prefix tells an operator what
  kind of context produced the row instead of "anonymous noise".
- Enforced at the transport push chokepoint so every path (shims, middleware,
  drainer) is covered: a missing request_id is filled with the shared
  per-process boot id during startup, then a unique orphan id once the first
  real request has been seen. Explicit ids are never touched.
- boot id is minted once per process in Init and passed to the transport, so
  /correlate?request_id=boot-<svc>-<id> returns the whole startup window
  (boot-error diagnosis becomes a real workflow).

Tests (Python executed green; Go vet/build clean, runs in CI): mint/classify,
boot-then-orphan transition, explicit id untouched, boot rows correlatable,
and a conformance check that no stream row across a mixed workload ever lacks
a request_id.

Refs #58
Expose the fields the StreamStore already indexes as first-class reader query
params, so discovery by something an operator knows (an event name, a status
code, a time window) is a fast indexed lookup instead of a scan or a
free-text search:

- /logs/sys gains ?event= and ?since=/?until=
- /logs/api gains ?status= and ?since=/?until=

Filters are honoured identically whether the stream is ring-only or persisted
(store reads add WHERE clauses on the indexed columns; ring reads filter in
memory with the same semantics), so behaviour doesn't change with persistence.

Go: QuerySyslog/QueryAPI now take a StreamQuery filter struct (no more
positional churn as filters grow); StreamStore.Query takes a since/until
window. Tests assert each filter on the ring and prove store/ring agree —
Python executed green; Go vet/build clean, runs in CI.

Refs #58
Multi-agent review follow-ups:

- Write-through resilience (parity + robustness): Python transport now wraps
  each stream-store insert and swallows failures to stderr, matching Go, so a
  persistence sink failure can never break the hot logging path (previously a
  raise could propagate through the HTTP shim's finally and corrupt the
  response). New test injects a failing store and asserts the ring copy
  survives and the push returns normally.
- Null-timestamp window agreement: stores now COALESCE(timestamp,'') in the
  since/until predicates so a timestamp-less row (e.g. a sentinel boot/orphan
  row) is filtered identically to the ring, which treats a missing timestamp
  as "". New ring-vs-store agreement test.
- Doc accuracy: drop the "byte-for-byte identical to what was pushed" claim
  from both StreamStore docs (JSON round-trip normalises numbers — int decodes
  to float64 in Go) and note the SQL LIKE wildcard caveat on the Python path
  filter.
- Document why Python _boot_over needs no lock (GIL + monotonic flag) where Go
  uses bootMu.
- Added coverage: combined filters compose (status + since), limit truncates
  to the newest N.

Refs #58
…rity)

Resolves the cross-language filter divergence from the Phase 1 review: the
Python ring + store matched method case-insensitively and path by substring,
while the Go SDK matched both exactly. /api?path=checkout returned a row in
Python and nothing in Go for the same deployment.

Python now matches method and path exactly in both the ring (store/ring.py)
and the persisted store (store/stream.py), so the two SDKs are interchangeable
on these filters. Tests updated to assert exact-match in both languages
(case-folded / substring queries now correctly return nothing).

Note: `level` is still lowercased on the Python filter side (tied to the
lowercase-severity convention) — a smaller, separate divergence left as-is.

Refs #58
@praveen-garg

Copy link
Copy Markdown
Contributor
  1. The name oversells what it measures. completeness does not tell a consumer whether
    this response was truncated. A ring that overflowed and silently dropped 3 000 rows
    reports exactly "ring" — identical to an empty ring that lost nothing. It's a static
    class label, not a per-response gap signal. At Phase 0 that's a fine scaffold, but the
    name invites readers to over-trust it. Worth a doc note (or renaming the value to
    describe the guarantee, e.g. durable/best-effort, rather than the mechanism store/ring).
  2. Go/Python divergence in where the default lives. Go hardcodes a nil-map fallback in
    the resolver itself (persistedStreams == nil && stream == "audit" → store), so a
    never-Init'd Go engine can never express "audit is a ring." Python keeps the default in
    the router constructor (frozenset({"audit"})), and
    test_empty_persist_streams_makes_audit_ring proves there's no hardcoded audit
    special-case. Harmless today (Init always seeds the map), but it's a latent asymmetry if
    Phase 1 ever wants audit-as-ring.
  3. Minor: Python's audit error path returns completeness+error but omits limit/offset
    that the success path carries, so "uniform shape" isn't literally uniform — consistent
    with the pre-existing error path, not a regression.

… shape

- Go: hoist the {"audit"} durability default into defaultPersistedStreams;
  Init seeds a per-engine copy and streamSource falls back to it, so the
  default lives in one declarative place with no hardcoded stream name in
  the resolver.
- Go/Python: document that completeness reports the stream's durability
  class, not a per-response gap signal — an overflowed ring still reads
  "ring"; per-response truncation honesty is deferred to Phase 1.
- Python: audit error path now carries limit/offset like its success path,
  with a test asserting the uniform shape.
Addresses the Phase 1 review feedback:

- completeness no longer lies on a failing sink: a swallowed stream-persist
  failure is recorded on the StreamStore (NoteWriteFailure / note_write_failure
  at the transport swallow point) and the reader degrades that stream's flag
  to "store-degraded" — reads bypass the ring once a store is attached, so a
  lost row is silently missing and plain "store" would assert durability the
  data no longer has. Sticky by design: the hole in history doesn't heal.
- /correlate reports per-stream "totals" (matching rows in the backing
  source) alongside "counts" (returned), so consumers can tell 100-of-100
  from 100-of-5000: filtered COUNT on the stream stores and the audit store
  (CountFiltered on SQLite/Postgres, count(**filters) in Python), ring
  match-count otherwise.
- Python drainer self-reports now persist like every other syslog push
  (write_drainer_syslog was the only path skipping the store, making drainer
  events vanish from the reader API when persistence is on — Go/Python
  parity; the store insert can't reintroduce the stdout deadlock).
- stream stores set PRAGMA synchronous=NORMAL (per-connection in Python; in
  migrate + DSN guidance in Go) and document the per-row write-through cost.
- level filters are exact-match in Python (like method/path, matching Go),
  and the Go slog handler writes lowercase levels — the canonical casing
  every other writer uses — so slog rows are findable cross-language.
- doc notes: lexicographic time-window assumption (canonical UTC timestamps),
  sentinel prefix-classification caveat, only boot/orphan are auto-minted.
- READMEs answer "is this the whole record or a recent window?" — the
  store / ring / store-degraded contract and counts-vs-totals truncation
  detection.
…reams default

Conflict resolution keeps both sides' semantics: Init seeds persistedStreams
from a per-engine copy of defaultPersistedStreams then extends it with
api/sys per config; streamSource keeps the nil-map fallback to the shared
default AND the store-degraded classification; the router _source docstring
merges the durability-class framing with the degraded contract, now pointing
at /correlate totals-vs-counts as the per-response truncation signal.
A local-build symlink slipped into the merge commit: the .gitignore
pattern 'fasten-core/target/' only matches directories, so a symlink by
the same name was not ignored. CI then failed with 'failed to create
directory fasten-core/target: Not a directory'. Remove it and drop the
trailing slash so the pattern covers both.
feat: Phase 1 — api/sys persistence, /correlate, sentinel request_id, structured filters
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.

2 participants