-
Notifications
You must be signed in to change notification settings - Fork 526
feat: span processor api refactor #2962
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
base: main
Are you sure you want to change the base?
Changes from all commits
ac0c3bd
a0a8c23
54fa6f2
445aa66
b72b031
afb4197
4954b0d
1d3e6d8
224a7ed
a62a9bd
5ce1ba4
5dae8a7
a28a461
edb290a
07840d8
9a2d119
48a3edf
e2edadb
4f9b012
ddcf5f9
28dc62b
b80d612
8601bd1
86fd3da
2dd7ccf
79270ce
d8f2f80
343aba2
2a284e5
2bab32c
cead725
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,10 +4,10 @@ use opentelemetry::trace::{ | |
SpanContext, SpanId, SpanKind, Status, TraceFlags, TraceId, TraceState, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you run the benchmark suite to do a performance regression against There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I've run the existing If the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we missing a benchmark? It seems like some of your changes should improve performance --> There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, some benchmarks running multiple span processors were missing, I added them un this PR. The comparison between the main branch and this branch for the benchmarks is in this PR description.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! This is exactly what I was hoping to see :D |
||
}; | ||
use opentelemetry_sdk::testing::trace::NoopSpanExporter; | ||
use opentelemetry_sdk::trace::SpanData; | ||
use opentelemetry_sdk::trace::{ | ||
BatchConfigBuilder, BatchSpanProcessor, SpanEvents, SpanLinks, SpanProcessor, | ||
}; | ||
use opentelemetry_sdk::trace::{FinishedSpan, SpanData}; | ||
use std::sync::Arc; | ||
use tokio::runtime::Runtime; | ||
|
||
|
@@ -62,7 +62,8 @@ fn criterion_benchmark(c: &mut Criterion) { | |
let spans = get_span_data(); | ||
handles.push(tokio::spawn(async move { | ||
for span in spans { | ||
span_processor.on_end(span); | ||
let mut span = FinishedSpan::new(span); | ||
span_processor.on_end(&mut span); | ||
tokio::task::yield_now().await; | ||
} | ||
})); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
use std::time::Duration; | ||
|
||
use criterion::{black_box, criterion_group, criterion_main, Criterion}; | ||
use opentelemetry::{ | ||
trace::{Span, Tracer, TracerProvider}, | ||
Context, KeyValue, | ||
}; | ||
use opentelemetry_sdk::trace as sdktrace; | ||
|
||
#[cfg(not(target_os = "windows"))] | ||
use pprof::criterion::{Output, PProfProfiler}; | ||
|
||
/* | ||
Adding results in comments for a quick reference. | ||
Chip: Apple M1 Max | ||
Total Number of Cores: 10 (8 performance and 2 efficiency) | ||
|
||
SpanProcessorApi/0_processors | ||
time: [385.15 ns 386.14 ns 387.25 ns] | ||
SpanProcessorApi/1_processors | ||
time: [385.73 ns 387.17 ns 388.85 ns] | ||
SpanProcessorApi/2_processors | ||
time: [384.84 ns 385.66 ns 386.50 ns] | ||
SpanProcessorApi/4_processors | ||
time: [386.78 ns 388.17 ns 389.58 ns] | ||
*/ | ||
|
||
#[derive(Debug)] | ||
struct NoopSpanProcessor; | ||
|
||
impl sdktrace::SpanProcessor for NoopSpanProcessor { | ||
fn on_start(&self, _span: &mut sdktrace::Span, _parent_cx: &Context) {} | ||
fn on_end(&self, _span: &mut sdktrace::FinishedSpan) {} | ||
fn force_flush(&self) -> opentelemetry_sdk::error::OTelSdkResult { | ||
Ok(()) | ||
} | ||
fn shutdown_with_timeout(&self, _timeout: Duration) -> opentelemetry_sdk::error::OTelSdkResult { | ||
Ok(()) | ||
} | ||
} | ||
|
||
fn create_tracer(span_processors_count: usize) -> sdktrace::SdkTracer { | ||
let mut builder = sdktrace::SdkTracerProvider::builder(); | ||
for _ in 0..span_processors_count { | ||
builder = builder.with_span_processor(NoopSpanProcessor); | ||
} | ||
builder.build().tracer("tracer") | ||
} | ||
|
||
fn create_span(tracer: &sdktrace::Tracer) -> sdktrace::Span { | ||
let mut span = tracer.start("foo"); | ||
span.set_attribute(KeyValue::new("key1", false)); | ||
span.set_attribute(KeyValue::new("key2", "hello")); | ||
span.set_attribute(KeyValue::new("key4", 123.456)); | ||
span.add_event("my_event", vec![KeyValue::new("key1", "value1")]); | ||
span | ||
} | ||
|
||
fn criterion_benchmark(c: &mut Criterion) { | ||
let mut group = c.benchmark_group("SpanProcessorApi"); | ||
for i in [0, 1, 2, 4] { | ||
group.bench_function(format!("{}_processors", i), |b| { | ||
let tracer = create_tracer(i); | ||
b.iter(|| { | ||
black_box(create_span(&tracer)); | ||
}); | ||
}); | ||
} | ||
} | ||
|
||
#[cfg(not(target_os = "windows"))] | ||
criterion_group! { | ||
name = benches; | ||
config = Criterion::default().with_profiler(PProfProfiler::new(100, Output::Flamegraph(None))) | ||
.warm_up_time(std::time::Duration::from_secs(1)) | ||
.measurement_time(std::time::Duration::from_secs(2)); | ||
targets = criterion_benchmark | ||
} | ||
|
||
#[cfg(target_os = "windows")] | ||
criterion_group! { | ||
name = benches; | ||
config = Criterion::default().warm_up_time(std::time::Duration::from_secs(1)) | ||
.measurement_time(std::time::Duration::from_secs(2)); | ||
targets = criterion_benchmark | ||
} | ||
|
||
criterion_main!(benches); |
Uh oh!
There was an error while loading. Please reload this page.