diff --git a/CHANGELOG.md b/CHANGELOG.md index 22f55118..ae3c75cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,9 @@ ## [7.0.0] - Unreleased +- Enhancement: Use fiber-local storage for logging context + - Replaces thread-local storage with Fiber[] for proper isolation in async environments + - Ensures logging context doesn't leak between fibers in the same thread + - Leverages Ruby 3.2+ fiber-local storage API + - Enhancement: Add yard-lint with comprehensive YARD documentation - Adds yard-lint gem for documentation linting - Documents all public classes, modules, and methods with YARD tags diff --git a/lib/shoryuken/logging.rb b/lib/shoryuken/logging.rb index d30fef44..a3e185e8 100644 --- a/lib/shoryuken/logging.rb +++ b/lib/shoryuken/logging.rb @@ -8,18 +8,35 @@ module Shoryuken # Provides logging functionality for Shoryuken. - # Manages the global logger instance and thread-local context. + # Manages the global logger instance and fiber-local context. module Logging - # Executes a block with a thread-local logging context + # Executes a block with a fiber-local logging context. + # Uses Fiber storage (Ruby 3.2+) for proper isolation in async environments. # # @param msg [String] the context message to set # @yield the block to execute within the context # @return [Object] the result of the block def self.with_context(msg) - Thread.current[:shoryuken_context] = msg + previous = context_storage[:shoryuken_context] + context_storage[:shoryuken_context] = msg yield ensure - Thread.current[:shoryuken_context] = nil + context_storage[:shoryuken_context] = previous + end + + # Returns the current logging context value + # + # @return [String, nil] the current context or nil if not set + def self.current_context + context_storage[:shoryuken_context] + end + + # Returns the Fiber class for fiber-local context storage. + # Uses Fiber[] and Fiber[]= (Ruby 3.2+) for proper isolation in async environments. + # + # @return [Class] the Fiber class + def self.context_storage + Fiber end # Initializes a new logger instance diff --git a/lib/shoryuken/logging/base.rb b/lib/shoryuken/logging/base.rb index d5d948cf..d9f42450 100644 --- a/lib/shoryuken/logging/base.rb +++ b/lib/shoryuken/logging/base.rb @@ -18,7 +18,7 @@ def tid # # @return [String] Formatted context string or empty string if no context def context - c = Thread.current[:shoryuken_context] + c = Shoryuken::Logging.current_context c ? " #{c}" : '' end end diff --git a/spec/lib/shoryuken/logging_spec.rb b/spec/lib/shoryuken/logging_spec.rb index c6df94c6..ad378c11 100644 --- a/spec/lib/shoryuken/logging_spec.rb +++ b/spec/lib/shoryuken/logging_spec.rb @@ -24,13 +24,17 @@ end describe '#context' do + after do + Shoryuken::Logging.context_storage[:shoryuken_context] = nil + end + it 'returns empty string when no context is set' do - Thread.current[:shoryuken_context] = nil + Shoryuken::Logging.context_storage[:shoryuken_context] = nil expect(formatter.context).to eq('') end it 'returns formatted context when context is set' do - Thread.current[:shoryuken_context] = 'test_context' + Shoryuken::Logging.context_storage[:shoryuken_context] = 'test_context' expect(formatter.context).to eq(' test_context') end end @@ -41,9 +45,13 @@ let(:time) { Time.new(2023, 8, 15, 10, 30, 45, '+00:00') } describe '#call' do + after do + Shoryuken::Logging.context_storage[:shoryuken_context] = nil + end + it 'formats log messages with timestamp' do allow(formatter).to receive(:tid).and_return('abc123') - Thread.current[:shoryuken_context] = nil + Shoryuken::Logging.context_storage[:shoryuken_context] = nil result = formatter.call('INFO', time, 'program', 'test message') expect(result).to eq("2023-08-15T10:30:45Z #{Process.pid} TID-abc123 INFO: test message\n") @@ -51,7 +59,7 @@ it 'includes context when present' do allow(formatter).to receive(:tid).and_return('abc123') - Thread.current[:shoryuken_context] = 'worker-1' + Shoryuken::Logging.context_storage[:shoryuken_context] = 'worker-1' result = formatter.call('ERROR', time, 'program', 'error message') expect(result).to eq("2023-08-15T10:30:45Z #{Process.pid} TID-abc123 worker-1 ERROR: error message\n") @@ -63,9 +71,13 @@ let(:formatter) { described_class.new } describe '#call' do + after do + Shoryuken::Logging.context_storage[:shoryuken_context] = nil + end + it 'formats log messages without timestamp' do allow(formatter).to receive(:tid).and_return('xyz789') - Thread.current[:shoryuken_context] = nil + Shoryuken::Logging.context_storage[:shoryuken_context] = nil result = formatter.call('DEBUG', Time.now, 'program', 'debug message') expect(result).to eq("pid=#{Process.pid} tid=xyz789 DEBUG: debug message\n") @@ -73,7 +85,7 @@ it 'includes context when present' do allow(formatter).to receive(:tid).and_return('xyz789') - Thread.current[:shoryuken_context] = 'queue-processor' + Shoryuken::Logging.context_storage[:shoryuken_context] = 'queue-processor' result = formatter.call('WARN', Time.now, 'program', 'warning message') expect(result).to eq("pid=#{Process.pid} tid=xyz789 queue-processor WARN: warning message\n") @@ -82,9 +94,13 @@ end describe '.with_context' do + after do + described_class.context_storage[:shoryuken_context] = nil + end + it 'sets context for the duration of the block' do described_class.with_context('test_context') do - expect(Thread.current[:shoryuken_context]).to eq('test_context') + expect(described_class.current_context).to eq('test_context') end end @@ -92,7 +108,7 @@ described_class.with_context('test_context') do # context is set end - expect(Thread.current[:shoryuken_context]).to be_nil + expect(described_class.current_context).to be_nil end it 'clears context even when an exception is raised' do @@ -102,7 +118,7 @@ end end.to raise_error(StandardError, 'test error') - expect(Thread.current[:shoryuken_context]).to be_nil + expect(described_class.current_context).to be_nil end it 'returns the value of the block' do @@ -111,6 +127,52 @@ end expect(result).to eq('block_result') end + + it 'preserves outer context in nested calls' do + described_class.with_context('outer') do + expect(described_class.current_context).to eq('outer') + + described_class.with_context('inner') do + expect(described_class.current_context).to eq('inner') + end + + expect(described_class.current_context).to eq('outer') + end + expect(described_class.current_context).to be_nil + end + + it 'restores outer context even when inner block raises' do + described_class.with_context('outer') do + expect do + described_class.with_context('inner') do + raise StandardError, 'inner error' + end + end.to raise_error(StandardError, 'inner error') + + expect(described_class.current_context).to eq('outer') + end + end + end + + describe '.current_context' do + after do + described_class.context_storage[:shoryuken_context] = nil + end + + it 'returns nil when no context is set' do + expect(described_class.current_context).to be_nil + end + + it 'returns the current context value' do + described_class.context_storage[:shoryuken_context] = 'test_value' + expect(described_class.current_context).to eq('test_value') + end + end + + describe '.context_storage' do + it 'returns Fiber for fiber-local storage' do + expect(described_class.context_storage).to eq(Fiber) + end end describe '.initialize_logger' do