From be01344514c533a1760a507b8d6133acffda22af Mon Sep 17 00:00:00 2001 From: Kayla Reopelle <87386821+kaylareopelle@users.noreply.github.com> Date: Fri, 15 Nov 2024 13:05:15 -0800 Subject: [PATCH 1/2] test: Add workaround for windows test failure (#1751) We have to instantiate the processor outside of the stub so that the stub can be applied to the processor, but the method we're stubbing runs on initialization, so we sometimes run out of expectations. If we explicitly set 'OTEL_RUBY_BLRP_START_THREAD_ON_BOOT' to 'false' for the 'removes the older log records from the batch if full' test, the stubbed `work` method will not run outside of the stub block. Co-authored-by: Tanna McClure <tannalynn@users.noreply.github.com> --- .../export/batch_log_record_processor_test.rb | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/logs_sdk/test/opentelemetry/sdk/logs/export/batch_log_record_processor_test.rb b/logs_sdk/test/opentelemetry/sdk/logs/export/batch_log_record_processor_test.rb index a7ed0c362..cff2b718d 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/export/batch_log_record_processor_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/export/batch_log_record_processor_test.rb @@ -186,20 +186,23 @@ def to_log_record_data end it 'removes the older log records from the batch if full' do - processor = BatchLogRecordProcessor.new(TestExporter.new, max_queue_size: 1, max_export_batch_size: 1) + # Windows intermittently fails if we don't set this + OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_BLRP_START_THREAD_ON_BOOT' => 'false') do + processor = BatchLogRecordProcessor.new(TestExporter.new, max_queue_size: 1, max_export_batch_size: 1) - # Don't actually try to export, we're looking at the log records array - processor.stub(:work, nil) do - older_log_record = TestLogRecord.new - newest_log_record = TestLogRecord.new + # Don't actually try to export, we're looking at the log records array + processor.stub(:work, nil) do + older_log_record = TestLogRecord.new + newest_log_record = TestLogRecord.new - processor.on_emit(older_log_record, mock_context) - processor.on_emit(newest_log_record, mock_context) + processor.on_emit(older_log_record, mock_context) + processor.on_emit(newest_log_record, mock_context) - records = processor.instance_variable_get(:@log_records) + records = processor.instance_variable_get(:@log_records) - assert_includes(records, newest_log_record) - refute_includes(records, older_log_record) + assert_includes(records, newest_log_record) + refute_includes(records, older_log_record) + end end end From aa6ecce6a15df71a5ad3005450d2d30f7b677348 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle <87386821+kaylareopelle@users.noreply.github.com> Date: Mon, 18 Nov 2024 13:33:06 -0800 Subject: [PATCH 2/2] feat: OpenTelemetry.logger_provider API, ProxyLoggers, Configuration, and Instrument Registry (#1725) * WIP: Log SDK configuration * feat: Add configuration patch for logs SDK * style: Update spacing * test: Add tests for logs api * feat: Update inheritance to get tests to pass * feat: Add Instrument Registry to LoggerProvider Create a registry for loggers to make sure a logger with an identical name and version is created only once and reused * feat: Rescue NameError for OTLP logs exporter When OTLP logs exporter not installed, rescue the error, emit a message and set the exporter to nil. * Remove skip instrumenting stuff * style: Rubocop * test: Add skip for intermittent failure * refactor: Remove delegate, mutex from ProxyLogger * fix: Do not emit logs if stopped 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. --- logs_api/lib/opentelemetry-logs-api.rb | 36 +++++++++- .../opentelemetry/internal/proxy_logger.rb | 56 +++++++++++++++ .../internal/proxy_logger_provider.rb | 60 ++++++++++++++++ logs_api/lib/opentelemetry/logs.rb | 10 +-- logs_api/test/opentelemetry_logs_api_test.rb | 72 +++++++++++++++++++ logs_sdk/lib/opentelemetry/sdk/logs.rb | 1 + .../sdk/logs/configuration_patch.rb | 71 ++++++++++++++++++ .../opentelemetry/sdk/logs/logger_provider.rb | 11 ++- .../sdk/logs/logger_provider_test.rb | 27 +++++++ sdk/lib/opentelemetry/sdk/configurator.rb | 12 +++- 10 files changed, 347 insertions(+), 9 deletions(-) create mode 100644 logs_api/lib/opentelemetry/internal/proxy_logger.rb create mode 100644 logs_api/lib/opentelemetry/internal/proxy_logger_provider.rb create mode 100644 logs_api/test/opentelemetry_logs_api_test.rb create mode 100644 logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb diff --git a/logs_api/lib/opentelemetry-logs-api.rb b/logs_api/lib/opentelemetry-logs-api.rb index 47ba36905..03b09d7fe 100644 --- a/logs_api/lib/opentelemetry-logs-api.rb +++ b/logs_api/lib/opentelemetry-logs-api.rb @@ -5,5 +5,37 @@ # SPDX-License-Identifier: Apache-2.0 require 'opentelemetry' -require_relative 'opentelemetry/logs' -require_relative 'opentelemetry/logs/version' +require 'opentelemetry/logs' +require 'opentelemetry/logs/version' +require 'opentelemetry/internal/proxy_logger_provider' +require 'opentelemetry/internal/proxy_logger' + +# OpenTelemetry is an open source observability framework, providing a +# general-purpose API, SDK, and related tools required for the instrumentation +# of cloud-native software, frameworks, and libraries. +# +# The OpenTelemetry module in the Logs API gem provides global accessors +# for logs-related objects. +module OpenTelemetry + @logger_provider = Internal::ProxyLoggerProvider.new + + # Register the global logger provider. + # + # @param [LoggerProvider] provider A logger provider to register as the + # global instance. + def logger_provider=(provider) + @mutex.synchronize do + if @logger_provider.instance_of? Internal::ProxyLoggerProvider + logger.debug("Upgrading default proxy logger provider to #{provider.class}") + @logger_provider.delegate = provider + end + @logger_provider = provider + end + end + + # @return [Object, Logs::LoggerProvider] registered logger provider or a + # default no-op implementation of the logger provider. + def logger_provider + @mutex.synchronize { @logger_provider } + end +end diff --git a/logs_api/lib/opentelemetry/internal/proxy_logger.rb b/logs_api/lib/opentelemetry/internal/proxy_logger.rb new file mode 100644 index 000000000..a392c3dbc --- /dev/null +++ b/logs_api/lib/opentelemetry/internal/proxy_logger.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Internal + # @api private + # + # {ProxyLogger} is an implementation of {OpenTelemetry::Logs::Logger}. It is returned from + # the ProxyLoggerProvider until a delegate logger provider is installed. After the delegate + # logger provider is installed, the ProxyLogger will delegate to the corresponding "real" + # logger. + class ProxyLogger < Logs::Logger + attr_writer :delegate + + # Returns a new {ProxyLogger} instance. + # + # @return [ProxyLogger] + def initialize + @delegate = nil + end + + def on_emit( + timestamp: nil, + observed_timestamp: nil, + severity_number: nil, + severity_text: nil, + body: nil, + trace_id: nil, + span_id: nil, + trace_flags: nil, + attributes: nil, + context: nil + ) + unless @delegate.nil? + return @delegate.on_emit( + timestamp: nil, + observed_timestamp: nil, + severity_number: nil, + severity_text: nil, + body: nil, + trace_id: nil, + span_id: nil, + trace_flags: nil, + attributes: nil, + context: nil + ) + end + + super + end + end + end +end diff --git a/logs_api/lib/opentelemetry/internal/proxy_logger_provider.rb b/logs_api/lib/opentelemetry/internal/proxy_logger_provider.rb new file mode 100644 index 000000000..0cbedc9d6 --- /dev/null +++ b/logs_api/lib/opentelemetry/internal/proxy_logger_provider.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Internal + # @api private + # + # {ProxyLoggerProvider} is an implementation of {OpenTelemetry::Logs::LoggerProvider}. + # It is the default global logger provider returned by OpenTelemetry.logger_provider. + # It delegates to a "real" LoggerProvider after the global logger provider is registered. + # It returns {ProxyLogger} instances until the delegate is installed. + class ProxyLoggerProvider < Logs::LoggerProvider + Key = Struct.new(:name, :version) + private_constant(:Key) + # Returns a new {ProxyLoggerProvider} instance. + # + # @return [ProxyLoggerProvider] + def initialize + super + + @mutex = Mutex.new + @registry = {} + @delegate = nil + end + + # Set the delegate logger provider. If this is called more than once, a warning will + # be logged and superfluous calls will be ignored. + # + # @param [LoggerProvider] provider The logger provider to delegate to + def delegate=(provider) + unless @delegate.nil? + OpenTelemetry.logger.warn 'Attempt to reset delegate in ProxyLoggerProvider ignored.' + return + end + + @mutex.synchronize do + @delegate = provider + @registry.each { |key, logger| logger.delegate = provider.logger(key.name, key.version) } + end + end + + # Returns a {Logger} instance. + # + # @param [optional String] name Instrumentation package name + # @param [optional String] version Instrumentation package version + # + # @return [Logger] + def logger(name = nil, version = nil) + @mutex.synchronize do + return @delegate.logger(name, version) unless @delegate.nil? + + @registry[Key.new(name, version)] ||= ProxyLogger.new + end + end + end + end +end diff --git a/logs_api/lib/opentelemetry/logs.rb b/logs_api/lib/opentelemetry/logs.rb index 0669bb21d..ade58537c 100644 --- a/logs_api/lib/opentelemetry/logs.rb +++ b/logs_api/lib/opentelemetry/logs.rb @@ -4,11 +4,6 @@ # # SPDX-License-Identifier: Apache-2.0 -require_relative 'logs/log_record' -require_relative 'logs/logger' -require_relative 'logs/logger_provider' -require_relative 'logs/severity_number' - module OpenTelemetry # The Logs API records a timestamped record with metadata. # In OpenTelemetry, any data that is not part of a distributed trace or a @@ -20,3 +15,8 @@ module OpenTelemetry module Logs end end + +require 'opentelemetry/logs/log_record' +require 'opentelemetry/logs/logger' +require 'opentelemetry/logs/logger_provider' +require 'opentelemetry/logs/severity_number' diff --git a/logs_api/test/opentelemetry_logs_api_test.rb b/logs_api/test/opentelemetry_logs_api_test.rb new file mode 100644 index 000000000..9034a2b30 --- /dev/null +++ b/logs_api/test/opentelemetry_logs_api_test.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require 'test_helper' + +describe OpenTelemetry do + class CustomLogRecord < OpenTelemetry::Logs::LogRecord + end + + class CustomLogger < OpenTelemetry::Logs::Logger + def on_emit(*) + CustomLogRecord.new + end + end + + class CustomLoggerProvider < OpenTelemetry::Logs::LoggerProvider + def logger(name = nil, version = nil) + CustomLogger.new + end + end + + describe '.logger_provider' do + after do + # Ensure we don't leak custom logger factories and loggers to other tests + OpenTelemetry.logger_provider = OpenTelemetry::Internal::ProxyLoggerProvider.new + end + + it 'returns a Logs::LoggerProvider by default' do + logger_provider = OpenTelemetry.logger_provider + _(logger_provider).must_be_kind_of(OpenTelemetry::Logs::LoggerProvider) + end + + it 'returns the same instance when accessed multiple times' do + _(OpenTelemetry.logger_provider).must_equal(OpenTelemetry.logger_provider) + end + + it 'returns user-specified logger provider' do + custom_logger_provider = CustomLoggerProvider.new + OpenTelemetry.logger_provider = custom_logger_provider + _(OpenTelemetry.logger_provider).must_equal(custom_logger_provider) + end + end + + describe '.logger_provider=' do + after do + # Ensure we don't leak custom logger factories and loggers to other tests + OpenTelemetry.logger_provider = OpenTelemetry::Internal::ProxyLoggerProvider.new + end + + it 'has a default proxy logger' do + refute_nil OpenTelemetry.logger_provider.logger + end + + it 'upgrades default loggers to *real* loggers' do + # proxy loggers do not emit any log records, nor does the API logger + # the on_emit method is empty + default_logger = OpenTelemetry.logger_provider.logger + _(default_logger.on_emit(body: 'test')).must_be_instance_of(NilClass) + OpenTelemetry.logger_provider = CustomLoggerProvider.new + _(default_logger.on_emit(body: 'test')).must_be_instance_of(CustomLogRecord) + end + + it 'upgrades the default logger provider to a *real* logger provider' do + default_logger_provider = OpenTelemetry.logger_provider + OpenTelemetry.logger_provider = CustomLoggerProvider.new + _(default_logger_provider.logger).must_be_instance_of(CustomLogger) + end + end +end diff --git a/logs_sdk/lib/opentelemetry/sdk/logs.rb b/logs_sdk/lib/opentelemetry/sdk/logs.rb index 6003e3228..b310d6aa5 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs.rb @@ -5,6 +5,7 @@ # SPDX-License-Identifier: Apache-2.0 require_relative 'logs/version' +require_relative 'logs/configuration_patch' require_relative 'logs/logger' require_relative 'logs/logger_provider' require_relative 'logs/log_record' diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb b/logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb new file mode 100644 index 000000000..6ca09e8aa --- /dev/null +++ b/logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require 'opentelemetry/sdk/configurator' + +module OpenTelemetry + module SDK + module Logs + # The ConfiguratorPatch implements a hook to configure the logs portion + # of the SDK. + module ConfiguratorPatch + def add_log_record_processor(log_record_processor) + @log_record_processors << log_record_processor + end + + private + + def initialize + super + @log_record_processors = [] + end + + # The logs_configuration_hook method is where we define the setup + # process for logs SDK. + def logs_configuration_hook + OpenTelemetry.logger_provider = Logs::LoggerProvider.new(resource: @resource) + configure_log_record_processors + end + + def configure_log_record_processors + processors = @log_record_processors.empty? ? wrapped_log_exporters_from_env.compact : @log_record_processors + processors.each { |p| OpenTelemetry.logger_provider.add_log_record_processor(p) } + end + + def wrapped_log_exporters_from_env + # TODO: set default to OTLP to match traces, default is console until other exporters merged + exporters = ENV.fetch('OTEL_LOGS_EXPORTER', 'console') + + exporters.split(',').map do |exporter| + case exporter.strip + when 'none' then nil + when 'console' then Logs::Export::SimpleLogRecordProcessor.new(Logs::Export::ConsoleLogRecordExporter.new) + when 'otlp' + otlp_protocol = ENV['OTEL_EXPORTER_OTLP_LOGS_PROTOCOL'] || ENV['OTEL_EXPORTER_OTLP_PROTOCOL'] || 'http/protobuf' + + if otlp_protocol != 'http/protobuf' + OpenTelemetry.logger.warn "The #{otlp_protocol} transport protocol is not supported by the OTLP exporter, log_records will not be exported." + nil + else + begin + Logs::Export::BatchLogRecordProcessor.new(OpenTelemetry::Exporter::OTLP::LogsExporter.new) + rescue NameError + OpenTelemetry.logger.warn 'The otlp logs exporter cannot be configured - please add opentelemetry-exporter-otlp-logs to your Gemfile. Logs will not be exported' + nil + end + end + else + OpenTelemetry.logger.warn "The #{exporter} exporter is unknown and cannot be configured, log records will not be exported" + nil + end + end + end + end + end + end +end + +OpenTelemetry::SDK::Configurator.prepend(OpenTelemetry::SDK::Logs::ConfiguratorPatch) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb index 2f28e2480..a06494772 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb @@ -9,6 +9,9 @@ module SDK module Logs # The SDK implementation of OpenTelemetry::Logs::LoggerProvider. class LoggerProvider < OpenTelemetry::Logs::LoggerProvider + Key = Struct.new(:name, :version) + private_constant(:Key) + UNEXPECTED_ERROR_MESSAGE = 'unexpected error in ' \ 'OpenTelemetry::SDK::Logs::LoggerProvider#%s' @@ -28,6 +31,8 @@ def initialize(resource: OpenTelemetry::SDK::Resources::Resource.create, log_rec @mutex = Mutex.new @resource = resource @stopped = false + @registry = {} + @registry_mutex = Mutex.new end # Returns an {OpenTelemetry::SDK::Logs::Logger} instance. @@ -44,7 +49,9 @@ def logger(name:, version: nil) "invalid name. Name provided: #{name.inspect}") end - Logger.new(name, version, self) + @registry_mutex.synchronize do + @registry[Key.new(name, version)] ||= Logger.new(name, version, self) + end end # Adds a new log record processor to this LoggerProvider's @@ -135,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, 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 a2ee0e59c..992a4e817 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb @@ -223,5 +223,32 @@ 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 + # On the first call, create a logger with an empty string for name and + # version and add to the registry. The second call returns that logger + # from the registry instead of creating a new instance. + it 'reuses the same logger if called twice when name and version are nil' do + logger = logger_provider.logger(name: nil, version: nil) + logger2 = logger_provider.logger(name: nil, version: nil) + + assert_instance_of(Logs::Logger, logger) + assert_same(logger, logger2) + end end end diff --git a/sdk/lib/opentelemetry/sdk/configurator.rb b/sdk/lib/opentelemetry/sdk/configurator.rb index ae1bbaf07..94a5020eb 100644 --- a/sdk/lib/opentelemetry/sdk/configurator.rb +++ b/sdk/lib/opentelemetry/sdk/configurator.rb @@ -126,13 +126,20 @@ def add_span_processor(span_processor) @span_processors << span_processor end + # Add a log record processor to the export pipeline + # + # @param [#emit, #shutdown, #force_flush] log_record_processor A log_record_processor + # that satisfies the duck type #emit, #shutdown, #force_flush. See + # {SimpleLogRecordProcessor} for an example. + def add_log_record_processor(log_record_processor); end + # @api private # The configure method is where we define the setup process. This allows # us to make certain guarantees about which systems and globals are setup # at each stage. The setup process is: # - setup logging # - setup propagation - # - setup tracer_provider and meter_provider + # - setup tracer_provider, meter_provider, and logger_provider # - install instrumentation def configure OpenTelemetry.logger = logger @@ -142,6 +149,7 @@ def configure tracer_provider.id_generator = @id_generator OpenTelemetry.tracer_provider = tracer_provider metrics_configuration_hook + logs_configuration_hook install_instrumentation end @@ -149,6 +157,8 @@ def configure def metrics_configuration_hook; end + def logs_configuration_hook; end + def tracer_provider @tracer_provider ||= Trace::TracerProvider.new(resource: @resource) end