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: OpenTelemetry.logger_provider API, ProxyLoggers, Configuration, and Instrument Registry #1725

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 34 additions & 2 deletions logs_api/lib/opentelemetry-logs-api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,37 @@
# SPDX-License-Identifier: Apache-2.0

require 'opentelemetry'
require_relative 'opentelemetry/logs'
require_relative 'opentelemetry/logs/version'
require 'opentelemetry/logs'
require 'opentelemetry/logs/version'
require 'opentelemetry/internal/proxy_logger_provider'
require 'opentelemetry/internal/proxy_logger'

# OpenTelemetry is an open source observability framework, providing a
# general-purpose API, SDK, and related tools required for the instrumentation
# of cloud-native software, frameworks, and libraries.
#
# The OpenTelemetry module in the Logs API gem provides global accessors
# for logs-related objects.
module OpenTelemetry
@logger_provider = Internal::ProxyLoggerProvider.new

# Register the global logger provider.
#
# @param [LoggerProvider] provider A logger provider to register as the
# global instance.
def logger_provider=(provider)
@mutex.synchronize do
if @logger_provider.instance_of? Internal::ProxyLoggerProvider
logger.debug("Upgrading default proxy logger provider to #{provider.class}")
@logger_provider.delegate = provider
end
@logger_provider = provider
end
end

# @return [Object, Logs::LoggerProvider] registered logger provider or a
# default no-op implementation of the logger provider.
def logger_provider
@mutex.synchronize { @logger_provider }
end
end
70 changes: 70 additions & 0 deletions logs_api/lib/opentelemetry/internal/proxy_logger.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# 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
@mutex = Mutex.new
Copy link
Member

Choose a reason for hiding this comment

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

Do we need a mutex here? We don't have it for the proxy tracer. I believe we can remove this because the delegate is set from proxy logger provider, which synchronizes access using its mutex.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great question! I don't think we need it here. I was copying the setup in the ProxyMeter and didn't cross-reference what was going on with the ProxyTracer.

Should we remove the mutex from the ProxyMeter too?

def delegate=(meter)
@mutex.synchronize do
if @delegate.nil?
@delegate = meter
@instrument_registry.each_value { |instrument| instrument.upgrade_with(meter) }
else
OpenTelemetry.logger.warn 'Attempt to reset delegate in ProxyMeter ignored.'
end
end
end

Copy link
Member

Choose a reason for hiding this comment

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

I think it makes sense to align the three. For the purposes of this PR we can remove it from the proxy logger.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great! Updated in 9e2b8ee

@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,
severity_number: nil,
severity_text: nil,
body: nil,
trace_id: nil,
span_id: nil,
trace_flags: nil,
attributes: nil,
context: nil
)
unless @delegate.nil?
return @delegate.on_emit(
timestamp: nil,
observed_timestamp: nil,
severity_number: nil,
severity_text: nil,
body: nil,
trace_id: nil,
span_id: nil,
trace_flags: nil,
attributes: nil,
context: nil
)
end

super
end
end
end
end
60 changes: 60 additions & 0 deletions logs_api/lib/opentelemetry/internal/proxy_logger_provider.rb
Original file line number Diff line number Diff line change
@@ -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
10 changes: 5 additions & 5 deletions logs_api/lib/opentelemetry/logs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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'
72 changes: 72 additions & 0 deletions logs_api/test/opentelemetry_logs_api_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# frozen_string_literal: true

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

require 'test_helper'

describe OpenTelemetry do
class CustomLogRecord < OpenTelemetry::Logs::LogRecord
end

class CustomLogger < OpenTelemetry::Logs::Logger
def on_emit(*)
CustomLogRecord.new
end
end

class CustomLoggerProvider < OpenTelemetry::Logs::LoggerProvider
def logger(name = nil, version = nil)
CustomLogger.new
end
end

describe '.logger_provider' do
after do
# Ensure we don't leak custom logger factories and loggers to other tests
OpenTelemetry.logger_provider = OpenTelemetry::Internal::ProxyLoggerProvider.new
end

it 'returns a Logs::LoggerProvider by default' do
logger_provider = OpenTelemetry.logger_provider
_(logger_provider).must_be_kind_of(OpenTelemetry::Logs::LoggerProvider)
end

it 'returns the same instance when accessed multiple times' do
_(OpenTelemetry.logger_provider).must_equal(OpenTelemetry.logger_provider)
end

it 'returns user-specified logger provider' do
custom_logger_provider = CustomLoggerProvider.new
OpenTelemetry.logger_provider = custom_logger_provider
_(OpenTelemetry.logger_provider).must_equal(custom_logger_provider)
end
end

describe '.logger_provider=' do
after do
# Ensure we don't leak custom logger factories and loggers to other tests
OpenTelemetry.logger_provider = OpenTelemetry::Internal::ProxyLoggerProvider.new
end

it 'has a default proxy logger' do
refute_nil OpenTelemetry.logger_provider.logger
end

it 'upgrades default loggers to *real* loggers' do
# proxy loggers do not emit any log records, nor does the API logger
# the on_emit method is empty
default_logger = OpenTelemetry.logger_provider.logger
_(default_logger.on_emit(body: 'test')).must_be_instance_of(NilClass)
OpenTelemetry.logger_provider = CustomLoggerProvider.new
_(default_logger.on_emit(body: 'test')).must_be_instance_of(CustomLogRecord)
end

