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

fix: set span end time when it exits #124

Merged
merged 2 commits into from
May 24, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 69 additions & 2 deletions benches/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,65 @@ use std::time::SystemTime;
use tracing::{trace, trace_span};
use tracing_subscriber::prelude::*;

fn many_enters(c: &mut Criterion) {
let mut group = c.benchmark_group("otel_many_enters");

group.bench_function("spec_baseline", |b| {
let provider = TracerProvider::default();
let tracer = provider.tracer("bench");
b.iter(|| {
fn dummy(tracer: &Tracer, cx: &Context) {
let mut span = tracer.start_with_context("child", cx);
for _ in 0..1000 {
span.add_event("name", Vec::new());
}
}

tracer.in_span("parent", |cx| dummy(&tracer, &cx));
});
});

{
let _subscriber = tracing_subscriber::registry()
.with(RegistryAccessLayer)
.set_default();
group.bench_function("no_data_baseline", |b| b.iter(enters_harness));
}

{
let _subscriber = tracing_subscriber::registry()
.with(OtelDataLayer)
.set_default();
group.bench_function("data_only_baseline", |b| b.iter(enters_harness));
}

{
let provider = TracerProvider::default();
let tracer = provider.tracer("bench");
let otel_layer = tracing_opentelemetry::layer()
.with_tracer(tracer)
.with_tracked_inactivity(false);
let _subscriber = tracing_subscriber::registry()
.with(otel_layer)
.set_default();

group.bench_function("full_without_timings", |b| b.iter(enters_harness));
}

{
let provider = TracerProvider::default();
let tracer = provider.tracer("bench");
let otel_layer = tracing_opentelemetry::layer()
.with_tracer(tracer)
.with_tracked_inactivity(true);
let _subscriber = tracing_subscriber::registry()
.with(otel_layer)
.set_default();

group.bench_function("full_with_timings", |b| b.iter(enters_harness));
}
}

fn many_children(c: &mut Criterion) {
let mut group = c.benchmark_group("otel_many_children");

Expand Down Expand Up @@ -231,6 +290,14 @@ fn tracing_harness() {
dummy();
}

fn enters_harness() {
let span = trace_span!("span");
for _ in 0..1000 {
let guard = span.enter();
_ = criterion::black_box(guard)
}
}

fn events_harness() {
fn dummy() {
let _child = trace_span!("child").entered();
Expand All @@ -249,12 +316,12 @@ fn events_harness() {
criterion_group! {
name = benches;
config = Criterion::default().with_profiler(PProfProfiler::new(100, Output::Flamegraph(None)));
targets = many_children, many_events
targets = many_enters, many_children, many_events
}
#[cfg(target_os = "windows")]
criterion_group! {
name = benches;
config = Criterion::default();
targets = many_children, many_events
targets = many_enters, many_children, many_events
}
criterion_main!(benches);
14 changes: 8 additions & 6 deletions src/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -970,13 +970,17 @@ where
}

fn on_exit(&self, id: &span::Id, ctx: Context<'_, S>) {
let span = ctx.span(id).expect("Span not found, this is a bug");
let mut extensions = span.extensions_mut();

if let Some(otel_data) = extensions.get_mut::<OtelData>() {
otel_data.builder.end_time = Some(crate::time::now());
}

if !self.tracked_inactivity {
return;
}

let span = ctx.span(id).expect("Span not found, this is a bug");
let mut extensions = span.extensions_mut();

if let Some(timings) = extensions.get_mut::<Timings>() {
let now = Instant::now();
timings.busy += (now - timings.last).as_nanos() as i64;
Expand Down Expand Up @@ -1165,9 +1169,7 @@ where
}

// Assign end time, build and start span, drop span to export
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Assign end time

I think that the comment should have been updated too

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops! Want to send a PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

=> #148

builder
.with_end_time(crate::time::now())
.start_with_context(&self.tracer, &parent_cx);
builder.start_with_context(&self.tracer, &parent_cx);
}
}

Expand Down
Loading