From f70094d949d64de8a1081340b39f0585aa95905a Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Tue, 27 Aug 2024 15:55:38 -0700 Subject: [PATCH 01/12] WIP: Log SDK configuration --- api/lib/opentelemetry.rb | 13 +++- logs_api/lib/opentelemetry-logs-api.rb | 22 ++++++ .../opentelemetry/internal/proxy_logger.rb | 68 +++++++++++++++++++ .../internal/proxy_logger_provider.rb | 60 ++++++++++++++++ sdk/lib/opentelemetry/sdk/configurator.rb | 13 +++- 5 files changed, 173 insertions(+), 3 deletions(-) create mode 100644 logs_api/lib/opentelemetry/internal/proxy_logger.rb create mode 100644 logs_api/lib/opentelemetry/internal/proxy_logger_provider.rb diff --git a/api/lib/opentelemetry.rb b/api/lib/opentelemetry.rb index 3234fd782..5e9f795c2 100644 --- a/api/lib/opentelemetry.rb +++ b/api/lib/opentelemetry.rb @@ -28,7 +28,7 @@ module OpenTelemetry # @return [Object, Logger] configured Logger or a default STDOUT Logger. def logger - @logger ||= Logger.new($stdout, level: ENV['OTEL_LOG_LEVEL'] || Logger::INFO) + @logger ||= create_logger end # @return [Callable] configured error handler or a default that logs the @@ -69,4 +69,15 @@ def tracer_provider def propagation @propagation ||= Context::Propagation::NoopTextMapPropagator.new end + + private + + def create_logger + logger = Logger.new($stdout, level: ENV['OTEL_LOG_LEVEL'] || Logger::INFO) + # @skip_instrumenting prevents Ruby Logger instrumentation from + # triggering a stack overflow. Logs emitted using OpenTelemetry.logger + # will not be turned into OpenTelemetry LogRecords. + logger.instance_variable_set(:@skip_instrumenting, true) + logger + end end diff --git a/logs_api/lib/opentelemetry-logs-api.rb b/logs_api/lib/opentelemetry-logs-api.rb index 47ba36905..723004ed2 100644 --- a/logs_api/lib/opentelemetry-logs-api.rb +++ b/logs_api/lib/opentelemetry-logs-api.rb @@ -7,3 +7,25 @@ require 'opentelemetry' require_relative 'opentelemetry/logs' require_relative 'opentelemetry/logs/version' + +# 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 provides global accessors for telemetry objects. +module OpenTelemetry + # Register the global logger provider. + # + # @param [LoggerProvider] provider A logger provider to register as the + # global instance. + def logger_provider=(provider) + puts 'nil logger provider' if provider.nil? + @logger_provider = provider + 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..10dfe0bbf --- /dev/null +++ b/logs_api/lib/opentelemetry/internal/proxy_logger.rb @@ -0,0 +1,68 @@ +# 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 + # Returns a new {ProxyLogger} instance. + # + # @return [ProxyLogger] + def initialize + super + @delegate = nil + end + + # Set the delegate Logger. If this is called more than once, a warning will + # be logged and superfluous calls will be ignored. + # + # @param [Logger] logger The Logger to delegate to + def delegate=(logger) + @mutex.synchronize do + if @delegate.nil? + @delegate = logger + @instrument_registry.each_value { |instrument| instrument.upgrade_with(logger) } + else + OpenTelemetry.logger.warn 'Attempt to reset delegate in ProxyLogger ignored.' + end + end + 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 + ) + @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 + ) + + 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/sdk/lib/opentelemetry/sdk/configurator.rb b/sdk/lib/opentelemetry/sdk/configurator.rb index ae1bbaf07..86cbd2df4 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 @@ -179,7 +189,6 @@ def wrapped_exporters_from_env # rubocop:disable Metrics/CyclomaticComplexity when 'none' then nil when 'otlp' otlp_protocol = ENV['OTEL_EXPORTER_OTLP_TRACES_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, spans will not be exported." nil From f6fb046a9caf700e80c422ae25d85ddab681802d Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Wed, 28 Aug 2024 08:56:58 -0700 Subject: [PATCH 02/12] feat: Add configuration patch for logs SDK --- logs_sdk/lib/opentelemetry/sdk/logs.rb | 1 + .../sdk/logs/configuration_patch.rb | 67 +++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb diff --git a/logs_sdk/lib/opentelemetry/sdk/logs.rb b/logs_sdk/lib/opentelemetry/sdk/logs.rb index fbd978c68..bc661d471 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..51c3d9d42 --- /dev/null +++ b/logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb @@ -0,0 +1,67 @@ +# 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 + # default is console until other exporters built + 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 + Logs::Export::BatchLogRecordProcessor.new(OpenTelemetry::Exporter::OTLP::LogsExporter.new) + # fetch_exporter(exporter, 'OpenTelemetry::Exporter::OTLP::Exporter') + 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) From 679c085452472faf2cdaf19d5d784588022aad99 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Wed, 28 Aug 2024 09:03:09 -0700 Subject: [PATCH 03/12] style: Update spacing --- .../lib/opentelemetry/sdk/logs/configuration_patch.rb | 11 +++++------ sdk/lib/opentelemetry/sdk/configurator.rb | 1 + 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb b/logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb index 51c3d9d42..3c12759ad 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb @@ -9,8 +9,8 @@ module OpenTelemetry module SDK module Logs - # The ConfiguratorPatch implements a hook to configure the logs - # portion of the SDK. + # 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 @@ -23,8 +23,8 @@ def initialize @log_record_processors = [] end - # The logs_configuration_hook method is where we define the setup process - # for logs SDK. + # 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 @@ -36,7 +36,7 @@ def configure_log_record_processors end def wrapped_log_exporters_from_env - # default is console until other exporters built + # 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| @@ -51,7 +51,6 @@ def wrapped_log_exporters_from_env nil else Logs::Export::BatchLogRecordProcessor.new(OpenTelemetry::Exporter::OTLP::LogsExporter.new) - # fetch_exporter(exporter, 'OpenTelemetry::Exporter::OTLP::Exporter') end else OpenTelemetry.logger.warn "The #{exporter} exporter is unknown and cannot be configured, log records will not be exported" diff --git a/sdk/lib/opentelemetry/sdk/configurator.rb b/sdk/lib/opentelemetry/sdk/configurator.rb index 86cbd2df4..94a5020eb 100644 --- a/sdk/lib/opentelemetry/sdk/configurator.rb +++ b/sdk/lib/opentelemetry/sdk/configurator.rb @@ -189,6 +189,7 @@ def wrapped_exporters_from_env # rubocop:disable Metrics/CyclomaticComplexity when 'none' then nil when 'otlp' otlp_protocol = ENV['OTEL_EXPORTER_OTLP_TRACES_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, spans will not be exported." nil From a6d7079dfce71f55184148c7821bd92834c589fa Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Mon, 9 Sep 2024 15:54:59 -0700 Subject: [PATCH 04/12] test: Add tests for logs api --- logs_api/test/opentelemetry-logs-api_test.rb | 67 ++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 logs_api/test/opentelemetry-logs-api_test.rb 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..9b094ab1f --- /dev/null +++ b/logs_api/test/opentelemetry-logs-api_test.rb @@ -0,0 +1,67 @@ +# 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 'upgrades default loggers to "real" loggers' do + skip 'copy from tracer, not sure if this is how it should work' + default_logger = OpenTelemetry.logger_provider.logger + _(default_logger.on_emit(body: 'test')).must_be_instance_of(OpenTelemetry::Logs::LogRecord) + 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 From c9554dfb72f8d1f1979db6905fe93c4d699304d5 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Mon, 9 Sep 2024 17:34:56 -0700 Subject: [PATCH 05/12] feat: Update inheritance to get tests to pass --- logs_api/lib/opentelemetry-logs-api.rb | 20 ++++++++++++++----- .../opentelemetry/internal/proxy_logger.rb | 6 +++--- logs_api/lib/opentelemetry/logs.rb | 10 +++++----- logs_api/test/opentelemetry-logs-api_test.rb | 13 ++++++++---- 4 files changed, 32 insertions(+), 17 deletions(-) diff --git a/logs_api/lib/opentelemetry-logs-api.rb b/logs_api/lib/opentelemetry-logs-api.rb index 723004ed2..03b09d7fe 100644 --- a/logs_api/lib/opentelemetry-logs-api.rb +++ b/logs_api/lib/opentelemetry-logs-api.rb @@ -5,22 +5,32 @@ # 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 provides global accessors for telemetry objects. +# 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) - puts 'nil logger provider' if provider.nil? - @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 diff --git a/logs_api/lib/opentelemetry/internal/proxy_logger.rb b/logs_api/lib/opentelemetry/internal/proxy_logger.rb index 10dfe0bbf..837c584a1 100644 --- a/logs_api/lib/opentelemetry/internal/proxy_logger.rb +++ b/logs_api/lib/opentelemetry/internal/proxy_logger.rb @@ -18,6 +18,7 @@ class ProxyLogger < Logs::Logger # @return [ProxyLogger] def initialize super + @mutex = Mutex.new @delegate = nil end @@ -29,7 +30,6 @@ def delegate=(logger) @mutex.synchronize do if @delegate.nil? @delegate = logger - @instrument_registry.each_value { |instrument| instrument.upgrade_with(logger) } else OpenTelemetry.logger.warn 'Attempt to reset delegate in ProxyLogger ignored.' end @@ -48,7 +48,7 @@ def on_emit( attributes: nil, context: nil ) - @delegate.on_emit( + return @delegate.on_emit( timestamp: nil, observed_timestamp: nil, severity_number: nil, @@ -59,7 +59,7 @@ def on_emit( trace_flags: nil, attributes: nil, context: nil - ) + ) unless @delegate.nil? super 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 index 9b094ab1f..9034a2b30 100644 --- a/logs_api/test/opentelemetry-logs-api_test.rb +++ b/logs_api/test/opentelemetry-logs-api_test.rb @@ -50,15 +50,20 @@ def logger(name = nil, version = nil) OpenTelemetry.logger_provider = OpenTelemetry::Internal::ProxyLoggerProvider.new end - it 'upgrades default loggers to "real" loggers' do - skip 'copy from tracer, not sure if this is how it should work' + 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(OpenTelemetry::Logs::LogRecord) + _(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 + 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) From 0e1ad908d3abc417e5aa5495a5373f7526575d8e Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Tue, 10 Sep 2024 16:00:19 -0700 Subject: [PATCH 06/12] 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 --- .../opentelemetry/sdk/logs/logger_provider.rb | 24 +++++++++--- .../sdk/logs/logger_provider_test.rb | 37 +++++++++++++++++++ 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb index 99e7df10e..0fccdfba9 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' @@ -25,6 +28,8 @@ def initialize(resource: OpenTelemetry::SDK::Resources::Resource.create) @mutex = Mutex.new @resource = resource @stopped = false + @registry = {} + @registry_mutex = Mutex.new end # Returns an {OpenTelemetry::SDK::Logs::Logger} instance. @@ -34,14 +39,21 @@ def initialize(resource: OpenTelemetry::SDK::Resources::Resource.create) # # @return [OpenTelemetry::SDK::Logs::Logger] def logger(name:, version: nil) - version ||= '' + 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 - if !name.is_a?(String) || name.empty? - OpenTelemetry.logger.warn('LoggerProvider#logger called with an ' \ - "invalid name. Name provided: #{name.inspect}") + @registry_mutex.synchronize do + @registry[Key.new(name, version)] ||= Logger.new(name, version, self) + end end - - Logger.new(name, version, self) end # Adds a new log record processor to this LoggerProvider's 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 023a9a67c..91108d749 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb @@ -215,4 +215,41 @@ def mock_context.value(key); OpenTelemetry::Trace::Span::INVALID; end # rubocop: 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 + + 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 From 6a624e6c4f68b8d417a3d5acb6586fff0bd21738 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Tue, 10 Sep 2024 16:04:25 -0700 Subject: [PATCH 07/12] 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. --- logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb b/logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb index 3c12759ad..83df0c912 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb @@ -50,7 +50,12 @@ def wrapped_log_exporters_from_env OpenTelemetry.logger.warn "The #{otlp_protocol} transport protocol is not supported by the OTLP exporter, log_records will not be exported." nil else - Logs::Export::BatchLogRecordProcessor.new(OpenTelemetry::Exporter::OTLP::LogsExporter.new) + 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" From dfe8f4451ab59433d793b3ad8439ee2438a4fea8 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Tue, 10 Sep 2024 16:07:54 -0700 Subject: [PATCH 08/12] Remove skip instrumenting stuff --- api/lib/opentelemetry.rb | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/api/lib/opentelemetry.rb b/api/lib/opentelemetry.rb index 5e9f795c2..3234fd782 100644 --- a/api/lib/opentelemetry.rb +++ b/api/lib/opentelemetry.rb @@ -28,7 +28,7 @@ module OpenTelemetry # @return [Object, Logger] configured Logger or a default STDOUT Logger. def logger - @logger ||= create_logger + @logger ||= Logger.new($stdout, level: ENV['OTEL_LOG_LEVEL'] || Logger::INFO) end # @return [Callable] configured error handler or a default that logs the @@ -69,15 +69,4 @@ def tracer_provider def propagation @propagation ||= Context::Propagation::NoopTextMapPropagator.new end - - private - - def create_logger - logger = Logger.new($stdout, level: ENV['OTEL_LOG_LEVEL'] || Logger::INFO) - # @skip_instrumenting prevents Ruby Logger instrumentation from - # triggering a stack overflow. Logs emitted using OpenTelemetry.logger - # will not be turned into OpenTelemetry LogRecords. - logger.instance_variable_set(:@skip_instrumenting, true) - logger - end end From b53e6a321c94b1b04cb02d6f2a820898ce7e08c7 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Wed, 11 Sep 2024 09:12:18 -0700 Subject: [PATCH 09/12] style: Rubocop --- .../opentelemetry/internal/proxy_logger.rb | 26 ++++++++++--------- ...test.rb => opentelemetry_logs_api_test.rb} | 0 .../sdk/logs/configuration_patch.rb | 2 +- .../sdk/logs/logger_provider_test.rb | 1 - 4 files changed, 15 insertions(+), 14 deletions(-) rename logs_api/test/{opentelemetry-logs-api_test.rb => opentelemetry_logs_api_test.rb} (100%) diff --git a/logs_api/lib/opentelemetry/internal/proxy_logger.rb b/logs_api/lib/opentelemetry/internal/proxy_logger.rb index 837c584a1..8673d40d5 100644 --- a/logs_api/lib/opentelemetry/internal/proxy_logger.rb +++ b/logs_api/lib/opentelemetry/internal/proxy_logger.rb @@ -48,18 +48,20 @@ def on_emit( attributes: nil, context: 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 - ) unless @delegate.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 diff --git a/logs_api/test/opentelemetry-logs-api_test.rb b/logs_api/test/opentelemetry_logs_api_test.rb similarity index 100% rename from logs_api/test/opentelemetry-logs-api_test.rb rename to logs_api/test/opentelemetry_logs_api_test.rb diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb b/logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb index 83df0c912..6ca09e8aa 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb @@ -53,7 +53,7 @@ def wrapped_log_exporters_from_env 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" + 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 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 91108d749..e9b5380da 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb @@ -216,7 +216,6 @@ def mock_context.value(key); OpenTelemetry::Trace::Span::INVALID; end # rubocop: 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 654385d1893e3c888f12baa03ae7d00c8851cca5 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Wed, 11 Sep 2024 19:01:31 -0700 Subject: [PATCH 10/12] test: Add skip for intermittent failure --- .../sdk/logs/export/batch_log_record_processor_test.rb | 1 + 1 file changed, 1 insertion(+) 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 64e81477a..d68407d55 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 @@ -469,6 +469,7 @@ def shutdown(timeout: nil) let(:processor) { BatchLogRecordProcessor.new(exporter) } it 'reports export failures' do + skip 'intermittent failure will be fixed in #1701' mock_logger = Minitest::Mock.new mock_logger.expect(:error, nil, [/Unable to export/]) mock_logger.expect(:error, nil, [/Result code: 1/]) From 9e2b8eeb87d3ca900bb687ddedd5421e527079a7 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Mon, 28 Oct 2024 16:27:08 -0700 Subject: [PATCH 11/12] refactor: Remove delegate, mutex from ProxyLogger --- .../lib/opentelemetry/internal/proxy_logger.rb | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/logs_api/lib/opentelemetry/internal/proxy_logger.rb b/logs_api/lib/opentelemetry/internal/proxy_logger.rb index 8673d40d5..a392c3dbc 100644 --- a/logs_api/lib/opentelemetry/internal/proxy_logger.rb +++ b/logs_api/lib/opentelemetry/internal/proxy_logger.rb @@ -13,29 +13,15 @@ module Internal # 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 - super - @mutex = Mutex.new @delegate = nil end - # Set the delegate Logger. If this is called more than once, a warning will - # be logged and superfluous calls will be ignored. - # - # @param [Logger] logger The Logger to delegate to - def delegate=(logger) - @mutex.synchronize do - if @delegate.nil? - @delegate = logger - else - OpenTelemetry.logger.warn 'Attempt to reset delegate in ProxyLogger ignored.' - end - end - end - def on_emit( timestamp: nil, observed_timestamp: nil, From 06e4b05744729af78047a22dcbd1fa18b26a064d Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Mon, 28 Oct 2024 16:43:41 -0700 Subject: [PATCH 12/12] 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. --- .../opentelemetry/sdk/logs/logger_provider.rb | 23 +++++------- .../sdk/logs/logger_provider_test.rb | 37 +++++++------------ 2 files changed, 24 insertions(+), 36 deletions(-) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb index b7faa567c..a06494772 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb @@ -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 @@ -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, 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 02214b375..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,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 @@ -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