Skip to content

Ingestion clock skew detection (#279)#282

Merged
Polliog merged 17 commits into
developfrom
feat/ingestion-clock-skew-279
Jul 17, 2026
Merged

Ingestion clock skew detection (#279)#282
Polliog merged 17 commits into
developfrom
feat/ingestion-clock-skew-279

Conversation

@Polliog

@Polliog Polliog commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Closes the silent failure mode behind #279.

The problem

A log ingested with a client-supplied time far from the server clock is stored and searchable, but threshold alert rules count logs in [now - time_window, now], so they can never see it. In #279 an Ubuntu host shipped through rsyslog into Fluent Bit, whose Lua filter built a time about 27 hours in the past. Ingestion worked, the rule was enabled, the webhook channel tested fine, and no alert ever fired. Nothing in the product said why. Diagnosis took a GitHub issue round trip and a manual SQL query comparing NOW() to the stored time.

What this does

Detects a time more than 24 hours in the past or more than 5 minutes ahead of the server clock, counts it, and surfaces it. The log is never rejected: backfilling historical data stays valid.

The 24 hour threshold is derived, not chosen. It is the maximum configurable alert time_window (alerts/routes.ts), so past that point no threshold rule in the product can match the log. That makes the warning an exact statement rather than a heuristic.

Detection is always on. There is no configuration to get wrong: a feature whose whole job is to make a silent failure visible should not ship with a knob that silently switches it off.

How it works

  • Detection rides the existing record-building pass in ingestionService.ingestLogs, the single choke point for every log path (batch, Fluent Bit/NDJSON, OTLP, receivers). Cost is one subtraction per record and one Date.now() per batch, no second pass.
  • One write: a single ingestion.timestamp_skew metering event per skewed batch, carrying the count and the worst delta per direction. No migration: metering_events already had metadata JSONB.
  • Two reads off that write: the admin ingestion health card for operators, and GET /api/v1/projects/:id/ingestion-health driving a warning banner on the project overview for tenants. The banner is the one that actually solves [BUG] Las alertas de umbral (Threshold) no se disparan para logs de Ubuntu #279, since it is where the person who misconfigured the shipper looks. Its copy carries the whole diagnosis, including the fix, because there is no public ingestion doc to link to yet.

An Invalid Date is deliberately never counted as skew: now - NaN is NaN and both comparisons are false. What the reservoir does with a malformed timestamp is a separate hole, and conflating the two would make this signal mean two different things.

Also fixed

METERING_ENABLED=false silently suppressed every ingestion.* counter, including ingestion.pii_rejected, the safety counter for records dropped because PII masking failed. That toggle governs usage metering, while the ingestion.* counters are operational health signals already excluded from tenant usage breakdowns by a prefix filter. They now bypass it. The buffer hard cap still applies: that is a memory valve, not a feature switch.

Notes for review

Two bugs worth knowing about, both caught in review and fixed here:

  • The banner was first placed inside the overview page's {:else if stats} branch, reachable only when isEmpty is false. But isEmpty derives from clock-windowed queries, exactly the ones skewed logs fall outside of, so in the precise [BUG] Las alertas de umbral (Threshold) no se disparan para logs de Ubuntu #279 repro (a misconfigured shipper as the project's only source) the page showed "No data yet" and hid the banner explaining it. It is now hoisted above the branch chain.
  • Nothing linked the keys written at ingest to the keys read by the endpoint, so renaming one would have left every test green while the banner rendered "up to 0 minutes in the past". There is now an integration test that ingests a real skewed log, flushes the recorder and reads it back through the endpoint.

The per-project aggregate casts metadata->>'maxPastMs' to float8 before MAX, since the field is text and a lexicographic max is a real bug ("9000000" sorts above "97200000"). There is a regression test for exactly that.

Verification

Targeted suites green: projects 102, metering 49, ingestion, config, frontend banner 7, shared 177, frontend 79. svelte-check clean of new errors. tsc --noEmit clean.

A full backend run with coverage is still executing as this PR opens; the previous full run over this branch was green (exit 0, coverage gate met) and the changes since were the metering gate, the SQL aggregation and the threshold constants, all covered by the targeted suites above. I will report the full result on this PR.

Deferred work is tracked with its rationale, including extending skew detection to spans and metrics, which have the same exposure and are not covered here.

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.25287% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
packages/backend/src/modules/projects/routes.ts 68.75% 5 Missing ⚠️

📢 Thoughts on this report? Let us know!

@Polliog

Polliog commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

Full backend run with coverage over the final state of this branch (84b4c99): exit code 0, so no failures and the 80% coverage gate is met. vitest exits non-zero on either, so the zero covers both.

That is the last verification this branch was waiting on. Everything else was already reported: projects 102, metering 49, ingestion and config suites, frontend banner 7, shared 177, frontend 79, svelte-check clean of new errors, tsc --noEmit clean.

@Polliog
Polliog merged commit 5a83e9b into develop Jul 17, 2026
8 checks passed
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.

1 participant