Skip to content

Commit c2999af

Browse files
committed
design: Toplevel exit and trace gating
design-docs/adr/0015-balanced-toplevel-lifecycle-and-trace-gating.md: design-docs/toplevel-exit-and-trace-gating-implementation-plan.md: Signed-off-by: Tzanko Matev <[email protected]>
1 parent 98f0f08 commit c2999af

File tree

2 files changed

+158
-0
lines changed

2 files changed

+158
-0
lines changed
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# ADR 0015: Balanced Toplevel Lifecycle and Unified Trace Gating
2+
3+
- **Status:** Proposed
4+
- **Date:** 2025-03-21
5+
- **Deciders:** codetracer recorder maintainers
6+
- **Consulted:** DX tooling stakeholders, runtime tracing SMEs
7+
- **Informed:** Support engineering, product analytics, replay consumers
8+
9+
## Context
10+
- The recorder seeds every trace with a synthetic `<toplevel>` call when `TraceWriter::start` is invoked from `TraceOutputPaths::configure_writer` (`codetracer-python-recorder/src/runtime/output_paths.rs`). That call models the Python process entrypoint but the runtime never emits the matching return edge.
11+
- CLI and API entrypoints (`codetracer_python_recorder/cli.py`, `codetracer_python_recorder/session.py`) already capture the script's exit status, yet the Rust runtime is oblivious to it, so the trace file looks like the script is still running when recording ends.
12+
- Runtime gating currently combines two orthogonal systems: the legacy activation controller (`codetracer-python-recorder/src/runtime/activation.rs`) that defers tracing until a configured file executes, and the newer `TraceFilterEngine` (`codetracer-python-recorder/src/runtime/tracer/filtering.rs`) that offers scope-level allow/deny decisions. Both mechanisms decide whether an event should be written, but they execute independently and cache their own state.
13+
- Because activation and filtering have separate caches and lifecycle hooks, downstream policies (value capture, IO flushes) see inconsistent state: a filter-suppressed frame still triggers activation bookkeeping, and activation suspensions do not propagate into the filter cache. The split also makes it hard to reason about which events will be recorded in the presence of chained filters.
14+
15+
## Problem
16+
- **Unbalanced traces:** Without a `<toplevel>` return, consumers reconstructing the call stack see a dangling activation at depth 0. This breaks invariants in `TraceWriter::finish_writing_trace_events`, forces replay tools to special-case the synthetic frame, and hides the script's exit code even though callers already have it.
17+
- **Duplicated gating logic:** Activation and filter decisions contradict one another in edge cases. For example:
18+
- When activation gates tracing until a file runs, the filter still caches a `TraceDecision::Trace` for the same code object, so subsequent resumes bypass activation because the filter short-circuits to "disable location".
19+
- When filters skip a frame, activation has no way to learn that the frame completed; its suspended/completed bookkeeping only triggers on return events that never fire for filter-disabled frames.
20+
- The divergent implementations increase bug surface area (e.g., dangling activations, stale filter caches) and make it challenging to add new recorder policies that need a consistent view of "is this event observable?".
21+
22+
## Decision
23+
1. **Emit a `<toplevel>` return carrying process exit status.**
24+
- Extend the PyO3 surface so `stop_tracing` accepts an optional integer exit code (default `None`). The Python helpers (`session.stop`, CLI `main`) will pass the script's final status.
25+
- Add a `TraceSessionGuard` helper on the Rust side that stores the provided exit status until `RuntimeTracer::finish` runs. When `finish` executes, it must:
26+
- Flush pending IO.
27+
- Record the exit status via `TraceWriter::register_return`, tagging the payload as `<exit>` when the code is unknown (e.g., interpreter crash) and serialising the integer otherwise.
28+
- Only then call the existing `finalise`/`cleanup` routines.
29+
- If tracing aborts early (`OnRecorderError::Disable` or fatal errors), emit a `<toplevel>` return with a synthetic reason (`"<disabled>"` / captured exception) so the stack always balances.
30+
2. **Unify activation and filter decisions behind a shared gate.**
31+
- Introduce a `TraceGate` service managed by `RuntimeTracer`. The gate combines activation state and filter results into a single `GateDecision { process_event, disable_location, activation_event }`.
32+
- `FilterCoordinator` becomes responsible for caching scope resolutions only when `TraceGate` reports that the frame was actually processed. When the gate denies an event, both activation and filter caches are notified so they can mark the code id as "ignored" in lockstep.
33+
- `ActivationController` exposes explicit transitions (`on_enter`, `on_suspend`, `on_exit`) rather than letting callbacks poke its internal flags. `TraceGate` translates filter outcomes into the appropriate activation transition (e.g., a filter skip counts as `on_exit` so suspended activations resume correctly).
34+
- All tracer callbacks (`on_py_start`, `on_py_return`, `on_py_yield`, etc.) ask the gate for a decision before doing work. They honour `disable_location` uniformly, so CPython stops invoking us for code objects that either filter or activation wants to suppress.
35+
- Document the merged semantics: activation remains a coarse gate (enabling/disabling the root frame), filters apply fine-grained scope policies, and both share a single cache lifetime tied to tracer flush/reset.
36+
3. **Update metadata and tooling expectations.**
37+
- Persist the recorded exit status into trace metadata (`trace_metadata.json`) so downstream tools can rely on it without scanning events.
38+
- Update docs and integration tests to assert that traces end at stack depth zero, even when activation suspends/resumes or filters drop frames.
39+
40+
## Consequences
41+
- **Benefits**
42+
- Trace consumers no longer see dangling `<toplevel>` activations, and they can surface the script exit status directly from the trace file.
43+
- Activation, filtering, value capture, and IO policies share a single gating decision, reducing state divergence and simplifying future features (e.g., per-filter activation windows).
44+
- Error paths become easier to reason about because every exit funnels through the same `<toplevel>` return emission.
45+
- **Costs**
46+
- API changes propagate through the Python bindings (`stop_tracing`, `TraceSession.stop`, CLI), requiring coordination with users embedding the recorder programmatically.
47+
- The gate abstraction adds code churn in `runtime/tracer/events.rs` and related helpers as callbacks adopt the new decision API.
48+
- Metadata writers must update to include the exit status.
49+
- **Risks**
50+
- Forgetting to pass the exit status from bespoke integrations (custom Python entrypoints) would regress behaviour back to "unknown exit". We mitigate this with a backwards-compatible default (`None` translates to `<unknown>` exit) and clear release notes.
51+
- A buggy gate implementation could over-disable callbacks, suppressing legitimate trace data. We will add regression tests covering activation+filter combinations (activation path inside a skipped scope, resumed generators, etc.) before rollout.
52+
- The PyO3 signature change may break ABI expectations if not versioned carefully. We will bump the crate minor version and document the new keyword argument.
53+
54+
## Alternatives
55+
- **Emit the `<toplevel>` return entirely on the Python side.** Rejected because it would duplicate writer logic in Python, bypass IO flush/value capture, and fail when users call the PyO3 API directly from Rust.
56+
- **Keep activation and filter gating separate but document the quirks.** Rejected: we already hit real bugs (unbalanced traces, stale caches), and layering more documentation will not solve the underlying inconsistency.
57+
- **Deprecate activation now that filters exist.** Rejected because activation provides a simple UX for "start tracing when my script begins", which filters alone cannot replace without writing bespoke configs.
58+
59+
## References
60+
- `codetracer-python-recorder/src/runtime/output_paths.rs`
61+
- `codetracer-python-recorder/src/runtime/tracer/events.rs`
62+
- `codetracer-python-recorder/src/runtime/activation.rs`
63+
- `codetracer-python-recorder/src/runtime/tracer/filtering.rs`
64+
- `codetracer_python_recorder/session.py`
65+
- `codetracer_python_recorder/cli.py`
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
# Toplevel Exit & Trace Gating – Implementation Plan
2+
3+
Plan owners: codetracer recorder maintainers
4+
Target ADR: 0015 – Balanced Toplevel Lifecycle and Unified Trace Gating
5+
Impacted components:
6+
- `codetracer-python-recorder/src/session.rs` and `codetracer_python_recorder/session.py`
7+
- `codetracer-python-recorder/src/runtime/tracer` (events, lifecycle, filtering)
8+
- `codetracer-python-recorder/src/runtime/activation.rs`
9+
- `codetracer-python-recorder/src/runtime/output_paths.rs` and metadata helpers
10+
- `codetracer-pure-python-recorder` parity shims (optional but strongly recommended)
11+
12+
## Goals
13+
- Always emit a `<toplevel>` return event whose payload reflects the process exit status (or a descriptive placeholder when unavailable).
14+
- Plumb exit codes from Python entrypoints through the PyO3 API into the Rust runtime without breaking existing integrations.
15+
- Replace the ad-hoc combination of activation and filter decisions with a single gate so callbacks make consistent trace/skip/disable choices.
16+
- Keep lifecycle bookkeeping (IO flush, value capture, activation teardown) in sync with the unified gate and the new exit record.
17+
- Extend metadata (`trace_metadata.json`) with the recorded exit status for downstream tooling.
18+
19+
## Non-Goals
20+
- No changes to the on-disk trace schema beyond the new return record payload; we keep the existing call/return/line structure.
21+
- No removal of activation support; the work only refactors it to cooperate with filters.
22+
- No immediate addition of exit-status reporting to the CLI JSON trailers (can be follow-up).
23+
- No attempt to refit the pure-Python recorder in the same PR; it may gain parity later but does not block landing the Rust changes.
24+
25+
## Current Gaps
26+
- `stop_tracing` (PyO3) accepts no arguments, so the runtime never learns the script exit status captured by `codetracer_python_recorder/cli.py`.
27+
- `RuntimeTracer::finish` only finalises writers; it does not record any return edge for the synthetic `<toplevel>` call emitted in `TraceWriter::start`.
28+
- Activation and filtering are checked independently inside each callback (`on_py_start`, `on_py_return`, etc.), leading to divergent cache state (`ActivationController::suspended` vs `FilterCoordinator::ignored_code_ids`).
29+
- Filter-driven `CallbackOutcome::DisableLocation` does not inform the activation controller, so the activation window can remain "active" after CPython stops issuing callbacks.
30+
- Metadata writers do not persist exit status, and tests assume partial stacks are acceptable.
31+
32+
## Workstreams
33+
34+
### WS1 – Public API & Session Plumbing
35+
**Scope:** Carry exit status from Python to Rust with backwards-compatible defaults.
36+
- Update `codetracer-python-recorder/src/session.rs::stop_tracing` to expose an optional `exit_code: Option<i32>` parameter (new keyword-only arg in Python).
37+
- Adjust `codetracer_python_recorder/session.py` so `TraceSession.stop` and the module-level `stop()` accept an optional exit code and forward it.
38+
- Modify `codetracer_python_recorder/cli.py::main` to pass the captured `exit_code` when stopping the session; preserve legacy behaviour (`None`) for callers that do not provide a code.
39+
- Add unit tests in `codetracer_python_recorder/tests` ensuring the new keyword argument is optional and that `stop(exit_code=123)` calls into the backend with the expected value (mocking PyO3 layer).
40+
41+
### WS2 – Runtime Exit State & `<toplevel>` Return Emission
42+
**Scope:** Store the exit status and emit the balancing return event.
43+
- Introduce a small struct (e.g., `SessionTermination`) inside `RuntimeTracer` to hold `exit_code: Option<i32>` plus a `reason: ExitReason`.
44+
- Extend the `Tracer` trait implementation with a new method (e.g., `set_exit_status`) or reuse `notify_failure` paths to capture both normal exit and disable scenarios.
45+
- In `RuntimeTracer::finish`, before `finalise`:
46+
- Call `record_return_value` with the exit payload.
47+
- Invoke `TraceWriter::register_return` / `mark_event`.
48+
- Ensure activation receives an `ActivationExitKind::Completed` for the toplevel code id.
49+
- Emit synthetic reasons (`"<disabled>"`, `"<panic>"`, etc.) when tracing stops due to errors. Reuse existing error-path metadata in `notify_failure` to populate the reason.
50+
- Update integration tests (Rust + Python) to assert that the final event sequence includes a `<toplevel>` call + return pair and that the return payload matches the script exit code.
51+
52+
### WS3 – Unified Trace Gate Abstraction
53+
**Scope:** Merge activation and filter decision paths.
54+
- Create `TraceGate` and `GateDecision` types under `runtime/tracer`.
55+
- API: `evaluate(py, code, event_kind) -> GateDecision`.
56+
- Decision carries: `process_event` (bool), `disable_location` (bool), `activation_transition` (enum).
57+
- Refactor `FilterCoordinator` so it exposes `decide_for_gate(py, code)` returning an enriched result (trace/skip plus cached scope resolution). The coordinator only updates caches when the gate confirms the event was processed.
58+
- Update `ActivationController` with methods `on_enter`, `on_suspend`, `on_exit`, and `reset`. Remove direct field mutations from callbacks.
59+
- Rewrite tracer callbacks (`on_py_start`, `on_py_return`, `on_py_resume`, etc.) to:
60+
1. Ask the gate for a decision.
61+
2. Exit early when `process_event == false` and return the proper `CallbackOutcome`.
62+
3. After recording the event, invoke any activation transition included in the decision.
63+
- Ensure `CallbackOutcome::DisableLocation` is only returned once per code id and that both activation and filter caches mark the frame as ignored thereafter.
64+
- Unit test the gate with synthetic `CodeObjectWrapper` fixtures covering combinations: inactive activation + allow filter, active activation + skip filter, suspended activation resumed, etc.
65+
66+
### WS4 – Lifecycle & Metadata Updates
67+
**Scope:** Keep writer lifecycle and metadata consistent with the new behaviour.
68+
- Update `LifecycleController::finalise` (or adjacent helper) to write the exit status into `trace_metadata.json` under a new field (e.g., `"process_exit_code"`). Ensure this runs only once per session.
69+
- Confirm `cleanup_partial_outputs` still executes when tracing disables early and that the exit record is only written for successful sessions.
70+
- Add a regression test around `TraceOutputPaths::configure_writer` + `RuntimeTracer::finish` verifying the events buffer contains both call and return entries for `<toplevel>`.
71+
- Update documentation (`design-docs/design-001.md`, user docs) to describe the exit-status metadata and the unified gating semantics.
72+
73+
### WS5 – Validation & Parity Follow-Up
74+
**Scope:** Prove end-to-end correctness and plan the optional pure-Python update.
75+
- Extend Python integration tests (`tests/python`) with scenarios:
76+
- CLI run that exits with non-zero status; assert trace contains `<toplevel>` return with the negative path.
77+
- Activation path configured alongside a filter that skips the same file; ensure tracing starts/stops exactly once and stack depth ends at zero.
78+
- Generator/coroutine workloads guarded by activation; confirm gate decisions do not regress existing balanced-call tests.
79+
- Run `just test` to cover all tests after refactors.
80+
- Document a follow-up issue to mirror `<toplevel>` return emission in `codetracer-pure-python-recorder`, keeping trace semantics aligned across products.
81+
82+
## Testing & Rollout Checklist
83+
- [ ] `just test`
84+
- [ ] Python integration tests covering exit-code propagation and activation+filter combinations
85+
- [ ] Manual smoke test: run CLI against a script returning exit code 3, inspect `trace.json` for `<toplevel>` return payload `3`
86+
- [ ] Update changelog / release notes highlighting the new API parameter and exit-status metadata
87+
- [ ] Notify downstream data pipeline owners that exit status is now available
88+
89+
## Risks & Mitigations
90+
- **Breaking API changes:** Ensure `stop_tracing` still works without arguments by providing a Python default (`exit_code: int | None = None`) and by releasing under a minor version bump.
91+
- **Gate regressions:** Add exhaustive unit tests plus targeted integration tests so we catch scenarios where activation or filters no longer fire.
92+
- **Performance impact:** Benchmark tracing hot paths after the refactor; the gate should add minimal overhead. Profile with `just bench` / existing benchmarks, and roll back micro-optimisations if regressions exceed 5%.
93+
- **Incomplete error coverage:** Make sure disable/error paths still flush IO and write metadata. Write explicit tests that trigger `OnRecorderError::Disable` to observe the synthetic `<toplevel>` return reason.

0 commit comments

Comments
 (0)