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

[Logs API] Remove global provider for Logs #1691

Merged
merged 13 commits into from
Apr 30, 2024
Merged
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
5 changes: 5 additions & 0 deletions opentelemetry-otlp/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@

[#1568]: https://github.com/open-telemetry/opentelemetry-rust/pull/1568

### Changed
- **Breaking** Remove global provider for Logs [#1691](https://github.com/open-telemetry/opentelemetry-rust/pull/1691/)
- The method OtlpLogPipeline::install_simple() and OtlpLogPipeline::install_batch() now return `LoggerProvider` instead of
`Logger`. Refer to the [basic-otlp](https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-otlp/examples/basic-otlp/src/main.rs) and [basic-otlp-http](https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs) examples for how to initialize OTLP Log Exporter to use with OpenTelemetryLogBridge and OpenTelemetryTracingBridge respectively.

## v0.15.0

### Added
Expand Down
8 changes: 3 additions & 5 deletions opentelemetry-otlp/examples/basic-otlp-http/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use std::error::Error;
use tracing::info;
use tracing_subscriber::prelude::*;

fn init_logs() -> Result<sdklogs::Logger, opentelemetry::logs::LogError> {
fn init_logs() -> Result<sdklogs::LoggerProvider, opentelemetry::logs::LogError> {
let service_name = env!("CARGO_BIN_NAME");
opentelemetry_otlp::new_pipeline()
.logging()
Expand Down Expand Up @@ -76,13 +76,11 @@ static COMMON_ATTRIBUTES: Lazy<[KeyValue; 4]> = Lazy::new(|| {
async fn main() -> Result<(), Box<dyn Error + Send + Sync + 'static>> {
let _ = init_tracer()?;
let _ = init_metrics()?;
let _ = init_logs();
let logger_provider = init_logs().unwrap();
lalitb marked this conversation as resolved.
Show resolved Hide resolved

let tracer = global::tracer("ex.com/basic");
let meter = global::meter("ex.com/basic");

// configure the global logger to use our opentelemetry logger
let logger_provider = opentelemetry::global::logger_provider();
let layer = OpenTelemetryTracingBridge::new(&logger_provider);
tracing_subscriber::registry().with(layer).init();

Expand All @@ -108,7 +106,7 @@ async fn main() -> Result<(), Box<dyn Error + Send + Sync + 'static>> {
histogram.record(5.5, COMMON_ATTRIBUTES.as_ref());

global::shutdown_tracer_provider();
global::shutdown_logger_provider();
logger_provider.shutdown();
global::shutdown_meter_provider();

Ok(())
Expand Down
9 changes: 3 additions & 6 deletions opentelemetry-otlp/examples/basic-otlp/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ fn init_metrics() -> Result<(), MetricsError> {
}
}

fn init_logs() -> Result<opentelemetry_sdk::logs::Logger, LogError> {
fn init_logs() -> Result<opentelemetry_sdk::logs::LoggerProvider, LogError> {
let service_name = env!("CARGO_BIN_NAME");
opentelemetry_otlp::new_pipeline()
.logging()
Expand Down Expand Up @@ -93,10 +93,7 @@ async fn main() -> Result<(), Box<dyn Error + Send + Sync + 'static>> {
let _ = init_metrics()?;

// Initialize logs, which sets the global loggerprovider.
let _ = init_logs();

// Retrieve the global LoggerProvider.
let logger_provider = global::logger_provider();
let logger_provider = init_logs().unwrap();

// Create a new OpenTelemetryLogBridge using the above LoggerProvider.
let otel_log_appender = OpenTelemetryLogBridge::new(&logger_provider);
Expand Down Expand Up @@ -141,7 +138,7 @@ async fn main() -> Result<(), Box<dyn Error + Send + Sync + 'static>> {
info!(target: "my-target", "hello from {}. My price is {}", "apple", 1.99);

global::shutdown_tracer_provider();
global::shutdown_logger_provider();
logger_provider.shutdown();
global::shutdown_meter_provider();

Ok(())
Expand Down
34 changes: 11 additions & 23 deletions opentelemetry-otlp/src/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,10 @@ use crate::exporter::http::HttpExporterBuilder;

use crate::{NoExporterConfig, OtlpPipeline};
use async_trait::async_trait;
use std::{borrow::Cow, fmt::Debug};
use std::fmt::Debug;

use opentelemetry::logs::LogError;

use opentelemetry::{
global,
logs::{LogError, LoggerProvider},
};
use opentelemetry_sdk::{export::logs::LogData, runtime::RuntimeChannel};

/// Compression algorithm to use, defaults to none.
Expand Down Expand Up @@ -147,7 +145,7 @@ impl OtlpLogPipeline<LogExporterBuilder> {
/// Returns a [`Logger`] with the name `opentelemetry-otlp` and the current crate version.
///
/// [`Logger`]: opentelemetry_sdk::logs::Logger
pub fn install_simple(self) -> Result<opentelemetry_sdk::logs::Logger, LogError> {
pub fn install_simple(self) -> Result<opentelemetry_sdk::logs::LoggerProvider, LogError> {
Ok(build_simple_with_exporter(
self.exporter_builder.build_log_exporter()?,
self.log_config,
Expand All @@ -163,7 +161,7 @@ impl OtlpLogPipeline<LogExporterBuilder> {
pub fn install_batch<R: RuntimeChannel>(
self,
runtime: R,
) -> Result<opentelemetry_sdk::logs::Logger, LogError> {
) -> Result<opentelemetry_sdk::logs::LoggerProvider, LogError> {
Ok(build_batch_with_exporter(
self.exporter_builder.build_log_exporter()?,
self.log_config,
Expand All @@ -176,27 +174,22 @@ impl OtlpLogPipeline<LogExporterBuilder> {
fn build_simple_with_exporter(
exporter: LogExporter,
log_config: Option<opentelemetry_sdk::logs::Config>,
) -> opentelemetry_sdk::logs::Logger {
) -> opentelemetry_sdk::logs::LoggerProvider {
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
let mut provider_builder =
opentelemetry_sdk::logs::LoggerProvider::builder().with_simple_exporter(exporter);
if let Some(config) = log_config {
provider_builder = provider_builder.with_config(config);
}
let provider = provider_builder.build();
let logger = provider
.logger_builder(Cow::Borrowed("opentelemetry-otlp"))
.with_version(Cow::Borrowed(env!("CARGO_PKG_VERSION")))
.build();
let _ = global::set_logger_provider(provider);
logger
// logger would be created in the tracing appender
provider_builder.build()
}

fn build_batch_with_exporter<R: RuntimeChannel>(
exporter: LogExporter,
log_config: Option<opentelemetry_sdk::logs::Config>,
runtime: R,
batch_config: Option<opentelemetry_sdk::logs::BatchConfig>,
) -> opentelemetry_sdk::logs::Logger {
) -> opentelemetry_sdk::logs::LoggerProvider {
let mut provider_builder = opentelemetry_sdk::logs::LoggerProvider::builder();
let batch_processor = opentelemetry_sdk::logs::BatchLogProcessor::builder(exporter, runtime)
.with_batch_config(batch_config.unwrap_or_default())
Expand All @@ -206,11 +199,6 @@ fn build_batch_with_exporter<R: RuntimeChannel>(
if let Some(config) = log_config {
provider_builder = provider_builder.with_config(config);
}
let provider = provider_builder.build();
let logger = provider
.logger_builder(Cow::Borrowed("opentelemetry-otlp"))
.with_version(Cow::Borrowed(env!("CARGO_PKG_VERSION")))
.build();
let _ = global::set_logger_provider(provider);
logger
// logger would be created in the tracing appender
provider_builder.build()
}
67 changes: 10 additions & 57 deletions opentelemetry-sdk/src/logs/log_emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,8 @@ mod tests {
use crate::Resource;

use super::*;
use opentelemetry::global::{logger, set_logger_provider, shutdown_logger_provider};
use opentelemetry::logs::{Logger, LoggerProvider as _};
use opentelemetry::logs::Logger;
use opentelemetry::logs::LoggerProvider as _;
use opentelemetry::{Key, KeyValue, Value};
use std::fmt::{Debug, Formatter};
use std::sync::atomic::AtomicU64;
Expand Down Expand Up @@ -481,74 +481,27 @@ mod tests {
// Arrange
let shutdown_called = Arc::new(Mutex::new(false));
let flush_called = Arc::new(Mutex::new(false));
let signal_to_end = Arc::new(Mutex::new(false));
let signal_to_thread_started = Arc::new(Mutex::new(false));
let logger_provider = LoggerProvider::builder()
.with_log_processor(LazyLogProcessor::new(
shutdown_called.clone(),
flush_called.clone(),
))
.build();
set_logger_provider(logger_provider);
//set_logger_provider(logger_provider);
let logger1 = logger_provider.logger("test-logger1");
let logger2 = logger_provider.logger("test-logger2");

// Act
let logger1 = logger("test-logger1");
let logger2 = logger("test-logger2");
// Acts
logger1.emit(LogRecord::default());
logger2.emit(LogRecord::default());

let signal_to_end_clone = signal_to_end.clone();
let signal_to_thread_started_clone = signal_to_thread_started.clone();

let handle = thread::spawn(move || {
let logger3 = logger("test-logger3");
loop {
// signal the main thread that this thread has started.
*signal_to_thread_started_clone.lock().unwrap() = true;
logger3.emit(LogRecord::default());
if *signal_to_end_clone.lock().unwrap() {
break;
}
}
});

// wait for the spawned thread to start before calling shutdown This is
// very important - if shutdown is called before the spawned thread
// obtains its logger, then the logger will be no-op one, and the test
// will pass, but it will not be testing the intended scenario.
while !*signal_to_thread_started.lock().unwrap() {
thread::sleep(std::time::Duration::from_millis(10));
}

// Intentionally *not* calling shutdown/flush on the provider, but
// instead relying on shutdown_logger_provider which causes the global
// provider to be dropped, leading to the sdk logger provider's drop to
// be called, which is expected to call shutdown on processors.
shutdown_logger_provider();
// explicitly calling shutdown on logger_provider. This will
// indeed do the shutdown, even if there are loggers still alive.
logger_provider.shutdown();

// Assert

// shutdown_logger_provider is necessary but not sufficient, as loggers
// hold on to the the provider (via inner provider clones).
assert!(!*shutdown_called.lock().unwrap());

// flush is never called by the sdk.
assert!(!*flush_called.lock().unwrap());

// Drop one of the logger. Not enough!
drop(logger1);
assert!(!*shutdown_called.lock().unwrap());

// drop logger2, which is the only remaining logger in this thread.
// Still not enough!
drop(logger2);
assert!(!*shutdown_called.lock().unwrap());

// now signal the spawned thread to end, which causes it to drop its
// logger. Since that is the last logger, the provider (inner provider)
// is finally dropped, triggering shutdown
*signal_to_end.lock().unwrap() = true;
handle.join().unwrap();
// shutdown is called.
assert!(*shutdown_called.lock().unwrap());

// flush is never called by the sdk.
Expand Down
10 changes: 10 additions & 0 deletions opentelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@
### Removed

- Remove `urlencoding` crate dependency. [#1613](https://github.com/open-telemetry/opentelemetry-rust/pull/1613)
- Remove global providers for Logs [$1691](https://github.com/open-telemetry/opentelemetry-rust/pull/1691)
LoggerProviders are not meant for end users to get loggers from. It is only required for the log bridges.
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
Below global constructs for the logs are removed from API:
- opentelemetry::global::logger
- opentelemetry::global::set_logger_provider
- opentelemetry::global::shutdown_logger_provider
- opentelemetry::global::logger_provider
- opentelemetry::global::GlobalLoggerProvider
- opentelemetry::global::ObjectSafeLoggerProvider
For creating appenders using Logging bridge API, refer to the opentelemetry-tracing-appender [example](https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-appender-tracing/examples/basic.rs)

### Changed

Expand Down
2 changes: 1 addition & 1 deletion opentelemetry/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ logs_level_enabled = ["logs"]
otel_unstable = []

[dev-dependencies]
opentelemetry_sdk = { path = "../opentelemetry-sdk" } # for documentation tests
opentelemetry_sdk = { path = "../opentelemetry-sdk", features = ["logs_level_enabled"]} # for documentation tests
criterion = { version = "0.3" }

[[bench]]
Expand Down
Loading
Loading