Skip to content

feat(telemetry): track per-feature usage counters on compile events#6519

Merged
masenf merged 9 commits into
reflex-dev:mainfrom
FarhanAliRaza:telemetry-features
May 19, 2026
Merged

feat(telemetry): track per-feature usage counters on compile events#6519
masenf merged 9 commits into
reflex-dev:mainfrom
FarhanAliRaza:telemetry-features

Conversation

@FarhanAliRaza
Copy link
Copy Markdown
Contributor

@FarhanAliRaza FarhanAliRaza commented May 15, 2026

Introduce a typed FeatureName registry and increment_feature helper so runtime call sites (uploads, cookies, storage, models, lifespan tasks, shared state, dynamic routes) and the compile-time collector (state-manager mode, CORS, background event handlers) feed a uniform counters map into the compile event. Always emit every known key so zeros are distinguishable from missing detectors.

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

New Feature Submission:

  • Does your submission pass the tests?
  • Have you linted your code locally prior to submission?

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

fixes ENG-9480

Introduce a typed FeatureName registry and `increment_feature` helper so
runtime call sites (uploads, cookies, storage, models, lifespan tasks,
shared state, dynamic routes) and the compile-time collector
(state-manager mode, CORS, background event handlers) feed a uniform
counters map into the compile event. Always emit every known key so
zeros are distinguishable from missing detectors.
@FarhanAliRaza FarhanAliRaza requested a review from a team as a code owner May 15, 2026 21:54
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 15, 2026

Merging this PR will not alter performance

✅ 24 untouched benchmarks


Comparing FarhanAliRaza:telemetry-features (d136224) with main (447db31)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (509855b) during the generation of this report, so 447db31 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 15, 2026

Greptile Summary

This PR introduces a typed FeatureName registry and increment_feature helper so that runtime call sites (uploads, cookies, storage, models, lifespan tasks, shared state, dynamic routes) and compile-time collectors (state-manager mode, CORS, background event handlers) feed a uniform counters map into the compile PostHog event, with every known key always present so zeros are distinguishable from missing detectors.

  • Adds FeatureName Literal, _KNOWN_FEATURES tuple, process-level _recorded_features dict, and increment_feature in telemetry_context.py; wires call sites across storage.py, shared.py, model.py, lifespan.py, app.py, and upload.py.
  • Introduces _collect_features_used in telemetry_accounting.py that merges persistent pre-context counts, config attestations, background-handler counts, and in-context counts into the final snapshot.
  • Adds comprehensive unit tests including an autouse fixture that resets _recorded_features between tests — an explicit acknowledgment that the global dict needs lifecycle management that the production code does not yet enforce.

Confidence Score: 3/5

Safe to merge for application correctness, but the telemetry data it produces will be inaccurate in multi-compile (hot-reload) scenarios due to two merging bugs in the accounting layer.

The _recorded_features global is never cleared after being merged into a compile payload, so pre-context import-time counts carry forward into every subsequent hot-reload compile unchanged. More critically, _collect_features_used uses dict.update(ctx.features_used) as its final merge step, which silently overwrites persistent counts with context counts for any key that appears in both — dropping the pre-context portion of that feature's total. Both issues affect the accuracy of the telemetry data being collected, and the autouse test fixture that explicitly resets _recorded_features between tests signals awareness of the problem without a matching production-side fix.

reflex/utils/telemetry_accounting.py (_collect_features_used merge logic) and reflex/utils/telemetry_context.py (_recorded_features lifecycle)

Important Files Changed

