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

3 participants