Skip to content

Commit

Permalink
Replace span relationship with a potentially remote parent context
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jmacd authored and krnowak committed Jan 31, 2020
1 parent 6b4acf4 commit 5821f91
Show file tree
Hide file tree
Showing 22 changed files with 319 additions and 232 deletions.
34 changes: 26 additions & 8 deletions api/testharness/harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
40 changes: 8 additions & 32 deletions api/trace/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}

Expand Down
27 changes: 24 additions & 3 deletions api/trace/current.go → api/trace/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +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()
}
File renamed without changes.
5 changes: 2 additions & 3 deletions api/trace/testtrace/b3_propagator_benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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++ {
Expand Down
5 changes: 2 additions & 3 deletions api/trace/testtrace/b3_propagator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})

Expand Down
3 changes: 2 additions & 1 deletion api/trace/testtrace/trace_context_propagator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
12 changes: 8 additions & 4 deletions api/trace/testtrace/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
Loading

0 comments on commit 5821f91

Please sign in to comment.