fix(ingest): unvalidated client event times create permanent future rollup buckets

[vishr]

Summary

Fanout stores client-supplied OTLP event timestamps without any sanity bound, and three rollup queries filter buckets with a lower bound only. A single source with a wrong clock — a device, a VM with broken NTP, a container — can write service_rollup buckets arbitrarily far into the future. Those buckets then match every windowed query forever, and the retention sweep never removes them because it only prunes the past.

This is not a mobile-SDK feature request. #172 settled that boundary and the docs reflect it (docs/operations.md, "Mobile boundary"). This is a data-integrity gap in the topology that #172 approved: mobile SDK -> customer-controlled Collector/gateway -> private Fanout. A gateway forwards client spans verbatim, so the device clock becomes Fanout's bucket key. Backend services with drifting clocks hit the same path.

Evidence

Ingest copies the client timestamp straight onto the row, unvalidated:

• internal/ingest/server.go:93 — StartUnixNanos: int64(sp.StartTimeUnixNano)
• internal/ingest/server.go:156 — TimeUnixNanos: int64(lr.TimeUnixNano)
• internal/ingest/server.go:207,231,255,282,305 — metric data-point timestamps

Span duration is clamped (internal/ingest/server.go:338), but the absolute timestamp is not, and there is no other clamp, skew check, or drift correction in internal/ingest/.

Rollups bucket on that event time:

• internal/query/duck.go:1232 — date_trunc('minute', start_time) AS bucket

Six query helpers bound their window below but not above. Three read rollups, three read raw tables — so this is not confined to the rollup path:

Site	Function	Table	Filter
duck.go:1684	LatencyOverview	service_rollup	bucket >= now() - INTERVAL n MINUTE
duck.go:1723	LogsSamples	logs	time >= now() - INTERVAL n MINUTE
duck.go:1756	Throughput	service_rollup	bucket >= now() - INTERVAL n MINUTE
duck.go:1787	ServiceThroughput	service_rollup	bucket >= now() - INTERVAL n MINUTE
duck.go:1819	ErrorRoutes	logs	time >= now() - INTERVAL n MINUTE
duck.go:1855	ErrorRouteDetails	spans	start_time >= now() - INTERVAL n MINUTE

A repo-wide search for now() - INTERVAL finds no upper bound on any of them; the only < now() in non-test code is the retention sweep.

Retention only prunes the past:

• internal/query/duck.go:551 — DELETE FROM %s WHERE bucket < now() - INTERVAL %d DAY

This prunes the three rollup tables only, and it prunes them by event time (bucket). Raw spans/logs are pruned separately by PruneParquetPass (internal/query/duck.go:529), which passes a cutoff of now - RetentionDays and compares it against batch.metadata.MaxIngestedNanos (internal/telemetry/parquet.go:578) — i.e. ingest time.

That asymmetry decides how bad each half is:

Store	Pruned by	Future-dated row
service_rollup / edge_rollup / endpoint_rollup	event time (bucket)	never pruned — permanent
raw spans / logs (Parquet)	ingest time (MaxIngestedNanos)	pruned RetentionDays after ingest

So the rollup-backed helpers are permanently wrong; the raw-table helpers are wrong for up to RetentionDays and then self-heal. Both still need the upper bound — the rollup case is just the one that never recovers on its own.

Failure scenario

A device (or host) with a clock set to 2035 sends 10k spans through the customer's gateway.

1. Ingest accepts them; rows carry start_time in 2035.
2. The rollup pass creates service_rollup buckets in 2035.
3. All six helpers above match those rows, because a lower bound alone is satisfied by any future timestamp. So does every other window size, permanently. The raw-table helpers (LogsSamples, ErrorRoutes, ErrorRouteDetails) are hit directly and do not need the rollup pass to have run.
4. ServiceThroughput divides the summed span count by windowMinutes, so it reports a spans-per-minute rate that includes ten years of spans that did not happen in that window.
5. The retention sweep never deletes the rows — bucket < now() - N DAY does not match a future bucket. The distortion is permanent until someone deletes rows by hand.

The mirror case is the same asymmetry running the other way. A span arriving with an event time older than RetentionDays is accepted and rolled up, and the next maintenance pass deletes its rollup row (bucket < now() - N DAY matches immediately) while the raw Parquet batch survives, because that batch's MaxIngestedNanos is the receipt time. The row is then visible to raw SQL but absent from every rollup-backed view — the two halves of the product disagree about whether the data exists, indefinitely.

Note that internal/observability/* is not affected — it passes explicit scope.Start/scope.End bounds (overview.go:66, performance.go:234, logs.go:51). The gap is confined to the six duck.go helpers above.

Related, lower priority: internal/query/sql.go:350 and internal/query/schema.go:47 tell users and the AI chat to write start_time/time > now() - INTERVAL ..., propagating the same lower-bound-only pattern into ad-hoc and generated SQL. Worth updating the guidance text alongside this, but it is not the bug.

Proposed fix

Two independent changes; either alone is an improvement, both are cheap.

1. Bound the queries (defensive, no behavior change for healthy data).

Add an upper bound to all six helpers, so a future-dated row cannot leak into a window that does not contain it:

WHERE bucket >= now() - INTERVAL %d MINUTE
  AND bucket <= now()

and the equivalent AND time <= now() / AND start_time <= now() for the raw-table helpers.

2. Reject or clamp implausible event times at ingest (root cause).

In internal/ingest/server.go, bound StartTimeUnixNano / TimeUnixNano against receipt time — which is already computed as now at server.go:73 — with a configurable tolerance. Suggested defaults, matching what other receivers do:

• future tolerance: a few minutes (reject or clamp anything beyond)
• past tolerance: RetentionDays (rows older than retention will be pruned anyway; accepting them is a lie)

Open question for whoever picks this up: clamp or drop? Clamping preserves the row but fabricates a timestamp; dropping loses data but keeps the time axis honest. A third option is to keep the row at its original time and exclude out-of-bounds rows from rollups only. Recommend deciding this explicitly rather than by implementation accident, and emitting a counter either way so operators can see it happening.

A follow-on worth considering separately: OTLP has no equivalent of Sentry's sent_at envelope header ("used for clock drift correction of the event timestamp" — https://develop.sentry.dev/sdk/data-model/envelopes/), so Fanout cannot compute a real per-source drift correction today. Bounding is the pragmatic fix; drift correction would need a non-standard header and should not block this.

Non-goals

This issue deliberately stays inside the boundary set by #172. It does not propose:

• a Fanout mobile SDK, or direct mobile-to-Fanout ingestion
• per-app or public-client credentials, attestation, or CORS
• RUM sessions, session.id as a first-class dimension, or crash/symbolication pipelines
• rate limiting or quotas on the OTLP listeners

Verification

• A span or log with an event time far in the future does not appear in any of the six helpers for a window that does not contain it — rollup-backed and raw-table-backed alike.
• ServiceThroughput reports a rate consistent with the rows actually inside the window.
• A span with an event time older than RetentionDays is handled per the decision above, and the outcome is observable (counter or log), not silent.
• Rows with plausible timestamps are unaffected; existing rollup and retention tests stay green.
• A future-dated row no longer creates a permanently unprunable rollup bucket.
• Raw and rollup views agree on whether an out-of-bounds row exists, rather than one pruning it and the other retaining it.
• Both OTLP/gRPC and OTLP/HTTP go through the same validation — no transport-specific behavior (per Support OTLP/HTTP as backend compatibility, not a mobile SDK #172 §3).
• just check and just test-race pass.