aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorEdouard CHIN <edouard.chin@shopify.com>2018-10-02 17:02:27 -0400
committerEdouard CHIN <edouard.chin@shopify.com>2018-10-02 17:17:23 -0400
commit05ad44eb89047ac13e31149fa6cbc1459c5545a9 (patch)
tree88a785a5de6ddeaf03e233b51d89fbdbf0cadc38
parent92fece96da28d9f641bc8a8db1187b91c94cabfb (diff)
downloadrails-05ad44eb89047ac13e31149fa6cbc1459c5545a9.tar.gz
rails-05ad44eb89047ac13e31149fa6cbc1459c5545a9.tar.bz2
rails-05ad44eb89047ac13e31149fa6cbc1459c5545a9.zip
Fix the LoggerSilence to work as described:
- Following the Rails guide which state that a logger needs to include the `ActiveSupport::LoggerSilence` as well as `ActiveSupport::LoggerThreadSafe` modules isn't enough and won't work. Here is a test cases with 3 tests that all fails https://gist.github.com/Edouard-chin/4a72930c2b1eafbbd72a80c66f102010 The problems are the following: 1) The logger needs to call `after_initialize` in order to setup some instance variables. 2) The silence doesn't actually work because the bare ruby Logger `add` method checks for the instance variable `@logger`. We need to override the `add` (like we used to in the ActiveSupport::Logger class). 3) Calling `debug?` `info?` etc... doesn't work as the bare ruby methods will check for the instance variable. Again we need to override this methods (like we used to in the ActiveSupport::Logger class) The LoggerSilence won't work without LoggerThreadSafe, but the later is not public API, the user shouldn't have to include it so I modified to include it automatically. Same for the `after_initialize` method. I find unuintitive to have to call it directly. I modified to instance the variables when the module get included.
-rw-r--r--activesupport/lib/active_support/logger.rb15
-rw-r--r--activesupport/lib/active_support/logger_silence.rb2
-rw-r--r--activesupport/lib/active_support/logger_thread_safe_level.rb29
-rw-r--r--activesupport/test/broadcast_logger_test.rb3
-rw-r--r--activesupport/test/silence_logger_test.rb35
-rw-r--r--guides/source/configuring.md3
6 files changed, 65 insertions, 22 deletions
diff --git a/activesupport/lib/active_support/logger.rb b/activesupport/lib/active_support/logger.rb
index 8152a182b4..b8555c887b 100644
--- a/activesupport/lib/active_support/logger.rb
+++ b/activesupport/lib/active_support/logger.rb
@@ -6,7 +6,6 @@ require "logger"
module ActiveSupport
class Logger < ::Logger
- include ActiveSupport::LoggerThreadSafeLevel
include LoggerSilence
# Returns true if the logger destination matches one of the sources
@@ -81,20 +80,6 @@ module ActiveSupport
def initialize(*args)
super
@formatter = SimpleFormatter.new
- after_initialize if respond_to? :after_initialize
- end
-
- def add(severity, message = nil, progname = nil, &block)
- return true if @logdev.nil? || (severity || UNKNOWN) < level
- super
- end
-
- Logger::Severity.constants.each do |severity|
- class_eval(<<-EOT, __FILE__, __LINE__ + 1)
- def #{severity.downcase}? # def debug?
- Logger::#{severity} >= level # DEBUG >= level
- end # end
- EOT
end
# Simple formatter which only displays the message.
diff --git a/activesupport/lib/active_support/logger_silence.rb b/activesupport/lib/active_support/logger_silence.rb
index 2f62cc13b9..b2444c1e34 100644
--- a/activesupport/lib/active_support/logger_silence.rb
+++ b/activesupport/lib/active_support/logger_silence.rb
@@ -2,6 +2,7 @@
require "active_support/concern"
require "active_support/core_ext/module/attribute_accessors"
+require "active_support/logger_thread_safe_level"
module LoggerSilence
extend ActiveSupport::Concern
@@ -22,6 +23,7 @@ module ActiveSupport
included do
cattr_accessor :silencer, default: true
+ include ActiveSupport::LoggerThreadSafeLevel
end
# Silences the logger for the duration of the block.
diff --git a/activesupport/lib/active_support/logger_thread_safe_level.rb b/activesupport/lib/active_support/logger_thread_safe_level.rb
index 22ab4cc28c..f16c90cfc6 100644
--- a/activesupport/lib/active_support/logger_thread_safe_level.rb
+++ b/activesupport/lib/active_support/logger_thread_safe_level.rb
@@ -1,14 +1,30 @@
# frozen_string_literal: true
require "active_support/concern"
+require "active_support/core_ext/module/attribute_accessors"
require "concurrent"
module ActiveSupport
module LoggerThreadSafeLevel # :nodoc:
extend ActiveSupport::Concern
+ included do
+ cattr_accessor :local_levels, default: Concurrent::Map.new(initial_capacity: 2), instance_accessor: false
+ end
+
+ Logger::Severity.constants.each do |severity|
+ class_eval(<<-EOT, __FILE__, __LINE__ + 1)
+ def #{severity.downcase}? # def debug?
+ Logger::#{severity} >= level # DEBUG >= level
+ end # end
+ EOT
+ end
+
def after_initialize
- @local_levels = Concurrent::Map.new(initial_capacity: 2)
+ ActiveSupport::Deprecation.warn(
+ "Logger don't need to call #after_initialize directly anymore. It will be deprecated without replacement in " \
+ "Rails 6.1."
+ )
end
def local_log_id
@@ -16,19 +32,24 @@ module ActiveSupport
end
def local_level
- @local_levels[local_log_id]
+ self.class.local_levels[local_log_id]
end
def local_level=(level)
if level
- @local_levels[local_log_id] = level
+ self.class.local_levels[local_log_id] = level
else
- @local_levels.delete(local_log_id)
+ self.class.local_levels.delete(local_log_id)
end
end
def level
local_level || super
end
+
+ def add(severity, message = nil, progname = nil, &block) # :nodoc:
+ return true if @logdev.nil? || (severity || UNKNOWN) < level
+ super
+ end
end
end
diff --git a/activesupport/test/broadcast_logger_test.rb b/activesupport/test/broadcast_logger_test.rb
index ea2e388b8f..02bc317c66 100644
--- a/activesupport/test/broadcast_logger_test.rb
+++ b/activesupport/test/broadcast_logger_test.rb
@@ -123,6 +123,8 @@ module ActiveSupport
end
class CustomLogger
+ include ActiveSupport::LoggerSilence
+
attr_reader :adds, :closed, :chevrons
attr_accessor :level, :progname, :formatter, :local_level
@@ -174,7 +176,6 @@ module ActiveSupport
end
class FakeLogger < CustomLogger
- include ActiveSupport::LoggerSilence
end
end
end
diff --git a/activesupport/test/silence_logger_test.rb b/activesupport/test/silence_logger_test.rb
new file mode 100644
index 0000000000..fc5c5c1161
--- /dev/null
+++ b/activesupport/test/silence_logger_test.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+require "abstract_unit"
+require "active_support/logger_silence"
+require "logger"
+
+class LoggerSilenceTest < ActiveSupport::TestCase
+ class MyLogger < ::Logger
+ include LoggerSilence
+ end
+
+ setup do
+ @io = StringIO.new
+ @logger = MyLogger.new(@io)
+ end
+
+ test "#silence silences the log" do
+ @logger.silence(Logger::ERROR) do
+ @logger.info("Foo")
+ end
+ @io.rewind
+
+ assert_empty @io.read
+ end
+
+ test "#debug? is true when setting the temporary level to Logger::DEBUG" do
+ @logger.level = Logger::INFO
+
+ @logger.silence(Logger::DEBUG) do
+ assert_predicate @logger, :debug?
+ end
+
+ assert_predicate @logger, :info?
+ end
+end
diff --git a/guides/source/configuring.md b/guides/source/configuring.md
index d23af2ca57..812565b207 100644
--- a/guides/source/configuring.md
+++ b/guides/source/configuring.md
@@ -119,11 +119,10 @@ defaults to `:debug` for all environments. The available log levels are: `:debug
* `config.logger` is the logger that will be used for `Rails.logger` and any related Rails logging such as `ActiveRecord::Base.logger`. It defaults to an instance of `ActiveSupport::TaggedLogging` that wraps an instance of `ActiveSupport::Logger` which outputs a log to the `log/` directory. You can supply a custom logger, to get full compatibility you must follow these guidelines:
* To support a formatter, you must manually assign a formatter from the `config.log_formatter` value to the logger.
* To support tagged logs, the log instance must be wrapped with `ActiveSupport::TaggedLogging`.
- * To support silencing, the logger must include `ActiveSupport::LoggerSilence` and `ActiveSupport::LoggerThreadSafeLevel` modules. The `ActiveSupport::Logger` class already includes these modules.
+ * To support silencing, the logger must include `ActiveSupport::LoggerSilence` module. The `ActiveSupport::Logger` class already includes these modules.
```ruby
class MyLogger < ::Logger
- include ActiveSupport::LoggerThreadSafeLevel
include ActiveSupport::LoggerSilence
end