Skip to content

Commit dca0e22

Browse files
committed
revise tests
1 parent 76de7ff commit dca0e22

File tree

4 files changed

+64
-76
lines changed

4 files changed

+64
-76
lines changed

opentelemetry-sdk/src/logs/log_emitter.rs

+54-73
Original file line numberDiff line numberDiff line change
@@ -269,14 +269,63 @@ mod tests {
269269

270270
use super::*;
271271
use opentelemetry::global::{logger, set_logger_provider, shutdown_logger_provider};
272-
use opentelemetry::logs::Logger;
273-
use opentelemetry::{Key, KeyValue, Value};
274272
use opentelemetry::logs::{Logger, LoggerProvider as _};
273+
use opentelemetry::{Key, KeyValue, Value};
275274
use std::fmt::{Debug, Formatter};
276275
use std::sync::atomic::AtomicU64;
277276
use std::sync::{Arc, Mutex};
278277
use std::thread;
279278

279+
struct ShutdownTestLogProcessor {
280+
is_shutdown: Arc<Mutex<bool>>,
281+
counter: Arc<AtomicU64>,
282+
}
283+
284+
impl Debug for ShutdownTestLogProcessor {
285+
fn fmt(&self, _f: &mut Formatter<'_>) -> std::fmt::Result {
286+
todo!()
287+
}
288+
}
289+
290+
impl ShutdownTestLogProcessor {
291+
pub(crate) fn new(counter: Arc<AtomicU64>) -> Self {
292+
ShutdownTestLogProcessor {
293+
is_shutdown: Arc::new(Mutex::new(false)),
294+
counter,
295+
}
296+
}
297+
}
298+
299+
impl LogProcessor for ShutdownTestLogProcessor {
300+
fn emit(&self, _data: LogData) {
301+
self.is_shutdown
302+
.lock()
303+
.map(|is_shutdown| {
304+
if !*is_shutdown {
305+
self.counter
306+
.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
307+
}
308+
})
309+
.expect("lock poisoned");
310+
}
311+
312+
fn force_flush(&self) -> LogResult<()> {
313+
Ok(())
314+
}
315+
316+
fn shutdown(&self) -> LogResult<()> {
317+
self.is_shutdown
318+
.lock()
319+
.map(|mut is_shutdown| *is_shutdown = true)
320+
.expect("lock poisoned");
321+
Ok(())
322+
}
323+
324+
#[cfg(feature = "logs_level_enabled")]
325+
fn event_enabled(&self, _level: Severity, _target: &str, _name: &str) -> bool {
326+
true
327+
}
328+
}
280329
#[test]
281330
fn test_logger_provider_default_resource() {
282331
let assert_resource = |provider: &super::LoggerProvider,
@@ -403,57 +452,6 @@ mod tests {
403452
assert_eq!(no_service_name.config().resource.len(), 0);
404453
}
405454

406-
struct ShutdownTestLogProcessor {
407-
is_shutdown: Arc<Mutex<bool>>,
408-
counter: Arc<AtomicU64>,
409-
}
410-
411-
impl Debug for ShutdownTestLogProcessor {
412-
fn fmt(&self, _f: &mut Formatter<'_>) -> std::fmt::Result {
413-
todo!()
414-
}
415-
}
416-
417-
impl ShutdownTestLogProcessor {
418-
pub(crate) fn new(counter: Arc<AtomicU64>) -> Self {
419-
ShutdownTestLogProcessor {
420-
is_shutdown: Arc::new(Mutex::new(false)),
421-
counter,
422-
}
423-
}
424-
}
425-
426-
impl LogProcessor for ShutdownTestLogProcessor {
427-
fn emit(&self, _data: LogData) {
428-
self.is_shutdown
429-
.lock()
430-
.map(|is_shutdown| {
431-
if !*is_shutdown {
432-
self.counter
433-
.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
434-
}
435-
})
436-
.expect("lock poisoned");
437-
}
438-
439-
fn force_flush(&self) -> LogResult<()> {
440-
Ok(())
441-
}
442-
443-
fn shutdown(&self) -> LogResult<()> {
444-
self.is_shutdown
445-
.lock()
446-
.map(|mut is_shutdown| *is_shutdown = true)
447-
.expect("lock poisoned");
448-
Ok(())
449-
}
450-
451-
#[cfg(feature = "logs_level_enabled")]
452-
fn event_enabled(&self, _level: Severity, _target: &str, _name: &str) -> bool {
453-
true
454-
}
455-
}
456-
457455
#[test]
458456
fn shutdown_test() {
459457
let counter = Arc::new(AtomicU64::new(0));
@@ -524,36 +522,19 @@ mod tests {
524522
thread::sleep(std::time::Duration::from_millis(10));
525523
}
526524

527-
// Intentionally *not* calling shutdown/flush on the provider, but
528-
// instead relying on shutdown_logger_provider which causes the global
529-
// provider to be dropped, leading to the sdk logger provider's drop to
530-
// be called, which is expected to call shutdown on processors.
525+
// shutdown logger provider explicitly call shutdown on the logger
531526
shutdown_logger_provider();
532527

533528
// Assert
534529

535-
// shutdown_logger_provider is necessary but not sufficient, as loggers
536-
// hold on to the the provider (via inner provider clones).
537-
assert!(!*shutdown_called.lock().unwrap());
530+
// shutdown_logger_provider should be shutdown regardless if the logger is dropped.
531+
assert!(*shutdown_called.lock().unwrap());
538532

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

542-
// Drop one of the logger. Not enough!
543-
drop(logger1);
544-
assert!(!*shutdown_called.lock().unwrap());
545-
546-
// drop logger2, which is the only remaining logger in this thread.
547-
// Still not enough!
548-
drop(logger2);
549-
assert!(!*shutdown_called.lock().unwrap());
550-
551-
// now signal the spawned thread to end, which causes it to drop its
552-
// logger. Since that is the last logger, the provider (inner provider)
553-
// is finally dropped, triggering shutdown
554536
*signal_to_end.lock().unwrap() = true;
555537
handle.join().unwrap();
556-
assert!(*shutdown_called.lock().unwrap());
557538

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

opentelemetry/src/global/logs.rs

+4
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ impl LoggerProvider for GlobalLoggerProvider {
9898
fn library_logger(&self, library: Arc<InstrumentationLibrary>) -> Self::Logger {
9999
BoxedLogger(self.provider.boxed_logger(library))
100100
}
101+
102+
fn shutdown(&self) -> Vec<LogResult<()>> {
103+
self.provider.shutdown()
104+
}
101105
}
102106

103107
static GLOBAL_LOGGER_PROVIDER: Lazy<RwLock<GlobalLoggerProvider>> =

opentelemetry/src/logs/logger.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,5 @@ impl<'a, T: LoggerProvider + ?Sized> LoggerBuilder<'a, T> {
150150
///
151151
/// Note that `shutdown` doesn't mean `Drop`(though `Drop` can call `shutdown`).
152152
/// It simply means the provider will emit anything in the buffer(if applicable) and stop processing
153-
fn shutdown(&self) -> Vec<LogResult<()>> {
154-
Vec::new() // noop
155-
}
153+
fn shutdown(&self) -> Vec<LogResult<()>>;
156154
}

opentelemetry/src/logs/noop.rs

+5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::{borrow::Cow, sync::Arc};
22

3+
use crate::logs::LogResult;
34
use crate::{
45
logs::{LogRecord, Logger, LoggerProvider},
56
InstrumentationLibrary, KeyValue,
@@ -32,6 +33,10 @@ impl LoggerProvider for NoopLoggerProvider {
3233
) -> Self::Logger {
3334
NoopLogger(())
3435
}
36+
37+
fn shutdown(&self) -> Vec<LogResult<()>> {
38+
vec![]
39+
}
3540
}
3641

3742
/// A no-op implementation of a [`Logger`]

0 commit comments

Comments
 (0)