Replace span relationship with a potentially remote parent context (#451

)

This PR removes the non-compliant ChildOf and FollowsFrom interfaces
and the Relation type, which were inherited from OpenTracing via the
initial prototype. Instead allow adding a span context to the go
context as a remote span context and use a simple algorithm for
figuring out an actual parent of the new span, which was proposed for
the OpenTelemetry specification.

Also add a way to ignore current span and remote span context in go
context, so we can force the tracer to create a new root span - a span
with a new trace ID.

That required some moderate changes in the opentracing bridge - first
reference with ChildOfRef reference type becomes a local parent, the
rest become links. This also fixes links handling in the meantime. The
downside of the approach proposed here is that we can only set the
remote parent when creating a span through the opentracing API.

Co-authored-by: Joshua MacDonald <[email protected]>

api/testharness/harness.go

@@ -125,37 +125,55 @@ func (h *Harness) TestTracer(subjectFactory func() trace.Tracer) {
 			e.Expect(csc.SpanID).NotToEqual(psc.SpanID)
 		})
 
-		t.Run("propagates a parent's trace ID through `ChildOf`", func(t *testing.T) {
+		t.Run("ignores parent's trace ID when new root is requested", func(t *testing.T) {
 			t.Parallel()
 
 			e := matchers.NewExpecter(t)
 			subject := subjectFactory()
 
-			_, parent := subject.Start(context.Background(), "parent")
-			_, child := subject.Start(context.Background(), "child", trace.ChildOf(parent.SpanContext()))
+			ctx, parent := subject.Start(context.Background(), "parent")
+			_, child := subject.Start(ctx, "child", trace.WithNewRoot())
 
 			psc := parent.SpanContext()
 			csc := child.SpanContext()
 
-			e.Expect(csc.TraceID).ToEqual(psc.TraceID)
+			e.Expect(csc.TraceID).NotToEqual(psc.TraceID)
 			e.Expect(csc.SpanID).NotToEqual(psc.SpanID)
 		})
 
-		t.Run("propagates a parent's trace ID through `FollowsFrom`", func(t *testing.T) {
+		t.Run("propagates remote parent's trace ID through the context", func(t *testing.T) {
 			t.Parallel()
 
 			e := matchers.NewExpecter(t)
 			subject := subjectFactory()
 
-			_, parent := subject.Start(context.Background(), "parent")
-			_, child := subject.Start(context.Background(), "child", trace.FollowsFrom(parent.SpanContext()))
+			_, remoteParent := subject.Start(context.Background(), "remote parent")
+			parentCtx := trace.ContextWithRemoteSpanContext(context.Background(), remoteParent.SpanContext())
+			_, child := subject.Start(parentCtx, "child")
 
-			psc := parent.SpanContext()
+			psc := remoteParent.SpanContext()
 			csc := child.SpanContext()
 
 			e.Expect(csc.TraceID).ToEqual(psc.TraceID)
 			e.Expect(csc.SpanID).NotToEqual(psc.SpanID)
 		})
+
+		t.Run("ignores remote parent's trace ID when new root is requested", func(t *testing.T) {
+			t.Parallel()
+
+			e := matchers.NewExpecter(t)
+			subject := subjectFactory()
+
+			_, remoteParent := subject.Start(context.Background(), "remote parent")
+			parentCtx := trace.ContextWithRemoteSpanContext(context.Background(), remoteParent.SpanContext())
+			_, child := subject.Start(parentCtx, "child", trace.WithNewRoot())
+
+			psc := remoteParent.SpanContext()
+			csc := child.SpanContext()
+
+			e.Expect(csc.TraceID).NotToEqual(psc.TraceID)
+			e.Expect(csc.SpanID).NotToEqual(psc.SpanID)
+		})
 	})
 
 	h.t.Run("#WithSpan", func(t *testing.T) {

api/trace/api.go

@@ -100,26 +100,11 @@ type StartConfig struct {
 	Attributes []core.KeyValue
 	StartTime  time.Time
 	Links      []Link
-	Relation   Relation
 	Record     bool
+	NewRoot    bool
 	SpanKind   SpanKind
 }
 
-// Relation is used to establish relationship between newly created span and the
-// other span. The other span could be related as a parent or linked or any other
-// future relationship type.
-type Relation struct {
-	core.SpanContext
-	RelationshipType
-}
-
-type RelationshipType int
-
-const (
-	ChildOfRelationship RelationshipType = iota
-	FollowsFromRelationship
-)
-
 // Link is used to establish relationship between two spans within the same Trace or
 // across different Traces. Few examples of Link usage.
 //   1. Batch Processing: A batch of elements may contain elements associated with one
@@ -216,23 +201,14 @@ func WithRecord() StartOption {
 	}
 }
 
-// ChildOf. TODO: do we need this?.
-func ChildOf(sc core.SpanContext) StartOption {
-	return func(c *StartConfig) {
-		c.Relation = Relation{
-			SpanContext:      sc,
-			RelationshipType: ChildOfRelationship,
-		}
-	}
-}
-
-// FollowsFrom. TODO: do we need this?.
-func FollowsFrom(sc core.SpanContext) StartOption {
+// WithNewRoot specifies that the current span or remote span context
+// in context passed to `Start` should be ignored when deciding about
+// a parent, which effectively means creating a span with new trace
+// ID. The current span and the remote span context may be added as
+// links to the span by the implementation.
+func WithNewRoot() StartOption {
 	return func(c *StartConfig) {
-		c.Relation = Relation{
-			SpanContext:      sc,
-			RelationshipType: FollowsFromRelationship,
-		}
+		c.NewRoot = true
 	}
 }
 

api/trace/current.go → api/trace/context.go

@@ -16,19 +16,42 @@ package trace
 
 import (
 	"context"
+
+	"go.opentelemetry.io/otel/api/core"
 )
 
-type currentSpanKeyType struct{}
+type traceContextKeyType int
 
-var currentSpanKey = &currentSpanKeyType{}
+const (
+	currentSpanKey traceContextKeyType = iota
+	remoteContextKey
+)
 
+// ContextWithSpan creates a new context with a current span set to
+// the passed span.
 func ContextWithSpan(ctx context.Context, span Span) context.Context {
 	return context.WithValue(ctx, currentSpanKey, span)
 }
 
+// SpanFromContext returns the current span stored in the context.
 func SpanFromContext(ctx context.Context) Span {
 	if span, has := ctx.Value(currentSpanKey).(Span); has {
 		return span
 	}
 	return NoopSpan{}
 }
+
+// ContextWithRemoteSpanContext creates a new context with a remote
+// span context set to the passed span context.
+func ContextWithRemoteSpanContext(ctx context.Context, sc core.SpanContext) context.Context {
+	return context.WithValue(ctx, remoteContextKey, sc)
+}
+
+// RemoteSpanContextFromContext returns the remote span context stored
+// in the context.
+func RemoteSpanContextFromContext(ctx context.Context) core.SpanContext {
+	if sc, ok := ctx.Value(remoteContextKey).(core.SpanContext); ok {
+		return sc
+	}
+	return core.EmptySpanContext()
+}

api/trace/testtrace/b3_propagator_benchmark_test.go

@@ -102,10 +102,9 @@ func BenchmarkInjectB3(b *testing.B) {
 				req, _ := http.NewRequest("GET", "http://example.com", nil)
 				ctx := context.Background()
 				if tt.parentSc.IsValid() {
-					ctx, _ = mockTracer.Start(ctx, "inject", trace.ChildOf(tt.parentSc))
-				} else {
-					ctx, _ = mockTracer.Start(ctx, "inject")
+					ctx = trace.ContextWithRemoteSpanContext(ctx, tt.parentSc)
 				}
+				ctx, _ = mockTracer.Start(ctx, "inject")
 				b.ReportAllocs()
 				b.ResetTimer()
 				for i := 0; i < b.N; i++ {

api/trace/testtrace/b3_propagator_test.go

@@ -104,10 +104,9 @@ func TestInjectB3(t *testing.T) {
 				req, _ := http.NewRequest("GET", "http://example.com", nil)
 				ctx := context.Background()
 				if tt.parentSc.IsValid() {
-					ctx, _ = mockTracer.Start(ctx, "inject", trace.ChildOf(tt.parentSc))
-				} else {
-					ctx, _ = mockTracer.Start(ctx, "inject")
+					ctx = trace.ContextWithRemoteSpanContext(ctx, tt.parentSc)
 				}
+				ctx, _ = mockTracer.Start(ctx, "inject")
 				propagator.Inject(ctx, req.Header)
 
 				for h, v := range tt.wantHeaders {

api/trace/testtrace/trace_context_propagator_benchmark_test.go

@@ -38,8 +38,8 @@ func injectSubBenchmarks(b *testing.B, fn func(context.Context, *testing.B)) {
 			SpanID:     spanID,
 			TraceFlags: core.TraceFlagsSampled,
 		}
-		ctx := context.Background()
-		ctx, _ = mockTracer.Start(ctx, "inject", trace.ChildOf(sc))
+		ctx := trace.ContextWithRemoteSpanContext(context.Background(), sc)
+		ctx, _ = mockTracer.Start(ctx, "inject")
 		fn(ctx, b)
 	})
 

api/trace/testtrace/trace_context_propagator_test.go

@@ -273,7 +273,8 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) {
 			req, _ := http.NewRequest("GET", "http://example.com", nil)
 			ctx := context.Background()
 			if tt.sc.IsValid() {
-				ctx, _ = mockTracer.Start(ctx, "inject", trace.ChildOf(tt.sc))
+				ctx = trace.ContextWithRemoteSpanContext(ctx, tt.sc)
+				ctx, _ = mockTracer.Start(ctx, "inject")
 			}
 			propagator.Inject(ctx, req.Header)
 

api/trace/testtrace/tracer.go

@@ -21,6 +21,8 @@ import (
 
 	"go.opentelemetry.io/otel/api/core"
 	"go.opentelemetry.io/otel/api/trace"
+
+	"go.opentelemetry.io/otel/internal/trace/parent"
 )
 
 var _ trace.Tracer = (*Tracer)(nil)
@@ -52,10 +54,9 @@ func (t *Tracer) Start(ctx context.Context, name string, opts ...trace.StartOpti
 	var traceID core.TraceID
 	var parentSpanID core.SpanID
 
-	if parentSpanContext := c.Relation.SpanContext; parentSpanContext.IsValid() {
-		traceID = parentSpanContext.TraceID
-		parentSpanID = parentSpanContext.SpanID
-	} else if parentSpanContext := trace.SpanFromContext(ctx).SpanContext(); parentSpanContext.IsValid() {
+	parentSpanContext, _, links := parent.GetSpanContextAndLinks(ctx, c.NewRoot)
+
+	if parentSpanContext.IsValid() {
 		traceID = parentSpanContext.TraceID
 		parentSpanID = parentSpanContext.SpanID
 	} else {
@@ -86,6 +87,9 @@ func (t *Tracer) Start(ctx context.Context, name string, opts ...trace.StartOpti
 	span.SetName(name)
 	span.SetAttributes(c.Attributes...)
 
+	for _, link := range links {
+		span.links[link.SpanContext] = link.Attributes
+	}
 	for _, link := range c.Links {
 		span.links[link.SpanContext] = link.Attributes
 	}