Skip to content

Commit

Permalink
Revert "RFC: add SpanContext.LocalRootSpanID (census-instrumentation#…
Browse files Browse the repository at this point in the history
…1029)"

This reverts commit 57c0993.

It was merged without updating the spec and resolving
census-instrumentation/opencensus-specs#229
  • Loading branch information
rghetia committed Feb 20, 2019
1 parent 57c0993 commit 3aefc67
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 50 deletions.
9 changes: 4 additions & 5 deletions trace/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,10 @@ func UnregisterExporter(e Exporter) {
// SpanData contains all the information collected by a Span.
type SpanData struct {
SpanContext
ParentSpanID SpanID
LocalRootSpanID SpanID
SpanKind int
Name string
StartTime time.Time
ParentSpanID SpanID
SpanKind int
Name string
StartTime time.Time
// The wall clock time of EndTime will be adjusted to always be offset
// from StartTime by the duration of the span.
EndTime time.Time
Expand Down
21 changes: 6 additions & 15 deletions trace/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,9 @@ type Span struct {
// It will be non-nil if we are exporting the span or recording events for it.
// Otherwise, data is nil, and the Span is simply a carrier for the
// SpanContext, so that the trace ID is propagated.
data *SpanData
mu sync.Mutex // protects the contents of *data (but not the pointer value.)
spanContext SpanContext
localRootSpanID SpanID
data *SpanData
mu sync.Mutex // protects the contents of *data (but not the pointer value.)
spanContext SpanContext

// lruAttributes are capped at configured limit. When the capacity is reached an oldest entry
// is removed to create room for a new entry.
Expand Down Expand Up @@ -170,16 +169,14 @@ func WithSampler(sampler Sampler) StartOption {
func StartSpan(ctx context.Context, name string, o ...StartOption) (context.Context, *Span) {
var opts StartOptions
var parent SpanContext
var localRoot SpanID
if p := FromContext(ctx); p != nil {
p.addChild()
parent = p.spanContext
localRoot = p.localRootSpanID
}
for _, op := range o {
op(&opts)
}
span := startSpanInternal(name, parent != SpanContext{}, parent, localRoot, false, opts)
span := startSpanInternal(name, parent != SpanContext{}, parent, false, opts)

ctx, end := startExecutionTracerTask(ctx, name)
span.executionTracerTaskEnd = end
Expand All @@ -198,16 +195,15 @@ func StartSpanWithRemoteParent(ctx context.Context, name string, parent SpanCont
for _, op := range o {
op(&opts)
}
span := startSpanInternal(name, parent != SpanContext{}, parent, SpanID{}, true, opts)
span := startSpanInternal(name, parent != SpanContext{}, parent, true, opts)
ctx, end := startExecutionTracerTask(ctx, name)
span.executionTracerTaskEnd = end
return NewContext(ctx, span), span
}

func startSpanInternal(name string, hasParent bool, parent SpanContext, localRoot SpanID, remoteParent bool, o StartOptions) *Span {
func startSpanInternal(name string, hasParent bool, parent SpanContext, remoteParent bool, o StartOptions) *Span {
span := &Span{}
span.spanContext = parent
span.localRootSpanID = localRoot

cfg := config.Load().(*Config)

Expand All @@ -217,10 +213,6 @@ func startSpanInternal(name string, hasParent bool, parent SpanContext, localRoo
span.spanContext.SpanID = cfg.IDGenerator.NewSpanID()
sampler := cfg.DefaultSampler

if localRoot == (SpanID{}) {
span.localRootSpanID = span.spanContext.SpanID
}

if !hasParent || remoteParent || o.Sampler != nil {
// If this span is the child of a local span and no Sampler is set in the
// options, keep the parent's TraceOptions.
Expand All @@ -244,7 +236,6 @@ func startSpanInternal(name string, hasParent bool, parent SpanContext, localRoo

span.data = &SpanData{
SpanContext: span.spanContext,
LocalRootSpanID: span.localRootSpanID,
StartTime: time.Now(),
SpanKind: o.SpanKind,
Name: name,
Expand Down
30 changes: 0 additions & 30 deletions trace/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,32 +230,6 @@ func TestStartSpanWithRemoteParent(t *testing.T) {
}
}

func TestLocalRootSpanID(t *testing.T) {
ctx, span1 := StartSpan(context.Background(), "span1")
if span1.localRootSpanID == (SpanID{}) {
t.Errorf("exporting root span: expected nonzero localRootSpanID")
}

_, span2 := StartSpan(ctx, "span2")
if err := checkChild(span1.spanContext, span2); err != nil {
t.Error(err)
}
if got, want := span2.localRootSpanID, span1.localRootSpanID; got != want {
t.Errorf("span2.localRootSpanID=%q; want %q (span1.localRootSpanID)", got, want)
}

_, span3 := StartSpanWithRemoteParent(context.Background(), "span3", span2.SpanContext())
if err := checkChild(span3.spanContext, span2); err != nil {
t.Error(err)
}
if span3.localRootSpanID == (SpanID{}) {
t.Errorf("exporting span with remote parent: expected nonzero localRootSpanID")
}
if got, want := span3.localRootSpanID, span2.localRootSpanID; got == want {
t.Errorf("span3.localRootSpanID=%q; expected different value to span2.localRootSpanID, got same", got)
}
}

// startSpan returns a context with a new Span that is recording events and will be exported.
func startSpan(o StartOptions) *Span {
_, span := StartSpanWithRemoteParent(context.Background(), "span0",
Expand Down Expand Up @@ -300,11 +274,7 @@ func endSpan(span *Span) (*SpanData, error) {
if got.SpanContext.SpanID == (SpanID{}) {
return nil, fmt.Errorf("exporting span: expected nonzero SpanID")
}
if got.LocalRootSpanID == (SpanID{}) {
return nil, fmt.Errorf("exporting span: expected nonzero LocalRootSpanID")
}
got.SpanContext.SpanID = SpanID{}
got.LocalRootSpanID = SpanID{}
if !checkTime(&got.StartTime) {
return nil, fmt.Errorf("exporting span: expected nonzero StartTime")
}
Expand Down

0 comments on commit 3aefc67

Please sign in to comment.