Skip to content
Open
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
2 changes: 1 addition & 1 deletion engine/src/invocation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ impl InvocationHandler {
// Tag internal vs user functions for filtering
"iii.function.kind" = if is_builtin { "internal" } else { "user" },
)
.with_parent_headers(traceparent.as_deref(), baggage.as_deref())
.with_parent_headers(traceparent.as_deref(), None, baggage.as_deref())
};

// Run the dispatch under the caller's OTel context whenever one was
Expand Down
86 changes: 76 additions & 10 deletions engine/src/telemetry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,34 +11,89 @@

pub use crate::workers::observability::otel::*;

use opentelemetry::KeyValue;
use opentelemetry::trace::TraceContextExt;
use tracing::Span;
use tracing_opentelemetry::OpenTelemetrySpanExt;

/// Extension trait for `tracing::Span` to simplify setting parent context from HTTP headers.
///
/// This trait provides a fluent API for setting the parent context of a span using
/// W3C Trace Context (`traceparent`) and Baggage headers.
/// W3C Trace Context (`traceparent` + `tracestate`) and Baggage headers.
///
/// # Example
/// ```ignore
/// use crate::telemetry::SpanExt;
///
/// let span = tracing::info_span!("my_operation")
/// .with_parent_headers(traceparent.as_deref(), baggage.as_deref());
/// .with_parent_headers(traceparent.as_deref(), tracestate.as_deref(), baggage.as_deref());
/// ```
pub trait SpanExt {
/// Sets the parent context of this span from optional traceparent and baggage headers.
/// Sets the parent context of this span from optional W3C trace-context and baggage headers.
///
/// If either `traceparent` or `baggage` is provided, the span's parent context will be
/// set using the extracted context. If both are `None`, the span is returned unchanged.
fn with_parent_headers(self, traceparent: Option<&str>, baggage: Option<&str>) -> Self;
/// If any of `traceparent`, `tracestate`, or `baggage` is provided, the span's parent
/// context is set using the extracted context. `tracestate` is only meaningful alongside
/// a valid `traceparent` (it is ignored without one, per the W3C spec). If all are `None`,
/// the span is returned unchanged.
fn with_parent_headers(
self,
traceparent: Option<&str>,
tracestate: Option<&str>,
baggage: Option<&str>,
) -> Self;
}

