From cff311c38c6b69b1446546d542cae69ab81cfaff Mon Sep 17 00:00:00 2001 From: "Kayla Reopelle (she/her)" <87386821+kaylareopelle@users.noreply.github.com> Date: Mon, 22 Jan 2024 10:37:41 -0800 Subject: [PATCH] feat: Create Logs SDK LoggerProvider (#1517) * feat: Create Logs SDK LoggerProvider * Add OpenTelemetry::SDK::Logs::Export constants * Create OpenTelemetry::SDK::Logs::LoggerProvider * Implement LoggerProvider#logger * Implement LoggerProvider#shutdown * Implement LoggerProvider#force_flush * Create no-op LogRecordProcessor * Update Export module description * Update #on_emit context arg description * Remove public reader for log_record_processors * LoggerProvider#logger: keyword args, require name * Remove attr_readers for scope and provider * Apply suggestions from code review Co-authored-by: Francis Bogsanyi * Remove unncessary error handling * Warn #add_log_record_processor calls when stopped * Remove "optional" marker from type description Co-authored-by: Olle Jonsson * Remove "optional" from type documentation * Apply suggestions from code review Co-authored-by: Francis Bogsanyi * Apply suggestions from code review Co-authored-by: Francis Bogsanyi --------- Co-authored-by: Francis Bogsanyi Co-authored-by: Olle Jonsson --- logs_sdk/Gemfile | 5 + logs_sdk/lib/opentelemetry-logs-sdk.rb | 2 +- logs_sdk/lib/opentelemetry/sdk/logs.rb | 4 + logs_sdk/lib/opentelemetry/sdk/logs/export.rb | 23 +++ .../sdk/logs/log_record_processor.rb | 47 +++++ logs_sdk/lib/opentelemetry/sdk/logs/logger.rb | 31 ++++ .../opentelemetry/sdk/logs/logger_provider.rb | 126 +++++++++++++ logs_sdk/opentelemetry-logs-sdk.gemspec | 15 +- logs_sdk/test/.rubocop.yml | 22 +-- .../sdk/logs/logger_provider_test.rb | 173 ++++++++++++++++++ logs_sdk/test/test_helper.rb | 2 + 11 files changed, 424 insertions(+), 26 deletions(-) create mode 100644 logs_sdk/lib/opentelemetry/sdk/logs/export.rb create mode 100644 logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb 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 f649e2f64..69362a59b 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 0865890b8..32c03df1f 100644 --- a/logs_sdk/lib/opentelemetry-logs-sdk.rb +++ b/logs_sdk/lib/opentelemetry-logs-sdk.rb @@ -4,5 +4,5 @@ # # SPDX-License-Identifier: Apache-2.0 +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 b4f503b18..4331257d3 100644 --- a/logs_sdk/lib/opentelemetry/sdk/logs.rb +++ b/logs_sdk/lib/opentelemetry/sdk/logs.rb @@ -5,6 +5,10 @@ # SPDX-License-Identifier: Apache-2.0 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 000000000..2a9f150d3 --- /dev/null +++ b/logs_sdk/lib/opentelemetry/sdk/logs/export.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module SDK + module Logs + # The Export module contains result codes for exporters + module Export + # The operation finished successfully. + SUCCESS = 0 + + # The operation finished with an error. + FAILURE = 1 + + # 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 000000000..6b51b1691 --- /dev/null +++ b/logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module SDK + module Logs + # LogRecordProcessor describes a duck type and provides a synchronous no-op hook for when a + # {LogRecord} is emitted. It is not required to subclass this + # class to provide an implementation of LogRecordProcessor, provided the interface is + # satisfied. + class LogRecordProcessor + # Called when a {LogRecord} is emitted. Subsequent calls are not + # permitted after shutdown is called. + # @param [LogRecord] log_record The emitted {LogRecord} + # @param [Context] context The {Context} + def on_emit(log_record, context); end + + # Export all log records to the configured `Exporter` that have not yet + # been exported. + # + # 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 spans. + # + # @param [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) + Export::SUCCESS + end + + # Called when {LoggerProvider#shutdown} is called. + # + # @param [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) + Export::SUCCESS + end + end + end + end +end 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 000000000..e1b855079 --- /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 + # The SDK implementation of OpenTelemetry::Logs::Logger + class Logger < OpenTelemetry::Logs::Logger + # @api private + # + # 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 The {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 000000000..d284a634c --- /dev/null +++ b/logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb @@ -0,0 +1,126 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module SDK + module Logs + # The SDK implementation of OpenTelemetry::Logs::LoggerProvider. + class LoggerProvider < OpenTelemetry::Logs::LoggerProvider + attr_reader :resource + + UNEXPECTED_ERROR_MESSAGE = 'unexpected error in ' \ + 'OpenTelemetry::SDK::Logs::LoggerProvider#%s' + + private_constant :UNEXPECTED_ERROR_MESSAGE + + # Returns a new LoggerProvider instance. + # + # @param [optional Resource] resource The resource to associate with + # new LogRecords created by {Logger}s created by this LoggerProvider. + # + # @return [OpenTelemetry::SDK::Logs::LoggerProvider] + def initialize(resource: OpenTelemetry::SDK::Resources::Resource.create) + @log_record_processors = [] + @mutex = Mutex.new + @resource = resource + @stopped = false + end + + # Returns an {OpenTelemetry::SDK::Logs::Logger} instance. + # + # @param [String] name Instrumentation package name + # @param [optional String] version Instrumentation package version + # + # @return [OpenTelemetry::SDK::Logs::Logger] + def logger(name:, version: nil) + version ||= '' + + if !name.is_a?(String) || name.empty? + OpenTelemetry.logger.warn('LoggerProvider#logger called with an ' \ + "invalid name. Name provided: #{name.inspect}") + end + + Logger.new(name, version, self) + end + + # Adds a new log record processor to this LoggerProvider's + # log_record_processors. + # + # @param [LogRecordProcessor] log_record_processor The + # {LogRecordProcessor} to add to this LoggerProvider. + def add_log_record_processor(log_record_processor) + @mutex.synchronize do + if @stopped + OpenTelemetry.logger.warn('calling LoggerProvider#' \ + 'add_log_record_processor after shutdown.') + return + end + @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 {LogRecordProcessor}s. + # + # 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 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('LoggerProvider#shutdown called multiple times.') + return 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 [Export::TIMEOUT] if remaining_timeout&.zero? + + processor.shutdown(timeout: remaining_timeout) + end + + @stopped = true + results.max || Export::SUCCESS + end + end + + # 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 {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 + # 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? + + processor.force_flush(timeout: remaining_timeout) + end + + results.max || Export::SUCCESS + end + end + end + end + end +end diff --git a/logs_sdk/opentelemetry-logs-sdk.gemspec b/logs_sdk/opentelemetry-logs-sdk.gemspec index a4607ee60..29910cb29 100644 --- a/logs_sdk/opentelemetry-logs-sdk.gemspec +++ b/logs_sdk/opentelemetry-logs-sdk.gemspec @@ -24,15 +24,18 @@ 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', '~> 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.0' - 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" diff --git a/logs_sdk/test/.rubocop.yml b/logs_sdk/test/.rubocop.yml index c575887d4..dbc7df36e 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 000000000..30313eab1 --- /dev/null +++ b/logs_sdk/test/opentelemetry/sdk/logs/logger_provider_test.rb @@ -0,0 +1,173 @@ +# 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 } + 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) 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 + ) + end + end + + 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) + + logger_provider.add_log_record_processor(mock_log_record_processor) + assert_equal(1, logger_provider.instance_variable_get(:@log_record_processors).length) + end + + describe 'when stopped' do + before { logger_provider.instance_variable_set(:@stopped, true) } + + it 'does not add the processor' do + assert_equal(0, logger_provider.instance_variable_get(:@log_record_processors).length) + + logger_provider.add_log_record_processor(mock_log_record_processor) + assert_equal(0, logger_provider.instance_variable_get(:@log_record_processors).length) + end + + it 'logs a warning' do + OpenTelemetry::TestHelpers.with_test_logger do |log_stream| + logger_provider.add_log_record_processor(mock_log_record_processor) + assert_match(/calling LoggerProvider#add_log_record_processor after shutdown/, + log_stream.string) + end + end + end + end + + describe '#logger' do + let(:error_text) { /LoggerProvider#logger called with an invalid name/ } + + it 'logs a warning if name is nil' do + OpenTelemetry::TestHelpers.with_test_logger do |log_stream| + logger_provider.logger(name: nil) + assert_match(error_text, 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(name: '') + assert_match(error_text, log_stream.string) + end + end + + it 'sets version to an empty string if nil' do + # :version is nil by default, but explicitly setting it here + # to make the test easier to read + logger = logger_provider.logger(name: 'name', version: nil) + assert_equal(logger.instance_variable_get(:@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: name, version: version) + assert_equal(logger.instance_variable_get(:@instrumentation_scope).name, name) + assert_equal(logger.instance_variable_get(:@instrumentation_scope).version, version) + end + end + + describe '#shutdown' do + 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(/.* called multiple times/, log_stream.string) + end + end + + it 'sends shutdown to the processor' do + 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_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.shutdown + + mock_log_record_processor.verify + mock_log_record_processor2.verify + end + + 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) + logger_provider.shutdown + logger_provider.shutdown + + 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 + it 'notifies the log record processor' do + 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) + + 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 + + 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 + end +end diff --git a/logs_sdk/test/test_helper.rb b/logs_sdk/test/test_helper.rb index 786faa1ec..59d743987 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)