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

Change to owned LoggerProvider #1455

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion opentelemetry-sdk/CHANGELOG.md
Expand Up @@ -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.

Expand All @@ -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>`
- `LoggerProviderInner` is no longer `pub (crate)`
- `Logger.provider()` now returns `&LoggerProvider` instead of an `Option<LoggerProvider>`

## v0.21.2

### Fixed
Expand Down
41 changes: 14 additions & 27 deletions opentelemetry-sdk/src/logs/log_emitter.rs
Expand Up @@ -13,10 +13,7 @@
#[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.
Expand Down Expand Up @@ -55,16 +52,11 @@
}

fn library_logger(&self, library: Arc<InstrumentationLibrary>) -> 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<LoggerProviderInner>) -> Self {
LoggerProvider { inner }
}

/// Create a new `LoggerProvider` builder.
pub fn builder() -> Builder {
Builder::default()
Expand All @@ -91,7 +83,7 @@
/// Shuts down this `LoggerProvider`, panicking on failure.
pub fn shutdown(&mut self) -> Vec<LogResult<()>> {
self.try_shutdown()
.expect("canont shutdown LoggerProvider when child Loggers are still active")
.expect("cannot shutdown LoggerProvider when child Loggers are still active")

Check warning on line 86 in opentelemetry-sdk/src/logs/log_emitter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/logs/log_emitter.rs#L86

Added line #L86 was not covered by tests
}

/// Attempts to shutdown this `LoggerProvider`, succeeding only when
Expand All @@ -108,7 +100,7 @@
}

#[derive(Debug)]
pub(crate) struct LoggerProviderInner {
struct LoggerProviderInner {
processors: Vec<Box<dyn LogProcessor>>,
config: Config,
}
Expand Down Expand Up @@ -179,13 +171,13 @@
/// [`LogRecord`]: opentelemetry::logs::LogRecord
pub struct Logger {
instrumentation_lib: Arc<InstrumentationLibrary>,
provider: Weak<LoggerProviderInner>,
provider: LoggerProvider,
}

impl Logger {
pub(crate) fn new(
instrumentation_lib: Arc<InstrumentationLibrary>,
provider: Weak<LoggerProviderInner>,
provider: LoggerProvider,
) -> Self {
Logger {
instrumentation_lib,
Expand All @@ -194,8 +186,8 @@
}

/// LoggerProvider associated with this logger.
pub fn provider(&self) -> Option<LoggerProvider> {
self.provider.upgrade().map(LoggerProvider::new)
pub fn provider(&self) -> &LoggerProvider {
&self.provider
}

/// Instrumentation library information of this logger.
Expand All @@ -207,16 +199,14 @@
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())
Expand All @@ -226,16 +216,13 @@
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() {
Expand Down
4 changes: 3 additions & 1 deletion opentelemetry/CHANGELOG.md
Expand Up @@ -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

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

Expand Down
2 changes: 1 addition & 1 deletion opentelemetry/src/logs/noop.rs
Expand Up @@ -42,6 +42,6 @@
fn emit(&self, _record: LogRecord) {}
#[cfg(feature = "logs_level_enabled")]
fn event_enabled(&self, _level: super::Severity, _target: &str) -> bool {
true
false

Check warning on line 45 in opentelemetry/src/logs/noop.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry/src/logs/noop.rs#L45

Added line #L45 was not covered by tests
}
}