Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace span relationship with a potentially remote parent context #451

Merged
merged 1 commit into from
Feb 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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: 25 additions & 2 deletions api/trace/current.go → api/trace/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor

@MrAlias MrAlias Jan 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will panic if ctx.Value(remoteContextKey) returns a nil interface.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My assertion this will cause a panic is incorrect. Forget I made this suggestion.

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