Skip to content

Commit

Permalink
Do not add invalid context to references (#521)
Browse files Browse the repository at this point in the history
* Do not add invalid context to references

Signed-off-by: Yuri Shkuro <ys@uber.com>

* Rename helper

Signed-off-by: Yuri Shkuro <ys@uber.com>
  • Loading branch information
yurishkuro committed Jul 13, 2020
1 parent 704ce28 commit 59fdc96
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 9 deletions.
8 changes: 6 additions & 2 deletions span_context.go
Expand Up @@ -212,10 +212,14 @@ func (c SpanContext) SetFirehose() {
}

func (c SpanContext) String() string {
var flags int32
if c.samplingState != nil {
flags = c.samplingState.stateFlags.Load()
}
if c.traceID.High == 0 {
return fmt.Sprintf("%016x:%016x:%016x:%x", c.traceID.Low, uint64(c.spanID), uint64(c.parentID), c.samplingState.stateFlags.Load())
return fmt.Sprintf("%016x:%016x:%016x:%x", c.traceID.Low, uint64(c.spanID), uint64(c.parentID), flags)
}
return fmt.Sprintf("%016x%016x:%016x:%016x:%x", c.traceID.High, c.traceID.Low, uint64(c.spanID), uint64(c.parentID), c.samplingState.stateFlags.Load())
return fmt.Sprintf("%016x%016x:%016x:%016x:%x", c.traceID.High, c.traceID.Low, uint64(c.spanID), uint64(c.parentID), flags)
}

// ContextFromString reconstructs the Context encoded in a string
Expand Down
17 changes: 10 additions & 7 deletions tracer.go
Expand Up @@ -216,10 +216,10 @@ func (t *Tracer) startSpanWithOptions(
options.StartTime = t.timeNow()
}

// Predicate whether the given span context is a valid reference
// which may be used as parent / debug ID / baggage items source
isValidReference := func(ctx SpanContext) bool {
return ctx.IsValid() || ctx.isDebugIDContainerOnly() || len(ctx.baggage) != 0
// Predicate whether the given span context is an empty reference
// or may be used as parent / debug ID / baggage items source
isEmptyReference := func(ctx SpanContext) bool {
return !ctx.IsValid() && !ctx.isDebugIDContainerOnly() && len(ctx.baggage) == 0
}

var references []Reference
Expand All @@ -235,7 +235,7 @@ func (t *Tracer) startSpanWithOptions(
reflect.ValueOf(ref.ReferencedContext)))
continue
}
if !isValidReference(ctxRef) {
if isEmptyReference(ctxRef) {
continue
}

Expand All @@ -245,14 +245,17 @@ func (t *Tracer) startSpanWithOptions(
continue
}

references = append(references, Reference{Type: ref.Type, Context: ctxRef})
if ctxRef.IsValid() {
// we don't want empty context that contains only debug-id or baggage
references = append(references, Reference{Type: ref.Type, Context: ctxRef})
}

if !hasParent {
parent = ctxRef
hasParent = ref.Type == opentracing.ChildOfRef
}
}
if !hasParent && isValidReference(parent) {
if !hasParent && !isEmptyReference(parent) {
// If ChildOfRef wasn't found but a FollowFromRef exists, use the context from
// the FollowFromRef as the parent
hasParent = true
Expand Down
1 change: 1 addition & 0 deletions tracer_test.go
Expand Up @@ -414,6 +414,7 @@ func TestThrottling_DebugHeader(t *testing.T) {

sp := tracer.StartSpan("root", opentracing.ChildOf(ctx)).(*Span)
assert.True(t, sp.context.IsDebug())
assert.Len(t, sp.References(), 0)
closer.Close()

tracer, closer = NewTracer("DOOP", NewConstSampler(true), NewNullReporter(),
Expand Down

0 comments on commit 59fdc96

Please sign in to comment.