Skip to content

fix(agent): apply the configured logger_level to the default logger - #376

Merged
matthv merged 2 commits into
mainfrom
fix/371-default-logger-level
Aug 26, 2026
Merged

fix(agent): apply the configured logger_level to the default logger#376
matthv merged 2 commits into
mainfrom
fix/371-default-logger-level

Conversation

@matthv

@matthv matthv commented Aug 26, 2026

Copy link
Copy Markdown
Member

What

The default logger's level was never applied: logger_service.rb stored @logger_level but built MonoLogger.new($stdout) without ever calling .level = on it. Since MonoLogger inherits from Logger (DEBUG threshold by default), every message got through regardless of config.logger_level — the setting only worked when a custom logger proc was supplied.

Also normalized get_level to be case-insensitive: forest_admin_rails/forest_admin_rpc_agent default logger_level to the lowercase 'info', while the LEVELS lookup table keys are capitalized ('Info', 'Warn', ...). Without this, the fix above would silently no-op for the (lowercase) default and for anyone typing e.g. 'warn' instead of 'Warn'.

Why

Fixes #371.

How tested

  • New spec: packages/forest_admin_agent/spec/lib/forest_admin_agent/services/logger_service_spec.rb
  • Full forest_admin_agent suite green locally (1162 examples, 0 failures)
  • rubocop clean on the changed files

🤖 Generated with Claude Code

Note

Apply configured logger_level to default logger in LoggerService

The default logger was not using the configured severity threshold at construction time. LoggerService#initialize now calls get_level(@logger_level) to set the default logger's level, and get_level normalizes input with to_s.capitalize before indexing into LEVELS, so case-insensitive names and symbols are accepted. Unknown levels fall back to Logger::INFO.

  • Adds RSpec coverage for default INFO fallback, threshold filtering, and custom logger delegation.
  • Risk: get_level now treats :debug and 'debug' as valid inputs where it previously did not; callers relying on strict case-sensitive matching may see different levels applied.

Changes since #376 opened

  • Added support for Fatal and Unknown logger severity levels to ForestAdminAgent::Services::LoggerService [54518d0]

Macroscope summarized b2f7091.

The default logger's level was never set, so MonoLogger (a Logger
subclass, DEBUG threshold by default) let every message through
regardless of config.logger_level. get_level() is also normalized
to be case-insensitive, since config.logger_level defaults to the
lowercase 'info' while the LEVELS lookup table is capitalized.

Fixes #371
@qltysh

qltysh Bot commented Aug 26, 2026

Copy link
Copy Markdown

Qlty


Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (1)

RatingFile% DiffUncovered Line #s
Coverage rating: A Coverage rating: A
..._admin_agent/lib/forest_admin_agent/services/logger_service.rb100.0%
Total100.0%
🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

@christophebrun-forest

Copy link
Copy Markdown
Member

Fix confirmed on my side: I ran the suite on the branch (1101 examples, 0 failures) and replayed the new specs against the unpatched logger_service.rb -> 4 of the 7 fail, so they genuinely pin the bug. MonoLogger only overrides initialize/the log device, so the inherited Logger#add does filter on @level, and Logger::DEBUG == 0 is truthy in Ruby, so || Logger::INFO doesn't swallow 'debug' (measured: level=0). No lowercase log('info', ...) call site exists in any packages/*/lib, so the capitalize normalization has no internal fallout.

One thing worth adding to the PR body / release note before merging: this changes the log verbosity for every user who never touched the setting. Both forest_admin_rails and forest_admin_rpc_agent ship setting :logger_level, default: 'info', and until now everything got through regardless -- including Debug. forest_admin_datasource_rpc in particular is a heavy Debug emitter (schema polling, collection creation, each RPC call), so those lines silently disappear on a patch-level upgrade.

That's exactly the intended fix, but it goes out as fix: with no migration note. Something like this in the description would be enough:

Debug messages are now filtered out by default. Set config.logger_level = 'debug' to get the previous verbosity back.

@christophebrun-forest christophebrun-forest left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just 2 suggestions

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Need to add the 2 missing statuses : FATAL and UNKNOW ; see the reason below.

@logger_level = logger_level
@logger = logger
@default_logger = MonoLogger.new($stdout)
@default_logger.level = get_level(@logger_level) || Logger::INFO

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If it's FATAL, we translate it to INFO; so, instead of aiming for less verbosity, we end up with maximum verbosity.

Without these, configuring logger_level: 'Fatal' (the quietest setting)
hit the LEVELS lookup miss and fell back to Info -- the opposite of
what was requested. Both now map to their Logger constants.

Found by @christophebrun-forest in review on #376.

@christophebrun-forest christophebrun-forest left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@matthv
matthv merged commit bb4ac89 into main Aug 26, 2026
56 checks passed
@matthv
matthv deleted the fix/371-default-logger-level branch August 26, 2026 13:30
forest-bot added a commit that referenced this pull request Aug 26, 2026
## [1.39.3](v1.39.2...v1.39.3) (2026-08-26)

### Bug Fixes

* **agent:** apply the configured logger_level to the default logger ([#376](#376)) ([bb4ac89](bb4ac89)), closes [#371](#371)
* **customizer:** mark a field sortable only when the sort decorator can order it (PRD-1036) ([#368](#368)) ([a04900d](a04900d))
@forest-bot

Copy link
Copy Markdown
Member

🎉 This PR is included in version 1.39.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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.

config.logger_level is ignored by the built-in default logger

3 participants