impl SpanExt for Span {
fn with_parent_headers(self, traceparent: Option<&str>, baggage: Option<&str>) -> Self {
if traceparent.is_some() || baggage.is_some() {
let parent_context = extract_context(traceparent, baggage);
let _ = self.set_parent(parent_context);
fn with_parent_headers(
self,
traceparent: Option<&str>,
tracestate: Option<&str>,
baggage: Option<&str>,
) -> Self {
if traceparent.is_some() || tracestate.is_some() || baggage.is_some() {
let parent_context = extract_context_with_state(traceparent, tracestate, baggage);
let tp_owned = traceparent.map(|s| s.to_string());
let parent_trace_id = parent_context.span().span_context().trace_id();
let parent_valid = parent_context.span().span_context().is_valid();
match self.set_parent(parent_context) {
Ok(()) => {
tracing::debug!(
traceparent = tp_owned.as_deref().unwrap_or("(none)"),
parent_trace_id = %parent_trace_id,
parent_context_valid = parent_valid,
"with_parent_headers: successfully set parent context"
);
// Record an event on this span so it shows up in OTel export
self.add_event(
"traceparent.propagated",
vec![
KeyValue::new("parent.trace_id", format!("{parent_trace_id}")),
KeyValue::new(
"traceparent",
tp_owned.as_deref().unwrap_or("(none)").to_string(),
),
],
);
}
Err(err) => {
tracing::warn!(
traceparent = tp_owned.as_deref().unwrap_or("(none)"),
parent_trace_id = %parent_trace_id,
error = %err,
"with_parent_headers: failed to set parent context"
);
self.add_event(
"traceparent.set_parent_failed",
vec![
KeyValue::new("error", err.to_string()),
KeyValue::new(
"traceparent",
tp_owned.as_deref().unwrap_or("(none)").to_string(),
),
],
);
}
}
}
self
}
Expand All @@ -53,7 +108,18 @@ mod tests {
let span = tracing::info_span!("telemetry-test");
let _span = span.with_parent_headers(
Some("00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01"),
None,
Some("user_id=123"),
);
}

#[test]
fn span_ext_accepts_w3c_tracestate_header() {
let span = tracing::info_span!("telemetry-test");
let _span = span.with_parent_headers(
Some("00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01"),
Some("congo=t61rcWkgMzE,rojo=00f067aa0ba902b7"),
None,
);
}
}
110 changes: 100 additions & 10 deletions engine/src/workers/observability/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -891,18 +891,25 @@ fn corrected_detail_spans(
}

fn build_span_tree(spans: Vec<otel::StoredSpan>) -> Vec<SpanTreeNode> {
// Span ids present in this set. A span whose parent is NOT present is a
// local trace root — covers traces entering iii from an external caller via
// an incoming `traceparent`, whose server span points at the remote caller's
// span (never stored here). Without this the whole subtree is orphaned and
// the trace detail view renders nothing.
let present_ids: std::collections::HashSet<String> =
spans.iter().map(|s| s.span_id.clone()).collect();
let mut children_map: HashMap<String, Vec<otel::StoredSpan>> = HashMap::new();
let mut roots: Vec<otel::StoredSpan> = Vec::new();

for span in spans {
match &span.parent_span_id {
Some(parent_id) => {
Some(parent_id) if present_ids.contains(parent_id) => {
children_map
.entry(parent_id.clone())
.or_default()
.push(span);
}
None => roots.push(span),
_ => roots.push(span),
}
}

Expand Down Expand Up @@ -1976,10 +1983,25 @@ impl ObservabilityWorker {
None
};

// A span is a trace root when it has no parent OR its parent is
// absent from the store. The latter covers traces that entered
// iii from an external caller via an incoming `traceparent`: the
// server span's parent is the remote caller's span, which lives
// in another service and is never stored here. Without this,
// root-only listing hides every distributed trace. Mirrors
// `build_span_tree`'s dangling-parent handling.
let present_span_ids: std::collections::HashSet<String> =
all_spans.iter().map(|s| s.span_id.clone()).collect();

let mut filtered: Vec<_> = all_spans
.into_iter()
// Root-only by default; `search_all_spans` widens to children too.
.filter(|s| search_all || s.parent_span_id.is_none())
.filter(|s| {
search_all
|| s.parent_span_id
.as_ref()
.is_none_or(|p| !present_span_ids.contains(p))
})
.filter(|s| {
// Exclude internal engine traces unless explicitly requested
if !include_internal {
Expand Down Expand Up @@ -3583,6 +3605,7 @@ mod tests {
instrumentation_scope_name: None,
instrumentation_scope_version: None,
flags: None,
trace_state: None,
}
}

Expand Down Expand Up @@ -5294,11 +5317,10 @@ mod tests {
// =========================================================================

#[test]
fn test_build_span_tree_orphan_child() {
// A child whose parent is not in the span list becomes an orphan
// The current implementation only puts spans with None parent in roots.
// Orphan children (with parent_span_id set but parent not in list) are
// not treated as roots; they go into children_map but never get collected.
fn test_build_span_tree_orphan_child_becomes_root() {
// A span whose parent_span_id is set but absent from the span list is a
// local trace root (e.g. the server span of a trace that entered iii via
// an incoming `traceparent`, whose parent lives in the remote caller).
let orphan = make_span(
"t1",
"s2",
Expand All @@ -5313,8 +5335,8 @@ mod tests {

let tree = build_span_tree(vec![orphan]);

// Since the span has a parent_span_id, it won't appear as a root
assert!(tree.is_empty());
assert_eq!(tree.len(), 1, "orphan with missing parent must be a root");
assert_eq!(tree[0].span.name, "orphan");
}

// =========================================================================
Expand Down Expand Up @@ -6281,6 +6303,74 @@ mod tests {
}
}

#[tokio::test]
#[serial]
async fn test_list_traces_treats_dangling_remote_parent_as_root() {
reset_observability_test_state();

let module = make_test_module(Arc::new(Engine::new()));
let span_storage = otel::get_span_storage().expect("span storage should exist");
span_storage.clear();

// A trace that entered iii from an external caller: the server span's
// parent is the remote caller's span id, which is never stored here.
// Its child (parent present in the store) is NOT a root.
span_storage.add_spans(vec![
make_span(
"t-remote",
"s-http",
Some("remoteparent0001"),
"POST /x",
"iii-engine",
1_000_000_000,
1_100_000_000,
"OK",
vec![],
),
make_span(
"t-remote",
"s-child",
Some("s-http"),
"execute fn",
"worker",
1_010_000_000,
1_090_000_000,
"OK",
vec![],
),
]);

let input = TracesListInput {
trace_id: None,
offset: Some(0),
limit: Some(10),
service_name: None,
name: None,
status: None,
min_duration_ms: None,
max_duration_ms: None,
start_time: None,
end_time: None,
sort_by: None,
sort_order: None,
attributes: None,
include_internal: Some(false),
search_all_spans: None,
};

let spans = match module.list_traces(input).await {
FunctionResult::Success(v) => serde_json::to_value(&v).unwrap()["spans"]
.as_array()
.expect("spans array")
.clone(),
_ => panic!("expected list_traces success"),
};

// Only the dangling-parent server span surfaces as a root.
assert_eq!(spans.len(), 1, "dangling-parent span must be a root");
assert_eq!(spans[0]["name"].as_str().unwrap(), "POST /x");
}

#[tokio::test]
#[serial]
async fn test_list_traces_sort_by_duration_ms_and_service_name() {
Expand Down
Loading
Loading