Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Judoscale::Reporter doesn't always follow Judoscale::Config.log_level #187

Closed
azanar opened this issue Nov 29, 2023 · 1 comment · Fixed by #190
Closed

Judoscale::Reporter doesn't always follow Judoscale::Config.log_level #187

azanar opened this issue Nov 29, 2023 · 1 comment · Fixed by #190

Comments

@azanar
Copy link

azanar commented Nov 29, 2023

Expected

In config/initializers/judoscale.rb:

if defined?(Judoscale)
  Judoscale.configure do |config|
    config.log_level = :warn
  end
end

In logs: no INFO messages

Actual

In logs:

INFO -- : [Judoscale] Reporting 40 metrics

Workaround

if defined?(Judoscale)
  Judoscale.configure do |config|
    config.log_level = :warn
    Judoscale::Reporter.instance.logger.log_level = ::Logger::Severity::WARN
  end
end

Analysis

There appears to be a race condition between Judoscale::Config.instance and Judoscale::Reporter.instance.logger.

When Judoscale::Reporter.instance.logger is called, that calls this code: https://github.com/judoscale/judoscale-ruby/blob/main/judoscale-ruby/lib/judoscale/logger.rb#L9

The first time that code is called, it takes Judoscale::Config.instance.log_level at that moment, and hands it to a new instance LoggerProxy. All later calls hand back the same instance of LoggerProxy. Any later changes to Judoscale::Config.instance.log_level won't be reflected.

The Logger instance wrapped by LoggerProxy has no log level set: https://github.com/judoscale/judoscale-ruby/blob/main/judoscale-ruby/lib/judoscale/config.rb#L83

Potential Solutions

  1. Assign and keep in sync Judoscale::Config.instance.log_level with Judoscale::Config.instance.logger
  2. Update Judoscale::Reporter.instance.logger when Judoscale::Config.instance.log_level changes.
@adamlogic
Copy link
Collaborator

This is the best issue! Thanks so much for the clear repro steps, analysis, and potential solutions. 💯

I'll do some testing and create a PR for this next week. 👍

adamlogic added a commit that referenced this issue Dec 8, 2023
…ialized

Fixes #187

Our own Railtie code makes calls to `logger` which might be run before a user's custom `Judoscale.configure` block. We need to allow configuring the log level at any time.
adamlogic added a commit that referenced this issue Dec 8, 2023
… initialized (#190)

Fixes #187

Our own Railtie code makes calls to `logger` which might be run before a user's custom `Judoscale.configure` block. We need to allow configuring the log level at any time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants