ENG-10963 feat(log): add stdlib logging pipeline in reflex_base.utils.log (1/5) - #6863
Conversation
Greptile SummaryAdds a standard-library logging pipeline with Rich, JSON-lines, deduplication, and file sinks, then routes existing console-level configuration through it.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains from the previously reported issues. The dotenv logger is now placed under the Reflex logging hierarchy during direct environment imports, and table output emits a structured JSON record in JSON mode; no blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/reflex-base/src/reflex_base/utils/log.py | Introduces the logging hierarchy, managed handlers, Rich and JSON formatting, deduplication, file output, and deprecation utilities without an eligible blocking issue. |
| packages/reflex-base/src/reflex_base/utils/console.py | Delegates log-level state to the new pipeline and makes interactive output—including tables—respect JSON mode. |
| packages/reflex-base/src/reflex_base/environment.py | Adds the JSON logging environment variable and routes dotenv dependency errors through a bootstrapped module logger. |
| packages/reflex-base/src/reflex_base/constants/base.py | Implements rank-based total ordering for LogLevel and maps values to standard-library logging levels. |
| tests/units/reflex_base/utils/test_log.py | Covers rendering, JSON serialization, level gating, deduplication, managed and library modes, bootstrap behavior, and lightweight imports. |
Reviews (11): Last reviewed commit: "fix(log): never attach sinks from the im..." | Re-trigger Greptile
Merging this PR will not alter performance
Performance Changes
Comparing Footnotes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c9743d5da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
All reported issues were addressed across 13 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
masenf
left a comment
There was a problem hiding this comment.
some legit reviewbot feedback here.
i'm going to skim the rest of the PRs to see if there's maybe some overlap i'm not seeing that will come together
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/reflex-base/src/reflex_base/utils/log.py">
<violation number="1" location="packages/reflex-base/src/reflex_base/utils/log.py:428">
P2: When a caller supplies an extra field named `logger`, `message`, `timestamp`, or `pid`, `emit_json_print` overwrites the canonical JSON metadata because `**fields` is expanded last. Filter reserved keys or expand extras before the canonical fields.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
reflex/init.py now eagerly imports reflex_base.utils.log to call bootstrap(), which drags in rich.console (26 ms), reflex_base.constants (29 ms), logging, json, shutil, pathlib, typing: main: 0.0014 s 58 modules rich_loaded=False That defeats the lazy_loader design in reflex/init.py. codspeed missed it because there's no import-time benchmark. I prototyped two fixes and measured both. The cheap one: since reparenting a package root fixes all descendants whenever it happens, bootstrap() doesn't need to run at import reflex at all — move the call into reflex_base/utils/log.py's module body (console.py already imports log at module scope, so it fires the moment reflex does anything) and drop the import from init.py: self-bootstrapping log.py: 0.0026 s 95 modules (parenting correct once console loads)
|
|
@masenf this is fixed. |
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/benchmarks/test_import.py">
<violation number="1" location="tests/benchmarks/test_import.py:25">
P3: Importing `reflex` now bootstraps the logging pipeline (`bootstrap()` runs at `reflex_base.utils.log` module scope, mutating loggers' `propagate`/level/handlers, and `import reflex` runs console auto-detection that can print a non-TTY warning / emit records). `_wipe_reflex_modules` + re-import run these side effects once per benchmark iteration, and the `finally` block only restores `sys.modules`; it does not restore the mutated logging/global state. If this test shares a process with other (benchmark or unit) tests, the re-imported reflex leaves the root/reflex loggers and console in a state that no longer matches the restored module objects, contaminating later tests. Include the benchmark scope in isolation, or snapshot/restore logger config as well as `sys.modules`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
85e40c8 to
a92bc1d
Compare
Standard python logging with per-module loggers: rich-rendering console handler preserving the legacy colors, JSON-lines handler behind REFLEX_LOG_JSON, record dedupe, and file logging. LogLevel gains a correct total ordering (the str mixin compared alphabetically) and to_logging_level(). console.py delegates set_log_level into the pipeline and respects JSON mode in print/rule/status/progress; reflex bootstraps the reflex-owned loggers on import.
…arkup
Address review feedback on the logging pipeline:
- Parent every package logger (reflex_base, reflex_cli,
reflex_components_*) under the top-level "reflex" logger, so all
reflex logging is tunable in one place (or per package) with standard
stdlib APIs. Propagation stays enabled in library mode.
- Attach handlers only in managed mode, i.e. under the reflex CLI or a
worker it spawned (REFLEX_MANAGED_LOGGING marker inherited through the
environment). A plain "import reflex" no longer touches handlers or
propagation; records flow to the root logger for the application to
handle. Managed mode cuts propagation so an app-side basicConfig
cannot double-emit records or break the --json output contract.
This also removes the _BootstrapHandler lazy-attach trick.
- Rich markup in log messages is now opt-in via extra={"rich": True};
plain records keep their literal brackets in every sink instead of
being markup-stripped.
- The dedupe filter stores key hashes instead of full messages.
- Drop the autouse conftest fixture that re-attached caplog to each
package root; propagation makes pytest capture work natively.
- share one full-logging file between the pipeline handler and the legacy console writer, instead of opening (and unlinking) two files - create the parent directory of a configured REFLEX_LOG_FILE path - keep the originating severity in JSON records emitted by the legacy console helpers, and route console.log/print_table/PoorProgress through JSON mode instead of writing rich text into the machine-readable stream - keep an application's own level configuration on the package loggers when bootstrap reparents them - drop the duplicate console._LOG_LEVEL state; the log module's level is the single source of truth
Eagerly importing reflex_base.utils.log from reflex/__init__.py dragged in rich and 159 other modules, defeating the lazy_loader design (median import time 53.7ms vs 1.2ms, 46x). Reparenting is permanent, so bootstrap() now runs in log.py's own module body — console.py imports it at module scope, so the hierarchy is fixed the moment reflex does anything. Also: print tracebacks with soft_wrap so file paths survive narrow terminals, and point deprecation JSON records at the user call site instead of the framework frame. Adds a cold-import CodSpeed benchmark so the next eager import shows up as a regression.
reflex/__init__.py imports the pipeline on 3.10 on purpose, so the 3.10-deprecation warning renders through the sinks; the import-lightness assertions only apply on 3.11+.
The dotted side-effect import bound `reflex_base` in reflex/__init__.py globals on 3.10 only, so `rx.reflex_base` existed on that version alone. Import the pipeline through a private alias and delete it, as the block already does for logging.
…branch The import only bought a "Warning:" prefix for worker subprocesses on a deprecated Python; the CLI parent has not enabled managed mode at import time anyway, so the warning goes through logging.lastResort regardless. reflex/__init__.py now has no knowledge of the pipeline on any version, and the lazy-import test needs no version branch.
bootstrap() ran configure() in managed mode, and configure() imports reflex_base.environment, which pulls in the component tree. Since the pipeline now bootstraps when log.py is first imported, a managed worker that touched reflex_base.vars before anything else closed a circular import (vars -> console -> log -> environment -> plugins -> components -> vars) and died with a partially initialized module. Import-time bootstrap now only reparents the package loggers, which is all it ever needed to do at import. Managed processes attach sinks via enable_managed_logging() (CLI) or ensure_configured() (workers, from get_config()), both of which run after imports have settled.
a92bc1d to
c8998f0
Compare
Adds
reflex_base.utils.log: a standard pythonloggingpipeline with a rich-rendering console handler (legacy colors preserved), a JSON-lines handler behindREFLEX_LOG_JSON, record deduplication, and file logging.LogLevelgains a correct total ordering (thestrmixin compared alphabetically, so--loglevel criticalprinted the system-info banner) andto_logging_level().console.set_log_leveldelegates into the pipeline;print/rule/status/progressrespect JSON mode;print_tableadded.import reflexbootstraps the reflex-owned loggers (sinks attach lazily on the first record).Stack (ENG-10963)
this → #6864 → #6865 → #6866 → #6867.
Merge in order; each PR is based on the previous branch.