Merge pull request #37 from bdurand/set_logger_tags_in_block

Set logger tags in block

CHANGELOG.md

@@ -1,3 +1,8 @@
+## 1.2.6
+
+* Fix Logger#tag so it only ads to the current block's logger tags instead of the global tags if called inside a `Logger#tag` block.
+* Add Logger#remove_tag
+
 ## 1.2.5
 
 * Fix logic with recursive reference guard in StructuredFormatter so it only suppresses Enumerable references.

README.md

@@ -59,8 +59,10 @@ You can specify tags that will only be applied to the logger in a block as well.
 ```ruby
 logger.tag(thread_id: Thread.current.object_id) do
   logger.info("here") # Will include the `thread_id` tag
+  logger.tag(count: 15)
+  logger.info("with count") # Will include the `count` tag
 end
-logger.info("there") # Will not include the `thread_id` tag
+logger.info("there") # Will not include the `thread_id` or `count` tag
 ```
 
 You can also set tags to `Proc` objects that will be evaluated when creating a log entry.

VERSION

@@ -1 +1 @@
-1.2.5
+1.2.6

lib/lumberjack.rb

@@ -67,6 +67,11 @@ def context
         current_context || Context.new
       end
     end
+
+    # Return true if inside a context block.
+    def context?
+      !!Thread.current[:lumberjack_context]
+    end
 
     # Return the tags from the current context or nil if there are no tags.
     def context_tags

lib/lumberjack/logger.rb

@@ -156,10 +156,10 @@ def tagged_logger!
     #   logger.add_entry(:warn, "Request took a long time")
     #   logger.add_entry(Logger::DEBUG){"Start processing with options #{options.inspect}"}
     def add_entry(severity, message, progname = nil, tags = nil)
-      begin        
+      begin
         severity = Severity.label_to_level(severity) unless severity.is_a?(Integer)
         return true unless device && severity && severity >= level
-        
+
         return true if Thread.current[:lumberjack_logging]
         Thread.current[:lumberjack_logging] = true
 
@@ -237,7 +237,7 @@ def fatal(message_or_progname_or_tags = nil, progname_or_tags = nil, &block)
     def fatal?
       level <= FATAL
     end
-    
+
     # Set the log level to fatal.
     def fatal!
       self.level = FATAL
@@ -252,7 +252,7 @@ def error(message_or_progname_or_tags = nil, progname_or_tags = nil, &block)
     def error?
       level <= ERROR
     end
-    
+
     # Set the log level to error.
     def error!
       self.level = ERROR
@@ -267,7 +267,7 @@ def warn(message_or_progname_or_tags = nil, progname_or_tags = nil, &block)
     def warn?
       level <= WARN
     end
-    
+
     # Set the log level to warn.
     def warn!
       self.level = WARN
@@ -282,7 +282,7 @@ def info(message_or_progname_or_tags = nil, progname_or_tags = nil, &block)
     def info?
       level <= INFO
     end
-    
+
     # Set the log level to info.
     def info!
       self.level = INFO
@@ -297,7 +297,7 @@ def debug(message_or_progname_or_tags = nil, progname_or_tags = nil, &block)
     def debug?
       level <= DEBUG
     end
-    
+
     # Set the log level to debug.
     def debug!
       self.level = DEBUG
@@ -349,16 +349,34 @@ def progname
     end
 
     # Set a hash of tags on logger. If a block is given, the tags will only be set
-    # for the duration of the block.
+    # for the duration of the block. If this method is called inside such a block,
+    # the tags will only be defined on the tags in that block. When the parent block
+    # exits, all the tags will be reverted. If there is no block, then the tags will
+    # be defined as global and apply to all log statements.
     def tag(tags, &block)
       tags = Tags.stringify_keys(tags)
+      thread_tags = thread_local_value(:lumberjack_logger_tags)
       if block
-        thread_tags = thread_local_value(:lumberjack_logger_tags)
-        value = (thread_tags ? thread_tags.merge(tags) : tags)
-        push_thread_local_value(:lumberjack_logger_tags, value, &block)
+        merged_tags = (thread_tags ? thread_tags.merge(tags) : tags)
+        push_thread_local_value(:lumberjack_logger_tags, merged_tags, &block)
+      elsif thread_tags
+        thread_tags.merge!(tags)
       else
         @tags.merge!(tags)
       end
+      nil
+    end
+
+    # Remove a tag from the current tag context. If this is called inside a block to a
+    # call to `tag`, the tags will only be removed for the duration of that block. Otherwise
+    # they will be removed from the global tags.
+    def remove_tag(*tag_names)
+      thread_tags = thread_local_value(:lumberjack_logger_tags)
+      if thread_tags
+        tag_names.each { |name| thread_tags.delete(name.to_s) }
+      else
+        tag_names.each { |name| @tags.delete(name.to_s) }
+      end
     end
 
     # Return all tags in scope on the logger including global tags set on the Lumberjack
@@ -474,7 +492,7 @@ def create_flusher_thread(flush_seconds) #:nodoc:
                 sleep(flush_seconds)
                 logger.flush if Time.now - logger.last_flushed_at >= flush_seconds
               rescue => e
-                STDERR.puts("Error flushing log: #{e.inspect}")
+                $stderr.puts("Error flushing log: #{e.inspect}")
               end
             end
           end

