From 860d5d86e7ace12bf2b2ca8e437d2d4fc68a6913 Mon Sep 17 00:00:00 2001 From: Anthony Mirabella Date: Thu, 18 Mar 2021 12:05:37 -0400 Subject: [PATCH] Add flag to determine whether SpanContext is remote (#1701) * Add remote property to SpanContext * Set SpanContext.remote when extracting context in TraceContext propagator * Ensure remote flag is set when inserting remote SpanContext into context * Ensure tests are expecting remote flag in SpanContext where appropriate * Update CHANGELOG.md * Apply PR feedback Co-authored-by: Tyler Yahn Co-authored-by: Tyler Yahn --- CHANGELOG.md | 1 + exporters/stdout/trace_test.go | 2 +- oteltest/tracer_test.go | 2 ++ propagation/trace_context.go | 1 + propagation/trace_context_test.go | 11 +++++++++++ sdk/trace/trace_test.go | 1 + trace/trace.go | 29 +++++++++++++++++++++++++++-- trace/trace_test.go | 5 +++++ 8 files changed, 49 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b062c7f833b..8ac5b86c978 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - A `ForceFlush` method to the `"go.opentelemetry.io/otel/sdk/trace".TracerProvider` to flush all registered `SpanProcessor`s. (#1608) - Added `WithDefaultSampler` and `WithSpanLimits` to tracer provider. (#1633) - Jaeger exporter falls back to `resource.Default`'s `service.name` if the exported Span does not have one. (#1673) +- `"go.opentelemetry.io/otel/trace".SpanContext` now has a `remote` property, and `IsRemote()` predicate, that is true when the `SpanContext` has been extracted from remote context data. (#1701) - A `Valid` method to the `"go.opentelemetry.io/otel/attribute".KeyValue` type. (#1703) ### Changed diff --git a/exporters/stdout/trace_test.go b/exporters/stdout/trace_test.go index 166b436fe9d..851bef0355a 100644 --- a/exporters/stdout/trace_test.go +++ b/exporters/stdout/trace_test.go @@ -83,7 +83,7 @@ func TestExporter_ExportSpan(t *testing.T) { `{` + `"Key":"key",` + `"Value":{"Type":"STRING","Value":"val"}` + - `}]},` + + `}],"Remote":false},` + `"ParentSpanID":"0000000000000000",` + `"SpanKind":1,` + `"Name":"/foo",` + diff --git a/oteltest/tracer_test.go b/oteltest/tracer_test.go index e1ab877f136..dc9e2d232f9 100644 --- a/oteltest/tracer_test.go +++ b/oteltest/tracer_test.go @@ -184,6 +184,8 @@ func TestTracer(t *testing.T) { parentSpanContext := parentSpan.SpanContext() remoteParentSpanContext := remoteParentSpan.SpanContext() parentCtx = trace.ContextWithRemoteSpanContext(parentCtx, remoteParentSpanContext) + // remote SpanContexts will be marked as remote + remoteParentSpanContext = remoteParentSpanContext.WithRemote(true) _, span := subject.Start(parentCtx, "child", trace.WithNewRoot()) diff --git a/propagation/trace_context.go b/propagation/trace_context.go index 18c8c385a65..cb38cf416c2 100644 --- a/propagation/trace_context.go +++ b/propagation/trace_context.go @@ -133,6 +133,7 @@ func (tc TraceContext) extract(carrier TextMapCarrier) trace.SpanContext { scc.TraceFlags = opts[0] & trace.FlagsSampled scc.TraceState = parseTraceState(carrier.Get(tracestateHeader)) + scc.Remote = true sc := trace.NewSpanContext(scc) if !sc.IsValid() { diff --git a/propagation/trace_context_test.go b/propagation/trace_context_test.go index ebbbd580e36..1b4ea62580f 100644 --- a/propagation/trace_context_test.go +++ b/propagation/trace_context_test.go @@ -40,6 +40,7 @@ func TestExtractValidTraceContextFromHTTPReq(t *testing.T) { wantSc: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: traceID, SpanID: spanID, + Remote: true, }), }, { @@ -49,6 +50,7 @@ func TestExtractValidTraceContextFromHTTPReq(t *testing.T) { TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled, + Remote: true, }), }, { @@ -58,6 +60,7 @@ func TestExtractValidTraceContextFromHTTPReq(t *testing.T) { TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled, + Remote: true, }), }, { @@ -67,6 +70,7 @@ func TestExtractValidTraceContextFromHTTPReq(t *testing.T) { TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled, + Remote: true, }), }, { @@ -75,6 +79,7 @@ func TestExtractValidTraceContextFromHTTPReq(t *testing.T) { wantSc: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: traceID, SpanID: spanID, + Remote: true, }), }, { @@ -84,6 +89,7 @@ func TestExtractValidTraceContextFromHTTPReq(t *testing.T) { TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled, + Remote: true, }), }, { @@ -93,6 +99,7 @@ func TestExtractValidTraceContextFromHTTPReq(t *testing.T) { TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled, + Remote: true, }), }, { @@ -102,6 +109,7 @@ func TestExtractValidTraceContextFromHTTPReq(t *testing.T) { TraceID: traceID, SpanID: spanID, TraceFlags: trace.FlagsSampled, + Remote: true, }), }, } @@ -302,6 +310,7 @@ func TestTraceStatePropagation(t *testing.T) { TraceID: traceID, SpanID: spanID, TraceState: state, + Remote: true, }), }, { @@ -314,6 +323,7 @@ func TestTraceStatePropagation(t *testing.T) { wantSc: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: traceID, SpanID: spanID, + Remote: true, }), }, { @@ -326,6 +336,7 @@ func TestTraceStatePropagation(t *testing.T) { wantSc: trace.NewSpanContext(trace.SpanContextConfig{ TraceID: traceID, SpanID: spanID, + Remote: true, }), }, } diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 4c4406d44c6..ee5858c0836 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -1385,6 +1385,7 @@ func TestReadOnlySpan(t *testing.T) { TraceID: tID, SpanID: sID, TraceFlags: 0x1, + Remote: true, }) ctx := trace.ContextWithRemoteSpanContext(context.Background(), parent) diff --git a/trace/trace.go b/trace/trace.go index a98c1a7b1aa..fc45a86388f 100644 --- a/trace/trace.go +++ b/trace/trace.go @@ -326,6 +326,7 @@ type SpanContextConfig struct { SpanID SpanID TraceFlags byte TraceState TraceState + Remote bool } // NewSpanContext constructs a SpanContext using values from the provided @@ -336,6 +337,7 @@ func NewSpanContext(config SpanContextConfig) SpanContext { spanID: config.SpanID, traceFlags: config.TraceFlags, traceState: config.TraceState, + remote: config.Remote, } } @@ -345,6 +347,7 @@ type SpanContext struct { spanID SpanID traceFlags byte traceState TraceState + remote bool } // IsValid returns if the SpanContext is valid. A valid span context has a @@ -353,6 +356,22 @@ func (sc SpanContext) IsValid() bool { return sc.HasTraceID() && sc.HasSpanID() } +// IsRemote indicates whether the SpanContext represents a remotely-created Span. +func (sc SpanContext) IsRemote() bool { + return sc.remote +} + +// WithRemote returns a copy of sc with the Remote property set to remote. +func (sc SpanContext) WithRemote(remote bool) SpanContext { + return SpanContext{ + traceID: sc.traceID, + spanID: sc.spanID, + traceFlags: sc.traceFlags, + traceState: sc.traceState, + remote: remote, + } +} + // TraceID returns the TraceID from the SpanContext. func (sc SpanContext) TraceID() TraceID { return sc.traceID @@ -370,6 +389,7 @@ func (sc SpanContext) WithTraceID(traceID TraceID) SpanContext { spanID: sc.spanID, traceFlags: sc.traceFlags, traceState: sc.traceState, + remote: sc.remote, } } @@ -390,6 +410,7 @@ func (sc SpanContext) WithSpanID(spanID SpanID) SpanContext { spanID: spanID, traceFlags: sc.traceFlags, traceState: sc.traceState, + remote: sc.remote, } } @@ -405,6 +426,7 @@ func (sc SpanContext) WithTraceFlags(flags byte) SpanContext { spanID: sc.spanID, traceFlags: flags, traceState: sc.traceState, + remote: sc.remote, } } @@ -435,6 +457,7 @@ func (sc SpanContext) WithTraceState(state TraceState) SpanContext { spanID: sc.spanID, traceFlags: sc.traceFlags, traceState: state, + remote: sc.remote, } } @@ -443,7 +466,8 @@ func (sc SpanContext) Equal(other SpanContext) bool { return sc.traceID == other.traceID && sc.spanID == other.spanID && sc.traceFlags == other.traceFlags && - sc.traceState.String() == other.traceState.String() + sc.traceState.String() == other.traceState.String() && + sc.remote == other.remote } // MarshalJSON implements a custom marshal function to encode a SpanContext. @@ -453,6 +477,7 @@ func (sc SpanContext) MarshalJSON() ([]byte, error) { SpanID: sc.spanID, TraceFlags: sc.traceFlags, TraceState: sc.traceState, + Remote: sc.remote, }) } @@ -487,7 +512,7 @@ func SpanContextFromContext(ctx context.Context) SpanContext { // ContextWithRemoteSpanContext returns a copy of parent with a remote set as // the remote span context. func ContextWithRemoteSpanContext(parent context.Context, remote SpanContext) context.Context { - return context.WithValue(parent, remoteContextKey, remote) + return context.WithValue(parent, remoteContextKey, remote.WithRemote(true)) } // RemoteSpanContextFromContext returns the remote span context from ctx. diff --git a/trace/trace_test.go b/trace/trace_test.go index a5dc019d94c..b4266ad058c 100644 --- a/trace/trace_test.go +++ b/trace/trace_test.go @@ -80,6 +80,8 @@ func TestContextRemoteSpanContext(t *testing.T) { want := SpanContext{traceID: [16]byte{1}, spanID: [8]byte{42}} ctx = ContextWithRemoteSpanContext(ctx, want) + want = want.WithRemote(true) + if got, ok := ctx.Value(remoteContextKey).(SpanContext); !ok { t.Errorf("failed to set SpanContext with %#v", want) } else if !assertSpanContextEqual(got, want) { @@ -92,6 +94,8 @@ func TestContextRemoteSpanContext(t *testing.T) { want = SpanContext{traceID: [16]byte{1}, spanID: [8]byte{43}} ctx = ContextWithRemoteSpanContext(ctx, want) + want = want.WithRemote(true) + if got, ok := ctx.Value(remoteContextKey).(SpanContext); !ok { t.Errorf("failed to set SpanContext with %#v", want) } else if !assertSpanContextEqual(got, want) { @@ -982,6 +986,7 @@ func assertSpanContextEqual(got SpanContext, want SpanContext) bool { return got.spanID == want.spanID && got.traceID == want.traceID && got.traceFlags == want.traceFlags && + got.remote == want.remote && assertTraceStateEqual(got.traceState, want.traceState) }