From 5cee6e4f135557bf0278ab9e92928569c709ce6f Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Wed, 9 Aug 2023 11:42:05 -0700 Subject: [PATCH 01/15] Implement LoggerProvider#logger --- logs_sdk/Gemfile | 5 ++ logs_sdk/lib/opentelemetry-logs-sdk.rb | 3 +- logs_sdk/lib/opentelemetry/sdk/logs.rb | 2 + logs_sdk/lib/opentelemetry/sdk/logs/logger.rb | 31 ++++++++++ .../opentelemetry/sdk/logs/logger_provider.rb | 34 +++++++++++ logs_sdk/opentelemetry-logs-sdk.gemspec | 5 +- logs_sdk/test/.rubocop.yml | 22 +------ .../sdk/logs/logger_provider_test.rb | 59 +++++++++++++++++++ logs_sdk/test/test_helper.rb | 2 + 9 files changed, 142 insertions(+), 21 deletions(-) create mode 100644 logs_sdk/lib/opentelemetry/sdk/logs/logger.rb create mode 100644 logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb create mode 100644 logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb diff --git a/logs_sdk/Gemfile b/logs_sdk/Gemfile index f649e2f64a..69362a59bf 100644 --- a/logs_sdk/Gemfile +++ b/logs_sdk/Gemfile @@ -7,3 +7,8 @@ source 'https://rubygems.org' gemspec + +gem 'opentelemetry-api', path: '../api' +gem 'opentelemetry-logs-api', path: '../logs_api' +gem 'opentelemetry-sdk', path: '../sdk' +gem 'opentelemetry-test-helpers', path: '../test_helpers' diff --git a/logs_sdk/lib/opentelemetry-logs-sdk.rb b/logs_sdk/lib/opentelemetry-logs-sdk.rb index 0865890b8a..2f6b645458 100644 --- a/logs_sdk/lib/opentelemetry-logs-sdk.rb +++ b/logs_sdk/lib/opentelemetry-logs-sdk.rb @@ -4,5 +4,6 @@ # # SPDX-License-Identifier: Apache-2.0 +require 'opentelemetry' +require 'opentelemetry/sdk' require 'opentelemetry/sdk/logs' -require 'opentelemetry/sdk/logs/version' diff --git a/logs_sdk/lib/opentelemetry/sdk/logs.rb b/logs_sdk/lib/opentelemetry/sdk/logs.rb index b4f503b18d..576d44e276 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs.rb @@ -5,6 +5,8 @@ # SPDX-License-Identifier: Apache-2.0 require_relative 'logs/version' +require_relative 'logs/logger' +require_relative 'logs/logger_provider' module OpenTelemetry module SDK diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb new file mode 100644 index 0000000000..22d4d5fd97 --- /dev/null +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module SDK + module Logs + # {OpenTelemetry::SDK::Logs::Logger} is the SDK implementation of {OpenTelemetry::Logs::Logger} + class Logger < OpenTelemetry::Logs::Logger + attr_reader :instrumentation_scope + + # @api private + # + # Returns a new {OpenTelemetry::SDK::Logs::Logger} instance. + # + # @param [String] name Instrumentation package name + # @param [String] version Instrumentation package version + # @param [LoggerProvider] logger_provider LoggerProvider that initialized + # the logger + # + # @return [OpenTelemetry::SDK::Logs::Logger] + def initialize(name, version, logger_provider) + @instrumentation_scope = InstrumentationScope.new(name, version) + @logger_provider = logger_provider + end + end + 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 new file mode 100644 index 0000000000..c43aeb5ef9 --- /dev/null +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module SDK + module Logs + # {LoggerProvider} is the SDK implementation of + # {OpenTelemetry::Logs::LoggerProvider}. + class LoggerProvider < OpenTelemetry::Logs::LoggerProvider + EMPTY_NAME_ERROR = 'LoggerProvider#logger called without '\ + 'providing a logger name.' + + # Returns a {OpenTelemetry::SDK::Logs::Logger} instance. + # + # @param [optional String] name Instrumentation package name + # @param [optional String] version Instrumentation package version + # + # @return [OpenTelemetry::SDK::Logs::Logger] + def logger(name = nil, version = nil) + name ||= '' + version ||= '' + + OpenTelemetry.logger.warn(EMPTY_NAME_ERROR) if name.empty? + + # registry mutex? is that needed here for async safety? + OpenTelemetry::SDK::Logs::Logger.new(name, version, self) + end + end + end + end +end diff --git a/logs_sdk/opentelemetry-logs-sdk.gemspec b/logs_sdk/opentelemetry-logs-sdk.gemspec index a4607ee600..9d307b7116 100644 --- a/logs_sdk/opentelemetry-logs-sdk.gemspec +++ b/logs_sdk/opentelemetry-logs-sdk.gemspec @@ -24,10 +24,13 @@ Gem::Specification.new do |spec| spec.require_paths = ['lib'] spec.required_ruby_version = '>= 3.0' - spec.add_dependency 'opentelemetry-logs-api', '~> 0.1.0' + spec.add_dependency 'opentelemetry-api' + spec.add_dependency 'opentelemetry-logs-api' + spec.add_dependency 'opentelemetry-sdk' spec.add_development_dependency 'bundler', '>= 1.17' spec.add_development_dependency 'minitest', '~> 5.0' + spec.add_development_dependency 'opentelemetry-test-helpers' spec.add_development_dependency 'rake', '~> 12.0' spec.add_development_dependency 'rubocop', '~> 1.51.0' spec.add_development_dependency 'simplecov', '~> 0.17' diff --git a/logs_sdk/test/.rubocop.yml b/logs_sdk/test/.rubocop.yml index c575887d4b..dbc7df36ef 100644 --- a/logs_sdk/test/.rubocop.yml +++ b/logs_sdk/test/.rubocop.yml @@ -1,24 +1,8 @@ -# inherit_from: .rubocop_todo.yml +inherit_from: ../.rubocop.yml -AllCops: - TargetRubyVersion: '3.0' - -Lint/UnusedMethodArgument: - Enabled: false -Metrics/AbcSize: +Metrics/BlockLength: Enabled: false Metrics/LineLength: Enabled: false -Metrics/MethodLength: - Max: 50 -Metrics/PerceivedComplexity: - Max: 30 -Metrics/CyclomaticComplexity: - Max: 20 -Metrics/ParameterLists: - Enabled: false -Naming/FileName: - Exclude: - - 'lib/opentelemetry-logs-sdk.rb' -Style/ModuleFunction: +Metrics/AbcSize: Enabled: false diff --git a/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb new file mode 100644 index 0000000000..447006c089 --- /dev/null +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require 'test_helper' + +describe OpenTelemetry::SDK::Logs::LoggerProvider do + let(:logger_provider) { OpenTelemetry::SDK::Logs::LoggerProvider.new } + + describe '#logger' do + it 'logs a warning if name is nil' do + OpenTelemetry::TestHelpers.with_test_logger do |log_stream| + logger_provider.logger(nil) + assert_match( + /#{OpenTelemetry::SDK::Logs::LoggerProvider::EMPTY_NAME_ERROR}/, + log_stream.string + ) + end + end + + it 'logs a warning if name is an empty string' do + OpenTelemetry::TestHelpers.with_test_logger do |log_stream| + logger_provider.logger('') + assert_match( + /#{OpenTelemetry::SDK::Logs::LoggerProvider::EMPTY_NAME_ERROR}/, + log_stream.string + ) + end + end + + it 'sets name to an empty string if nil' do + logger = logger_provider.logger(nil) + assert_equal(logger.instrumentation_scope.name, '') + end + + it 'sets version to an empty string if nil' do + logger = logger_provider.logger('name', nil) + assert_equal(logger.instrumentation_scope.version, '') + end + + it 'creates a new logger with the passed-in name and version' do + name = 'name' + version = 'version' + logger = logger_provider.logger(name, version) + assert_equal(logger.instrumentation_scope.name, name) + assert_equal(logger.instrumentation_scope.version, version) + end + + it 'creates a new logger when name and version are missing' do + logger = logger_provider.logger + logger2 = logger_provider.logger + + refute_same(logger, logger2) + assert_instance_of(OpenTelemetry::SDK::Logs::Logger, logger) + end + end +end diff --git a/logs_sdk/test/test_helper.rb b/logs_sdk/test/test_helper.rb index 786faa1eca..59d743987f 100644 --- a/logs_sdk/test/test_helper.rb +++ b/logs_sdk/test/test_helper.rb @@ -9,6 +9,8 @@ SimpleCov.minimum_coverage 85 require 'opentelemetry-logs-api' +require 'opentelemetry-logs-sdk' +require 'opentelemetry-test-helpers' require 'minitest/autorun' OpenTelemetry.logger = Logger.new(File::NULL) From 1f548c9d8eecba1cabc7fa08f9f77da3b09c713c Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Wed, 9 Aug 2023 12:27:32 -0700 Subject: [PATCH 02/15] Associate a Resource with LoggerProvider --- .../lib/opentelemetry/sdk/logs/logger_provider.rb | 14 +++++++++++++- .../opentelemetry/sdk/logs/logger_provider_test.rb | 9 +++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb index c43aeb5ef9..60bcf05800 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb @@ -10,9 +10,21 @@ module Logs # {LoggerProvider} is the SDK implementation of # {OpenTelemetry::Logs::LoggerProvider}. class LoggerProvider < OpenTelemetry::Logs::LoggerProvider + attr_reader :resource + EMPTY_NAME_ERROR = 'LoggerProvider#logger called without '\ 'providing a logger name.' + # Returns a new {LoggerProvider} instance. + # + # @param [optional Resource] resource The resource to associate with new + # LogRecords created by Loggers created by this LoggerProvider + # + # @return [LoggerProvider] + def initialize(resource: OpenTelemetry::SDK::Resources::Resource.create) + @resource = resource + end + # Returns a {OpenTelemetry::SDK::Logs::Logger} instance. # # @param [optional String] name Instrumentation package name @@ -25,7 +37,7 @@ def logger(name = nil, version = nil) OpenTelemetry.logger.warn(EMPTY_NAME_ERROR) if name.empty? - # registry mutex? is that needed here for async safety? + # Q: @registry_mutex.synchronize invokes similar code within a block in the TracerProvider. Is that needed here for async safety? OpenTelemetry::SDK::Logs::Logger.new(name, version, self) 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 447006c089..f0275c70ce 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb @@ -9,6 +9,15 @@ describe OpenTelemetry::SDK::Logs::LoggerProvider do let(:logger_provider) { OpenTelemetry::SDK::Logs::LoggerProvider.new } + describe 'resource association' do + let(:resource) { OpenTelemetry::SDK::Resources::Resource.create('hi' => 1) } + let(:logger_provider) { OpenTelemetry::SDK::Logs::LoggerProvider.new(resource: resource) } + + it 'allows a resource to be associated with the logger provider' do + assert_instance_of(OpenTelemetry::SDK::Resources::Resource, logger_provider.resource) + end + end + describe '#logger' do it 'logs a warning if name is nil' do OpenTelemetry::TestHelpers.with_test_logger do |log_stream| From 5ff70fe643ef2067f729916a926a88fb91a67a8e Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Wed, 9 Aug 2023 13:33:16 -0700 Subject: [PATCH 03/15] resource, processors, mutex --- .../opentelemetry/sdk/logs/logger_provider.rb | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb index 60bcf05800..8bba66b479 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb @@ -19,10 +19,17 @@ class LoggerProvider < OpenTelemetry::Logs::LoggerProvider # # @param [optional Resource] resource The resource to associate with new # LogRecords created by Loggers created by this LoggerProvider + # @param [optional Array] log_record_processors to associate with the + # LoggerProvider # # @return [LoggerProvider] - def initialize(resource: OpenTelemetry::SDK::Resources::Resource.create) + def initialize( + resource: OpenTelemetry::SDK::Resources::Resource.create, + log_record_processors: [] + ) + @mutex = Mutex.new @resource = resource + @log_record_processors = log_record_processors end # Returns a {OpenTelemetry::SDK::Logs::Logger} instance. @@ -37,8 +44,19 @@ def logger(name = nil, version = nil) OpenTelemetry.logger.warn(EMPTY_NAME_ERROR) if name.empty? - # Q: @registry_mutex.synchronize invokes similar code within a block in the TracerProvider. Is that needed here for async safety? - OpenTelemetry::SDK::Logs::Logger.new(name, version, self) + # Q: Why does the TracerProvider have both @mutex and @registry_mutex? + @mutex.synchronize do + OpenTelemetry::SDK::Logs::Logger.new(name, version, self) + end + end + + # Adds a new LogRecordProcessor to this {LoggerProvider}. + # + # @param log_record_processor the new LogRecordProcessor to be added + def add_log_record_processor(log_record_processor) + @mutex.synchronize do + @log_record_processors = @log_record_processors.dup.push(log_record_processor) + end end end end From a6c9243c9e94f7328257b266647b9d226257b8b7 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Mon, 14 Aug 2023 16:26:37 -0700 Subject: [PATCH 04/15] Write test for add_log_record_processor --- .../test/opentelemetry/sdk/logs/logger_provider_test.rb | 9 +++++++++ 1 file changed, 9 insertions(+) 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 f0275c70ce..57d670dc96 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb @@ -18,6 +18,15 @@ end end + describe '#add_log_record_processor' do + it 'adds the processor to the providers processors' do + mock_span_processor = Minitest::Mock.new + assert_equal(0, logger_provider.instance_variable_get(:@log_record_processors).length) + logger_provider.add_log_record_processor(mock_span_processor) + assert_equal(1, logger_provider.instance_variable_get(:@log_record_processors).length) + end + end + describe '#logger' do it 'logs a warning if name is nil' do OpenTelemetry::TestHelpers.with_test_logger do |log_stream| From 64b38297750e81886c2e3fe5069b561723f5edd1 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Mon, 14 Aug 2023 16:44:30 -0700 Subject: [PATCH 05/15] Implement LoggerProvider#shutdown --- logs_sdk/lib/opentelemetry/sdk/logs.rb | 2 + logs_sdk/lib/opentelemetry/sdk/logs/export.rb | 28 ++++++++++ .../sdk/logs/log_record_processor.rb | 18 +++++++ .../opentelemetry/sdk/logs/logger_provider.rb | 38 +++++++++++++- .../sdk/logs/logger_provider_test.rb | 52 +++++++++++++++++++ 5 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 logs_sdk/lib/opentelemetry/sdk/logs/export.rb create mode 100644 logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb diff --git a/logs_sdk/lib/opentelemetry/sdk/logs.rb b/logs_sdk/lib/opentelemetry/sdk/logs.rb index 576d44e276..4331257d39 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs.rb @@ -7,6 +7,8 @@ require_relative 'logs/version' require_relative 'logs/logger' require_relative 'logs/logger_provider' +require_relative 'logs/log_record_processor' +require_relative 'logs/export' module OpenTelemetry module SDK diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/export.rb b/logs_sdk/lib/opentelemetry/sdk/logs/export.rb new file mode 100644 index 0000000000..6477ecefdd --- /dev/null +++ b/logs_sdk/lib/opentelemetry/sdk/logs/export.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module SDK + module Logs + # The Export module contains the built-in exporters and log record + # processors for the OpenTelemetry reference implementation. + module Export + # Result codes for the LogRecordExporter#export method and the LogRecordProcessor#force_flush and LogRecordProcessor#shutdown methods. + + # The operation finished successfully. + SUCCESS = 0 + + # The operation finished with an error. + FAILURE = 1 + + # Additional result code for the LogRecordProcessor#force_flush and LogRecordProcessor#shutdown methods. + + # The operation timed out. + TIMEOUT = 2 + end + end + end +end diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb b/logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb new file mode 100644 index 0000000000..f86fc8ac93 --- /dev/null +++ b/logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module SDK + module Logs + # + class LogRecordProcessor + def shutdown(timeout: nil) + # TODO: implement + end + end + 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 8bba66b479..b4a5f13f9d 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb @@ -27,9 +27,10 @@ def initialize( resource: OpenTelemetry::SDK::Resources::Resource.create, log_record_processors: [] ) + @log_record_processors = log_record_processors @mutex = Mutex.new @resource = resource - @log_record_processors = log_record_processors + @stopped = false end # Returns a {OpenTelemetry::SDK::Logs::Logger} instance. @@ -58,6 +59,41 @@ def add_log_record_processor(log_record_processor) @log_record_processors = @log_record_processors.dup.push(log_record_processor) end end + + # Attempts to stop all the activity for this {LoggerProvider}. Calls + # LogRecordProcessor#shutdown for all registered LogRecordProcessors. + # + # This operation may block until all the Log Records are processed. Must + # be called before turning off the main application to ensure all data + # are processed and exported. + # + # After this is called all the newly created {LogRecord}s will be no-op. + # + # @param [optional Numeric] timeout An optional timeout in seconds. + # @return [Integer] Export::SUCCESS if no error occurred, Export::FAILURE if + # a non-specific failure occurred, Export::TIMEOUT if a timeout occurred. + def shutdown(timeout: nil) + @mutex.synchronize do + if @stopped + OpenTelemetry.logger.warn( + 'calling LoggerProvider#shutdown multiple times.' + ) + return OpenTelemetry::SDK::Logs::Export::FAILURE + end + + start_time = OpenTelemetry::Common::Utilities.timeout_timestamp + results = @log_record_processors.map do |processor| + remaining_timeout = OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time) + break [OpenTelemetry::SDK::Logs::Export::TIMEOUT] if remaining_timeout&.zero? + + # this needs an argument, but I'm having trouble passing the arg to the expect + processor.shutdown + end + + @stopped = true + results.max || OpenTelemetry::SDK::Logs::Export::SUCCESS + end + end 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 57d670dc96..941c82493e 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb @@ -74,4 +74,56 @@ assert_instance_of(OpenTelemetry::SDK::Logs::Logger, logger) end end + + describe '#shutdown' do + # TODO: Figure out why the argument isn't working on expect/in method + let(:processor) { OpenTelemetry::SDK::Logs::LogRecordProcessor.new } + let(:provider) do + OpenTelemetry::SDK::Logs::LoggerProvider.new( + log_record_processors: [processor] + ) + end + + it 'logs a warning if called twice' do + OpenTelemetry::TestHelpers.with_test_logger do |log_stream| + provider.shutdown + assert provider.instance_variable_get(:@stopped) + assert_empty(log_stream.string) + provider.shutdown + assert_match(/calling .* multiple times/, log_stream.string) + end + end + + it 'sends shutdown to the processor' do + mock_span_processor = Minitest::Mock.new + mock_span_processor.expect(:shutdown, nil) + provider.add_log_record_processor(mock_span_processor) + provider.shutdown + mock_span_processor.verify + end + + it 'sends shutdown to multiple processors' do + mock_span_processor = Minitest::Mock.new + mock_span_processor2 = Minitest::Mock.new + mock_span_processor.expect(:shutdown, nil) + mock_span_processor2.expect(:shutdown, nil) + + provider.instance_variable_set(:@log_record_processors, [mock_span_processor, mock_span_processor2]) + + provider.shutdown + + mock_span_processor.verify + mock_span_processor2.verify + end + + it 'only notifies the processor once' do + mock_span_processor = Minitest::Mock.new + + mock_span_processor.expect(:shutdown, nil) + provider.add_log_record_processor(mock_span_processor) + provider.shutdown + provider.shutdown + mock_span_processor.verify + end + end end From 0a7cf84cd2b5370e41995b575b227f3a7695319d Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Tue, 15 Aug 2023 11:26:36 -0700 Subject: [PATCH 06/15] LoggerProvider#shutdown with arg on processor --- .../opentelemetry/sdk/logs/logger_provider.rb | 1 + .../sdk/logs/logger_provider_test.rb | 49 ++++++++++--------- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb index b4a5f13f9d..68c7050c6b 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb @@ -87,6 +87,7 @@ def shutdown(timeout: nil) break [OpenTelemetry::SDK::Logs::Export::TIMEOUT] if remaining_timeout&.zero? # this needs an argument, but I'm having trouble passing the arg to the expect + # processor.shutdown(timeout: remaining_timeout) processor.shutdown 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 941c82493e..fdfc510d9c 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb @@ -77,44 +77,49 @@ describe '#shutdown' do # TODO: Figure out why the argument isn't working on expect/in method - let(:processor) { OpenTelemetry::SDK::Logs::LogRecordProcessor.new } - let(:provider) do - OpenTelemetry::SDK::Logs::LoggerProvider.new( - log_record_processors: [processor] - ) - end + let(:mock_log_record_processor) { Minitest::Mock.new } it 'logs a warning if called twice' do OpenTelemetry::TestHelpers.with_test_logger do |log_stream| - provider.shutdown - assert provider.instance_variable_get(:@stopped) + logger_provider.shutdown + assert logger_provider.instance_variable_get(:@stopped) assert_empty(log_stream.string) - provider.shutdown + logger_provider.shutdown assert_match(/calling .* multiple times/, log_stream.string) end end it 'sends shutdown to the processor' do - mock_span_processor = Minitest::Mock.new - mock_span_processor.expect(:shutdown, nil) - provider.add_log_record_processor(mock_span_processor) - provider.shutdown - mock_span_processor.verify + # mock_log_record_processor.expect(:shutdown, nil, [{timeout: nil}]) + mock_log_record_processor.expect(:shutdown, nil) + logger_provider.add_log_record_processor(mock_log_record_processor) + logger_provider.shutdown + mock_log_record_processor.verify end it 'sends shutdown to multiple processors' do - mock_span_processor = Minitest::Mock.new - mock_span_processor2 = Minitest::Mock.new - mock_span_processor.expect(:shutdown, nil) - mock_span_processor2.expect(:shutdown, nil) + mock_log_record_processor2 = Minitest::Mock.new + # mock_log_record_processor.expect(:shutdown, nil, [{timeout: nil}]) + # mock_log_record_processor2.expect(:shutdown, nil, [{timeout: nil}]) + mock_log_record_processor.expect(:shutdown, nil) + mock_log_record_processor2.expect(:shutdown, nil) - provider.instance_variable_set(:@log_record_processors, [mock_span_processor, mock_span_processor2]) + logger_provider.instance_variable_set(:@log_record_processors, [mock_log_record_processor, mock_log_record_processor2]) + logger_provider.shutdown - provider.shutdown + mock_log_record_processor.verify + mock_log_record_processor2.verify + end - mock_span_processor.verify - mock_span_processor2.verify + it 'only notifies the processor once' do + # mock_log_record_processor.expect(:shutdown, nil, [{timeout: nil}]) + mock_log_record_processor.expect(:shutdown, nil) + logger_provider.add_log_record_processor(mock_log_record_processor) + logger_provider.shutdown + logger_provider.shutdown + mock_log_record_processor.verify end + end it 'only notifies the processor once' do mock_span_processor = Minitest::Mock.new From f4585718e47b2b5a4171de50b59ace3559387eac Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Tue, 15 Aug 2023 11:27:09 -0700 Subject: [PATCH 07/15] Temp class comment on LogRecordProcessor --- logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb b/logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb index f86fc8ac93..3661d00953 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb @@ -7,7 +7,7 @@ module OpenTelemetry module SDK module Logs - # + # Presently no-op LogRecordProcessor class LogRecordProcessor def shutdown(timeout: nil) # TODO: implement From e87f3c1758fa68b43605e00a8e800c4be988b72d Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Tue, 15 Aug 2023 11:28:57 -0700 Subject: [PATCH 08/15] LogRecordProcessor#force_flush --- .../sdk/logs/log_record_processor.rb | 4 +++ .../opentelemetry/sdk/logs/logger_provider.rb | 31 ++++++++++++++++ .../sdk/logs/logger_provider_test.rb | 35 +++++++++++++------ 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb b/logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb index 3661d00953..7451684c60 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb @@ -12,6 +12,10 @@ class LogRecordProcessor def shutdown(timeout: nil) # TODO: implement end + + def force_flush(timeout: nil) + # TODO: implement + end 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 68c7050c6b..a76e5639cf 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb @@ -95,6 +95,37 @@ def shutdown(timeout: nil) results.max || OpenTelemetry::SDK::Logs::Export::SUCCESS end end + + # Immediately export all log records that have not yet been exported + # for all the registered LogRecordProcessors. + # + # This method should only be called in cases where it is absolutely + # necessary, such as when using some FaaS providers that may suspend + # the process after an invocation, but before the `Processor` exports + # the completed log records. + # + # @param [optional Numeric] timeout An optional timeout in seconds. + # @return [Integer] Export::SUCCESS if no error occurred, Export::FAILURE if + # a non-specific failure occurred, Export::TIMEOUT if a timeout occurred. + def force_flush(timeout: nil) + @mutex.synchronize do + return Export::SUCCESS if @stopped + + start_time = OpenTelemetry::Common::Utilities.timeout_timestamp + results = @log_record_processors.map do |processor| + remaining_timeout = OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time) + return Export::TIMEOUT if remaining_timeout&.zero? + + # TODO: Fix the issue with the test not accepting the arg + # processor.force_flush(timeout: remaining_timeout) + processor.force_flush + end + + results.max || Export::SUCCESS + end + rescue StandardError + Export::FAILURE + end 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 fdfc510d9c..e28f1c31ae 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb @@ -19,10 +19,11 @@ end describe '#add_log_record_processor' do - it 'adds the processor to the providers processors' do - mock_span_processor = Minitest::Mock.new + let(:mock_log_record_processor) { Minitest::Mock.new } + + it "adds the processor to the logger provider's processors" do assert_equal(0, logger_provider.instance_variable_get(:@log_record_processors).length) - logger_provider.add_log_record_processor(mock_span_processor) + logger_provider.add_log_record_processor(mock_log_record_processor) assert_equal(1, logger_provider.instance_variable_get(:@log_record_processors).length) end end @@ -121,14 +122,28 @@ end end - it 'only notifies the processor once' do - mock_span_processor = Minitest::Mock.new + describe '#force_flush' do + let(:mock_log_record_processor) { Minitest::Mock.new } + let(:mock_log_record_processor2) { Minitest::Mock.new } - mock_span_processor.expect(:shutdown, nil) - provider.add_log_record_processor(mock_span_processor) - provider.shutdown - provider.shutdown - mock_span_processor.verify + it 'notifies the log record processor' do + # mock_log_record_processor.expect(:force_flush, nil, [{timeout: nil}]) + mock_log_record_processor.expect(:force_flush, nil) + logger_provider.add_log_record_processor(mock_log_record_processor) + logger_provider.force_flush + mock_log_record_processor.verify + end + + it 'supports multiple log record processors' do + # mock_log_record_processor.expect(:force_flush, nil, [{timeout: nil}]) + # mock_log_record_processor2.expect(:force_flush, nil, [{timeout: nil}]) + mock_log_record_processor.expect(:force_flush, nil) + mock_log_record_processor2.expect(:force_flush, nil) + logger_provider.add_log_record_processor(mock_log_record_processor) + logger_provider.add_log_record_processor(mock_log_record_processor2) + logger_provider.force_flush + mock_log_record_processor.verify + mock_log_record_processor2.verify end end end From 0d6935bb0590ec1668b280cca54bf61dff1ab7f6 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Wed, 16 Aug 2023 14:03:45 -0700 Subject: [PATCH 09/15] Cleanup --- logs_sdk/lib/opentelemetry/sdk/logs/export.rb | 6 +- logs_sdk/lib/opentelemetry/sdk/logs/logger.rb | 7 +- .../opentelemetry/sdk/logs/logger_provider.rb | 25 ++++--- .../sdk/logs/logger_provider_test.rb | 65 ++++++++++--------- 4 files changed, 55 insertions(+), 48 deletions(-) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/export.rb b/logs_sdk/lib/opentelemetry/sdk/logs/export.rb index 6477ecefdd..b50b2e678b 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/export.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/export.rb @@ -10,7 +10,8 @@ module Logs # The Export module contains the built-in exporters and log record # processors for the OpenTelemetry reference implementation. module Export - # Result codes for the LogRecordExporter#export method and the LogRecordProcessor#force_flush and LogRecordProcessor#shutdown methods. + # Result codes for the LoggerProvider#force_flush and + # LoggerProvider#shutdown methods. # The operation finished successfully. SUCCESS = 0 @@ -18,7 +19,8 @@ module Export # The operation finished with an error. FAILURE = 1 - # Additional result code for the LogRecordProcessor#force_flush and LogRecordProcessor#shutdown methods. + # Additional result code for the LoggerProvider#force_flush and + # LoggerProvider#shutdown methods. # The operation timed out. TIMEOUT = 2 diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb index 22d4d5fd97..bced1804be 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb @@ -7,7 +7,8 @@ module OpenTelemetry module SDK module Logs - # {OpenTelemetry::SDK::Logs::Logger} is the SDK implementation of {OpenTelemetry::Logs::Logger} + # {OpenTelemetry::SDK::Logs::Logger} is the SDK implementation of + # {OpenTelemetry::Logs::Logger} class Logger < OpenTelemetry::Logs::Logger attr_reader :instrumentation_scope @@ -17,8 +18,8 @@ class Logger < OpenTelemetry::Logs::Logger # # @param [String] name Instrumentation package name # @param [String] version Instrumentation package version - # @param [LoggerProvider] logger_provider LoggerProvider that initialized - # the logger + # @param [LoggerProvider] logger_provider LoggerProvider that + # initialized the logger # # @return [OpenTelemetry::SDK::Logs::Logger] def initialize(name, version, logger_provider) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb index a76e5639cf..38012f423c 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb @@ -17,10 +17,10 @@ class LoggerProvider < OpenTelemetry::Logs::LoggerProvider # Returns a new {LoggerProvider} instance. # - # @param [optional Resource] resource The resource to associate with new - # LogRecords created by Loggers created by this LoggerProvider - # @param [optional Array] log_record_processors to associate with the - # LoggerProvider + # @param [optional Resource] resource The resource to associate with + # new LogRecords created by Loggers created by this LoggerProvider. + # @param [optional Array] log_record_processors Log Record Processors to + # associate with the LoggerProvider. # # @return [LoggerProvider] def initialize( @@ -45,15 +45,16 @@ def logger(name = nil, version = nil) OpenTelemetry.logger.warn(EMPTY_NAME_ERROR) if name.empty? - # Q: Why does the TracerProvider have both @mutex and @registry_mutex? @mutex.synchronize do OpenTelemetry::SDK::Logs::Logger.new(name, version, self) end end - # Adds a new LogRecordProcessor to this {LoggerProvider}. + # Adds a new LogRecordProcessor to this {LoggerProvider}'s + # log_record_processors. # - # @param log_record_processor the new LogRecordProcessor to be added + # @param [LogRecordProcessor] log_record_processor The new + # LogRecordProcessor to add. def add_log_record_processor(log_record_processor) @mutex.synchronize do @log_record_processors = @log_record_processors.dup.push(log_record_processor) @@ -76,7 +77,7 @@ def shutdown(timeout: nil) @mutex.synchronize do if @stopped OpenTelemetry.logger.warn( - 'calling LoggerProvider#shutdown multiple times.' + 'LoggerProvider#shutdown called multiple times.' ) return OpenTelemetry::SDK::Logs::Export::FAILURE end @@ -86,9 +87,7 @@ def shutdown(timeout: nil) remaining_timeout = OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time) break [OpenTelemetry::SDK::Logs::Export::TIMEOUT] if remaining_timeout&.zero? - # this needs an argument, but I'm having trouble passing the arg to the expect - # processor.shutdown(timeout: remaining_timeout) - processor.shutdown + processor.shutdown(timeout: remaining_timeout) end @stopped = true @@ -116,9 +115,7 @@ def force_flush(timeout: nil) remaining_timeout = OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time) return Export::TIMEOUT if remaining_timeout&.zero? - # TODO: Fix the issue with the test not accepting the arg - # processor.force_flush(timeout: remaining_timeout) - processor.force_flush + processor.force_flush(timeout: remaining_timeout) end results.max || Export::SUCCESS 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 e28f1c31ae..425c213330 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb @@ -8,23 +8,35 @@ describe OpenTelemetry::SDK::Logs::LoggerProvider do let(:logger_provider) { OpenTelemetry::SDK::Logs::LoggerProvider.new } + let(:mock_log_record_processor) { Minitest::Mock.new } + let(:mock_log_record_processor2) { Minitest::Mock.new } describe 'resource association' do let(:resource) { OpenTelemetry::SDK::Resources::Resource.create('hi' => 1) } - let(:logger_provider) { OpenTelemetry::SDK::Logs::LoggerProvider.new(resource: resource) } + let(:logger_provider) do + OpenTelemetry::SDK::Logs::LoggerProvider.new(resource: resource) + end it 'allows a resource to be associated with the logger provider' do - assert_instance_of(OpenTelemetry::SDK::Resources::Resource, logger_provider.resource) + assert_instance_of( + OpenTelemetry::SDK::Resources::Resource, logger_provider.resource + ) end end describe '#add_log_record_processor' do - let(:mock_log_record_processor) { Minitest::Mock.new } - it "adds the processor to the logger provider's processors" do - assert_equal(0, logger_provider.instance_variable_get(:@log_record_processors).length) + assert_equal( + 0, + logger_provider.instance_variable_get(:@log_record_processors).length + ) + logger_provider.add_log_record_processor(mock_log_record_processor) - assert_equal(1, logger_provider.instance_variable_get(:@log_record_processors).length) + + assert_equal( + 1, + logger_provider.instance_variable_get(:@log_record_processors).length + ) end end @@ -77,35 +89,31 @@ end describe '#shutdown' do - # TODO: Figure out why the argument isn't working on expect/in method - let(:mock_log_record_processor) { Minitest::Mock.new } - it 'logs a warning if called twice' do OpenTelemetry::TestHelpers.with_test_logger do |log_stream| logger_provider.shutdown assert logger_provider.instance_variable_get(:@stopped) assert_empty(log_stream.string) logger_provider.shutdown - assert_match(/calling .* multiple times/, log_stream.string) + assert_match(/.* called multiple times/, log_stream.string) end end it 'sends shutdown to the processor' do - # mock_log_record_processor.expect(:shutdown, nil, [{timeout: nil}]) - mock_log_record_processor.expect(:shutdown, nil) + mock_log_record_processor.expect(:shutdown, nil, timeout: nil) logger_provider.add_log_record_processor(mock_log_record_processor) logger_provider.shutdown mock_log_record_processor.verify end it 'sends shutdown to multiple processors' do - mock_log_record_processor2 = Minitest::Mock.new - # mock_log_record_processor.expect(:shutdown, nil, [{timeout: nil}]) - # mock_log_record_processor2.expect(:shutdown, nil, [{timeout: nil}]) - mock_log_record_processor.expect(:shutdown, nil) - mock_log_record_processor2.expect(:shutdown, nil) + mock_log_record_processor.expect(:shutdown, nil, timeout: nil) + mock_log_record_processor2.expect(:shutdown, nil, timeout: nil) - logger_provider.instance_variable_set(:@log_record_processors, [mock_log_record_processor, mock_log_record_processor2]) + logger_provider.instance_variable_set( + :@log_record_processors, + [mock_log_record_processor, mock_log_record_processor2] + ) logger_provider.shutdown mock_log_record_processor.verify @@ -113,35 +121,34 @@ end it 'only notifies the processor once' do - # mock_log_record_processor.expect(:shutdown, nil, [{timeout: nil}]) - mock_log_record_processor.expect(:shutdown, nil) + mock_log_record_processor.expect(:shutdown, nil, timeout: nil) + logger_provider.add_log_record_processor(mock_log_record_processor) logger_provider.shutdown logger_provider.shutdown + mock_log_record_processor.verify end end describe '#force_flush' do - let(:mock_log_record_processor) { Minitest::Mock.new } - let(:mock_log_record_processor2) { Minitest::Mock.new } - it 'notifies the log record processor' do - # mock_log_record_processor.expect(:force_flush, nil, [{timeout: nil}]) - mock_log_record_processor.expect(:force_flush, nil) + mock_log_record_processor.expect(:force_flush, nil, timeout: nil) + logger_provider.add_log_record_processor(mock_log_record_processor) logger_provider.force_flush + mock_log_record_processor.verify end it 'supports multiple log record processors' do - # mock_log_record_processor.expect(:force_flush, nil, [{timeout: nil}]) - # mock_log_record_processor2.expect(:force_flush, nil, [{timeout: nil}]) - mock_log_record_processor.expect(:force_flush, nil) - mock_log_record_processor2.expect(:force_flush, nil) + mock_log_record_processor.expect(:force_flush, nil, timeout: nil) + mock_log_record_processor2.expect(:force_flush, nil, timeout: nil) + logger_provider.add_log_record_processor(mock_log_record_processor) logger_provider.add_log_record_processor(mock_log_record_processor2) logger_provider.force_flush + mock_log_record_processor.verify mock_log_record_processor2.verify end From 893e1b89f205706ea6938ddf66f4a46776e07859 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Wed, 16 Aug 2023 14:08:22 -0700 Subject: [PATCH 10/15] Bump versions --- logs_sdk/opentelemetry-logs-sdk.gemspec | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/logs_sdk/opentelemetry-logs-sdk.gemspec b/logs_sdk/opentelemetry-logs-sdk.gemspec index 9d307b7116..9513cf7761 100644 --- a/logs_sdk/opentelemetry-logs-sdk.gemspec +++ b/logs_sdk/opentelemetry-logs-sdk.gemspec @@ -29,13 +29,13 @@ Gem::Specification.new do |spec| spec.add_dependency 'opentelemetry-sdk' spec.add_development_dependency 'bundler', '>= 1.17' - spec.add_development_dependency 'minitest', '~> 5.0' - spec.add_development_dependency 'opentelemetry-test-helpers' - spec.add_development_dependency 'rake', '~> 12.0' - spec.add_development_dependency 'rubocop', '~> 1.51.0' - spec.add_development_dependency 'simplecov', '~> 0.17' + spec.add_development_dependency 'minitest', '~> 5.19' + spec.add_development_dependency 'opentelemetry-test-helpers', '~> 0.4' + spec.add_development_dependency 'rake', '~> 13.0' + spec.add_development_dependency 'rubocop', '~> 1.56' + spec.add_development_dependency 'simplecov', '~> 0.22' spec.add_development_dependency 'yard', '~> 0.9' - spec.add_development_dependency 'yard-doctest', '~> 0.1.6' + spec.add_development_dependency 'yard-doctest', '~> 0.1.17' if spec.respond_to?(:metadata) spec.metadata['changelog_uri'] = "https://open-telemetry.github.io/opentelemetry-ruby/opentelemetry-logs-sdk/v#{OpenTelemetry::SDK::Logs::VERSION}/file.CHANGELOG.html" From 97a378b69c3a0a4a54010d06e4084eebd5732187 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Fri, 18 Aug 2023 11:40:58 -0700 Subject: [PATCH 11/15] Address feedback from fallwith --- logs_sdk/lib/opentelemetry/sdk/logs/export.rb | 10 ++-------- logs_sdk/lib/opentelemetry/sdk/logs/logger.rb | 2 +- logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb | 4 +--- logs_sdk/opentelemetry-logs-sdk.gemspec | 6 +++--- .../opentelemetry/sdk/logs/logger_provider_test.rb | 2 +- 5 files changed, 8 insertions(+), 16 deletions(-) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/export.rb b/logs_sdk/lib/opentelemetry/sdk/logs/export.rb index b50b2e678b..cb8c846eff 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/export.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/export.rb @@ -7,21 +7,15 @@ module OpenTelemetry module SDK module Logs - # The Export module contains the built-in exporters and log record - # processors for the OpenTelemetry reference implementation. + # The export module contains result codes for LoggerProvider#force_flush + # and LoggerProvider#shutdown module Export - # Result codes for the LoggerProvider#force_flush and - # LoggerProvider#shutdown methods. - # The operation finished successfully. SUCCESS = 0 # The operation finished with an error. FAILURE = 1 - # Additional result code for the LoggerProvider#force_flush and - # LoggerProvider#shutdown methods. - # The operation timed out. TIMEOUT = 2 end diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb index bced1804be..10bab0bf5d 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb @@ -10,7 +10,7 @@ module Logs # {OpenTelemetry::SDK::Logs::Logger} is the SDK implementation of # {OpenTelemetry::Logs::Logger} class Logger < OpenTelemetry::Logs::Logger - attr_reader :instrumentation_scope + attr_reader :instrumentation_scope, :logger_provider # @api private # diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb index 38012f423c..41b5fb3c9d 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb @@ -76,9 +76,7 @@ def add_log_record_processor(log_record_processor) def shutdown(timeout: nil) @mutex.synchronize do if @stopped - OpenTelemetry.logger.warn( - 'LoggerProvider#shutdown called multiple times.' - ) + OpenTelemetry.logger.warn('LoggerProvider#shutdown called multiple times.') return OpenTelemetry::SDK::Logs::Export::FAILURE end diff --git a/logs_sdk/opentelemetry-logs-sdk.gemspec b/logs_sdk/opentelemetry-logs-sdk.gemspec index 9513cf7761..29910cb296 100644 --- a/logs_sdk/opentelemetry-logs-sdk.gemspec +++ b/logs_sdk/opentelemetry-logs-sdk.gemspec @@ -24,9 +24,9 @@ Gem::Specification.new do |spec| spec.require_paths = ['lib'] spec.required_ruby_version = '>= 3.0' - spec.add_dependency 'opentelemetry-api' - spec.add_dependency 'opentelemetry-logs-api' - spec.add_dependency 'opentelemetry-sdk' + spec.add_dependency 'opentelemetry-api', '~> 1.2' + spec.add_dependency 'opentelemetry-logs-api', '~> 0.1' + spec.add_dependency 'opentelemetry-sdk', '~> 1.3' spec.add_development_dependency 'bundler', '>= 1.17' spec.add_development_dependency 'minitest', '~> 5.19' 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 425c213330..dfad3136d0 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb @@ -120,7 +120,7 @@ mock_log_record_processor2.verify end - it 'only notifies the processor once' do + it 'subsequent shutdown attempts do not reach the processor' do mock_log_record_processor.expect(:shutdown, nil, timeout: nil) logger_provider.add_log_record_processor(mock_log_record_processor) From a4d9197d22de1989e5353051e4cb9a61a68fd092 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Tue, 22 Aug 2023 12:21:30 -0700 Subject: [PATCH 12/15] Add more tests --- .../sdk/logs/logger_provider_test.rb | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) 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 dfad3136d0..1d188416b9 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb @@ -120,7 +120,7 @@ mock_log_record_processor2.verify end - it 'subsequent shutdown attempts do not reach the processor' do + it 'does not allow subsequent shutdown attempts to reach the processor' do mock_log_record_processor.expect(:shutdown, nil, timeout: nil) logger_provider.add_log_record_processor(mock_log_record_processor) @@ -129,6 +129,13 @@ mock_log_record_processor.verify end + + it 'returns a timeout code if the countdown reaches zero' do + OpenTelemetry::Common::Utilities.stub :maybe_timeout, 0 do + logger_provider.add_log_record_processor(mock_log_record_processor) + assert_equal(OpenTelemetry::SDK::Logs::Export::TIMEOUT, logger_provider.shutdown) + end + end end describe '#force_flush' do @@ -152,5 +159,25 @@ mock_log_record_processor.verify mock_log_record_processor2.verify end + + it 'returns a success status code if called while stopped' do + logger_provider.add_log_record_processor(mock_log_record_processor) + logger_provider.instance_variable_set(:@stopped, true) + assert_equal(OpenTelemetry::SDK::Logs::Export::SUCCESS, logger_provider.force_flush) + end + + it 'returns a timeout code when the timeout countdown reaches zero' do + OpenTelemetry::Common::Utilities.stub :maybe_timeout, 0 do + logger_provider.add_log_record_processor(mock_log_record_processor) + assert_equal(OpenTelemetry::SDK::Logs::Export::TIMEOUT, logger_provider.force_flush) + end + end + + it 'returns a failure code when an error is raised' do + OpenTelemetry::Common::Utilities.stub :maybe_timeout, -> { raise StandardError.new, 'fail' } do + logger_provider.add_log_record_processor(mock_log_record_processor) + assert_equal(OpenTelemetry::SDK::Logs::Export::FAILURE, logger_provider.force_flush) + end + end end end From 1863a178f9818441f996f2ecc102f5612c3bff50 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Tue, 22 Aug 2023 14:36:28 -0700 Subject: [PATCH 13/15] Update tests and error handling --- logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb | 4 +++- logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb index 41b5fb3c9d..87ecd4f255 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb @@ -14,6 +14,7 @@ class LoggerProvider < OpenTelemetry::Logs::LoggerProvider EMPTY_NAME_ERROR = 'LoggerProvider#logger called without '\ 'providing a logger name.' + FORCE_FLUSH_ERROR = 'unexpected error in OpenTelemetry::SDK::Logs::LoggerProvider#force_flush' # Returns a new {LoggerProvider} instance. # @@ -118,7 +119,8 @@ def force_flush(timeout: nil) results.max || Export::SUCCESS end - rescue StandardError + rescue StandardError => e + OpenTelemetry.handle_error(exception: e, message: FORCE_FLUSH_ERROR) Export::FAILURE 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 1d188416b9..42e6663177 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb @@ -174,8 +174,7 @@ end it 'returns a failure code when an error is raised' do - OpenTelemetry::Common::Utilities.stub :maybe_timeout, -> { raise StandardError.new, 'fail' } do - logger_provider.add_log_record_processor(mock_log_record_processor) + OpenTelemetry::Common::Utilities.stub :timeout_timestamp, -> { raise StandardError.new, 'fail' } do assert_equal(OpenTelemetry::SDK::Logs::Export::FAILURE, logger_provider.force_flush) end end From b10d32cc7193ebd1342b97c8cc14afc1adfacddd Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Tue, 22 Aug 2023 15:42:02 -0700 Subject: [PATCH 14/15] Docs updates --- logs_sdk/lib/opentelemetry/sdk/logs/logger.rb | 9 ++-- .../opentelemetry/sdk/logs/logger_provider.rb | 48 +++++++++---------- .../sdk/logs/logger_provider_test.rb | 11 +---- 3 files changed, 31 insertions(+), 37 deletions(-) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb index 10bab0bf5d..5d3707c4d4 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger.rb @@ -7,18 +7,19 @@ module OpenTelemetry module SDK module Logs - # {OpenTelemetry::SDK::Logs::Logger} is the SDK implementation of - # {OpenTelemetry::Logs::Logger} + # 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. + # Returns a new {OpenTelemetry::SDK::Logs::Logger} instance. This should + # not be called directly. New loggers should be created using + # {LoggerProvider#logger}. # # @param [String] name Instrumentation package name # @param [String] version Instrumentation package version - # @param [LoggerProvider] logger_provider LoggerProvider that + # @param [LoggerProvider] logger_provider The {LoggerProvider} that # initialized the logger # # @return [OpenTelemetry::SDK::Logs::Logger] diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb index 87ecd4f255..5d1c00f1d3 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb @@ -7,23 +7,23 @@ module OpenTelemetry module SDK module Logs - # {LoggerProvider} is the SDK implementation of - # {OpenTelemetry::Logs::LoggerProvider}. + # The SDK implementation of OpenTelemetry::Logs::LoggerProvider. class LoggerProvider < OpenTelemetry::Logs::LoggerProvider - attr_reader :resource + attr_reader :resource, :log_record_processors EMPTY_NAME_ERROR = 'LoggerProvider#logger called without '\ - 'providing a logger name.' - FORCE_FLUSH_ERROR = 'unexpected error in OpenTelemetry::SDK::Logs::LoggerProvider#force_flush' + 'providing a logger name.' + FORCE_FLUSH_ERROR = 'unexpected error in ' \ + 'OpenTelemetry::SDK::Logs::LoggerProvider#force_flush' - # Returns a new {LoggerProvider} instance. + # Returns a new LoggerProvider instance. # # @param [optional Resource] resource The resource to associate with # new LogRecords created by Loggers created by this LoggerProvider. - # @param [optional Array] log_record_processors Log Record Processors to - # associate with the LoggerProvider. + # @param [optional Array] log_record_processors The log record + # processors to associate with this LoggerProvider. # - # @return [LoggerProvider] + # @return [OpenTelemetry::SDK::Logs::LoggerProvider] def initialize( resource: OpenTelemetry::SDK::Resources::Resource.create, log_record_processors: [] @@ -34,7 +34,7 @@ def initialize( @stopped = false end - # Returns a {OpenTelemetry::SDK::Logs::Logger} instance. + # Creates an {OpenTelemetry::SDK::Logs::Logger} instance. # # @param [optional String] name Instrumentation package name # @param [optional String] version Instrumentation package version @@ -51,25 +51,25 @@ def logger(name = nil, version = nil) end end - # Adds a new LogRecordProcessor to this {LoggerProvider}'s + # Adds a new log record processor to this LoggerProvider's # log_record_processors. # - # @param [LogRecordProcessor] log_record_processor The new - # LogRecordProcessor to add. + # @param [LogRecordProcessor] log_record_processor The + # {LogRecordProcessor} to add to this LoggerProvider. def add_log_record_processor(log_record_processor) @mutex.synchronize do - @log_record_processors = @log_record_processors.dup.push(log_record_processor) + @log_record_processors = log_record_processors.dup.push(log_record_processor) end end - # Attempts to stop all the activity for this {LoggerProvider}. Calls - # LogRecordProcessor#shutdown for all registered LogRecordProcessors. + # Attempts to stop all the activity for this LoggerProvider. Calls + # {LogRecordProcessor#shutdown} for all registered {LogRecordProcessor}s. # - # This operation may block until all the Log Records are processed. Must + # This operation may block until all log records are processed. Must # be called before turning off the main application to ensure all data # are processed and exported. # - # After this is called all the newly created {LogRecord}s will be no-op. + # After this is called all newly created {LogRecord}s will be no-op. # # @param [optional Numeric] timeout An optional timeout in seconds. # @return [Integer] Export::SUCCESS if no error occurred, Export::FAILURE if @@ -82,7 +82,7 @@ def shutdown(timeout: nil) end start_time = OpenTelemetry::Common::Utilities.timeout_timestamp - results = @log_record_processors.map do |processor| + results = log_record_processors.map do |processor| remaining_timeout = OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time) break [OpenTelemetry::SDK::Logs::Export::TIMEOUT] if remaining_timeout&.zero? @@ -94,13 +94,13 @@ def shutdown(timeout: nil) end end - # Immediately export all log records that have not yet been exported - # for all the registered LogRecordProcessors. + # Immediately export all {LogRecord}s that have not yet been exported + # for all the registered {LogRecordProcessor}s. # # This method should only be called in cases where it is absolutely # necessary, such as when using some FaaS providers that may suspend - # the process after an invocation, but before the `Processor` exports - # the completed log records. + # the process after an invocation, but before the {LogRecordProcessor} + # exports the completed {LogRecord}s. # # @param [optional Numeric] timeout An optional timeout in seconds. # @return [Integer] Export::SUCCESS if no error occurred, Export::FAILURE if @@ -110,7 +110,7 @@ def force_flush(timeout: nil) return Export::SUCCESS if @stopped start_time = OpenTelemetry::Common::Utilities.timeout_timestamp - results = @log_record_processors.map do |processor| + results = log_record_processors.map do |processor| remaining_timeout = OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time) return Export::TIMEOUT if remaining_timeout&.zero? 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 42e6663177..57a5581817 100644 --- a/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb @@ -26,17 +26,10 @@ describe '#add_log_record_processor' do it "adds the processor to the logger provider's processors" do - assert_equal( - 0, - logger_provider.instance_variable_get(:@log_record_processors).length - ) + assert_equal(0, logger_provider.log_record_processors.length) logger_provider.add_log_record_processor(mock_log_record_processor) - - assert_equal( - 1, - logger_provider.instance_variable_get(:@log_record_processors).length - ) + assert_equal(1, logger_provider.log_record_processors.length) end end From 2ecac1c29c5c3563ca20e8208194e496afad29e5 Mon Sep 17 00:00:00 2001 From: Kayla Reopelle Date: Tue, 22 Aug 2023 15:53:49 -0700 Subject: [PATCH 15/15] Add links --- logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb index 5d1c00f1d3..a16b678849 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb @@ -19,9 +19,9 @@ class LoggerProvider < OpenTelemetry::Logs::LoggerProvider # Returns a new LoggerProvider instance. # # @param [optional Resource] resource The resource to associate with - # new LogRecords created by Loggers created by this LoggerProvider. - # @param [optional Array] log_record_processors The log record - # processors to associate with this LoggerProvider. + # new LogRecords created by {Logger}s created by this LoggerProvider. + # @param [optional Array] log_record_processors The + # {LogRecordProcessor}s to associate with this LoggerProvider. # # @return [OpenTelemetry::SDK::Logs::LoggerProvider] def initialize(