Preserve RegistrationContext in telemetry worker threads - #6960
Conversation
…hreads test_get_config_loads_once_for_shared_context patches reflex_base.config._load_config and asserts it ran exactly once. The counter was process-wide, so any background thread that called get_config() inside the patched window was counted as a duplicate load of the shared context. The telemetry worker thread does exactly that: _process_event -> _send -> _prepare_event -> get_event_defaults -> get_bun_version -> path_ops.get_bun_path -> get_config(). That thread carries no RegistrationContext, so ensure_context() creates a fresh one and loads the config for it. When an event queued by an earlier test drains during this test, load_count reaches 2 and the assert fails (seen on Windows CI). Count only loads made from the test's worker threads, via a thread-local marker, so an unserialized load path for the shared context still fails the test while unrelated background loads are ignored.
The telemetry worker thread carries no RegistrationContext, so a config lookup during event collection (_prepare_event -> get_event_defaults -> get_bun_version -> path_ops.get_bun_path -> get_config()) attached a throwaway context to the worker and re-imported rxconfig.py off-thread — duplicating work the caller had already done, against a config the app is not using, and mutating sys.path from a background thread while doing it. _submit now captures the caller's RegistrationContext (without attaching one to a caller that has none) and _run_suppressed attaches it for the duration of the job, resetting it afterwards so a context never leaks into the next job on the reused worker thread. Collection therefore reuses the already-loaded Config instead of loading its own. Verified across the unit suite: instrumenting _load_config to log calls from non-main threads reported one such load before this change and none after.
Greptile SummaryThis PR propagates the submitting thread’s RegistrationContext into telemetry jobs and resets that temporary attachment after execution.
Confidence Score: 4/5The PR is not yet safe to merge because context-free telemetry jobs can still leak worker-local configuration and registration state into later jobs. The single telemetry worker is reused, while the new cleanup resets only a propagated submitter context; a context created by get_config() during a context-free job remains attached and is visible to subsequent context-free work. Files Needing Attention: reflex/utils/telemetry.py
|
| Filename | Overview |
|---|---|
| reflex/utils/telemetry.py | Adds submitter-context propagation and token-based restoration, but the previously reported context-free worker state leak remains. |
| tests/units/test_telemetry.py | Covers propagation and cleanup after a context-bearing job, but does not exercise a context-free job that creates its own worker context. |
| tests/units/test_config.py | Restricts load counting to the test’s participating threads to avoid unrelated telemetry activity. |
| news/6960.bugfix.md | Documents the intended telemetry context-preservation behavior. |
Reviews (4): Last reviewed commit: "telemetry: document the context-less-job..." | Re-trigger Greptile
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
Review completed against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
resolved issues via documenting the edge case instead of adding a bunch of complexity for a case which is rarely hit and doesn't really cause significant issue when it is hit. |
Type of change
Description
This PR fixes an issue where telemetry event collection in worker threads would lose the submitting thread's
RegistrationContext, causing the worker to create a throwaway context and re-importrxconfig.pyunnecessarily.The Problem:
When telemetry jobs are submitted via
_submit(), they run on a background executor thread that has noRegistrationContextattached. During event collection (e.g.,get_bun_path()), a config lookup would triggerensure_context(), which attaches a new context to the worker thread and re-importsrxconfig.py. This is wasteful since the app's config is already loaded in the submitting thread's context.The Solution:
RegistrationContext(if any) at submission time via new_current_registration_context()helper_run_suppressed()in the worker threadChanges:
reflex/utils/telemetry.py:_current_registration_context()to safely retrieve the caller's context without creating one_run_suppressed()to accept and manage the registration context around job execution_submit()to capture and forward the contexttests/units/test_telemetry.py: Added two new tests verifying context preservation and cleanuptests/units/test_config.py: Updated existing test to avoid counting unrelated background thread loadsTesting
test_submit_runs_job_in_callers_registration_context()to verify the submitting thread's context is available during job executiontest_submit_leaves_worker_context_clean_between_jobs()to verify the context is properly detached after each jobtest_get_config_loads_once_for_shared_context()to exclude unrelated background threads from load countinghttps://claude.ai/code/session_01Ex7f8t8niToPKPyqXh4Eeq