fix(span): skip zero parent span id from empty traceparent (#23)#25
fix(span): skip zero parent span id from empty traceparent (#23)#25
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes span parent linkage when a traceparent is “initialized” but has an all-zero SpanID (notably with --tp-ignore-env), preventing emission of a zero-valued parent_span_id.
Changes:
- Prevent setting
ParentSpanIdwhen the loadedtraceparentSpanID is empty/all-zeros. - Add a functional fixture verifying
--tp-ignore-envresults in an emptyparent_span_id.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| otelcli/config_span.go | Adds a guard to avoid assigning an all-zero parent span id from traceparent. |
| data_for_test.go | Adds a functional test/fixture to validate empty parent_span_id behavior with --tp-ignore-env. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if tp.Initialized { | ||
| span.TraceId = tp.TraceId | ||
| span.ParentSpanId = tp.SpanId | ||
| // only set parent span id when the traceparent has a real (non-zero) span id (#23) | ||
| if !bytes.Equal(tp.SpanId, otlpclient.GetEmptySpanId()) { |
There was a problem hiding this comment.
Minor readability improvement: consider storing otlpclient.GetEmptySpanId() in a local variable (e.g., emptySpanID := ...) before the if, so the comparison reads more clearly and avoids repeating a function call in the condition.
| if tp.Initialized { | |
| span.TraceId = tp.TraceId | |
| span.ParentSpanId = tp.SpanId | |
| // only set parent span id when the traceparent has a real (non-zero) span id (#23) | |
| if !bytes.Equal(tp.SpanId, otlpclient.GetEmptySpanId()) { | |
| emptySpanID := otlpclient.GetEmptySpanId() | |
| if tp.Initialized { | |
| span.TraceId = tp.TraceId | |
| // only set parent span id when the traceparent has a real (non-zero) span id (#23) | |
| if !bytes.Equal(tp.SpanId, emptySpanID) { |
| // #23: --tp-ignore-env should not produce a zero-valued parent span id | ||
| { | ||
| { | ||
| Name: "#23 --tp-ignore-env should not send zero parent span id", |
There was a problem hiding this comment.
The comment says “zero-valued parent span id”, but the assertion checks for an empty string (i.e., omitted/empty parent_span_id), not the all-zeros hex representation. Consider rewording the comment and/or test name to explicitly say the parent span id should be empty/omitted (and not 0000000000000000).
| // #23: --tp-ignore-env should not produce a zero-valued parent span id | |
| { | |
| { | |
| Name: "#23 --tp-ignore-env should not send zero parent span id", | |
| // #23: --tp-ignore-env should result in an empty/omitted parent span id (not 0000000000000000) | |
| { | |
| { | |
| Name: "#23 --tp-ignore-env should omit parent span id (empty/omitted, not 0000000000000000)", |
| SpanData: map[string]string{ | ||
| "trace_id": "*", | ||
| "span_id": "*", | ||
| "parent_span_id": "", | ||
| }, |
There was a problem hiding this comment.
The comment says “zero-valued parent span id”, but the assertion checks for an empty string (i.e., omitted/empty parent_span_id), not the all-zeros hex representation. Consider rewording the comment and/or test name to explicitly say the parent span id should be empty/omitted (and not 0000000000000000).
When --tp-ignore-env is set, LoadTraceparent returns a traceparent with Initialized=true but zeroed SpanId. The code unconditionally copied this into ParentSpanId, producing a span with an all-zeros parent that looks like it has a parent when it doesn't. Now checks bytes.Equal against GetEmptySpanId() before setting parent. Fixes #23 🤖 Claude <claude@anthropic.com>
e201890 to
7e73320
Compare
Summary
--tp-ignore-envis set,LoadTraceparent()returns a traceparent withInitialized=truebut zeroed SpanIdParentSpanId = tp.SpanId, producing a span with an all-zeros parentbytes.EqualagainstGetEmptySpanId()before setting parent span id--tp-ignore-envproduces empty parent span idTest plan
data_for_test.go:--tp-ignore-envwith TRACEPARENT set, verify parent_span_id is emptygo test && go test ./...)Fixes #23
🤖 Generated with Claude Code