Merge pull request #36 from Invoca/TECH-4747_update_contextual_logger…

…_to_normalize_context_keys_to_symbols

Tech 4747 update contextual logger to normalize context keys to symbols

CHANGELOG.md

@@ -4,6 +4,10 @@ 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).
 
+## [0.11.0] - 2020-09-15
+### Changed
+- Updated contextual logger to normalize keys to symbols and warn on string keys
+
 ## [0.10.0] - 2020-09-02
 ### Added
 - Added support and tests for all combinations of `progname`, `message`, and `context`:
@@ -82,6 +86,7 @@ are already passed to the formatter as arguments so that the formatter can decid
  - Extracted `ContextualLogger.normalize_log_level` into a public class method so we can call it elsewhere where we allow log_level to be
    configured to text values like 'debug'.
 
+[0.11.0]: https://github.com/Invoca/contextual_logger/compare/v0.10.0...v0.11.0
 [0.10.0]: https://github.com/Invoca/contextual_logger/compare/v0.9.1...v0.10.0
 [0.9.1]: https://github.com/Invoca/contextual_logger/compare/v0.9.0...v0.9.1
 [0.9.0]: https://github.com/Invoca/contextual_logger/compare/v0.8.0...v0.9.0

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    contextual_logger (0.10.0)
+    contextual_logger (0.11.0)
       activesupport
       json
 

lib/contextual_logger/logger_with_context.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'active_support/core_ext/hash/keys'
+
 module ContextualLogger
   # A logger that deep_merges additional context and then delegates to the given logger.
   # Keeps it own log level (called override_level) that may be set independently of the logger it delegates to.
@@ -13,7 +15,7 @@ def initialize(logger, context, level: nil)
       logger.is_a?(LoggerMixin) or raise ArgumentError, "logger must include ContextualLogger::LoggerMixin (got #{logger.inspect})"
       @logger = logger
       self.level = level
-      @context = context
+      @context = normalize_context(context)
       @merged_context_cache = {}  # so we don't have to merge every time
     end
 
@@ -36,6 +38,29 @@ def write_entry_to_log(severity, timestamp, progname, message, context:)
       @logger.write_entry_to_log(severity, timestamp, progname, message, context: merged_context)
     end
 
+    private
+
+    def normalize_context(context)
+      if warn_on_string_keys(context)
+        context.deep_symbolize_keys
+      else
+        context
+      end
+    end
+
+    def warn_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')
+      end
+    end
+
+    def deep_key_has_string?(hash)
+      hash.any? do |key, value|
+        key.is_a?(String) ||
+          (value.is_a?(Hash) && deep_key_has_string?(value))
+      end
+    end
+
     class << self
       def for_log_source(logger, log_source, level: nil)
         new(logger, { log_source: log_source }, level: level)

lib/contextual_logger/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module ContextualLogger
-  VERSION = '0.10.0'
+  VERSION = '0.11.0'
 end

spec/lib/contextual_logger/logger_with_context_spec.rb

@@ -99,6 +99,27 @@
       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")
+      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" } } } })
+      end
+
+      it "should return a deprecation warning" 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
+      end
+    end
+
     context "when base logger doesn't include LoggerMixin" do
       let(:base_logger) { Object.new }