Skip to content

Commit 74dec5b

Browse files
nikhilmantri0902nikhilmantri0902
committed
refactor: replace parent_span_context with parent_span_is_remote boolean
- Replace Option<SpanContext> with bool for parent_span_is_remote field - Eliminates unnecessary cloning of SpanContext objects - More efficient memory usage and cleaner API - Updated build_span_flags function to accept boolean parameter - Updated all test files to use new boolean field - Maintains same functionality with better performance Addresses reviewer feedback for more efficient implementation.
1 parent 6222815 commit 74dec5b

File tree

10 files changed

+28
-58
lines changed

10 files changed

+28
-58
lines changed

‎opentelemetry-otlp/src/exporter/http/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1047,7 +1047,7 @@ mod tests {
10471047
SpanData {
10481048
span_context,
10491049
parent_span_id: SpanId::from(0),
1050-
parent_span_context: None,
1050+
parent_span_is_remote: false,
10511051
span_kind: SpanKind::Internal,
10521052
name: Cow::Borrowed("test_span"),
10531053
start_time: SystemTime::UNIX_EPOCH,

‎opentelemetry-proto/src/transform/trace.rs‎

Lines changed: 14 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,15 @@
11
#[cfg(feature = "gen-tonic-messages")]
2-
/// Builds span flags based on the parent span context's remote property.
2+
/// Builds span flags based on the parent span's remote property.
33
/// This follows the OTLP specification for span flags.
44
pub fn build_span_flags(
5-
parent_span_context: Option<&opentelemetry::trace::SpanContext>,
5+
parent_span_is_remote: bool,
66
base_flags: u32,
77
) -> u32 {
88
use crate::proto::tonic::trace::v1::SpanFlags;
99
let mut flags = base_flags;
1010
flags |= SpanFlags::ContextHasIsRemoteMask as u32;
11-
if let Some(parent_ctx) = parent_span_context {
12-
if parent_ctx.is_remote() {
13-
flags |= SpanFlags::ContextIsRemoteMask as u32;
14-
}
11+
if parent_span_is_remote {
12+
flags |= SpanFlags::ContextIsRemoteMask as u32;
1513
}
1614
flags
1715
}
@@ -60,7 +58,7 @@ pub mod tonic {
6058
attributes: Attributes::from(link.attributes).0,
6159
dropped_attributes_count: link.dropped_attributes_count,
6260
flags: super::build_span_flags(
63-
Some(&link.span_context),
61+
link.span_context.is_remote(),
6462
link.span_context.trace_flags().to_u8() as u32,
6563
),
6664
}
@@ -81,7 +79,7 @@ pub mod tonic {
8179
}
8280
},
8381
flags: super::build_span_flags(
84-
source_span.parent_span_context.as_ref(),
82+
source_span.parent_span_is_remote,
8583
source_span.span_context.trace_flags().to_u8() as u32,
8684
),
8785
name: source_span.name.into_owned(),
@@ -143,7 +141,7 @@ pub mod tonic {
143141
}
144142
},
145143
flags: super::build_span_flags(
146-
source_span.parent_span_context.as_ref(),
144+
source_span.parent_span_is_remote,
147145
source_span.span_context.trace_flags().to_u8() as u32,
148146
),
149147
name: source_span.name.into_owned(),
@@ -228,29 +226,13 @@ mod span_flags_tests {
228226

229227
#[test]
230228
fn test_build_span_flags_local_parent() {
231-
let local_parent = SpanContext::new(
232-
TraceId::from(123),
233-
SpanId::from(456),
234-
TraceFlags::default(),
235-
false, // is_remote = false
236-
TraceState::default(),
237-
);
238-
239-
let flags = super::build_span_flags(Some(&local_parent), 0);
229+
let flags = super::build_span_flags(false, 0); // is_remote = false
240230
assert_eq!(flags, SpanFlags::ContextHasIsRemoteMask as u32); // 0x100
241231
}
242232

243233
#[test]
244234
fn test_build_span_flags_remote_parent() {
245-
let remote_parent = SpanContext::new(
246-
TraceId::from(123),
247-
SpanId::from(456),
248-
TraceFlags::default(),
249-
true, // is_remote = true
250-
TraceState::default(),
251-
);
252-
253-
let flags = super::build_span_flags(Some(&remote_parent), 0);
235+
let flags = super::build_span_flags(true, 0); // is_remote = true
254236
assert_eq!(
255237
flags,
256238
(SpanFlags::ContextHasIsRemoteMask as u32) | (SpanFlags::ContextIsRemoteMask as u32)
@@ -259,21 +241,13 @@ mod span_flags_tests {
259241

260242
#[test]
261243
fn test_build_span_flags_no_parent() {
262-
let flags = super::build_span_flags(None, 0);
244+
let flags = super::build_span_flags(false, 0); // no parent = false
263245
assert_eq!(flags, SpanFlags::ContextHasIsRemoteMask as u32); // 0x100
264246
}
265247

266248
#[test]
267249
fn test_build_span_flags_preserves_base_flags() {
268-
let local_parent = SpanContext::new(
269-
TraceId::from(123),
270-
SpanId::from(456),
271-
TraceFlags::default(),
272-
false,
273-
TraceState::default(),
274-
);
275-
276-
let flags = super::build_span_flags(Some(&local_parent), 0x01); // SAMPLED flag
250+
let flags = super::build_span_flags(false, 0x01); // SAMPLED flag, local parent
277251
assert_eq!(flags, 0x01 | (SpanFlags::ContextHasIsRemoteMask as u32)); // 0x101
278252
}
279253

@@ -296,7 +270,7 @@ mod span_flags_tests {
296270
TraceState::default(),
297271
),
298272
parent_span_id: SpanId::from(456),
299-
parent_span_context: Some(local_parent),
273+
parent_span_is_remote: false,
300274
span_kind: opentelemetry::trace::SpanKind::Internal,
301275
name: Cow::Borrowed("test_span"),
302276
start_time: std::time::SystemTime::now(),
@@ -332,7 +306,7 @@ mod span_flags_tests {
332306
TraceState::default(),
333307
),
334308
parent_span_id: SpanId::from(456),
335-
parent_span_context: Some(remote_parent),
309+
parent_span_is_remote: true,
336310
span_kind: opentelemetry::trace::SpanKind::Internal,
337311
name: Cow::Borrowed("test_span"),
338312
start_time: std::time::SystemTime::now(),
@@ -381,7 +355,7 @@ mod tests {
381355
SpanData {
382356
span_context,
383357
parent_span_id: SpanId::from(0),
384-
parent_span_context: None,
358+
parent_span_is_remote: false,
385359
span_kind: SpanKind::Internal,
386360
name: Cow::Borrowed("test_span"),
387361
start_time: now(),

‎opentelemetry-sdk/CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
## vNext
44

55
- **Feature**: Add span flags support for `isRemote` property in OTLP exporter ([#3153](https://github.com/open-telemetry/opentelemetry-rust/pull/3153))
6-
- Added `parent_span_context` field to `SpanData` to store parent span context information
6+
- Added `parent_span_is_remote` field to `SpanData` to store parent span remote flag
77
- Implemented `build_span_flags` function that sets OTLP span flags based on parent span's `isRemote` property
88
- Updated span and link transformations to properly set flags field (0x100 for local, 0x300 for remote)
99
- Added comprehensive tests for span flags functionality

‎opentelemetry-sdk/benches/batch_span_processor.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ fn get_span_data() -> Vec<SpanData> {
2222
TraceState::default(),
2323
),
2424
parent_span_id: SpanId::from(12),
25-
parent_span_context: None,
25+
parent_span_is_remote: false,
2626
span_kind: SpanKind::Client,
2727
name: Default::default(),
2828
start_time: now(),

‎opentelemetry-sdk/src/testing/trace/span_exporters.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ pub fn new_test_export_span_data() -> SpanData {
2121
TraceState::default(),
2222
),
2323
parent_span_id: SpanId::INVALID,
24-
parent_span_context: None,
24+
parent_span_is_remote: false,
2525
span_kind: SpanKind::Internal,
2626
name: "opentelemetry".into(),
2727
start_time: opentelemetry::time::now(),

‎opentelemetry-sdk/src/trace/export.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,8 @@ pub struct SpanData {
8282
pub span_context: SpanContext,
8383
/// Span parent id
8484
pub parent_span_id: SpanId,
85-
/// Parent span context (for span flags)
86-
pub parent_span_context: Option<SpanContext>,
85+
/// Parent span is remote flag (for span flags)
86+
pub parent_span_is_remote: bool,
8787
/// Span kind
8888
pub span_kind: SpanKind,
8989
/// Span name

‎opentelemetry-sdk/src/trace/span.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ pub struct Span {
2727
pub(crate) struct SpanData {
2828
/// Span parent id
2929
pub(crate) parent_span_id: SpanId,
30-
/// Parent span context (for span flags)
31-
pub(crate) parent_span_context: Option<SpanContext>,
30+
/// Parent span is remote flag (for span flags)
31+
pub(crate) parent_span_is_remote: bool,
3232
/// Span kind
3333
pub(crate) span_kind: SpanKind,
3434
/// Span name
@@ -256,7 +256,7 @@ fn build_export_data(
256256
crate::trace::SpanData {
257257
span_context,
258258
parent_span_id: data.parent_span_id,
259-
parent_span_context: data.parent_span_context,
259+
parent_span_is_remote: data.parent_span_is_remote,
260260
span_kind: data.span_kind,
261261
name: data.name,
262262
start_time: data.start_time,
@@ -289,7 +289,7 @@ mod tests {
289289
let tracer = provider.tracer("opentelemetry");
290290
let data = SpanData {
291291
parent_span_id: SpanId::from(0),
292-
parent_span_context: None,
292+
parent_span_is_remote: false,
293293
span_kind: trace::SpanKind::Internal,
294294
name: "opentelemetry".into(),
295295
start_time: opentelemetry::time::now(),

‎opentelemetry-sdk/src/trace/span_processor.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -968,7 +968,7 @@ mod tests {
968968
let unsampled = SpanData {
969969
span_context: SpanContext::empty_context(),
970970
parent_span_id: SpanId::INVALID,
971-
parent_span_context: None,
971+
parent_span_is_remote: false,
972972
span_kind: SpanKind::Internal,
973973
name: "opentelemetry".into(),
974974
start_time: opentelemetry::time::now(),
@@ -1130,7 +1130,7 @@ mod tests {
11301130
SpanData {
11311131
span_context: SpanContext::empty_context(),
11321132
parent_span_id: SpanId::INVALID,
1133-
parent_span_context: None,
1133+
parent_span_is_remote: false,
11341134
span_kind: SpanKind::Internal,
11351135
name: name.to_string().into(),
11361136
start_time: opentelemetry::time::now(),

‎opentelemetry-sdk/src/trace/tracer.rs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,7 @@ impl SdkTracer {
137137
sc,
138138
Some(SpanData {
139139
parent_span_id: psc.span_id(),
140-
parent_span_context: if psc.is_valid() {
141-
Some(psc.clone())
142-
} else {
143-
None
144-
},
140+
parent_span_is_remote: psc.is_valid() && psc.is_remote(),
145141
span_kind: builder.span_kind.take().unwrap_or(SpanKind::Internal),
146142
name,
147143
start_time,

‎opentelemetry-zipkin/src/exporter/model/span.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ mod tests {
154154
Default::default(),
155155
),
156156
parent_span_id: SpanId::from(1),
157-
parent_span_context: None,
157+
parent_span_is_remote: false,
158158
span_kind: SpanKind::Client,
159159
name: "".into(),
160160
start_time: now(),

0 commit comments

Comments
 (0)