Skip to content

Commit

Permalink
fix: Do not emit logs if stopped
Browse files Browse the repository at this point in the history
Previously, a no-op Logger was returned when LoggerProvider#logger was
called after the provider was stopped.

Now, when the provider is stopped, the on_emit method will return early
and not emit any log records.

This more closely follows the behavior in the TracerProvider.
  • Loading branch information
kaylareopelle committed Oct 28, 2024
1 parent 9e2b8ee commit 06e4b05
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 36 deletions.
23 changes: 10 additions & 13 deletions logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,20 +42,15 @@ def initialize(resource: OpenTelemetry::SDK::Resources::Resource.create, log_rec
#
# @return [OpenTelemetry::SDK::Logs::Logger]
def logger(name:, version: nil)
if @stopped
OpenTelemetry.logger.warn('calling LoggerProvider#logger after shutdown, a noop logger will be returned')
OpenTelemetry::Logs::Logger.new
else
version ||= ''

if !name.is_a?(String) || name.empty?
OpenTelemetry.logger.warn('LoggerProvider#logger called with an ' \
"invalid name. Name provided: #{name.inspect}")
end
version ||= ''

@registry_mutex.synchronize do
@registry[Key.new(name, version)] ||= Logger.new(name, version, self)
end
if !name.is_a?(String) || name.empty?
OpenTelemetry.logger.warn('LoggerProvider#logger called with an ' \
"invalid name. Name provided: #{name.inspect}")
end

@registry_mutex.synchronize do
@registry[Key.new(name, version)] ||= Logger.new(name, version, self)
end
end

Expand Down Expand Up @@ -147,6 +142,8 @@ def on_emit(timestamp: nil,
instrumentation_scope: nil,
context: nil)

return if @stopped

log_record = LogRecord.new(timestamp: timestamp,
observed_timestamp: observed_timestamp,
severity_text: severity_text,
Expand Down
37 changes: 14 additions & 23 deletions logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,20 @@ def mock_context.value(key); OpenTelemetry::Trace::Span::INVALID; end # rubocop:
mock_log_record_processor.verify
end
end

it 'does not emit if the provider is stopped' do
logger_provider.instance_variable_set(:@stopped, true)
mock_log_record = Minitest::Mock.new

OpenTelemetry::SDK::Logs::LogRecord.stub :new, mock_log_record do
logger_provider.add_log_record_processor(mock_log_record_processor)
mock_log_record_processor.expect(:on_emit, nil, [mock_log_record, mock_context])

logger_provider.on_emit(**args)
# The verify should fail because on_emit should never call new on LogRecord
assert_raises(MockExpectationError) { mock_log_record_processor.verify }
end
end
end

describe 'instrument registry' do
Expand All @@ -236,28 +250,5 @@ def mock_context.value(key); OpenTelemetry::Trace::Span::INVALID; end # rubocop:
assert_instance_of(Logs::Logger, logger)
assert_same(logger, logger2)
end

describe 'when stopped' do
it 'logs a warning' do
logger_provider.instance_variable_set(:@stopped, true)

OpenTelemetry::TestHelpers.with_test_logger do |log_stream|
logger_provider.logger(name: '')
assert_match(
/calling LoggerProvider#logger after shutdown/,
log_stream.string
)
end
end

it 'does not add a new logger to the registry' do
before_stopped_size = logger_provider.instance_variable_get(:@registry).keys.size
logger_provider.instance_variable_set(:@stopped, true)
logger_provider.logger(name: 'new_logger')
after_stopped_size = logger_provider.instance_variable_get(:@registry).keys.size

assert_equal(before_stopped_size, after_stopped_size)
end
end
end
end

0 comments on commit 06e4b05

Please sign in to comment.