Filename Overview
reflex/utils/telemetry_context.py Introduces FeatureName Literal type, _KNOWN_FEATURES tuple, process-level _recorded_features dict, and increment_feature helper; the module-level _recorded_features dict is never reset between compile events.
reflex/utils/telemetry_accounting.py Adds _collect_features_used which merges persistent and context feature dicts; the final features.update(ctx.features_used) overwrites (not adds) persistent values for keys that appear in both dicts, silently dropping pre-context counts.
reflex/istate/storage.py Adds increment_feature calls in Cookie.new, LocalStorage.new, and SessionStorage.new; these trigger at class-definition (import) time so they correctly land in the persistent dict before any compile context.
reflex/istate/shared.py Adds increment_feature("shared_state_count") at the end of SharedState.init_subclass; fires at class-definition time and correctly accumulates in the persistent dict.
reflex/app.py Increments dynamic_routes_count inside add_page when route has dynamic args; since add_page is typically called before any compile context is active, this lands in the persistent dict and is subject to the non-clearing concern.
reflex/app_mixins/lifespan.py Increments lifespan_tasks_count for non-reflex-module tasks using a module filter; module attribution for asyncio.Task registrations could misattribute, but the common callable path is correct.
reflex/model.py Increments db_model_count when a model class is registered via the @Model.create_all decorator; fires at class definition time, correctly lands in persistent dict.
tests/units/utils/test_telemetry_accounting.py Adds comprehensive tests for the new feature counter system; autouse fixture explicitly clears _recorded_features between tests, confirming this dict needs manual management that production code doesn't provide.
packages/reflex-components-core/src/reflex_components_core/core/upload.py Adds increment_feature("upload_count") in Upload.create(); fires during page compilation which is inside the telemetry context, so it correctly lands in ctx.features_used.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Import time / class definitions] -->|Cookie, SharedState, Model.add| B[increment_feature]
    C[App setup / add_page] -->|dynamic routes| B
    D[register_lifespan_task] -->|user tasks only| B

    B -->|No active context| E[_recorded_features\nglobal dict]
    B -->|Active context| F[ctx.features_used\nper-compile dict]

    G[Compile starts] --> H[TelemetryContext created & entered]
    H --> I[_compile_page / Upload.create] -->|upload_count| F

    J[record_compile called] --> K[_collect_features_used]
    E -->|features.update| K
    L[_record_config_attestations\nstate-manager, CORS] --> K
    M[_record_background_handler_count\nwalk user states] --> K
    F -->|features.update OVERWRITES| K

    K --> N[features_used snapshot\nall _KNOWN_FEATURES initialized to 0]
    N --> O[PostHog compile event]

    style E fill:#f9c,stroke:#c66
    style F fill:#9cf,stroke:#66c
    style K fill:#fc9,stroke:#c96
Loading

Reviews (1): Last reviewed commit: "feat(telemetry): track per-feature usage..." | Re-trigger Greptile

Comment thread reflex/utils/telemetry_accounting.py Outdated
Comment thread reflex/utils/telemetry_accounting.py Outdated
Comment thread reflex/app.py Outdated
Move TelemetryContext into reflex_base and drop the scattered
`increment_feature` call sites in Upload, lifespan, shared state,
storage classes, and Model. Compile-event collection now walks the
live app (states, pages, config, ModelRegistry) at compile end so the
counters reflect actual final state rather than import-time markers
that could double-count under reload or get out of sync.
…collector

Drop the intermediate ``_ComponentWalk`` TypedDict in favor of a plain
tuple, pass the resolved ``Config`` and ``get_route_args`` into the
feature collector instead of re-resolving them, and lean on the
``_KNOWN_FEATURES`` zero-fill to drop the redundant storage-counter
seeding pass.
Alek99
Alek99 previously requested changes May 15, 2026
Comment thread reflex/utils/telemetry_accounting.py
State classes that inherit Cookie/LocalStorage/SessionStorage fields were
re-counting the parent's fields on each descendant. Walk only the fields
declared on each class so the per-feature counters reflect distinct
storage vars across the state tree.
Comment thread tests/integration/test_telemetry_compile.py Outdated
Comment thread tests/units/utils/test_telemetry_accounting.py Outdated
Comment thread tests/units/utils/test_telemetry_accounting.py
Comment thread tests/units/utils/test_telemetry_accounting.py Outdated
FarhanAliRaza and others added 4 commits May 19, 2026 00:16
Co-authored-by: Masen Furer <m_github@0x26.net>
Rename `_TelAcct*`, `_Storage*`, `_Shared*`, `_BgState`, `_UserWalkState`,
`_StubMemoWrapper`, and `_PlainStub` to their non-underscored equivalents,
and construct `Upload`/`StyledUpload` via `create()` instead of
`object.__new__`.
Comment thread reflex/utils/telemetry_accounting.py Outdated
Comment thread reflex/utils/telemetry_accounting.py Outdated
The compile-event walk was paying for an ``isinstance(node, Upload)``
on every component in every page tree to derive ``upload_count``, which
profiled at ~20% of total walk overhead. ``Upload.is_used`` is already
maintained by upload call sites for the upload-endpoint mount gate;
read it directly off the class instead.

- Restore ``_count_components`` to its single-purpose, class-name-only
  shape (no tuple return, no Upload special case).
- Drop the ``upload_count`` parameter from ``_collect_features_used``;
  it now reads ``int(Upload.is_used)``.
- No change to ``Upload`` itself.
@masenf
Copy link
Copy Markdown
Collaborator

masenf commented May 19, 2026

we can keep the walk for now, overall it's not contributing a significant amount to the runtime; we can optimize it later if it turns out to be more significant

@masenf masenf dismissed Alek99’s stale review May 19, 2026 23:12

issue was fixed and Alek is at BTS concert

@masenf masenf merged commit 5d65aa9 into reflex-dev:main May 19, 2026
70 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.

3 participants