Merge pull request #78 from Invoca/TECH-19528/support-predicates-for-debug-warn-info-error-fatal

ColinDKelley · web-flow · commit 6398a304b913 · 2025-09-12T13:57:41.000-07:00
TECH-19528: LoggerWithContext support for predicates: debug? warn? info? error? fatal?
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -8,13 +8,13 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        ruby: [3.1, 3.2, 3.3]
+        ruby: [3.3, 3.4]
         gemfile:
         - Gemfile
-        - gemfiles/activesupport_6_0.gemfile
-        - gemfiles/activesupport_6_1.gemfile
         - gemfiles/activesupport_7_0.gemfile
         - gemfiles/activesupport_7_1.gemfile
+        - gemfiles/activesupport_7_2.gemfile
+        - gemfiles/activesupport_8_0.gemfile
     env:
       BUNDLE_GEMFILE: ${{ matrix.gemfile }}
     steps:
diff --git a/.ruby-version b/.ruby-version
@@ -1 +1 @@
-3.1.6
+3.3.8
diff --git a/Appraisals b/Appraisals
@@ -2,4 +2,4 @@
 
 require 'appraisal/matrix'
 
-appraisal_matrix(activesupport: "6.0")
+appraisal_matrix(activesupport: "7.0")
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,12 @@ Inspired by [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 Note: this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [1.5.0] - 2025-09-12
+### Added
+- Added LoggerWithContext support for `debug?`, `info?`, `warn?`, `error?`, `fatal?`.
+### Changed
+- Raise ArgumentError if context keys are not symbols. (Was documented to start in v1.0.0 but was not implemented until now.)
+
 ## [1.4.0] - 2024-07-10
 ### Added
 - Added support for `activesupport` 7.1 by providing a mixin for extending Broadcast loggers.
diff --git a/Gemfile b/Gemfile
@@ -9,6 +9,7 @@ gem 'coveralls', require: false
 gem 'appraisal'
 gem 'appraisal-matrix'
 gem 'bump', '~> 0.6.1'
+gem 'mutex_m'
 gem 'pry'
 gem 'rake'
 gem 'rubocop', '0.54.0'
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -1,36 +1,40 @@
 PATH
   remote: .
   specs:
-    contextual_logger (1.4.0)
+    contextual_logger (1.5.0)
       activesupport (>= 6.0)
       json
 
 GEM
   remote: https://rubygems.org/
   specs:
-    activesupport (7.1.3.4)
+    activesupport (8.0.2.1)
       base64
+      benchmark (>= 0.3)
       bigdecimal
-      concurrent-ruby (~> 1.0, >= 1.0.2)
+      concurrent-ruby (~> 1.0, >= 1.3.1)
       connection_pool (>= 2.2.5)
       drb
       i18n (>= 1.6, < 2)
+      logger (>= 1.4.2)
       minitest (>= 5.1)
-      mutex_m
-      tzinfo (~> 2.0)
+      securerandom (>= 0.3)
+      tzinfo (~> 2.0, >= 2.0.5)
+      uri (>= 0.13.1)
     appraisal (2.5.0)
       bundler
       rake
       thor (>= 0.14.0)
     appraisal-matrix (0.2.0)
       appraisal (~> 2.2)
     ast (2.4.2)
-    base64 (0.2.0)
-    bigdecimal (3.1.8)
+    base64 (0.3.0)
+    benchmark (0.4.1)
+    bigdecimal (3.2.3)
     bump (0.6.1)
     coderay (1.1.3)
-    concurrent-ruby (1.3.3)
-    connection_pool (2.4.1)
+    concurrent-ruby (1.3.5)
+    connection_pool (2.5.4)
     coveralls (0.8.23)
       json (>= 1.8, < 3)
       simplecov (~> 0.16.1)
@@ -39,13 +43,14 @@ GEM
       tins (~> 1.6)
     diff-lcs (1.5.0)
     docile (1.4.0)
-    drb (2.2.1)
-    i18n (1.14.5)
+    drb (2.2.3)
+    i18n (1.14.7)
       concurrent-ruby (~> 1.0)
     json (2.6.3)
+    logger (1.7.0)
     method_source (1.0.0)
-    minitest (5.24.1)
-    mutex_m (0.2.0)
+    minitest (5.25.5)
+    mutex_m (0.3.0)
     parallel (1.23.0)
     parser (3.2.2.3)
       ast (~> 2.4.1)
@@ -85,6 +90,7 @@ GEM
     ruby-prof-flamegraph (0.3.0)
       ruby-prof (~> 0.13)
     ruby-progressbar (1.13.0)
+    securerandom (0.4.1)
     simplecov (0.16.1)
       docile (~> 1.1)
       json (>= 1.8, < 3)
@@ -93,12 +99,13 @@ GEM
     sync (0.5.0)
     term-ansicolor (1.7.1)
       tins (~> 1.0)
-    thor (1.3.1)
+    thor (1.4.0)
     tins (1.32.1)
       sync
     tzinfo (2.0.6)
       concurrent-ruby (~> 1.0)
     unicode-display_width (1.8.0)
+    uri (1.0.3)
 
 PLATFORMS
   ruby
@@ -109,6 +116,7 @@ DEPENDENCIES
   bump (~> 0.6.1)
   contextual_logger!
   coveralls
+  mutex_m
   pry
   rake
   rspec
diff --git a/gemfiles/activesupport_7_0.gemfile b/gemfiles/activesupport_7_0.gemfile
@@ -4,8 +4,9 @@ source "https://rubygems.org"
 
 gem "coveralls", require: false
 gem "appraisal"
-gem "appraisal-matrix", "0.1.0"
+gem "appraisal-matrix"
 gem "bump", "~> 0.6.1"
+gem "mutex_m"
 gem "pry"
 gem "rake"
 gem "rubocop", "0.54.0"
diff --git a/gemfiles/activesupport_7_1.gemfile b/gemfiles/activesupport_7_1.gemfile
@@ -4,8 +4,9 @@ source "https://rubygems.org"
 
 gem "coveralls", require: false
 gem "appraisal"
-gem "appraisal-matrix", "0.1.0"
+gem "appraisal-matrix"
 gem "bump", "~> 0.6.1"
+gem "mutex_m"
 gem "pry"
 gem "rake"
 gem "rubocop", "0.54.0"
diff --git a/gemfiles/activesupport_7_2.gemfile b/gemfiles/activesupport_7_2.gemfile
@@ -0,0 +1,20 @@
+# This file was generated by Appraisal
+
+source "https://rubygems.org"
+
+gem "coveralls", require: false
+gem "appraisal"
+gem "appraisal-matrix"
+gem "bump", "~> 0.6.1"
+gem "mutex_m"
+gem "pry"
+gem "rake"
+gem "rubocop", "0.54.0"
+gem "rubocop-git"
+gem "ruby-prof"
+gem "ruby-prof-flamegraph"
+gem "rspec"
+gem "rspec_junit_formatter"
+gem "activesupport", "~> 7.2.0"
+
+gemspec path: "../"
diff --git a/gemfiles/activesupport_8_0.gemfile b/gemfiles/activesupport_8_0.gemfile
@@ -0,0 +1,20 @@
+# This file was generated by Appraisal
+
+source "https://rubygems.org"
+
+gem "coveralls", require: false
+gem "appraisal"
+gem "appraisal-matrix"
+gem "bump", "~> 0.6.1"
+gem "mutex_m"
+gem "pry"
+gem "rake"
+gem "rubocop", "0.54.0"
+gem "rubocop-git"
+gem "ruby-prof"
+gem "ruby-prof-flamegraph"
+gem "rspec"
+gem "rspec_junit_formatter"
+gem "activesupport", "~> 8.0.0"
+
+gemspec path: "../"
diff --git a/lib/contextual_logger.rb b/lib/contextual_logger.rb
@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require 'logger' # required first to get ::Logger defined for ActiveSupport 7.0
 require 'active_support'
 require 'active_support/core_ext/module/delegation'
 require 'json'
@@ -11,12 +12,12 @@
 module ContextualLogger
   LOG_LEVEL_NAMES_TO_SEVERITY =
   {
-    debug:  Logger::Severity::DEBUG,
-    info:   Logger::Severity::INFO,
-    warn:   Logger::Severity::WARN,
-    error:  Logger::Severity::ERROR,
-    fatal:  Logger::Severity::FATAL,
-    unknown: Logger::Severity::UNKNOWN
+    debug:  ::Logger::Severity::DEBUG,
+    info:   ::Logger::Severity::INFO,
+    warn:   ::Logger::Severity::WARN,
+    error:  ::Logger::Severity::ERROR,
+    fatal:  ::Logger::Severity::FATAL,
+    unknown: ::Logger::Severity::UNKNOWN
   }.freeze
 
   class << self
diff --git a/lib/contextual_logger/logger_with_context.rb b/lib/contextual_logger/logger_with_context.rb
@@ -40,6 +40,12 @@ def level=(override_level)
       @override_level = (ContextualLogger.normalize_log_level(override_level) if override_level)
     end
 
+    ::ContextualLogger::LOG_LEVEL_NAMES_TO_SEVERITY.except(:unknown).each do |severity, log_level|
+      define_method("#{severity}?") do
+        log_level_enabled?(log_level)
+      end
+    end
+
     def write_entry_to_log(severity, timestamp, progname, message, context:)
       merged_context =
         if context.any?
@@ -54,16 +60,13 @@ def write_entry_to_log(severity, timestamp, progname, message, context:)
     private
 
     def normalize_context(context)
-      if warn_on_string_keys(context)
-        context.deep_symbolize_keys
-      else
-        context
-      end
+      raise_on_string_keys(context)
+      context
     end
 
-    def warn_on_string_keys(context)
+    def raise_on_string_keys(context)
       if deep_key_has_string?(context)
-        ActiveSupport::Deprecation.warn('Context keys must use symbols not strings. This will be asserted as of contextual_logger v1.0.0')
+        raise ArgumentError, "context keys must use symbols not strings: #{context.inspect}"
       end
     end
 
diff --git a/lib/contextual_logger/version.rb b/lib/contextual_logger/version.rb
@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module ContextualLogger
-  VERSION = '1.4.0'
+  VERSION = '1.5.0'
 end
diff --git a/spec/lib/contextual_logger/logger_with_context_spec.rb b/spec/lib/contextual_logger/logger_with_context_spec.rb
@@ -126,24 +126,60 @@
       end
     end
 
-    context "when string passed as context key" do
-      it "returns context with a symbol key" do
-        context_with_string_key = { "log_source" => "redis_client" }
-        string_context = ContextualLogger::LoggerWithContext.new(base_logger, context_with_string_key)
-        expect(string_context.context).to eq(log_source: "redis_client")
+    describe "log level predicates" do
+      it "delegates to DEBUG level" do
+        subject.level = Logger::Severity::DEBUG
+        expect(subject.debug?).to eq(true)
+        expect(subject.info?).to eq(true)
+        expect(subject.warn?).to eq(true)
+        expect(subject.error?).to eq(true)
+        expect(subject.fatal?).to eq(true)
+      end
+
+      it "delegates to INFO level" do
+        subject.level = Logger::Severity::INFO
+        expect(subject.debug?).to eq(false)
+        expect(subject.info?).to eq(true)
+        expect(subject.warn?).to eq(true)
+        expect(subject.error?).to eq(true)
+        expect(subject.fatal?).to eq(true)
+      end
+
+      it "delegates to WARN level" do
+        subject.level = nil
+        base_logger.level = Logger::Severity::WARN
+        expect(subject.debug?).to eq(false)
+        expect(subject.info?).to eq(false)
+        expect(subject.warn?).to eq(true)
+        expect(subject.error?).to eq(true)
+        expect(subject.fatal?).to eq(true)
       end
 
-      it "returns a deep context with symbol key" do
-        context_with_string_key_levels = { log_source: { level1: { level2: { "level3" => "redis_client" } } } }
-        string_context = ContextualLogger::LoggerWithContext.new(base_logger, context_with_string_key_levels)
-        expect(string_context.context)
-          .to eq({ log_source: { level1: { level2: { level3: "redis_client" } } } })
+      it "delegates to ERROR level" do
+        subject.level = Logger::Severity::ERROR
+        expect(subject.debug?).to eq(false)
+        expect(subject.info?).to eq(false)
+        expect(subject.warn?).to eq(false)
+        expect(subject.error?).to eq(true)
+        expect(subject.fatal?).to eq(true)
       end
 
-      it "should return a deprecation warning" do
+      it "delegates to FATAL level" do
+        subject.level = Logger::Severity::FATAL
+        expect(subject.debug?).to eq(false)
+        expect(subject.info?).to eq(false)
+        expect(subject.warn?).to eq(false)
+        expect(subject.error?).to eq(false)
+        expect(subject.fatal?).to eq(true)
+      end
+    end
+
+    context "when string passed as context key" do
+      it "raises ArgumentError" do
         context_with_string_key = { "log_source" => "redis_client" }
-        expect { ContextualLogger::LoggerWithContext.new(base_logger, context_with_string_key) }
-          .to output(/DEPRECATION WARNING: Context keys must use symbols not strings/).to_stderr
+        expect do
+          ContextualLogger::LoggerWithContext.new(base_logger, context_with_string_key)
+        end.to raise_exception(ArgumentError, /context keys must use symbols not strings: \{"log_source" ?=> ?"redis_client"\}/)
       end
     end
 
diff --git a/spec/lib/contextual_logger_spec.rb b/spec/lib/contextual_logger_spec.rb
@@ -120,13 +120,13 @@ def expect_log_line_to_be_written(log_line)
         expect(log_message_levels).to eq(["error", "fatal", "unknown"])
         # note: context lands in `progname` arg
         if ::ActiveSupport::VERSION::STRING < "7.1"
-          expect(console_log_stream.string.gsub(/\[[^\]]+\]/, '[]')).to eq(<<~EOS)
-            D, [] DEBUG -- {:service=>\"test_service\"}: debug message
-            I, []  INFO -- {:service=>\"test_service\"}: info message
-            W, []  WARN -- {:service=>\"test_service\"}: warn message
-            E, [] ERROR -- {:service=>\"test_service\"}: error message
-            F, [] FATAL -- {:service=>\"test_service\"}: fatal message
-            A, []   ANY -- {:service=>\"test_service\"}: unknown message
+          expect(console_log_stream.string.gsub(/\[[^\]]+\]/, '[]').gsub(':service=>', 'service: ')).to eq(<<~EOS)
+            D, [] DEBUG -- {service: \"test_service\"}: debug message
+            I, []  INFO -- {service: \"test_service\"}: info message
+            W, []  WARN -- {service: \"test_service\"}: warn message
+            E, [] ERROR -- {service: \"test_service\"}: error message
+            F, [] FATAL -- {service: \"test_service\"}: fatal message
+            A, []   ANY -- {service: \"test_service\"}: unknown message
           EOS
         else
           expect(log_stream.string.gsub(/"timestamp":"[^"]+"/, '<time>')).to eq(<<~EOS)
@@ -142,7 +142,7 @@ def expect_log_line_to_be_written(log_line)
         expect(log_message_levels).to eq(["error"])
         # note: context lands in `progname` arg
         if ::ActiveSupport::VERSION::STRING < "7.1"
-          expect(console_log_stream.string.gsub(/\[[^\]]+\]/, '[]')).to eq("E, [] ERROR -- {:service=>\"test_service\"}: error message\n")
+          expect(console_log_stream.string.gsub(/\[[^\]]+\]/, '[]').gsub(':service=>', 'service: ')).to eq("E, [] ERROR -- {service: \"test_service\"}: error message\n")
         else
           expect(console_log_stream.string.gsub(/"timestamp":"[^"]+"/, '<time>')).to eq(<<~EOS)
             {"message":"error message","severity":"ERROR",<time>,"service":"test_service"}

Original file line number	Diff line number	Diff line change
`@@ -2,4 +2,4 @@`
`2`	`2`
`3`	`3`	`require 'appraisal/matrix'`
`4`	`4`
`5`		`-appraisal_matrix(activesupport: "6.0")`
	`5`	`+appraisal_matrix(activesupport: "7.0")`