Skip to content

Report prerequisite exposures and add an opt-in deferred tracking buffer - #135

Merged
madhuchavva merged 16 commits into
mainfrom
tracking-telemetry
Sep 5, 2026
Merged

Report prerequisite exposures and add an opt-in deferred tracking buffer#135
madhuchavva merged 16 commits into
mainfrom
tracking-telemetry

Conversation

@madhuchavva

@madhuchavva madhuchavva commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Closes #133

Summary

Two related changes:

  • Prerequisite telemetry (bug fix). When an experiment decides a prerequisite feature, the SDK now reports it. Previously eval_prereqs dropped every callback, so users were bucketed into real experiments that never showed up in analytics.
  • Deferred tracking (new, opt-in). A server can buffer the exposures an evaluation produces and forward them to a client SDK, which fires them in the browser with setDeferredTrackingCalls() + fireDeferredTrackingCalls(). This is the missing piece for SSR and remote-evaluation setups.

Replaces #133 with a different structure: callbacks live on the EvaluationContext (like the JS SDK's global context), so nested evaluations — prerequisites included — inherit them automatically instead of depending on every call site to pass them along. The deferred buffer is owned by the caller rather than stored on UserContext.

What changes for users

Prerequisite telemetry:

  • on_experiment_viewed fires for prerequisite experiment assignments, including when the gate ultimately fails (matches the JS SDK).
  • on_feature_usage fires for every feature an evaluation touches, prerequisites included — once per key per evaluation.
  • The sync client's subscribe() and get_all_results() now see prerequisite experiments.
  • The async client's subscriptions still fire only from run(). It has no per-user change-detection, so firing on every eval would repeat every subscriber callback on every request. (The JS multi-user client has no eval-time subscriptions either.)

Deferred tracking:

  • Sync: GrowthBook(defer_tracking=True), then get_deferred_tracking_calls() and clear_deferred_tracking_calls(). One instance per request when serving multiple users, as the README already prescribes.
  • Async: pass a per-request TrackingBuffer to the eval methods. The caller owns the buffer, so concurrent requests cannot mix.
  • Buffering is independent of on_experiment_viewed — both fire (same semantics as the Go SDK, Track passthrough and prerequisite exposures; deferred tracking; user context on ExperimentCallback (breaking) growthbook-golang#84).
  • Entries are JSON round-tripped when recorded, so the buffer is always json.dumps-ready. An exposure carrying a non-JSON value (say, a datetime attribute) is dropped with a log line; the rest of the batch is unaffected.
  • Receiving the forwarded user context requires JS SDK 1.7.0+.

Smaller fixes that fell out of review:

  • The tracking dedupe key is now a tuple of fields. The old concatenated string let two different exposures collide if a value contained the separator.
  • core.eval_feature / core.run_experiment still accept the old tracking_cb / callback_subscription keyword arguments. They are deprecated, emit a DeprecationWarning, and apply only to that call.
  • Experiment.to_dict() no longer turns an explicit coverage of 0 into 1.

Performance

Measured with python tests/scripts/benchmark_eval_overhead.py (500k sequential in-memory evals, no callbacks configured, best of 5):

path main this PR
default-value 1.29 us/eval 1.42 us/eval
experiment-rule 6.33 us/eval ~6.4 us/eval

The cheapest path costs about 0.13 us more per evaluation; a realistic experiment path is within ~1%. The remaining cost is the EvaluationContext carrying the callback fields — the mechanism that fixes the dropped prerequisite exposures — and is accepted deliberately (there is a Performance note in the CHANGELOG). Usage bookkeeping and the deprecated-kwarg shim are skipped entirely when unused.

Test plan

  • Full suite: pytest tests/ -q --ignore=tests/test_live.py — 972 passed. (Two test_typing.py mypy failures are a local urllib3-stub environment issue; they fail identically on main and pass in CI.)
  • pyright: 0 errors. Corpus freshness check: green.
  • New trackingCalls section in cases.json — 5 cases run by both the sync and async suites. It is a Python-local extension listed in the freshness checker, so it gets drift-checked automatically if the JS corpus adopts it.
  • End-to-end: tests/scripts/simulate_deferred_tracking.py runs SSR and concurrent-API scenarios against the published @growthbook/growthbook 1.7.0 in Node. 60 requests, 148 exposures fired with the exposure-time user context; expected values are derived from the SDK's own assignments, not hard-coded. Injected regressions (a dropped prerequisite exposure, a corrupted user) are caught.
  • tests/test_tracking_telemetry.py covers the buffer API: snapshot and JSON-drop semantics, dedupe collisions, legacy-kwarg scoping, async per-request isolation.

Follow-ups

  • Propose the trackingCalls section upstream in the JS SDK's shared cases.json — no SDK's spec asserts tracking today, which is how this bug stayed invisible everywhere.
  • Go SDK: revisit its buffer living on Client with clone-sharing, toward an eval-scoped collector like this design.
  • Report two JS SDK quirks upstream: a dedupe hit re-defers an already-fired exposure, and setting only an eventLogger silently disables deferral.

@greptile-apps

greptile-apps Bot commented Sep 4, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge; no outstanding or newly introduced actionable failures remain.

The changes since the previous review are empty, no repository-rule violations were identified, and the only previous finding was resolved after Greptile accepted that concurrent reuse falls outside the synchronous client's documented single-user contract.

Reviews (2): Last reviewed commit: "trim call overhead on the no-telemetry e..." | Re-trigger Greptile

Comment thread growthbook/growthbook.py
@madhuchavva

Copy link
Copy Markdown
Contributor Author

Re the summary above: the single P1 was discussed and resolved in the inline thread — the scenario (one sync instance shared across concurrent requests) is outside the sync client's single-user contract (instance-level attributes already race there), the README prescribes per-request instances, and the concurrent multi-user case is the async client's caller-owned per-request TrackingBuffer. Greptile concurred in the thread.

@madhuchavva

Copy link
Copy Markdown
Contributor Author

@greptileai

@madhuchavva
madhuchavva merged commit 4dac52f into main Sep 5, 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