From e75494d29fb9554f71f6746249a7a6e013fe2b82 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Sun, 25 Feb 2024 17:46:35 -0800 Subject: [PATCH 1/3] Fix span kind being always internal --- 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..dc37ab1a80 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -1,8 +1,12 @@ # Changelog - ## vNext +### Fixed + +- [#10000](https://github.com/open-telemetry/opentelemetry-rust/pull/1000) + 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..7e12b01eb3 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_eq!(span.is_recording(), true); + 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_eq!(span.span_context.is_remote(), false); + 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_eq!(span.span_context.is_remote(), false); + 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_eq!(span.span_context.is_remote(), false); + 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(&[]), ) From 95aef04ed92870bd1104f770ddec2b81ad575607 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Sun, 25 Feb 2024 17:51:05 -0800 Subject: [PATCH 2/3] PR number real --- opentelemetry-sdk/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index dc37ab1a80..53de7614bc 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- [#10000](https://github.com/open-telemetry/opentelemetry-rust/pull/1000) +- [#1576](https://github.com/open-telemetry/opentelemetry-rust/pull/1576) Fix Span kind is always set to "internal". ## v0.22.0 From c26a807072b84b3d1072577b73a5a082d655fec1 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Sun, 25 Feb 2024 18:06:43 -0800 Subject: [PATCH 3/3] clippy fix --- opentelemetry-sdk/src/trace/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/trace/mod.rs b/opentelemetry-sdk/src/trace/mod.rs index 7e12b01eb3..9a1b1b9157 100644 --- a/opentelemetry-sdk/src/trace/mod.rs +++ b/opentelemetry-sdk/src/trace/mod.rs @@ -74,7 +74,7 @@ mod tests { let tracer = provider.tracer("test_tracer"); tracer.in_span("span_name", |cx| { let span = cx.span(); - assert_eq!(span.is_recording(), true); + 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![]); @@ -94,7 +94,7 @@ mod tests { assert_eq!(span.events.len(), 1); assert_eq!(span.events[0].name, "test-event"); assert_eq!(span.span_context.trace_flags(), TraceFlags::SAMPLED); - assert_eq!(span.span_context.is_remote(), false); + assert!(!span.span_context.is_remote()); assert_eq!(span.status, Status::Unset); } @@ -127,7 +127,7 @@ mod tests { assert_eq!(span.events.len(), 1); assert_eq!(span.events[0].name, "test-event"); assert_eq!(span.span_context.trace_flags(), TraceFlags::SAMPLED); - assert_eq!(span.span_context.is_remote(), false); + assert!(!span.span_context.is_remote()); let status_expected = Status::error("cancelled"); assert_eq!(span.status, status_expected); } @@ -165,7 +165,7 @@ mod tests { assert_eq!(span.events.len(), 1); assert_eq!(span.events[0].name, "test-event"); assert_eq!(span.span_context.trace_flags(), TraceFlags::SAMPLED); - assert_eq!(span.span_context.is_remote(), false); + assert!(!span.span_context.is_remote()); assert_eq!(span.status, Status::Ok); }