Skip to content

Commit

Permalink
Honor explicit parent spans for events (#92)
Browse files Browse the repository at this point in the history
## Motivation

Currently, when an event is created, no matter what is set as its
parent, `OpenTelemetryLayer` will call `lookup_current` and use that as
the parent span. It should honor the configured parent, if any, or be
ignored, if the parent is explicitly set to `None`.

## Solution

The code is changed so that the parent is correctly resolved. The logic
is somewhat similar to `OpenTelemetryLayer::parent_context`.
  • Loading branch information
mladedav authored Feb 16, 2024
1 parent 8b20ca4 commit 101dc89
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 1 deletion.
7 changes: 6 additions & 1 deletion src/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -996,7 +996,12 @@ where
/// [`Error`]: opentelemetry::trace::StatusCode::Error
fn on_event(&self, event: &Event<'_>, ctx: Context<'_, S>) {
// Ignore events that are not in the context of a span
if let Some(span) = ctx.lookup_current() {
if let Some(span) = event.parent().and_then(|id| ctx.span(id)).or_else(|| {
event
.is_contextual()
.then(|| ctx.lookup_current())
.flatten()
}) {
// Performing read operations before getting a write lock to avoid a deadlock
// See https://github.com/tokio-rs/tracing/issues/763
#[cfg(feature = "tracing-log")]
Expand Down
98 changes: 98 additions & 0 deletions tests/parents.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
use futures_util::future::BoxFuture;
use opentelemetry::trace::TracerProvider as _;
use opentelemetry_sdk::{
export::trace::{ExportResult, SpanData, SpanExporter},
trace::{Tracer, TracerProvider},
};
use std::sync::{Arc, Mutex};
use tracing::level_filters::LevelFilter;
use tracing::Subscriber;
use tracing_opentelemetry::layer;
use tracing_subscriber::prelude::*;

#[derive(Clone, Default, Debug)]
struct TestExporter(Arc<Mutex<Vec<SpanData>>>);

impl SpanExporter for TestExporter {
fn export(&mut self, mut batch: Vec<SpanData>) -> BoxFuture<'static, ExportResult> {
let spans = self.0.clone();
Box::pin(async move {
if let Ok(mut inner) = spans.lock() {
inner.append(&mut batch);
}
Ok(())
})
}
}

fn test_tracer() -> (Tracer, TracerProvider, TestExporter, impl Subscriber) {
let exporter = TestExporter::default();
let provider = TracerProvider::builder()
.with_simple_exporter(exporter.clone())
.build();
let tracer = provider.tracer("test");

let subscriber = tracing_subscriber::registry()
.with(layer().with_tracer(tracer.clone()).with_filter(LevelFilter::DEBUG))
.with(tracing_subscriber::fmt::layer().with_filter(LevelFilter::TRACE));

(tracer, provider, exporter, subscriber)
}

#[test]
fn explicit_parents_of_events() {
let (_tracer, provider, exporter, subscriber) = test_tracer();

tracing::subscriber::with_default(subscriber, || {
let root = tracing::debug_span!("root").entered();

tracing::debug!("1");
tracing::debug!(parent: &root, "2");
tracing::debug!(parent: None, "3");

let child = tracing::debug_span!(parent: &root, "child");
child.in_scope(|| {
tracing::debug!("4");
tracing::debug!(parent: &root, "5");
tracing::debug!(parent: &child, "6");
tracing::debug!(parent: None, "7");
});

tracing::debug!("8");
tracing::debug!(parent: &root, "9");
tracing::debug!(parent: &child, "10");
tracing::debug!(parent: None, "11");

let root = root.exit();

tracing::debug!("12");
tracing::debug!(parent: &root, "13");
tracing::debug!(parent: &child, "14");
tracing::debug!(parent: None, "15");
});

drop(provider); // flush all spans
let spans = exporter.0.lock().unwrap();

assert_eq!(spans.len(), 2);

{
// Check the root span
let expected_root_events = ["1", "2", "5", "8", "9", "13"];

let root_span = spans.iter().find(|s| s.name == "root").unwrap();
let actual_events: Vec<_> = root_span.events.iter().map(|event| event.name.to_string()).collect();

assert_eq!(&expected_root_events, &actual_events[..]);
}

{
// Check the child span
let expected_child_events = ["4", "6", "10", "14"];

let child_span = spans.iter().find(|s| s.name == "child").unwrap();
let actual_events: Vec<_> = child_span.events.iter().map(|event| event.name.to_string()).collect();

assert_eq!(&expected_child_events, &actual_events[..]);
}
}

0 comments on commit 101dc89

Please sign in to comment.