Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Create Logs SDK LoggerProvider #2

Closed
wants to merge 15 commits into from
Closed
5 changes: 5 additions & 0 deletions logs_sdk/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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'
3 changes: 2 additions & 1 deletion logs_sdk/lib/opentelemetry-logs-sdk.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
#
# SPDX-License-Identifier: Apache-2.0

require 'opentelemetry'
require 'opentelemetry/sdk'
require 'opentelemetry/sdk/logs'
require 'opentelemetry/sdk/logs/version'
4 changes: 4 additions & 0 deletions logs_sdk/lib/opentelemetry/sdk/logs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions logs_sdk/lib/opentelemetry/sdk/logs/export.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# 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 LoggerProvider#force_flush
# and LoggerProvider#shutdown
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
22 changes: 22 additions & 0 deletions logs_sdk/lib/opentelemetry/sdk/logs/log_record_processor.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# frozen_string_literal: true

# Copyright The OpenTelemetry Authors
#
# SPDX-License-Identifier: Apache-2.0

module OpenTelemetry
module SDK
module Logs
# Presently no-op LogRecordProcessor
class LogRecordProcessor
def shutdown(timeout: nil)
# TODO: implement
end

def force_flush(timeout: nil)
# TODO: implement
end
end
end
end
end
32 changes: 32 additions & 0 deletions logs_sdk/lib/opentelemetry/sdk/logs/logger.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# 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, :logger_provider

# @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
127 changes: 127 additions & 0 deletions logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
# 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
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.
# @param [optional Array] log_record_processors Log Record Processors to
# associate with the LoggerProvider.
#
# @return [LoggerProvider]
def initialize(
resource: OpenTelemetry::SDK::Resources::Resource.create,
log_record_processors: []
)
@log_record_processors = log_record_processors
@mutex = Mutex.new
@resource = resource
@stopped = false
end

# 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?

@mutex.synchronize do
OpenTelemetry::SDK::Logs::Logger.new(name, version, self)
end
end

# Adds a new LogRecordProcessor to this {LoggerProvider}'s
# log_record_processors.
#
# @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)
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is copied from TracerProvider#add_span_processor. @fallwith and I were chatting about this LOC. He was asking about the use of .dup.push here.

Does dup.push provide atomicity in a way that the push method alone couldn't deliver? Or is there a different reason dup.push was used?

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('LoggerProvider#shutdown called multiple times.')
return OpenTelemetry::SDK::Logs::Export::FAILURE
end
Comment on lines +79 to +82
Copy link
Owner Author

@kaylareopelle kaylareopelle Aug 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code for LoggerProvider#shutdown and LoggerProvider#force_flush are drawn mainly from TracerProvider methods of the same names.

Why does #shutdown return a failure status code if called when @stopped is true, but #force_flush returns a success status code?

JS, for example, does similar things for both #force_flush and #shutdown.

The specs on #force_flush (very similar for Logs and Traces), don't describe the desired behavior in cases where the method is called, but the provider has stopped/shutdown.

Copy link
Owner Author

@kaylareopelle kaylareopelle Aug 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the spec states:

After the call to Shutdown, subsequent attempts to get a Logger are not allowed. SDKs SHOULD return a valid no-op Logger for these calls, if possible.
Source

This is also true for TracerProvider#shutdown (source). Should I update this code in the Logs SDK to return OpenTelemetry::Logs::LoggerProvider::NOOP_LOGGER? If so, should action be taken on the Trace SDK?

I think we should keep the Logs SDK and the Trace SDK as similar as possible. Some things like this might be better handled as issues to tackle in both libraries after the initial implementation. I'm open to keeping things in sync for now and making dual changes later on.


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?

processor.shutdown(timeout: remaining_timeout)
end

@stopped = true
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?

processor.force_flush(timeout: remaining_timeout)
end

results.max || Export::SUCCESS
end
rescue StandardError
Export::FAILURE
end
end
end
end
end
15 changes: 9 additions & 6 deletions logs_sdk/opentelemetry-logs-sdk.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
22 changes: 3 additions & 19 deletions logs_sdk/test/.rubocop.yml
Original file line number Diff line number Diff line change
@@ -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
Loading