Skip to content

Commit 28b354b

Browse files
committed
refactor: only return span instead of entire context
1 parent 0172924 commit 28b354b

File tree

1 file changed

+35
-10
lines changed

1 file changed

+35
-10
lines changed

opentelemetry/src/context.rs

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -478,11 +478,11 @@ impl Drop for ContextGuard {
478478
fn drop(&mut self) {
479479
let id = self.cx_pos;
480480
if id > ContextStack::BASE_POS && id < ContextStack::MAX_POS {
481-
// Extract the context to drop outside of borrow_mut to avoid panic
482-
// when the context's drop implementation (e.g., Span::drop) calls Context::current()
483-
let _context_to_drop =
481+
// Extract only the span to drop outside of borrow_mut to avoid panic
482+
// when the span's drop implementation calls Context::current()
483+
let _to_drop =
484484
CURRENT_CONTEXT.with(|context_stack| context_stack.borrow_mut().pop_id(id));
485-
// Context is automatically dropped here, outside of borrow_mut scope
485+
// Span (if any) is automatically dropped here, outside of borrow_mut scope
486486
}
487487
}
488488
}
@@ -533,6 +533,12 @@ struct ContextStack {
533533
_marker: PhantomData<*const ()>,
534534
}
535535

536+
// Type alias for what pop_id returns - only return the span when trace feature is enabled
537+
#[cfg(feature = "trace")]
538+
type PopIdReturn = Option<Arc<SynchronizedSpan>>;
539+
#[cfg(not(feature = "trace"))]
540+
type PopIdReturn = ();
541+
536542
impl ContextStack {
537543
const BASE_POS: u16 = 0;
538544
const MAX_POS: u16 = u16::MAX;
@@ -561,7 +567,7 @@ impl ContextStack {
561567
}
562568

563569
#[inline(always)]
564-
fn pop_id(&mut self, pos: u16) -> Option<Context> {
570+
fn pop_id(&mut self, pos: u16) -> PopIdReturn {
565571
if pos == ContextStack::BASE_POS || pos == ContextStack::MAX_POS {
566572
// The empty context is always at the bottom of the [`ContextStack`]
567573
// and cannot be popped, and the overflow position is invalid, so do
@@ -575,7 +581,10 @@ impl ContextStack {
575581
"Attempted to pop the overflow position which is not allowed"
576582
}
577583
);
584+
#[cfg(feature = "trace")]
578585
return None;
586+
#[cfg(not(feature = "trace"))]
587+
return;
579588
}
580589
let len: u16 = self.stack.len() as u16;
581590
// Are we at the top of the [`ContextStack`]?
@@ -588,10 +597,17 @@ impl ContextStack {
588597
// empty context is always at the bottom of the stack if the
589598
// [`ContextStack`] is not empty.
590599
if let Some(Some(next_cx)) = self.stack.pop() {
591-
// Return the old context to be dropped outside of borrow_mut
592-
Some(std::mem::replace(&mut self.current_cx, next_cx))
600+
let old_cx = std::mem::replace(&mut self.current_cx, next_cx);
601+
// Extract and return only the span to avoid cloning the entire Context
602+
#[cfg(feature = "trace")]
603+
return old_cx.span;
604+
#[cfg(not(feature = "trace"))]
605+
return;
593606
} else {
594-
None
607+
#[cfg(feature = "trace")]
608+
return None;
609+
#[cfg(not(feature = "trace"))]
610+
return;
595611
}
596612
} else {
597613
// This is an out of order pop.
@@ -603,10 +619,19 @@ impl ContextStack {
603619
stack_length = len,
604620
message = "Attempted to pop beyond the end of the context stack"
605621
);
622+
#[cfg(feature = "trace")]
606623
return None;
624+
#[cfg(not(feature = "trace"))]
625+
return;
626+
}
627+
// Clear out the entry at the given id and extract its span
628+
#[cfg(feature = "trace")]
629+
return self.stack[pos as usize].take().and_then(|cx| cx.span);
630+
#[cfg(not(feature = "trace"))]
631+
{
632+
self.stack[pos as usize] = None;
633+
return;
607634
}
608-
// Clear out the entry at the given id and return it
609-
self.stack[pos as usize].take()
610635
}
611636
}
612637

0 commit comments

Comments
 (0)