lumberjack.gemspec

@@ -11,8 +11,7 @@ Gem::Specification.new do |spec|
   # Specify which files should be added to the gem when it is released.
   # The `git ls-files -z` loads the files in the RubyGem that have been added into git.
   ignore_files = %w(
-    .gitignore
-    .travis.yml
+    .
     Appraisals
     Gemfile
     Gemfile.lock

spec/device/rolling_log_file_spec.rb

@@ -6,15 +6,15 @@
     Lumberjack::Device::SizeRollingLogFile #needed by jruby
     create_tmp_dir
   end
-  
+
   after :all do
     delete_tmp_dir
   end
-  
+
   before :each do
     delete_tmp_files
   end
-  
+
   let(:entry){ Lumberjack::LogEntry.new(Time.now, 1, "New log entry", nil, $$, nil) }
 
   it "should check for rolling the log file on flush" do
@@ -24,7 +24,7 @@
     device.flush
     device.close
   end
-  
+
   it "should roll the file by archiving the existing file and opening a new stream and calling after_roll" do
     log_file = File.join(tmp_dir, "test_2.log")
     device = Lumberjack::Device::RollingLogFile.new(log_file, :template => ":message", :buffer_size => 32767, :min_roll_check => 0)
@@ -38,7 +38,7 @@
     expect(File.read("#{log_file}.rolled")).to eq("New log entry#{Lumberjack::LINE_SEPARATOR}")
     expect(File.read(log_file)).to eq("Another log entry#{Lumberjack::LINE_SEPARATOR}")
   end
-  
+
   it "should reopen the file if the stream inode doesn't match the file path inode" do
     log_file = File.join(tmp_dir, "test_3.log")
     device = Lumberjack::Device::RollingLogFile.new(log_file, :template => ":message", :min_roll_check => 0)
@@ -52,7 +52,7 @@
     expect(File.read("#{log_file}.rolled")).to eq("New log entry#{Lumberjack::LINE_SEPARATOR}")
     expect(File.read(log_file)).to eq("Another log entry#{Lumberjack::LINE_SEPARATOR}")
   end
-  
+
   it "should roll the file properly with multiple thread and processes using it" do
     log_file = File.join(tmp_dir, "test_4.log")
     process_count = 8
@@ -61,7 +61,7 @@
     max_size = 128
     severity = Logger::INFO
     message = "This is a test message that is written to the log file to indicate what the state of the application is."
-    
+
     logger_test = lambda do
       device = Lumberjack::Device::SizeRollingLogFile.new(log_file, :max_size => max_size, :template => ":message", :buffer_size => 32767, :min_roll_check => 0)
       threads = []
@@ -77,7 +77,7 @@
       threads.each{|thread| thread.value}
       device.close
     end
-    
+
     # Process.fork is unavailable on jruby so we need to use the java threads instead.
     if RUBY_PLATFORM.match(/java/)
       outer_threads = []
@@ -91,7 +91,7 @@
       end
       Process.waitall
     end
-    
+
     line_count = 0
     file_count = 0
     Dir.glob("#{log_file}*").each do |file|
@@ -102,10 +102,10 @@
         expect(line).to eq(message)
       end
     end
-    
+
     expect(file_count).to be > 3
   end
-  
+
   it "should only keep a specified number of archived log files" do
     log_file = File.join(tmp_dir, "test_5.log")
     device = Lumberjack::Device::RollingLogFile.new(log_file, :template => ":message", :keep => 2, :buffer_size => 32767, :min_roll_check => 0)

spec/logger_spec.rb

@@ -492,6 +492,31 @@
         expect(lines[2]).to eq 'three - 3 - [foo:bar] [baz:thing] [tag:d]'
       end
 
+      it "should add and remove tags only in a tag block" do
+        logger.tag(baz: "boo", count: 1) do
+          logger.info("one")
+          logger.tag(foo: "bar", count: 2)
+          logger.info("two")
+        end
+        logger.info("three")
+
+        lines = output.string.split(n)
+        expect(lines[0]).to eq 'one - 1 - [baz:boo]'
+        expect(lines[1]).to eq 'two - 2 - [baz:boo] [foo:bar]'
+        expect(lines[2]).to eq 'three -  -'
+      end
+
+      it "should add and remove tags in the global scope if there is no block" do
+        logger.tag(count: 1, foo: "bar")
+        logger.info("one")
+        logger.remove_tag(:foo)
+        logger.info("two")
+
+        lines = output.string.split(n)
+        expect(lines[0]).to eq 'one - 1 - [foo:bar]'
+        expect(lines[1]).to eq 'two - 1 -'
+      end
+
       it "should apply a tag formatter to the tags" do
         logger.tag_formatter.add(:foo, &:reverse).add(:count) { |val| val * 100 }
         logger.info("message", count: 2, foo: "abc")

spec/lumberjack_spec.rb

@@ -16,6 +16,14 @@
       expect(context).to_not eq(Lumberjack.context)
     end
 
+    it "should determine if it is inside a context block" do
+      expect(Lumberjack.context?).to eq false
+      Lumberjack.context do
+        expect(Lumberjack.context?).to eq true
+      end
+      expect(Lumberjack.context?).to eq false
+    end
+
     it "should inherit parent context tags in sub blocks" do
       Lumberjack.context do
         Lumberjack.tag(foo: "bar")