From 5542f37b465cfa7b341257763e5f6c1c16ccb896 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Mon, 26 Feb 2024 09:33:25 -0800 Subject: [PATCH] Fix span kind being always internal (#1576) --- opentelemetry-sdk/CHANGELOG.md | 6 ++- opentelemetry-sdk/src/trace/mod.rs | 68 +++++++++++++++++++++++++-- opentelemetry-sdk/src/trace/tracer.rs | 3 +- 3 files changed, 69 insertions(+), 8 deletions(-) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 1b50c5b8a0..53de7614bc 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -1,8 +1,12 @@ # Changelog - ## vNext +### Fixed + +- [#1576](https://github.com/open-telemetry/opentelemetry-rust/pull/1576) + Fix Span kind is always set to "internal". + ## v0.22.0 ### Deprecated diff --git a/opentelemetry-sdk/src/trace/mod.rs b/opentelemetry-sdk/src/trace/mod.rs index a49259c0a5..9a1b1b9157 100644 --- a/opentelemetry-sdk/src/trace/mod.rs +++ b/opentelemetry-sdk/src/trace/mod.rs @@ -52,7 +52,7 @@ mod tests { }; use opentelemetry::testing::trace::TestSpan; use opentelemetry::trace::{ - SamplingDecision, SamplingResult, SpanKind, TraceContextExt, TraceState, + SamplingDecision, SamplingResult, SpanKind, Status, TraceContextExt, TraceState, }; use opentelemetry::{ trace::{ @@ -63,7 +63,7 @@ mod tests { }; #[test] - fn in_span() { + fn tracer_in_span() { // Arrange let exporter = InMemorySpanExporterBuilder::new().build(); let provider = TracerProvider::builder() @@ -72,7 +72,13 @@ mod tests { // Act let tracer = provider.tracer("test_tracer"); - tracer.in_span("span_name", |_cx| {}); + tracer.in_span("span_name", |cx| { + let span = cx.span(); + assert!(span.is_recording()); + span.update_name("span_name_updated"); + span.set_attribute(KeyValue::new("attribute1", "value1")); + span.add_event("test-event".to_string(), vec![]); + }); provider.force_flush(); @@ -82,8 +88,14 @@ mod tests { .expect("Spans are expected to be exported."); assert_eq!(exported_spans.len(), 1); let span = &exported_spans[0]; - assert_eq!(span.name, "span_name"); + assert_eq!(span.name, "span_name_updated"); assert_eq!(span.instrumentation_lib.name, "test_tracer"); + assert_eq!(span.attributes.len(), 1); + assert_eq!(span.events.len(), 1); + assert_eq!(span.events[0].name, "test-event"); + assert_eq!(span.span_context.trace_flags(), TraceFlags::SAMPLED); + assert!(!span.span_context.is_remote()); + assert_eq!(span.status, Status::Unset); } #[test] @@ -97,7 +109,46 @@ mod tests { // Act let tracer = provider.tracer("test_tracer"); let mut span = tracer.start("span_name"); - span.set_attribute(KeyValue::new("key1", "value1")); + span.set_attribute(KeyValue::new("attribute1", "value1")); + span.add_event("test-event".to_string(), vec![]); + span.set_status(Status::error("cancelled")); + drop(span); + provider.force_flush(); + + // Assert + let exported_spans = exporter + .get_finished_spans() + .expect("Spans are expected to be exported."); + assert_eq!(exported_spans.len(), 1); + let span = &exported_spans[0]; + assert_eq!(span.name, "span_name"); + assert_eq!(span.instrumentation_lib.name, "test_tracer"); + assert_eq!(span.attributes.len(), 1); + assert_eq!(span.events.len(), 1); + assert_eq!(span.events[0].name, "test-event"); + assert_eq!(span.span_context.trace_flags(), TraceFlags::SAMPLED); + assert!(!span.span_context.is_remote()); + let status_expected = Status::error("cancelled"); + assert_eq!(span.status, status_expected); + } + + #[test] + fn tracer_span_builder() { + // Arrange + let exporter = InMemorySpanExporterBuilder::new().build(); + let provider = TracerProvider::builder() + .with_span_processor(SimpleSpanProcessor::new(Box::new(exporter.clone()))) + .build(); + + // Act + let tracer = provider.tracer("test_tracer"); + let mut span = tracer + .span_builder("span_name") + .with_kind(SpanKind::Server) + .start(&tracer); + span.set_attribute(KeyValue::new("attribute1", "value1")); + span.add_event("test-event".to_string(), vec![]); + span.set_status(Status::Ok); drop(span); provider.force_flush(); @@ -108,7 +159,14 @@ mod tests { assert_eq!(exported_spans.len(), 1); let span = &exported_spans[0]; assert_eq!(span.name, "span_name"); + assert_eq!(span.span_kind, SpanKind::Server); assert_eq!(span.instrumentation_lib.name, "test_tracer"); + assert_eq!(span.attributes.len(), 1); + assert_eq!(span.events.len(), 1); + assert_eq!(span.events[0].name, "test-event"); + assert_eq!(span.span_context.trace_flags(), TraceFlags::SAMPLED); + assert!(!span.span_context.is_remote()); + assert_eq!(span.status, Status::Ok); } #[test] diff --git a/opentelemetry-sdk/src/trace/tracer.rs b/opentelemetry-sdk/src/trace/tracer.rs index dcfe1e565d..2ca1af30c0 100644 --- a/opentelemetry-sdk/src/trace/tracer.rs +++ b/opentelemetry-sdk/src/trace/tracer.rs @@ -190,7 +190,6 @@ impl opentelemetry::trace::Tracer for Tracer { .span_id .take() .unwrap_or_else(|| config.id_generator.new_span_id()); - let span_kind = builder.span_kind.take().unwrap_or(SpanKind::Internal); let trace_id; let mut psc = &SpanContext::empty_context(); @@ -219,7 +218,7 @@ impl opentelemetry::trace::Tracer for Tracer { Some(parent_cx), trace_id, &builder.name, - &span_kind, + builder.span_kind.as_ref().unwrap_or(&SpanKind::Internal), builder.attributes.as_ref().unwrap_or(&Vec::new()), builder.links.as_deref().unwrap_or(&[]), )