From ab570713d783b1dac3edea013c2d6f929cc03549 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Mon, 15 Jul 2024 16:31:56 -0700 Subject: [PATCH] feat: Create LogRecord in LoggerProvider#on_emit * Follow the pattern in TracerProvider#internal_start_span * Update spacing in LogRecordData * Remove resource references outside LoggerProvider * Remove Logger attr_readers for logger_provider and instrumentation_scope --- .../lib/opentelemetry/sdk/logs/log_record.rb | 19 ++++---- .../opentelemetry/sdk/logs/log_record_data.rb | 27 +++++------ logs_sdk/lib/opentelemetry/sdk/logs/logger.rb | 28 +++++------- .../opentelemetry/sdk/logs/logger_provider.rb | 29 ++++++++++-- .../opentelemetry/sdk/logs/log_record_test.rb | 39 ++-------------- .../sdk/logs/logger_provider_test.rb | 45 ++++++++++++++++--- .../opentelemetry/sdk/logs/logger_test.rb | 6 --- 7 files changed, 103 insertions(+), 90 deletions(-) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/log_record.rb b/logs_sdk/lib/opentelemetry/sdk/logs/log_record.rb index 149eb4184a..255158df8d 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/log_record.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/log_record.rb @@ -43,11 +43,13 @@ class LogRecord < OpenTelemetry::Logs::LogRecord # current context. # @param [optional String] span_id The span ID associated with the # current context. - # @param [optional TraceFlags] trace_flags The trace flags associated - # with the current context. - # @param [optional OpenTelemetry::SDK::Logs::Logger] logger The logger that - # created the {LogRecord}. Used to set `resource` and - # `instrumentation_scope`. + # @param [optional OpenTelemetry::Trace::TraceFlags] trace_flags The + # trace flags associated with the current context. + # @param [optional OpenTelemetry::SDK::Resources::Resource] recource The + # source of the log, desrived from the LoggerProvider. + # @param [optional OpenTelemetry::SDK::InstrumentationScope] instrumentation_scope + # The instrumentation scope, derived from the emitting Logger + # # # @return [LogRecord] def initialize( @@ -60,7 +62,8 @@ def initialize( trace_id: nil, span_id: nil, trace_flags: nil, - logger: nil + resource: nil, + instrumentation_scope: nil ) @timestamp = timestamp @observed_timestamp = observed_timestamp || timestamp || Time.now @@ -71,8 +74,8 @@ def initialize( @trace_id = trace_id @span_id = span_id @trace_flags = trace_flags - @resource = logger&.resource - @instrumentation_scope = logger&.instrumentation_scope + @resource = resource + @instrumentation_scope = instrumentation_scope @total_recorded_attributes = @attributes&.size || 0 end diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/log_record_data.rb b/logs_sdk/lib/opentelemetry/sdk/logs/log_record_data.rb index c8bf23f910..5bf304f85c 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/log_record_data.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/log_record_data.rb @@ -8,19 +8,20 @@ module OpenTelemetry module SDK module Logs # LogRecordData is a Struct containing {LogRecord} data for export. - LogRecordData = Struct.new(:timestamp, # optional Integer nanoseconds since Epoch - :observed_timestamp, # Integer nanoseconds since Epoch - :severity_text, # optional String - :severity_number, # optional Integer - :body, # optional String, Numeric, Boolean, Array, Hash{String => String, Numeric, Boolean, - # Array} - :attributes, # optional Hash{String => String, Numeric, Boolean, Array} - :trace_id, # optional String (16-byte binary) - :span_id, # optional String (8-byte binary) - :trace_flags, # optional Integer (8-bit byte of bit flags) - :resource, # optional OpenTelemetry::SDK::Resources::Resource - :instrumentation_scope, # OpenTelemetry::SDK::InstrumentationScope + LogRecordData = Struct.new(:timestamp, # optional Integer nanoseconds since Epoch + :observed_timestamp, # Integer nanoseconds since Epoch + :severity_text, # optional String + :severity_number, # optional Integer + :body, # optional String, Numeric, Boolean, Array, Hash{String => String, Numeric, Boolean, + # Array} + :attributes, # optional Hash{String => String, Numeric, Boolean, + # Array} + :trace_id, # optional String (16-byte binary) + :span_id, # optional String (8-byte binary) + :trace_flags, # optional Integer (8-bit byte of bit flags) + :resource, # optional OpenTelemetry::SDK::Resources::Resource + :instrumentation_scope, # optional OpenTelemetry::SDK::InstrumentationScope :total_recorded_attributes) # Integer end end diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb index a8d8edf99f..15fa3bcaf3 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb @@ -9,8 +9,6 @@ module SDK module Logs # The SDK implementation of OpenTelemetry::Logs::Logger class Logger < OpenTelemetry::Logs::Logger - attr_reader :instrumentation_scope, :logger_provider - # @api private # # Returns a new {OpenTelemetry::SDK::Logs::Logger} instance. This should @@ -28,10 +26,6 @@ def initialize(name, version, logger_provider) @logger_provider = logger_provider end - def resource - logger_provider.resource - end - # Emit a {LogRecord} to the processing pipeline. # # @param [optional Time] timestamp Time when the event occurred. @@ -78,18 +72,18 @@ def on_emit(timestamp: nil, current_span = OpenTelemetry::Trace.current_span(context) span_context = current_span.context unless current_span == OpenTelemetry::Trace::Span::INVALID - log_record = LogRecord.new(timestamp: timestamp, - observed_timestamp: observed_timestamp, - severity_text: severity_text, - severity_number: severity_number, - body: body, - attributes: attributes, - trace_id: trace_id || span_context&.trace_id, - span_id: span_id || span_context&.span_id, - trace_flags: trace_flags || span_context&.trace_flags, - logger: self) - logger_provider.on_emit(log_record, context) + @logger_provider.on_emit(timestamp: timestamp, + observed_timestamp: observed_timestamp, + severity_text: severity_text, + severity_number: severity_number, + body: body, + attributes: attributes, + trace_id: trace_id || span_context&.trace_id, + span_id: span_id || span_context&.span_id, + trace_flags: trace_flags || span_context&.trace_flags, + instrumentation_scope: @instrumentation_scope, + context: context) end end end diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb index d767c9c6b0..99e7df10e5 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb @@ -9,8 +9,6 @@ module SDK module Logs # The SDK implementation of OpenTelemetry::Logs::LoggerProvider. class LoggerProvider < OpenTelemetry::Logs::LoggerProvider - attr_reader :resource - UNEXPECTED_ERROR_MESSAGE = 'unexpected error in ' \ 'OpenTelemetry::SDK::Logs::LoggerProvider#%s' @@ -121,8 +119,31 @@ def force_flush(timeout: nil) end end - # Call the on_emit methods for each log_record_processor - def on_emit(log_record, context) + # @api private + def on_emit(timestamp: nil, + observed_timestamp: nil, + severity_text: nil, + severity_number: nil, + body: nil, + attributes: nil, + trace_id: nil, + span_id: nil, + trace_flags: nil, + instrumentation_scope: nil, + context: nil) + + log_record = LogRecord.new(timestamp: timestamp, + observed_timestamp: observed_timestamp, + severity_text: severity_text, + severity_number: severity_number, + body: body, + attributes: attributes, + trace_id: trace_id, + span_id: span_id, + trace_flags: trace_flags, + resource: @resource, + instrumentation_scope: instrumentation_scope) + @log_record_processors.each { |processor| processor.on_emit(log_record, context) } end end diff --git a/logs_sdk/test/opentelemetry/sdk/logs/log_record_test.rb b/logs_sdk/test/opentelemetry/sdk/logs/log_record_test.rb index 72f6c4493e..1d9711a946 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/log_record_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/log_record_test.rb @@ -75,7 +75,8 @@ trace_id: span_context.trace_id, span_id: span_context.span_id, trace_flags: span_context.trace_flags, - logger: logger + resource: logger.instance_variable_get(:@logger_provider).instance_variable_get(:@resource), + instrumentation_scope: logger.instance_variable_get(:@instrumentation_scope) } end @@ -91,40 +92,8 @@ assert_equal(args[:trace_id], log_record_data.trace_id) assert_equal(args[:span_id], log_record_data.span_id) assert_equal(args[:trace_flags], log_record_data.trace_flags) - assert_equal(args[:logger].resource, log_record_data.resource) - assert_equal(args[:logger].instrumentation_scope, log_record_data.instrumentation_scope) - end - end - - describe 'attributes set through logger' do - let(:logger_provider) { Logs::LoggerProvider.new } - let(:resource) { OpenTelemetry::SDK::Resources::Resource.create } - let(:instrumentation_scope) { OpenTelemetry::SDK::InstrumentationScope.new('name', 'version') } - let(:logger) { Logs::Logger.new(resource, instrumentation_scope, logger_provider) } - let(:args) { { logger: logger } } - - describe 'resource' do - it 'is set to the resource of the logger given on initialization' do - assert_equal(logger.resource, log_record.resource) - end - end - - describe 'instrumentation_scope' do - it 'is set to the instrumentation_scope of the logger given on initialization' do - assert_equal(logger.instrumentation_scope, log_record.instrumentation_scope) - end - end - - describe 'when logger is nil' do - let(:logger) { nil } - - it 'sets the resource to nil' do - assert_nil(log_record.resource) - end - - it 'sets the instrumentation_scope to nil' do - assert_nil(log_record.instrumentation_scope) - end + assert_equal(args[:resource], log_record_data.resource) + assert_equal(args[:instrumentation_scope], log_record_data.instrumentation_scope) end end end diff --git a/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb index bfbc109dab..fda91bb9bb 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb @@ -19,7 +19,7 @@ it 'allows a resource to be associated with the logger provider' do assert_instance_of( - OpenTelemetry::SDK::Resources::Resource, logger_provider.resource + OpenTelemetry::SDK::Resources::Resource, logger_provider.instance_variable_get(:@resource) ) end end @@ -172,16 +172,47 @@ end describe '#on_emit' do - it 'sends the log record to the processors' do - mock_log_record = Minitest::Mock.new + let(:mock_context) do mock_context = Minitest::Mock.new def mock_context.value(key); OpenTelemetry::Trace::Span::INVALID; end # rubocop:disable Style/SingleLineMethods - logger_provider.add_log_record_processor(mock_log_record_processor) - mock_log_record_processor.expect(:on_emit, nil, [mock_log_record, mock_context]) + mock_context + end + + let(:args) do + span_context = OpenTelemetry::Trace::SpanContext.new + { + timestamp: Time.now, + observed_timestamp: Time.now + 1, + severity_text: 'DEBUG', + severity_number: 0, + body: 'body', + attributes: { 'a' => 'b' }, + trace_id: span_context.trace_id, + span_id: span_context.span_id, + trace_flags: span_context.trace_flags, + instrumentation_scope: OpenTelemetry::SDK::InstrumentationScope, + context: mock_context, + } + end + + it 'creates a new log record' do + output = 'union station' + OpenTelemetry::SDK::Logs::LogRecord.stub(:new, ->(_) { puts output }) do + assert_output(/#{output}/) { logger_provider.on_emit(**args) } + end + end - logger_provider.on_emit(mock_log_record, mock_context) - mock_log_record_processor.verify + it 'sends the log record to the processors' do + 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) + mock_log_record_processor.verify + end end end end diff --git a/logs_sdk/test/opentelemetry/sdk/logs/logger_test.rb b/logs_sdk/test/opentelemetry/sdk/logs/logger_test.rb index cb15516778..6fae3a85f8 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/logger_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_test.rb @@ -10,12 +10,6 @@ let(:logger_provider) { OpenTelemetry::SDK::Logs::LoggerProvider.new } let(:logger) { logger_provider.logger(name: 'default_logger') } - describe '#resource' do - it 'returns the resource associated with the logger_provider' do - assert_equal(logger.resource, logger_provider.resource) - end - end - describe '#on_emit' do it 'creates a new LogRecord' do output = 'chocolate cherry'