From bdf902bfe76b43d46fe1e7bffe2b5d741eb0eba9 Mon Sep 17 00:00:00 2001 From: Harold Dost Date: Tue, 26 Dec 2023 22:17:58 +0100 Subject: [PATCH 1/2] Change to owned TracerProvider The interface to an item shouldn't take an inner value since it's considered inner, this also allows for further optimizations in the future as it hides the complexity from the user. Rational: This removes exposing the inner which doesn't need to be provided outside of the class. The advantage of this approach is that it's a cleaner implementation. This also removes a weak reference upgrade from the hotpath since we need to have a strong reference in order to access the information. Relates #1209 --- opentelemetry-sdk/src/logs/log_emitter.rs | 41 ++++++++--------------- 1 file changed, 14 insertions(+), 27 deletions(-) diff --git a/opentelemetry-sdk/src/logs/log_emitter.rs b/opentelemetry-sdk/src/logs/log_emitter.rs index 557ed552c5..983d50827b 100644 --- a/opentelemetry-sdk/src/logs/log_emitter.rs +++ b/opentelemetry-sdk/src/logs/log_emitter.rs @@ -13,10 +13,7 @@ use opentelemetry::{ #[cfg(feature = "logs_level_enabled")] use opentelemetry::logs::Severity; -use std::{ - borrow::Cow, - sync::{Arc, Weak}, -}; +use std::{borrow::Cow, sync::Arc}; #[derive(Debug, Clone)] /// Creator for `Logger` instances. @@ -55,16 +52,11 @@ impl opentelemetry::logs::LoggerProvider for LoggerProvider { } fn library_logger(&self, library: Arc) -> Self::Logger { - Logger::new(library, Arc::downgrade(&self.inner)) + Logger::new(library, self.clone()) } } impl LoggerProvider { - /// Build a new logger provider. - pub(crate) fn new(inner: Arc) -> Self { - LoggerProvider { inner } - } - /// Create a new `LoggerProvider` builder. pub fn builder() -> Builder { Builder::default() @@ -91,7 +83,7 @@ impl LoggerProvider { /// Shuts down this `LoggerProvider`, panicking on failure. pub fn shutdown(&mut self) -> Vec> { self.try_shutdown() - .expect("canont shutdown LoggerProvider when child Loggers are still active") + .expect("cannot shutdown LoggerProvider when child Loggers are still active") } /// Attempts to shutdown this `LoggerProvider`, succeeding only when @@ -108,7 +100,7 @@ impl LoggerProvider { } #[derive(Debug)] -pub(crate) struct LoggerProviderInner { +struct LoggerProviderInner { processors: Vec>, config: Config, } @@ -179,13 +171,13 @@ impl Builder { /// [`LogRecord`]: opentelemetry::logs::LogRecord pub struct Logger { instrumentation_lib: Arc, - provider: Weak, + provider: LoggerProvider, } impl Logger { pub(crate) fn new( instrumentation_lib: Arc, - provider: Weak, + provider: LoggerProvider, ) -> Self { Logger { instrumentation_lib, @@ -194,8 +186,8 @@ impl Logger { } /// LoggerProvider associated with this logger. - pub fn provider(&self) -> Option { - self.provider.upgrade().map(LoggerProvider::new) + pub fn provider(&self) -> &LoggerProvider { + &self.provider } /// Instrumentation library information of this logger. @@ -207,16 +199,14 @@ impl Logger { impl opentelemetry::logs::Logger for Logger { /// Emit a `LogRecord`. fn emit(&self, record: LogRecord) { - let provider = match self.provider() { - Some(provider) => provider, - None => return, - }; + let provider = self.provider(); + let config = provider.config(); + let processors = provider.log_processors(); let trace_context = Context::map_current(|cx| { cx.has_active_span() .then(|| TraceContext::from(cx.span().span_context())) }); - let config = provider.config(); - for processor in provider.log_processors() { + for p in processors { let mut record = record.clone(); if let Some(ref trace_context) = trace_context { record.trace_context = Some(trace_context.clone()) @@ -226,16 +216,13 @@ impl opentelemetry::logs::Logger for Logger { resource: config.resource.clone(), instrumentation: self.instrumentation_library().clone(), }; - processor.emit(data); + p.emit(data); } } #[cfg(feature = "logs_level_enabled")] fn event_enabled(&self, level: Severity, target: &str) -> bool { - let provider = match self.provider() { - Some(provider) => provider, - None => return false, - }; + let provider = self.provider(); let mut enabled = false; for processor in provider.log_processors() { From 18ee0883a74583cd8de852249321775cbd71244d Mon Sep 17 00:00:00 2001 From: Harold Dost Date: Tue, 6 Feb 2024 14:53:43 +0100 Subject: [PATCH 2/2] opentelemetry: Disable events for NoopLogger NoopLogger should be using a little resources as possible ideally none. This should help accomplish that. --- opentelemetry-sdk/CHANGELOG.md | 7 ++++++- opentelemetry/CHANGELOG.md | 4 +++- opentelemetry/src/logs/noop.rs | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index b594320fdf..2d3045fa8d 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -30,7 +30,7 @@ - **Breaking** Remove `TextMapCompositePropagator` [#1373](https://github.com/open-telemetry/opentelemetry-rust/pull/1373). Use `TextMapCompositePropagator` in opentelemetry API. - [#1375](https://github.com/open-telemetry/opentelemetry-rust/pull/1375/) Fix metric collections during PeriodicReader shutdown -- **Breaking** [#1480](https://github.com/open-telemetry/opentelemetry-rust/pull/1480) Remove fine grained `BatchConfig` configurations from `BatchLogProcessorBuilder` and `BatchSpanProcessorBuilder`. Use `BatchConfigBuilder` to construct a `BatchConfig` instance and pass it using `BatchLogProcessorBuilder::with_batch_config` or `BatchSpanProcessorBuilder::with_batch_config`. +- **Breaking** [#1480](https://github.com/open-telemetry/opentelemetry-rust/pull/1480) Remove fine grained `BatchConfig` configurations from `BatchLogProcessorBuilder` and `BatchSpanProcessorBuilder`. Use `BatchConfigBuilder` to construct a `BatchConfig` instance and pass it using `BatchLogProcessorBuilder::with_batch_config` or `BatchSpanProcessorBuilder::with_batch_config`. - **Breaking** [#1480](https://github.com/open-telemetry/opentelemetry-rust/pull/1480) Remove mutating functions from `BatchConfig`, use `BatchConfigBuilder` to construct a `BatchConfig` instance. - **Breaking** [#1495](https://github.com/open-telemetry/opentelemetry-rust/pull/1495) Remove Batch LogRecord&Span Processor configuration via non-standard environment variables. Use the following table to migrate from the no longer supported non-standard environment variables to the standard ones. @@ -41,6 +41,11 @@ | OTEL_BSP_SCHEDULE_DELAY_MILLIS | OTEL_BSP_SCHEDULE_DELAY | | OTEL_BSP_EXPORT_TIMEOUT_MILLIS | OTEL_BSP_EXPORT_TIMEOUT | +- **Breaking** [1455](https://github.com/open-telemetry/opentelemetry-rust/pull/1455) Make the LoggerProvider Owned + - `Logger` now takes an Owned Logger instead of a `Weak` + - `LoggerProviderInner` is no longer `pub (crate)` + - `Logger.provider()` now returns `&LoggerProvider` instead of an `Option` + ## v0.21.2 ### Fixed diff --git a/opentelemetry/CHANGELOG.md b/opentelemetry/CHANGELOG.md index b57647e5ab..9387106a5e 100644 --- a/opentelemetry/CHANGELOG.md +++ b/opentelemetry/CHANGELOG.md @@ -6,7 +6,7 @@ - [#1410](https://github.com/open-telemetry/opentelemetry-rust/pull/1410) Add experimental synchronous gauge. This is behind the feature flag, and can be enabled by enabling the feature `otel_unstable` for opentelemetry crate. -- [#1410](https://github.com/open-telemetry/opentelemetry-rust/pull/1410) Guidelines to add new unstable/experimental features. +- [#1410](https://github.com/open-telemetry/opentelemetry-rust/pull/1410) Guidelines to add new unstable/experimental features. ### Changed @@ -17,6 +17,8 @@ gains, and avoids `IndexMap` dependency. This affects `body` and `attributes` of `LogRecord`. [#1353](https://github.com/open-telemetry/opentelemetry-rust/pull/1353) - Add `TextMapCompositePropagator` [#1373](https://github.com/open-telemetry/opentelemetry-rust/pull/1373) +- Turned off events for `NoopLogger` to save on operations + [1455](https://github.com/open-telemetry/opentelemetry-rust/pull/1455) ### Removed diff --git a/opentelemetry/src/logs/noop.rs b/opentelemetry/src/logs/noop.rs index 55d3d617a3..c99b891145 100644 --- a/opentelemetry/src/logs/noop.rs +++ b/opentelemetry/src/logs/noop.rs @@ -42,6 +42,6 @@ impl Logger for NoopLogger { fn emit(&self, _record: LogRecord) {} #[cfg(feature = "logs_level_enabled")] fn event_enabled(&self, _level: super::Severity, _target: &str) -> bool { - true + false } }