feat: OpenTelemetry.logger_provider API, ProxyLoggers, Configuration, and Instrument Registry #1725

kaylareopelle · 2024-09-10T23:15:55Z

This PR has a few different features that are all somewhat related:

Adds the APIs to set a global logger provider using OpenTelemetry.logger_provider
- Related to this, introduces OpenTelemetry::Internal::ProxyLoggerProvider and OpenTelemetry::Internal::ProxyLogger classes for defaults.
- Like in the main SDK with traces, these proxies are upgraded to real LoggerProviders/Loggers when set.
Adds the ability to configure the logs exporter using OTEL_LOGS_EXPORTER. The options available are console, none, and otlp. The OTLP exporter option will fail gracefully unless the yet to be released opentelemetry-exporter-otlp-logs gem is available. This takes the ConfigurationPatch approach, similar to feat: Update metrics configuration patch #1548
Adds a thread-safe registry for loggers to access the same logger based on name and version

Closes #1699
Closes #1702

Create a registry for loggers to make sure a logger with an identical name and version is created only once and reused

When OTLP logs exporter not installed, rescue the error, emit a message and set the exporter to nil.

…pentelemetry-ruby into logs-configuration-patch

mwear · 2024-10-11T21:19:44Z

logs_api/lib/opentelemetry/internal/proxy_logger.rb

+      # @return [ProxyLogger]
+      def initialize
+        super
+        @mutex = Mutex.new


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.

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?

opentelemetry-ruby/metrics_api/lib/opentelemetry/internal/proxy_meter.rb

Lines 28 to 37 in 6727391

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

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

Great! Updated in 9e2b8ee

mwear · 2024-10-11T21:26:16Z

logs_sdk/lib/opentelemetry/sdk/logs/logger_provider.rb

@@ -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


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

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.

opentelemetry-ruby/sdk/lib/opentelemetry/sdk/trace/tracer_provider.rb

Lines 72 to 75 in a1f1629

if @stopped

OpenTelemetry.logger.warn('calling Tracer#shutdown multiple times.')

return Export::FAILURE

end

What do you think?

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

opentelemetry-ruby/sdk/lib/opentelemetry/sdk/trace/tracer_provider.rb

Lines 146 to 167 in a1f1629

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.

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

Previously, a no-op Logger was returned when LoggerProvider#logger was called after the provider was stopped. Now, when the provider is stopped, the on_emit method will return early and not emit any log records. This more closely follows the behavior in the TracerProvider.

mwear

LGTM

kaylareopelle added 8 commits August 27, 2024 15:55

WIP: Log SDK configuration

f70094d

feat: Add configuration patch for logs SDK

f6fb046

style: Update spacing

679c085

test: Add tests for logs api

a6d7079

feat: Update inheritance to get tests to pass

c9554df

feat: Add Instrument Registry to LoggerProvider

0e1ad90

Create a registry for loggers to make sure a logger with an identical name and version is created only once and reused

feat: Rescue NameError for OTLP logs exporter

6a624e6

When OTLP logs exporter not installed, rescue the error, emit a message and set the exporter to nil.

Remove skip instrumenting stuff

dfe8f44

kaylareopelle added the logs label Sep 10, 2024

kaylareopelle and others added 4 commits September 10, 2024 16:16

Merge branch 'main' into logs-configuration-patch

234cc95

style: Rubocop

b53e6a3

Merge branch 'logs-configuration-patch' of github.com:kaylareopelle/o…

34c8f44

…pentelemetry-ruby into logs-configuration-patch

test: Add skip for intermittent failure

654385d

kaylareopelle changed the title ~~feat: Logs APIs, ProxyLoggers, Configuration, and Instrument Registry~~ feat: OpenTelemetry.logger_provider API, ProxyLoggers, Configuration, and Instrument Registry Sep 12, 2024

kaylareopelle marked this pull request as ready for review September 12, 2024 05:42

kaylareopelle requested review from fbogsany, mwear, robertlaurin, dazuma, ericmustin, arielvalentin, ahayworth, plantfansam and robbkidd as code owners September 12, 2024 05:42

kaylareopelle mentioned this pull request Sep 13, 2024

OTLP Exporter for Logs #1525

Open

kaylareopelle self-assigned this Sep 27, 2024

mwear reviewed Oct 11, 2024

View reviewed changes

kaylareopelle and others added 3 commits October 15, 2024 11:54

Merge branch 'main' into logs-configuration-patch

acf5cdb

refactor: Remove delegate, mutex from ProxyLogger

9e2b8ee

Merge branch 'main' into logs-configuration-patch

936e149

mwear approved these changes Nov 15, 2024

View reviewed changes

Merge branch 'main' into logs-configuration-patch

a3434c8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: OpenTelemetry.logger_provider API, ProxyLoggers, Configuration, and Instrument Registry #1725

feat: OpenTelemetry.logger_provider API, ProxyLoggers, Configuration, and Instrument Registry #1725

kaylareopelle commented Sep 10, 2024 •

edited

Loading

mwear Oct 11, 2024

kaylareopelle Oct 16, 2024

mwear Oct 25, 2024

kaylareopelle Oct 28, 2024

mwear Oct 11, 2024

kaylareopelle Oct 16, 2024 •

edited

Loading

mwear Oct 25, 2024

kaylareopelle Oct 28, 2024

mwear left a comment

	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

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

	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

feat: OpenTelemetry.logger_provider API, ProxyLoggers, Configuration, and Instrument Registry #1725

Are you sure you want to change the base?

feat: OpenTelemetry.logger_provider API, ProxyLoggers, Configuration, and Instrument Registry #1725

Conversation

kaylareopelle commented Sep 10, 2024 • edited Loading

mwear Oct 11, 2024

Choose a reason for hiding this comment

kaylareopelle Oct 16, 2024

Choose a reason for hiding this comment

mwear Oct 25, 2024

Choose a reason for hiding this comment

kaylareopelle Oct 28, 2024

Choose a reason for hiding this comment

mwear Oct 11, 2024

Choose a reason for hiding this comment

kaylareopelle Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

mwear Oct 25, 2024

Choose a reason for hiding this comment

kaylareopelle Oct 28, 2024

Choose a reason for hiding this comment

mwear left a comment

Choose a reason for hiding this comment

kaylareopelle commented Sep 10, 2024 •

edited

Loading

kaylareopelle Oct 16, 2024 •

edited

Loading