Skip to content

fix: apply log_level directive to env filter for tracing subscriber#247

Open
gaius-qi wants to merge 1 commit into
ai-dynamo:mainfrom
gaius-qi:feature/env-filter
Open

fix: apply log_level directive to env filter for tracing subscriber#247
gaius-qi wants to merge 1 commit into
ai-dynamo:mainfrom
gaius-qi:feature/env-filter

Conversation

@gaius-qi
Copy link
Copy Markdown
Contributor

@gaius-qi gaius-qi commented Apr 21, 2026

The tracing subscriber previously used EnvFilter::from_default_env() together with with_max_level(log_level). When RUST_LOG was unset, EnvFilter defaulted to ERROR and overrode the configured log_level, making it ineffective.

Summary by CodeRabbit

  • Chores
    • Updated internal logging configuration setup for improved consistency and maintainability.

The tracing subscriber previously used `EnvFilter::from_default_env()`
together with `with_max_level(log_level)`. When `RUST_LOG` was unset,
`EnvFilter` defaulted to `ERROR` and overrode the configured
`log_level`, making it ineffective.

Signed-off-by: Gaius <gaius.qi@gmail.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 21, 2026

Walkthrough

The tracing subscriber configuration in the main entry point was refactored to use a single EnvFilter directive for log-level limiting, consolidating the previous approach that relied on both with_env_filter() and with_max_level() methods.

Changes

Cohort / File(s) Summary
Tracing Configuration
modelexpress_server/src/main.rs
Refactored EnvFilter construction to combine default environment filter with log-level directive via add_directive(), replacing separate with_max_level() call.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A hop through logs, now filtered clean,
Where directives reign supreme,
No more conflicting calls to sort,
Just one EnvFilter to report! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: applying a log_level directive to the env filter for the tracing subscriber configuration.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@modelexpress_server/src/main.rs`:
- Around line 51-53: The current logging setup unconditionally adds a directive
via EnvFilter::from_default_env().add_directive(log_level.into()) which
overrides RUST_LOG; change to use EnvFilter::from_env_lossy() combined with
setting the fallback via with_default_directive(log_level.into()) so the
configured log_level is only applied when RUST_LOG is unset/invalid. Locate the
EnvFilter usage (EnvFilter::from_default_env and add_directive) and replace the
construction with EnvFilter::from_env_lossy(...) / or use
EnvFilter::from_env_lossy() and pass it into
FmtSubscriber::builder().with_env_filter(...), or use the builder helper
with_default_directive(log_level.into()) before calling from_env_lossy to ensure
the configured level is a default rather than an override.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c682f952-3cc0-4479-85e0-36262b2aa136

📥 Commits

Reviewing files that changed from the base of the PR and between 94b8e75 and 0a5586f.

📒 Files selected for processing (1)
  • modelexpress_server/src/main.rs

Comment thread modelexpress_server/src/main.rs
@gaius-qi
Copy link
Copy Markdown
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant