From 60341f4f7331df0ff0280fdd4effcfb804a9801c Mon Sep 17 00:00:00 2001 From: Anna Garcia Date: Thu, 17 Sep 2026 22:53:02 -0400 Subject: [PATCH] docs(traces): evict aged spans only at the live-span bound Co-Authored-By: Claude Opus 5 (1M context) --- .../.openspec.yaml | 2 + .../design.md | 61 +++++++++++++++++ .../proposal.md | 65 +++++++++++++++++++ .../specs/traces/spec.md | 43 ++++++++++++ .../tasks.md | 21 ++++++ openspec/specs/traces/spec.md | 34 ++++++---- 6 files changed, 215 insertions(+), 11 deletions(-) create mode 100644 openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/.openspec.yaml create mode 100644 openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/design.md create mode 100644 openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/proposal.md create mode 100644 openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/specs/traces/spec.md create mode 100644 openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/tasks.md diff --git a/openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/.openspec.yaml b/openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/.openspec.yaml new file mode 100644 index 0000000..f2cbbe6 --- /dev/null +++ b/openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-18 diff --git a/openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/design.md b/openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/design.md new file mode 100644 index 0000000..362dd02 --- /dev/null +++ b/openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/design.md @@ -0,0 +1,61 @@ +## Context + +`Live span bounds` pairs a count bound (`maxLiveSpans`) with an age bound (`maxSpanAgeMs`). The +age bound was added only so a span leak cannot pin the count bound forever. The current text +applies it everywhere: any over-age live span is evicted at the next span start, end, or flush +tick. `posthog-node` implements that, sweeping on every `startSpan`. A review of the +`posthog-python` port (PostHog/posthog-python#953) showed the cost: a long span that its caller +does end is dropped, and its already-exported children are orphaned. The Python port changed to +sweep only at the count bound. + +## Goals / Non-Goals + +**Goals:** +- Keep leak recovery at the count bound exactly as strong as today. +- Stop dropping long spans that end normally below the bound. +- One rule both implementations follow, testable at the boundary. + +**Non-Goals:** +- Changing `maxSpanAgeMs`'s name, type, or default. +- Adding a timer, or a per-span lifetime independent of the bound. +- Surfacing leaks below the bound some other way. + +## Decisions + +### Sweep only when `startSpan` finds the bound reached + +**Decision.** At the bound, evict every live span whose age is at least `maxSpanAgeMs`, then +refuse with a no-op handle if, and only if, the bound is still reached. Never evict for age at +any other time. + +**Alternatives considered.** +- *Keep eviction everywhere (current text).* Drops legitimate long work and orphans children, + for no memory benefit: live bookkeeping is an id and a monotonic timestamp per span, already + capped by the count bound. +- *Make eviction below the bound optional (`MAY`).* Leaves `posthog-node` compliant with no + change, but lets two SDKs keep disagreeing about whether the same span is exported, which is + how this divergence arose. +- *Evict at end or on the flush tick as well.* Same data loss as today, only later. + +### Age boundary is inclusive + +Both SDKs evict a span whose age equals `maxSpanAgeMs` (the sweep stops at the first entry +strictly newer than the cutoff). The text says "at least" so a conformance test at the exact +boundary matches the implementations. + +## Risks / Trade-offs + +- [A leak below the bound is no longer counted or warned about] → Its cost is bounded by + `maxLiveSpans`; at the bound, the refusal and eviction both warn, as before. +- [A span that outlives `maxSpanAgeMs` in a process at the bound is still evicted] → + Unchanged from today and documented, including the mobile-background case. + +## Migration Plan + +`posthog-python` already implements the new rule in its unmerged traces stack. `posthog-node` +moves the sweep behind the bound check in `@posthog/core`, shipped as a patch. No public API +changes. Rollback is restoring the unconditional sweep. + +## Open Questions + +None. diff --git a/openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/proposal.md b/openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/proposal.md new file mode 100644 index 0000000..7a97c28 --- /dev/null +++ b/openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/proposal.md @@ -0,0 +1,65 @@ +## Why + +`Live span bounds` requires that no live span outlive `maxSpanAgeMs`: an over-age span is evicted +and never exported, checked lazily at the next span start, span end, or flush tick. The rule exists +for one reason, which the requirement states: eviction "prevents a span leak from permanently +disabling tracing for the rest of the process". + +Evicting below the count bound does nothing for that goal, and it drops real data. A review of the +`posthog-python` traces pipeline (PostHog/posthog-python#953) reproduced the failure: a root span +older than `maxSpanAgeMs` that its caller does end is never exported, and its children, already +exported, arrive as orphans whose `parentSpanId` never lands. The default is one hour, and a +batch-job or migration wrapper span longer than that is realistic. The reviewer's observation was +that "the leak-recovery goal is served equally well by sweeping only when +`len(_live_spans) >= max_live_spans`". + +Below the bound, an old live span costs nothing: every implementation tracks only an id and a +monotonic timestamp per live span, never the span itself, so a leaked handle stays collectable. +The count bound already caps that bookkeeping. + +The two SDKs that implement traces currently disagree: + +| SDK | When over-age spans are evicted | +| --- | --- | +| `posthog-node` (shared core, `_evictAgedSpans()` in `packages/core/src/traces/index.ts`) | on every `startSpan` — matches the current text | +| `posthog-python` (unmerged traces stack ending at PostHog/posthog-python#957) | only when `startSpan` finds the count bound reached — adopted from the #953 review | + +Node's behavior is also nondeterministic in the case it should settle: whether an over-age span +survives depends on whether any other span happened to start before it ended. + +## What Changes + +- Evict over-age spans only when `startSpan` finds the live-span count at `maxLiveSpans`, and + return a no-op handle if, and only if, the bound is still reached after that sweep. +- Forbid age eviction below the bound, so a long span that ends is exported with its full + duration. +- Recast `maxSpanAgeMs` as the leak-recovery threshold for the count bound, not a standalone + per-span lifetime. +- Narrow the mobile-background note: such a span is evicted only if the process reaches the bound + meanwhile. +- Add a scenario for a long span below the bound. The two existing scenarios still hold as + written. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `traces`: `Live span bounds` evicts over-age spans only at the count bound. + +## Impact + +- `posthog-python`: the unmerged traces stack already implements this. +- `posthog-node`: the sweep moves behind the bound check in `@posthog/core`, shipped + as a patch (PostHog/posthog-js, branch `fix/traces-age-sweep-at-bound`). No API change: + `maxSpanAgeMs` keeps its name, type, and default. +- Users lose no spans they get today. Spans that ran past `maxSpanAgeMs` and ended, previously + dropped with their children orphaned, are now exported. +- Leak recovery is unchanged: once leaks fill the bound, the next `startSpan` after they age out + evicts them and records a real span, as before. +- Leaks below the bound are no longer evicted, so they are no longer counted as drops or warned + about until the bound is reached. Their cost is an id and a timestamp each, capped by the bound. +- No other SDK implements traces yet, so no further ports are affected. diff --git a/openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/specs/traces/spec.md b/openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/specs/traces/spec.md new file mode 100644 index 0000000..40a215d --- /dev/null +++ b/openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/specs/traces/spec.md @@ -0,0 +1,43 @@ +## MODIFIED Requirements + +### Requirement: Live span bounds + +The SDK SHALL bound live spans: at most `maxLiveSpans` concurrently. `maxSpanAgeMs` is the +leak-recovery threshold for that bound. Its default SHALL comfortably exceed the platform's +realistic worst-case trace duration — on the order of an hour, not minutes (production PostHog +traces routinely exceed 10 minutes); mobile ports MAY choose a larger default since backgrounded +time counts. Age SHALL be the monotonic elapsed time since handle creation, independent of any +caller-supplied `startTime` — a backdated span is not instantly evicted, a future-dated +leaked span cannot evade the bound, and wall-clock jumps do not affect it. + +At the count bound, `startSpan` SHALL first evict from live accounting every live span whose +age is at least `maxSpanAgeMs` — an evicted handle becomes a no-op (never exported) — and SHALL +return a no-op handle if, and only if, the bound is still reached. Eviction prevents a span leak +from permanently disabling tracing for the rest of the process. The SDK SHALL NOT evict a span +for age at any other time: a live span past `maxSpanAgeMs` that has not been evicted at the +bound SHALL, when it ends, go through the same end-time gates and export path as any other +span. Evicting it would drop legitimate long work (a batch job, a migration) and leave its +already-exported children pointing at a `parentSpanId` that never lands. No dedicated timer is required. A span spanning a long mobile background interval can +still be evicted if the process reaches the count bound meanwhile; that is expected behavior, +not a defect. All span drops — live-bound refusals, age evictions, queue overflow, and +end-time gate drops — SHALL increment a single per-instance dropped-spans counter, with a +warning at most once per flush interval naming the count and reason mix. + +#### Scenario: leaked spans are bounded +- **GIVEN** the live-span bound is reached because spans are started but never ended +- **WHEN** the app calls `startSpan` again +- **THEN** it receives a no-op handle, the dropped count increments, and SDK memory does not + grow further + +#### Scenario: age eviction re-enables tracing +- **GIVEN** `maxLiveSpans` handles leaked (started, never ended) longer than `maxSpanAgeMs` + ago +- **WHEN** the age bound passes and the app calls `startSpan` +- **THEN** the leaked spans are evicted (never exported) and the new span is a real span + +#### Scenario: a long span below the bound is exported +- **GIVEN** fewer than `maxLiveSpans` live spans, one of them started longer than + `maxSpanAgeMs` ago +- **WHEN** the app starts another span, then ends the old one +- **THEN** the old span is not evicted, and it passes the end-time gates and is exported with + its full duration like any other span diff --git a/openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/tasks.md b/openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/tasks.md new file mode 100644 index 0000000..c440fae --- /dev/null +++ b/openspec/changes/archive/2026-09-18-evict-aged-spans-only-at-live-bound/tasks.md @@ -0,0 +1,21 @@ +## 1. Establish the Contract + +- [x] 1.1 Read when each traces implementation evicts over-age spans: `posthog-node` on every `startSpan`, `posthog-python` only at the count bound. +- [x] 1.2 Confirm the requirement's stated purpose for age eviction is leak recovery at the count bound. +- [x] 1.3 Confirm live-span bookkeeping holds only ids and timestamps in both SDKs, so an old live span below the bound costs nothing. +- [x] 1.4 Reproduce the loss: a span older than `maxSpanAgeMs` that ends is dropped under the current text, and its exported children are orphaned (PostHog/posthog-python#953 review). +- [x] 1.5 Confirm both SDKs evict a span whose age equals `maxSpanAgeMs`, and word the boundary to match. + +## 2. Write the Delta + +- [x] 2.1 Rewrite `Live span bounds` to evict over-age spans only when `startSpan` finds the bound reached, return a no-op handle if, and only if, the bound is still reached, and forbid age eviction at any other time. +- [x] 2.2 Narrow the mobile-background note to the at-bound case. +- [x] 2.3 Add the `a long span below the bound is exported` scenario. + +## 3. Validate and Review + +- [x] 3.1 Run `openspec validate --specs --strict --no-interactive` and `openspec validate evict-aged-spans-only-at-live-bound --strict`. +- [x] 3.2 Run `git diff --check`. +- [x] 3.3 Prepare the matching `posthog-node` change in PostHog/posthog-js (`fix/traces-age-sweep-at-bound`), with a test that fails on the current sweep. +- [x] 3.4 Decide whether acceptance scenarios belong in `acceptance/`. Spec scenarios suffice: `acceptance/` carries no traces feature file yet. +- [x] 3.5 Archive into the canonical `traces` spec. diff --git a/openspec/specs/traces/spec.md b/openspec/specs/traces/spec.md index 81e3a0b..0bcf035 100644 --- a/openspec/specs/traces/spec.md +++ b/openspec/specs/traces/spec.md @@ -726,18 +726,23 @@ but breaks assembled traces). ### Requirement: Live span bounds -The SDK SHALL bound live spans: at most `maxLiveSpans` concurrently, and no live span older -than `maxSpanAgeMs`. The default SHALL comfortably exceed the platform's realistic worst-case -trace duration — on the order of an hour, not minutes (production PostHog traces routinely -exceed 10 minutes); mobile ports MAY choose a larger default since backgrounded time counts. -Age SHALL be the monotonic elapsed time since handle creation, independent of any +The SDK SHALL bound live spans: at most `maxLiveSpans` concurrently. `maxSpanAgeMs` is the +leak-recovery threshold for that bound. Its default SHALL comfortably exceed the platform's +realistic worst-case trace duration — on the order of an hour, not minutes (production PostHog +traces routinely exceed 10 minutes); mobile ports MAY choose a larger default since backgrounded +time counts. Age SHALL be the monotonic elapsed time since handle creation, independent of any caller-supplied `startTime` — a backdated span is not instantly evicted, a future-dated -leaked span cannot evade the bound, and wall-clock jumps do not affect it. At the count bound, `startSpan` -returns a no-op handle. A live span exceeding the age bound SHALL be evicted from live -accounting and its handle becomes a no-op (never exported) — eviction prevents a span leak -from permanently disabling tracing for the rest of the process. Eviction MAY be evaluated -lazily — at the next span start, span end, or flush tick — no dedicated timer is required. A span legitimately spanning -a long mobile background interval will be evicted by this rule; that is expected behavior, +leaked span cannot evade the bound, and wall-clock jumps do not affect it. + +At the count bound, `startSpan` SHALL first evict from live accounting every live span whose +age is at least `maxSpanAgeMs` — an evicted handle becomes a no-op (never exported) — and SHALL +return a no-op handle if, and only if, the bound is still reached. Eviction prevents a span leak +from permanently disabling tracing for the rest of the process. The SDK SHALL NOT evict a span +for age at any other time: a live span past `maxSpanAgeMs` that has not been evicted at the +bound SHALL, when it ends, go through the same end-time gates and export path as any other +span. Evicting it would drop legitimate long work (a batch job, a migration) and leave its +already-exported children pointing at a `parentSpanId` that never lands. No dedicated timer is required. A span spanning a long mobile background interval can +still be evicted if the process reaches the count bound meanwhile; that is expected behavior, not a defect. All span drops — live-bound refusals, age evictions, queue overflow, and end-time gate drops — SHALL increment a single per-instance dropped-spans counter, with a warning at most once per flush interval naming the count and reason mix. @@ -754,6 +759,13 @@ warning at most once per flush interval naming the count and reason mix. - **WHEN** the age bound passes and the app calls `startSpan` - **THEN** the leaked spans are evicted (never exported) and the new span is a real span +#### Scenario: a long span below the bound is exported +- **GIVEN** fewer than `maxLiveSpans` live spans, one of them started longer than + `maxSpanAgeMs` ago +- **WHEN** the app starts another span, then ends the old one +- **THEN** the old span is not evicted, and it passes the end-time gates and is exported with + its full duration like any other span + ### Requirement: Flush triggers The SDK SHALL flush on each applicable trigger: (1) a repeating timer at `flushIntervalMs`; (2)