it 'upgrades the default logger provider to a *real* logger provider' do
default_logger_provider = OpenTelemetry.logger_provider
OpenTelemetry.logger_provider = CustomLoggerProvider.new
_(default_logger_provider.logger).must_be_instance_of(CustomLogger)
end
end
end
1 change: 1 addition & 0 deletions logs_sdk/lib/opentelemetry/sdk/logs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
71 changes: 71 additions & 0 deletions logs_sdk/lib/opentelemetry/sdk/logs/configuration_patch.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# frozen_string_literal: true

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

require 'opentelemetry/sdk/configurator'

module OpenTelemetry
module SDK
module Logs
# The ConfiguratorPatch implements a hook to configure the logs portion
# of the SDK.
module ConfiguratorPatch
def add_log_record_processor(log_record_processor)
@log_record_processors << log_record_processor
end

private

def initialize
super
@log_record_processors = []
end

# The logs_configuration_hook method is where we define the setup
# process for logs SDK.
def logs_configuration_hook
OpenTelemetry.logger_provider = Logs::LoggerProvider.new(resource: @resource)
configure_log_record_processors
end

def configure_log_record_processors
processors = @log_record_processors.empty? ? wrapped_log_exporters_from_env.compact : @log_record_processors
processors.each { |p| OpenTelemetry.logger_provider.add_log_record_processor(p) }
end

def wrapped_log_exporters_from_env
# TODO: set default to OTLP to match traces, default is console until other exporters merged
exporters = ENV.fetch('OTEL_LOGS_EXPORTER', 'console')

exporters.split(',').map do |exporter|
case exporter.strip
when 'none' then nil
when 'console' then Logs::Export::SimpleLogRecordProcessor.new(Logs::Export::ConsoleLogRecordExporter.new)
when 'otlp'
otlp_protocol = ENV['OTEL_EXPORTER_OTLP_LOGS_PROTOCOL'] || ENV['OTEL_EXPORTER_OTLP_PROTOCOL'] || 'http/protobuf'

if otlp_protocol != 'http/protobuf'
OpenTelemetry.logger.warn "The #{otlp_protocol} transport protocol is not supported by the OTLP exporter, log_records will not be exported."
nil
else
begin
Logs::Export::BatchLogRecordProcessor.new(OpenTelemetry::Exporter::OTLP::LogsExporter.new)
rescue NameError
OpenTelemetry.logger.warn 'The otlp logs exporter cannot be configured - please add opentelemetry-exporter-otlp-logs to your Gemfile. Logs will not be exported'
nil
end
end
else
OpenTelemetry.logger.warn "The #{exporter} exporter is unknown and cannot be configured, log records will not be exported"
nil
end
end
end
end
end
end
end

OpenTelemetry::SDK::Configurator.prepend(OpenTelemetry::SDK::Logs::ConfiguratorPatch)
24 changes: 18 additions & 6 deletions logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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.
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

Is this case necessary? I ask because it differs from what we're doing in the TracerProvider.

Copy link
Contributor Author

@kaylareopelle kaylareopelle Oct 16, 2024

Choose a reason for hiding this comment

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

It's been a minute since I wrote this, but I believe I did it to adhere to this part of the shutdown protocol:

Shutdown MUST be called only once for each LoggerProvider instance. 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)

There's similar verbiage for the Trace SDK.

I'm not sure if we adhere to this part of the Trace spec elsewhere. Maybe this part does the job well enough? But I'm not sure if that would sufficiently covers an attempt to get a tracer after shutdown.

if @stopped
OpenTelemetry.logger.warn('calling Tracer#shutdown multiple times.')
return Export::FAILURE
end

What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I think the other piece of the puzzle (for traces) is here:

if result.recording? && !@stopped
trace_flags = result.sampled? ? OpenTelemetry::Trace::TraceFlags::SAMPLED : OpenTelemetry::Trace::TraceFlags::DEFAULT
context = OpenTelemetry::Trace::SpanContext.new(trace_id: trace_id, span_id: span_id, trace_flags: trace_flags, tracestate: result.tracestate)
attributes = attributes&.merge(result.attributes) || result.attributes.dup
Span.new(
context,
parent_context,
parent_span,
name,
kind,
parent_span_id,
@span_limits,
@span_processors,
attributes,
links,
start_timestamp,
@resource,
instrumentation_scope
)
else
OpenTelemetry::Trace.non_recording_span(OpenTelemetry::Trace::SpanContext.new(trace_id: trace_id, span_id: span_id, tracestate: result.tracestate))
end

My interpretation of the spec is that after calling shutdown on a tracer provider all tracers issued by it, past or future, must be non-operational. That is, they will create no-op spans from there on out. They do not necessarily need to be an instance of a no-op tracer, if that makes sense.

I think we need to ensure that all loggers return early from on_emit after shutdown is called, then it doesn't really matter what kind of logger we return from the logger provider. For consistency, we can follow what we're doing for tracing. I'm open to other considerations though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hadn't thought about it that way! Thanks for breaking that down. I've updated the code to use this approach. See: 06e4b05

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
Expand Down
Loading
Loading