diff --git a/api/testharness/harness.go b/api/testharness/harness.go index e82c1cc1b13..80e62476621 100644 --- a/api/testharness/harness.go +++ b/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) { diff --git a/api/trace/api.go b/api/trace/api.go index 11c7f541427..fc337098c89 100644 --- a/api/trace/api.go +++ b/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 } } diff --git a/api/trace/current.go b/api/trace/context.go similarity index 52% rename from api/trace/current.go rename to api/trace/context.go index 55cd2d56528..66b003682fb 100644 --- a/api/trace/current.go +++ b/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 = ¤tSpanKeyType{} +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() +} diff --git a/api/trace/current_test.go b/api/trace/context_test.go similarity index 100% rename from api/trace/current_test.go rename to api/trace/context_test.go diff --git a/api/trace/testtrace/b3_propagator_benchmark_test.go b/api/trace/testtrace/b3_propagator_benchmark_test.go index 930b674d70f..b6c7c7117e5 100644 --- a/api/trace/testtrace/b3_propagator_benchmark_test.go +++ b/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++ { diff --git a/api/trace/testtrace/b3_propagator_test.go b/api/trace/testtrace/b3_propagator_test.go index 444dfccdabc..827e1c0ed2d 100644 --- a/api/trace/testtrace/b3_propagator_test.go +++ b/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 { diff --git a/api/trace/testtrace/trace_context_propagator_benchmark_test.go b/api/trace/testtrace/trace_context_propagator_benchmark_test.go index ba688930fa8..f4340b695d4 100644 --- a/api/trace/testtrace/trace_context_propagator_benchmark_test.go +++ b/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) }) diff --git a/api/trace/testtrace/trace_context_propagator_test.go b/api/trace/testtrace/trace_context_propagator_test.go index ba99c78c8f3..5711c2403f7 100644 --- a/api/trace/testtrace/trace_context_propagator_test.go +++ 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) diff --git a/api/trace/testtrace/tracer.go b/api/trace/testtrace/tracer.go index deb3221bf04..5295a138ea5 100644 --- a/api/trace/testtrace/tracer.go +++ b/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 } diff --git a/api/trace/testtrace/tracer_test.go b/api/trace/testtrace/tracer_test.go index 3c7907fd8f1..a4daa775450 100644 --- a/api/trace/testtrace/tracer_test.go +++ b/api/trace/testtrace/tracer_test.go @@ -21,6 +21,7 @@ import ( "time" "go.opentelemetry.io/otel/api/core" + "go.opentelemetry.io/otel/api/key" "go.opentelemetry.io/otel/api/testharness" "go.opentelemetry.io/otel/api/trace" "go.opentelemetry.io/otel/api/trace/testtrace" @@ -74,17 +75,17 @@ func TestTracer(t *testing.T) { e.Expect(attributes[attr2.Key]).ToEqual(attr2.Value) }) - t.Run("uses the parent's span context from ChildOf", func(t *testing.T) { + t.Run("uses the current span from context as parent", func(t *testing.T) { t.Parallel() e := matchers.NewExpecter(t) subject := testtrace.NewTracer() - _, parent := subject.Start(context.Background(), "parent") - parentSpanContext := parent.SpanContext() + parent, parentSpan := subject.Start(context.Background(), "parent") + parentSpanContext := parentSpan.SpanContext() - _, span := subject.Start(context.Background(), "child", trace.ChildOf(parentSpanContext)) + _, span := subject.Start(parent, "child") testSpan, ok := span.(*testtrace.Span) e.Expect(ok).ToBeTrue() @@ -95,18 +96,19 @@ func TestTracer(t *testing.T) { e.Expect(testSpan.ParentSpanID()).ToEqual(parentSpanContext.SpanID) }) - t.Run("defers to ChildOf if the provided context also contains a parent span", func(t *testing.T) { + t.Run("uses the current span from context as parent, even if it has remote span context", func(t *testing.T) { t.Parallel() e := matchers.NewExpecter(t) subject := testtrace.NewTracer() - _, parent := subject.Start(context.Background(), "parent") - parentSpanContext := parent.SpanContext() + parent, parentSpan := subject.Start(context.Background(), "parent") + _, remoteParentSpan := subject.Start(context.Background(), "remote not-a-parent") + parent = trace.ContextWithRemoteSpanContext(parent, remoteParentSpan.SpanContext()) + parentSpanContext := parentSpan.SpanContext() - ctx, _ := subject.Start(context.Background(), "should be ignored") - _, span := subject.Start(ctx, "child", trace.ChildOf(parentSpanContext)) + _, span := subject.Start(parent, "child") testSpan, ok := span.(*testtrace.Span) e.Expect(ok).ToBeTrue() @@ -117,47 +119,101 @@ func TestTracer(t *testing.T) { e.Expect(testSpan.ParentSpanID()).ToEqual(parentSpanContext.SpanID) }) - t.Run("uses the parent's span context from FollowsFrom", func(t *testing.T) { + t.Run("uses the remote span context from context as parent, if current span is missing", func(t *testing.T) { t.Parallel() e := matchers.NewExpecter(t) subject := testtrace.NewTracer() - _, parent := subject.Start(context.Background(), "parent") - parentSpanContext := parent.SpanContext() + _, remoteParentSpan := subject.Start(context.Background(), "remote parent") + parent := trace.ContextWithRemoteSpanContext(context.Background(), remoteParentSpan.SpanContext()) + remoteParentSpanContext := remoteParentSpan.SpanContext() - _, span := subject.Start(context.Background(), "child", trace.FollowsFrom(parentSpanContext)) + _, span := subject.Start(parent, "child") testSpan, ok := span.(*testtrace.Span) e.Expect(ok).ToBeTrue() childSpanContext := testSpan.SpanContext() - e.Expect(childSpanContext.TraceID).ToEqual(parentSpanContext.TraceID) + e.Expect(childSpanContext.TraceID).ToEqual(remoteParentSpanContext.TraceID) + e.Expect(childSpanContext.SpanID).NotToEqual(remoteParentSpanContext.SpanID) + e.Expect(testSpan.ParentSpanID()).ToEqual(remoteParentSpanContext.SpanID) + }) + + t.Run("creates new root when both current span and remote span context are missing", func(t *testing.T) { + t.Parallel() + + e := matchers.NewExpecter(t) + + subject := testtrace.NewTracer() + + _, parentSpan := subject.Start(context.Background(), "not-a-parent") + _, remoteParentSpan := subject.Start(context.Background(), "remote not-a-parent") + parentSpanContext := parentSpan.SpanContext() + remoteParentSpanContext := remoteParentSpan.SpanContext() + + _, span := subject.Start(context.Background(), "child") + + testSpan, ok := span.(*testtrace.Span) + e.Expect(ok).ToBeTrue() + + childSpanContext := testSpan.SpanContext() + e.Expect(childSpanContext.TraceID).NotToEqual(parentSpanContext.TraceID) + e.Expect(childSpanContext.TraceID).NotToEqual(remoteParentSpanContext.TraceID) e.Expect(childSpanContext.SpanID).NotToEqual(parentSpanContext.SpanID) - e.Expect(testSpan.ParentSpanID()).ToEqual(parentSpanContext.SpanID) + e.Expect(childSpanContext.SpanID).NotToEqual(remoteParentSpanContext.SpanID) + e.Expect(testSpan.ParentSpanID().IsValid()).ToBeFalse() }) - t.Run("defers to FollowsFrom if the provided context also contains a parent span", func(t *testing.T) { + t.Run("creates new root when requested, even if both current span and remote span context are in context", func(t *testing.T) { t.Parallel() e := matchers.NewExpecter(t) subject := testtrace.NewTracer() - _, parent := subject.Start(context.Background(), "parent") - parentSpanContext := parent.SpanContext() + parentCtx, parentSpan := subject.Start(context.Background(), "not-a-parent") + _, remoteParentSpan := subject.Start(context.Background(), "remote not-a-parent") + parentSpanContext := parentSpan.SpanContext() + remoteParentSpanContext := remoteParentSpan.SpanContext() + parentCtx = trace.ContextWithRemoteSpanContext(parentCtx, remoteParentSpanContext) - ctx, _ := subject.Start(context.Background(), "should be ignored") - _, span := subject.Start(ctx, "child", trace.FollowsFrom(parentSpanContext)) + _, span := subject.Start(parentCtx, "child", trace.WithNewRoot()) testSpan, ok := span.(*testtrace.Span) e.Expect(ok).ToBeTrue() childSpanContext := testSpan.SpanContext() - e.Expect(childSpanContext.TraceID).ToEqual(parentSpanContext.TraceID) + e.Expect(childSpanContext.TraceID).NotToEqual(parentSpanContext.TraceID) + e.Expect(childSpanContext.TraceID).NotToEqual(remoteParentSpanContext.TraceID) e.Expect(childSpanContext.SpanID).NotToEqual(parentSpanContext.SpanID) - e.Expect(testSpan.ParentSpanID()).ToEqual(parentSpanContext.SpanID) + e.Expect(childSpanContext.SpanID).NotToEqual(remoteParentSpanContext.SpanID) + e.Expect(testSpan.ParentSpanID().IsValid()).ToBeFalse() + + expectedLinks := []trace.Link{ + { + SpanContext: parentSpanContext, + Attributes: []core.KeyValue{ + key.String("ignored-on-demand", "current"), + }, + }, + { + SpanContext: remoteParentSpanContext, + Attributes: []core.KeyValue{ + key.String("ignored-on-demand", "remote"), + }, + }, + } + tsLinks := testSpan.Links() + gotLinks := make([]trace.Link, 0, len(tsLinks)) + for sc, attributes := range tsLinks { + gotLinks = append(gotLinks, trace.Link{ + SpanContext: sc, + Attributes: attributes, + }) + } + e.Expect(gotLinks).ToMatchInAnyOrder(expectedLinks) }) t.Run("uses the links provided through LinkedTo", func(t *testing.T) { diff --git a/bridge/opentracing/bridge.go b/bridge/opentracing/bridge.go index 62a0ceb2ec9..7583b604895 100644 --- a/bridge/opentracing/bridge.go +++ b/bridge/opentracing/bridge.go @@ -28,6 +28,7 @@ import ( otlog "github.com/opentracing/opentracing-go/log" otelcore "go.opentelemetry.io/otel/api/core" + otelkey "go.opentelemetry.io/otel/api/key" oteltrace "go.opentelemetry.io/otel/api/trace" "go.opentelemetry.io/otel/bridge/opentracing/migration" @@ -309,15 +310,18 @@ func (t *BridgeTracer) StartSpan(operationName string, opts ...ot.StartSpanOptio for _, opt := range opts { opt.Apply(&sso) } - // TODO: handle links, needs SpanData to be in the API first? - bRelation, _ := otSpanReferencesToBridgeRelationAndLinks(sso.References) + parentBridgeSC, links := otSpanReferencesToParentAndLinks(sso.References) attributes, kind, hadTrueErrorTag := otTagsToOtelAttributesKindAndError(sso.Tags) checkCtx := migration.WithDeferredSetup(context.Background()) + if parentBridgeSC != nil { + checkCtx = oteltrace.ContextWithRemoteSpanContext(checkCtx, parentBridgeSC.otelSpanContext) + } checkCtx2, otelSpan := t.setTracer.tracer().Start(checkCtx, operationName, func(opts *oteltrace.StartConfig) { opts.Attributes = attributes opts.StartTime = sso.StartTime - opts.Relation = bRelation.ToOtelRelation() + opts.Links = links opts.Record = true + opts.NewRoot = false opts.SpanKind = kind }) if checkCtx != checkCtx2 { @@ -328,9 +332,15 @@ func (t *BridgeTracer) StartSpan(operationName string, opts ...ot.StartSpanOptio if hadTrueErrorTag { otelSpan.SetStatus(codes.Unknown) } + // One does not simply pass a concrete pointer to function + // that takes some interface. In case of passing nil concrete + // pointer, we get an interface with non-nil type (because the + // pointer type is known) and a nil value. Which means + // interface is not nil, but calling some interface function + // on it will most likely result in nil pointer dereference. var otSpanContext ot.SpanContext - if bRelation.spanContext != nil { - otSpanContext = bRelation.spanContext + if parentBridgeSC != nil { + otSpanContext = parentBridgeSC } sctx := newBridgeSpanContext(otelSpan.SpanContext(), otSpanContext) span := &bridgeSpan{ @@ -440,53 +450,59 @@ func otTagToOtelCoreKey(k string) otelcore.Key { return otelcore.Key(k) } -type bridgeRelation struct { - spanContext *bridgeSpanContext - relationshipType oteltrace.RelationshipType -} - -func (r bridgeRelation) ToOtelRelation() oteltrace.Relation { - if r.spanContext == nil { - return oteltrace.Relation{} - } - return oteltrace.Relation{ - SpanContext: r.spanContext.otelSpanContext, - RelationshipType: r.relationshipType, +func otSpanReferencesToParentAndLinks(references []ot.SpanReference) (*bridgeSpanContext, []oteltrace.Link) { + var ( + parent *bridgeSpanContext + links []oteltrace.Link + ) + for _, reference := range references { + bridgeSC, ok := reference.ReferencedContext.(*bridgeSpanContext) + if !ok { + // We ignore foreign ot span contexts, + // sorry. We have no way of getting any + // TraceID and SpanID out of it for form a + // otelcore.SpanContext for otelcore.Link. And + // we can't make it a parent - it also needs a + // valid otelcore.SpanContext. + continue + } + if parent != nil { + links = append(links, otSpanReferenceToOtelLink(bridgeSC, reference.Type)) + } else { + if reference.Type == ot.ChildOfRef { + parent = bridgeSC + } else { + links = append(links, otSpanReferenceToOtelLink(bridgeSC, reference.Type)) + } + } } + return parent, links } -func otSpanReferencesToBridgeRelationAndLinks(references []ot.SpanReference) (bridgeRelation, []*bridgeSpanContext) { - if len(references) == 0 { - return bridgeRelation{}, nil - } - first := references[0] - relation := bridgeRelation{ - spanContext: mustGetBridgeSpanContext(first.ReferencedContext), - relationshipType: otSpanReferenceTypeToOtelRelationshipType(first.Type), +func otSpanReferenceToOtelLink(bridgeSC *bridgeSpanContext, refType ot.SpanReferenceType) oteltrace.Link { + return oteltrace.Link{ + SpanContext: bridgeSC.otelSpanContext, + Attributes: otSpanReferenceTypeToOtelLinkAttributes(refType), } - var links []*bridgeSpanContext - for _, reference := range references[1:] { - links = append(links, mustGetBridgeSpanContext(reference.ReferencedContext)) - } - return relation, links } -func mustGetBridgeSpanContext(ctx ot.SpanContext) *bridgeSpanContext { - ourCtx, ok := ctx.(*bridgeSpanContext) - if !ok { - panic("oops, some foreign span context here") +func otSpanReferenceTypeToOtelLinkAttributes(refType ot.SpanReferenceType) []otelcore.KeyValue { + return []otelcore.KeyValue{ + otelkey.String("ot-span-reference-type", otSpanReferenceTypeToString(refType)), } - return ourCtx } -func otSpanReferenceTypeToOtelRelationshipType(srt ot.SpanReferenceType) oteltrace.RelationshipType { - switch srt { +func otSpanReferenceTypeToString(refType ot.SpanReferenceType) string { + switch refType { case ot.ChildOfRef: - return oteltrace.ChildOfRelationship + // "extra", because first child-of reference is used + // as a parent, so this function isn't even called for + // it. + return "extra-child-of" case ot.FollowsFromRef: - return oteltrace.FollowsFromRelationship + return "follows-from-ref" default: - panic("fix yer code, it uses bogus opentracing reference type") + return fmt.Sprintf("unknown-%d", int(refType)) } } diff --git a/bridge/opentracing/internal/mock.go b/bridge/opentracing/internal/mock.go index 53411424806..187e6fc0584 100644 --- a/bridge/opentracing/internal/mock.go +++ b/bridge/opentracing/internal/mock.go @@ -26,6 +26,7 @@ import ( otelcorrelation "go.opentelemetry.io/otel/api/correlation" otelkey "go.opentelemetry.io/otel/api/key" oteltrace "go.opentelemetry.io/otel/api/trace" + otelparent "go.opentelemetry.io/otel/internal/trace/parent" "go.opentelemetry.io/otel/bridge/opentracing/migration" ) @@ -146,14 +147,8 @@ func (t *MockTracer) getParentSpanID(ctx context.Context, spanOpts *oteltrace.St } func (t *MockTracer) getParentSpanContext(ctx context.Context, spanOpts *oteltrace.StartConfig) otelcore.SpanContext { - if spanOpts.Relation.RelationshipType == oteltrace.ChildOfRelationship && - spanOpts.Relation.SpanContext.IsValid() { - return spanOpts.Relation.SpanContext - } - if parentSpanContext := oteltrace.SpanFromContext(ctx).SpanContext(); parentSpanContext.IsValid() { - return parentSpanContext - } - return otelcore.EmptySpanContext() + spanCtx, _, _ := otelparent.GetSpanContextAndLinks(ctx, spanOpts.NewRoot) + return spanCtx } func (t *MockTracer) getSpanID() otelcore.SpanID { diff --git a/example/grpc/middleware/tracing/tracing.go b/example/grpc/middleware/tracing/tracing.go index ce65955d278..b3d4f812133 100644 --- a/example/grpc/middleware/tracing/tracing.go +++ b/example/grpc/middleware/tracing/tracing.go @@ -50,10 +50,9 @@ func UnaryServerInterceptor(ctx context.Context, req interface{}, info *grpc.Una tr := global.TraceProvider().Tracer("example/grpc") ctx, span := tr.Start( - ctx, + trace.ContextWithRemoteSpanContext(ctx, spanCtx), "hello-api-op", trace.WithAttributes(serverSpanAttrs...), - trace.ChildOf(spanCtx), trace.WithSpanKind(trace.SpanKindServer), ) defer span.End() diff --git a/example/http-stackdriver/server/server.go b/example/http-stackdriver/server/server.go index bbda5a30c67..601a2f1535a 100644 --- a/example/http-stackdriver/server/server.go +++ b/example/http-stackdriver/server/server.go @@ -63,10 +63,9 @@ func main() { }))) ctx, span := tr.Start( - req.Context(), + trace.ContextWithRemoteSpanContext(req.Context(), spanCtx), "hello", trace.WithAttributes(attrs...), - trace.ChildOf(spanCtx), ) defer span.End() diff --git a/example/http/server/server.go b/example/http/server/server.go index 94b958f3e2a..3c7f7970270 100644 --- a/example/http/server/server.go +++ b/example/http/server/server.go @@ -57,10 +57,9 @@ func main() { }))) ctx, span := tr.Start( - req.Context(), + trace.ContextWithRemoteSpanContext(req.Context(), spanCtx), "hello", trace.WithAttributes(attrs...), - trace.ChildOf(spanCtx), ) defer span.End() diff --git a/internal/trace/mock_tracer.go b/internal/trace/mock_tracer.go index 98d973f8b5b..0c680ef3c8b 100644 --- a/internal/trace/mock_tracer.go +++ b/internal/trace/mock_tracer.go @@ -22,6 +22,7 @@ import ( "go.opentelemetry.io/otel/api/core" apitrace "go.opentelemetry.io/otel/api/trace" + "go.opentelemetry.io/otel/internal/trace/parent" ) // MockTracer is a simple tracer used for testing purpose only. @@ -45,9 +46,9 @@ func (mt *MockTracer) WithSpan(ctx context.Context, name string, body func(conte return body(ctx) } -// Start starts a MockSpan. It creates a new Span based on Relation SpanContext option. -// TracdID is used from Relation Span Context and SpanID is assigned. -// If Relation SpanContext option is not specified then random TraceID is used. +// Start starts a MockSpan. It creates a new Span based on Parent SpanContext option. +// TracdID is used from Parent Span Context and SpanID is assigned. +// If Parent SpanContext option is not specified then random TraceID is used. // No other options are supported. func (mt *MockTracer) Start(ctx context.Context, name string, o ...apitrace.StartOption) (context.Context, apitrace.Span) { var opts apitrace.StartConfig @@ -56,14 +57,17 @@ func (mt *MockTracer) Start(ctx context.Context, name string, o ...apitrace.Star } var span *MockSpan var sc core.SpanContext - if !opts.Relation.SpanContext.IsValid() { + + parentSpanContext, _, _ := parent.GetSpanContextAndLinks(ctx, opts.NewRoot) + + if !parentSpanContext.IsValid() { sc = core.SpanContext{} _, _ = rand.Read(sc.TraceID[:]) if mt.Sampled { sc.TraceFlags = core.TraceFlagsSampled } } else { - sc = opts.Relation.SpanContext + sc = parentSpanContext } binary.BigEndian.PutUint64(sc.SpanID[:], atomic.AddUint64(mt.StartSpanID, 1)) diff --git a/internal/trace/parent/parent.go b/internal/trace/parent/parent.go new file mode 100644 index 00000000000..bda728699b2 --- /dev/null +++ b/internal/trace/parent/parent.go @@ -0,0 +1,41 @@ +package parent + +import ( + "context" + + "go.opentelemetry.io/otel/api/core" + "go.opentelemetry.io/otel/api/key" + "go.opentelemetry.io/otel/api/trace" +) + +func GetSpanContextAndLinks(ctx context.Context, ignoreContext bool) (core.SpanContext, bool, []trace.Link) { + lsctx := trace.SpanFromContext(ctx).SpanContext() + rsctx := trace.RemoteSpanContextFromContext(ctx) + + if ignoreContext { + links := addLinkIfValid(nil, lsctx, "current") + links = addLinkIfValid(links, rsctx, "remote") + + return core.EmptySpanContext(), false, links + } + if lsctx.IsValid() { + links := addLinkIfValid(nil, rsctx, "remote") + return lsctx, false, links + } + if rsctx.IsValid() { + return rsctx, true, nil + } + return core.EmptySpanContext(), false, nil +} + +func addLinkIfValid(links []trace.Link, sc core.SpanContext, kind string) []trace.Link { + if !sc.IsValid() { + return links + } + return append(links, trace.Link{ + SpanContext: sc, + Attributes: []core.KeyValue{ + key.String("ignored-on-demand", kind), + }, + }) +} diff --git a/plugin/othttp/handler.go b/plugin/othttp/handler.go index a1840e670a6..972809fc7f9 100644 --- a/plugin/othttp/handler.go +++ b/plugin/othttp/handler.go @@ -146,19 +146,20 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // TODO: do something with the correlation context sc, _ := h.prop.Extract(r.Context(), r.Header) + ctx := r.Context() if sc.IsValid() { // not a valid span context, so no link / parent relationship to establish var opt trace.StartOption if h.public { // If the endpoint is a public endpoint, it should start a new trace // and incoming remote sctx should be added as a link. opt = trace.LinkedTo(sc) + opts = append(opts, opt) } else { // not a private endpoint, so assume child relationship - opt = trace.ChildOf(sc) + ctx = trace.ContextWithRemoteSpanContext(ctx, sc) } - opts = append(opts, opt) } - ctx, span := h.tracer.Start(r.Context(), h.operation, opts...) + ctx, span := h.tracer.Start(ctx, h.operation, opts...) defer span.End() readRecordFunc := func(int64) {} diff --git a/sdk/trace/batch_span_processor_test.go b/sdk/trace/batch_span_processor_test.go index 4c87894d1af..763317023b0 100644 --- a/sdk/trace/batch_span_processor_test.go +++ b/sdk/trace/batch_span_processor_test.go @@ -187,7 +187,8 @@ func generateSpan(t *testing.T, tr apitrace.Tracer, option testOption) { for i := 0; i < option.genNumSpans; i++ { binary.BigEndian.PutUint64(sc.TraceID[0:8], uint64(i+1)) - _, span := tr.Start(context.Background(), option.name, apitrace.ChildOf(sc)) + ctx := apitrace.ContextWithRemoteSpanContext(context.Background(), sc) + _, span := tr.Start(ctx, option.name) span.End() } } diff --git a/sdk/trace/simple_span_processor_test.go b/sdk/trace/simple_span_processor_test.go index 8d57f5cfb90..952c9c494d4 100644 --- a/sdk/trace/simple_span_processor_test.go +++ b/sdk/trace/simple_span_processor_test.go @@ -65,7 +65,8 @@ func TestSimpleSpanProcessorOnEnd(t *testing.T) { SpanID: sid, TraceFlags: 0x1, } - _, span := tr.Start(context.Background(), "OnEnd", apitrace.ChildOf(sc)) + ctx := apitrace.ContextWithRemoteSpanContext(context.Background(), sc) + _, span := tr.Start(ctx, "OnEnd") span.End() wantTraceID := tid diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 9438343a215..1d5151393c5 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -175,7 +175,7 @@ func TestSampling(t *testing.T) { tr := p.Tracer("test") var sampled int for i := 0; i < total; i++ { - var opts []apitrace.StartOption + ctx := context.Background() if tc.parent { psc := core.SpanContext{ TraceID: idg.NewTraceID(), @@ -184,9 +184,9 @@ func TestSampling(t *testing.T) { if tc.sampledParent { psc.TraceFlags = core.TraceFlagsSampled } - opts = append(opts, apitrace.ChildOf(psc)) + ctx = apitrace.ContextWithRemoteSpanContext(ctx, psc) } - _, span := tr.Start(context.Background(), "test", opts...) + _, span := tr.Start(ctx, "test") if span.SpanContext().IsSampled() { sampled++ } @@ -208,21 +208,22 @@ func TestSampling(t *testing.T) { } } -func TestStartSpanWithChildOf(t *testing.T) { +func TestStartSpanWithParent(t *testing.T) { tp, _ := NewProvider() - tr := tp.Tracer("SpanWith ChildOf") + tr := tp.Tracer("SpanWithParent") + ctx := context.Background() sc1 := core.SpanContext{ TraceID: tid, SpanID: sid, TraceFlags: 0x0, } - _, s1 := tr.Start(context.Background(), "span1-unsampled-parent1", apitrace.ChildOf(sc1)) + _, s1 := tr.Start(apitrace.ContextWithRemoteSpanContext(ctx, sc1), "span1-unsampled-parent1") if err := checkChild(sc1, s1); err != nil { t.Error(err) } - _, s2 := tr.Start(context.Background(), "span2-unsampled-parent1", apitrace.ChildOf(sc1)) + _, s2 := tr.Start(apitrace.ContextWithRemoteSpanContext(ctx, sc1), "span2-unsampled-parent1") if err := checkChild(sc1, s2); err != nil { t.Error(err) } @@ -233,60 +234,18 @@ func TestStartSpanWithChildOf(t *testing.T) { TraceFlags: 0x1, //Tracestate: testTracestate, } - _, s3 := tr.Start(context.Background(), "span3-sampled-parent2", apitrace.ChildOf(sc2)) + _, s3 := tr.Start(apitrace.ContextWithRemoteSpanContext(ctx, sc2), "span3-sampled-parent2") if err := checkChild(sc2, s3); err != nil { t.Error(err) } - ctx, s4 := tr.Start(context.Background(), "span4-sampled-parent2", apitrace.ChildOf(sc2)) + ctx2, s4 := tr.Start(apitrace.ContextWithRemoteSpanContext(ctx, sc2), "span4-sampled-parent2") if err := checkChild(sc2, s4); err != nil { t.Error(err) } s4Sc := s4.SpanContext() - _, s5 := tr.Start(ctx, "span5-implicit-childof-span4") - if err := checkChild(s4Sc, s5); err != nil { - t.Error(err) - } -} - -func TestStartSpanWithFollowsFrom(t *testing.T) { - tp, _ := NewProvider() - tr := tp.Tracer("SpanWith FollowsFrom") - - sc1 := core.SpanContext{ - TraceID: tid, - SpanID: sid, - TraceFlags: 0x0, - } - _, s1 := tr.Start(context.Background(), "span1-unsampled-parent1", apitrace.FollowsFrom(sc1)) - if err := checkChild(sc1, s1); err != nil { - t.Error(err) - } - - _, s2 := tr.Start(context.Background(), "span2-unsampled-parent1", apitrace.FollowsFrom(sc1)) - if err := checkChild(sc1, s2); err != nil { - t.Error(err) - } - - sc2 := core.SpanContext{ - TraceID: tid, - SpanID: sid, - TraceFlags: 0x1, - //Tracestate: testTracestate, - } - _, s3 := tr.Start(context.Background(), "span3-sampled-parent2", apitrace.FollowsFrom(sc2)) - if err := checkChild(sc2, s3); err != nil { - t.Error(err) - } - - ctx, s4 := tr.Start(context.Background(), "span4-sampled-parent2", apitrace.FollowsFrom(sc2)) - if err := checkChild(sc2, s4); err != nil { - t.Error(err) - } - - s4Sc := s4.SpanContext() - _, s5 := tr.Start(ctx, "span5-implicit-childof-span4") + _, s5 := tr.Start(ctx2, "span5-implicit-childof-span4") if err := checkChild(s4Sc, s5); err != nil { t.Error(err) } @@ -574,15 +533,15 @@ func TestLinksOverLimit(t *testing.T) { func TestSetSpanName(t *testing.T) { te := &testExporter{} tp, _ := NewProvider(WithSyncer(te)) + ctx := context.Background() want := "SpanName-1" - _, span := tp.Tracer("SetSpanName").Start(context.Background(), "SpanName-1", - apitrace.ChildOf(core.SpanContext{ - TraceID: tid, - SpanID: sid, - TraceFlags: 1, - }), - ) + ctx = apitrace.ContextWithRemoteSpanContext(ctx, core.SpanContext{ + TraceID: tid, + SpanID: sid, + TraceFlags: 1, + }) + _, span := tp.Tracer("SetSpanName").Start(ctx, "SpanName-1") got, err := endSpan(te, span) if err != nil { t.Fatal(err) @@ -662,13 +621,15 @@ func startSpan(tp *Provider, trName string, args ...apitrace.StartOption) apitra } // startNamed Span is a test utility func that starts a span with a -// passed name and with ChildOf option. remote span context contains -// TraceFlags with sampled bit set. This allows the span to be -// automatically sampled. +// passed name and with remote span context as parent. The remote span +// context contains TraceFlags with sampled bit set. This allows the +// span to be automatically sampled. func startNamedSpan(tp *Provider, trName, name string, args ...apitrace.StartOption) apitrace.Span { - args = append(args, apitrace.ChildOf(remoteSpanContext()), apitrace.WithRecord()) + ctx := context.Background() + ctx = apitrace.ContextWithRemoteSpanContext(ctx, remoteSpanContext()) + args = append(args, apitrace.WithRecord()) _, span := tp.Tracer(trName).Start( - context.Background(), + ctx, name, args..., ) @@ -678,7 +639,7 @@ func startNamedSpan(tp *Provider, trName, name string, args ...apitrace.StartOpt // endSpan is a test utility function that ends the span in the context and // returns the exported export.SpanData. // It requires that span be sampled using one of these methods -// 1. Passing parent span context using ChildOf option +// 1. Passing parent span context in context // 2. Use WithSampler(AlwaysSample()) // 3. Configuring AlwaysSample() as default sampler // @@ -739,9 +700,10 @@ func TestEndSpanTwice(t *testing.T) { func TestStartSpanAfterEnd(t *testing.T) { spans := make(fakeExporter) tp, _ := NewProvider(WithConfig(Config{DefaultSampler: AlwaysSample()}), WithSyncer(spans)) + ctx := context.Background() tr := tp.Tracer("SpanAfterEnd") - ctx, span0 := tr.Start(context.Background(), "parent", apitrace.ChildOf(remoteSpanContext())) + ctx, span0 := tr.Start(apitrace.ContextWithRemoteSpanContext(ctx, remoteSpanContext()), "parent") ctx1, span1 := tr.Start(ctx, "span-1") span1.End() // Start a new span with the context containing span-1 @@ -852,17 +814,18 @@ func TestExecutionTracerTaskEnd(t *testing.T) { tID, _ := core.TraceIDFromHex("0102030405060708090a0b0c0d0e0f") sID, _ := core.SpanIDFromHex("0001020304050607") + ctx := context.Background() + ctx = apitrace.ContextWithRemoteSpanContext(ctx, + core.SpanContext{ + TraceID: tID, + SpanID: sID, + TraceFlags: 0, + }, + ) _, apiSpan = tr.Start( - context.Background(), + ctx, "foo", - apitrace.ChildOf( - core.SpanContext{ - TraceID: tID, - SpanID: sID, - TraceFlags: 0, - }, - ), ) s = apiSpan.(*span) s.executionTracerTaskEnd = executionTracerTaskEnd diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index 0fe7e0fc3b8..18d65078d39 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -17,8 +17,8 @@ package trace import ( "context" - "go.opentelemetry.io/otel/api/core" apitrace "go.opentelemetry.io/otel/api/trace" + "go.opentelemetry.io/otel/internal/trace/parent" ) type tracer struct { @@ -30,29 +30,23 @@ var _ apitrace.Tracer = &tracer{} func (tr *tracer) Start(ctx context.Context, name string, o ...apitrace.StartOption) (context.Context, apitrace.Span) { var opts apitrace.StartConfig - var parent core.SpanContext - var remoteParent bool - //TODO [rghetia] : Add new option for parent. If parent is configured then use that parent. for _, op := range o { op(&opts) } - if relation := opts.Relation; relation.SpanContext != core.EmptySpanContext() { - switch relation.RelationshipType { - case apitrace.ChildOfRelationship, apitrace.FollowsFromRelationship: - parent = relation.SpanContext - remoteParent = true - default: - // Future relationship types may have different behavior, - // e.g., adding a `Link` instead of setting the `parent` + parentSpanContext, remoteParent, links := parent.GetSpanContextAndLinks(ctx, opts.NewRoot) + + if p := apitrace.SpanFromContext(ctx); p != nil { + if sdkSpan, ok := p.(*span); ok { + sdkSpan.addChild() } - } else if p, ok := apitrace.SpanFromContext(ctx).(*span); ok { - p.addChild() - parent = p.spanContext } - span := startSpanInternal(tr, name, parent, remoteParent, opts) + span := startSpanInternal(tr, name, parentSpanContext, remoteParent, opts) + for _, l := range links { + span.addLink(l) + } for _, l := range opts.Links { span.addLink(l) }