Repository: open-telemetry/opentelemetry-go
Pull request #1655 (2021-03-05 02:45:02)

Add tests for propagation of Sampler Tracestate changes
Sampler specification indicates that SamplingResult.Tracestate should be associated with the SpanContext of the newly created span.

See
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#sampler

codecov[bot]: # [Codecov](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1655?src=pr&el=h1) Report
> Merging [#1655](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1655?src=pr&el=desc) (4a2e94f) into [main](https://codecov.io/gh/open-telemetry/opentelemetry-go/commit/992837f195eacbd84f0596bfaa79470dba7408e5?el=desc) (992837f) will **increase** coverage by `0.0%`.
> The diff coverage is `100.0%`.

[![Impacted file tree graph](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1655/graphs/tree.svg?width=650&height=150&src=pr&token=8efTmh4kvf)](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1655?src=pr&el=tree)

```diff
@@          Coverage Diff          @@
##            main   #1655   +/-   ##
=====================================
  Coverage   77.3%   77.3%           
=====================================
  Files        128     128           
  Lines       6671    6672    +1     
=====================================
+ Hits        5157    5158    +1     
  Misses      1267    1267           
  Partials     247     247           
```


| [Impacted Files](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1655?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdk/trace/span.go](https://codecov.io/gh/open-telemetry/opentelemetry-go/pull/1655/diff?src=pr&el=tree#diff-c2RrL3RyYWNlL3NwYW4uZ28=) | `90.7% <100.0%> (+<0.1%)` | :arrow_up: |


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7eb3c8d2..a9510374 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Removed the exported `SimpleSpanProcessor` and `BatchSpanProcessor` structs.
    These are now returned as a SpanProcessor interface from their respective constructors. (#1638)
 
+### Fixed
+
+- `SamplingResult.TraceState` is correctly propagated to a newly created
+  span's `SpanContext`. (#1655)
+
 ## [0.18.0] - 2020-03-03
 
 ### Added
@@ -41,7 +46,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ### Changed
 
 - Replaced interface `oteltest.SpanRecorder` with its existing implementation
-  `StandardSpanRecorder` (#1542).
+  `StandardSpanRecorder`. (#1542)
 - Default span limit values to 128. (#1535)
 - Rename `MaxEventsPerSpan`, `MaxAttributesPerSpan` and `MaxLinksPerSpan` to `EventCountLimit`, `AttributeCountLimit` and `LinkCountLimit`, and move these fields into `SpanLimits`. (#1535)
 - Renamed the `otel/label` package to `otel/attribute`. (#1541)
diff --git a/sdk/trace/span.go b/sdk/trace/span.go
index 0bb4155b..9f03f5d3 100644
--- a/sdk/trace/span.go
+++ b/sdk/trace/span.go
@@ -543,6 +543,7 @@ func startSpanInternal(ctx context.Context, tr *tracer, name string, parent trac
 		kind:         o.SpanKind,
 	}
 	sampled := makeSamplingDecision(data)
+	span.spanContext.TraceState = sampled.Tracestate
 
 	if !span.spanContext.IsSampled() && !o.Record {
 		return span


Test patch:
diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go
index 93dde221..4a3aeffb 100644
--- a/sdk/trace/trace_test.go
+++ b/sdk/trace/trace_test.go
@@ -58,12 +58,22 @@ var (
 	sid trace.SpanID
 
 	handler *storingHandler = &storingHandler{}
+
+	k1, k2, k3    attribute.Key
+	kv1, kv2, kv3 attribute.KeyValue
 )
 
 func init() {
 	tid, _ = trace.TraceIDFromHex("01020304050607080102040810203040")
 	sid, _ = trace.SpanIDFromHex("0102040810203040")
 
+	k1 = attribute.Key("k1")
+	kv1 = k1.String("v1")
+	k2 = attribute.Key("k2")
+	kv2 = k2.String("v2")
+	k3 = attribute.Key("k3")
+	kv3 = k3.String("v3")
+
 	otel.SetErrorHandler(handler)
 }
 
@@ -1526,3 +1536,145 @@ func TestAddLinksWithMoreAttributesThanLimit(t *testing.T) {
 		t.Errorf("Link: -got +want %s", diff)
 	}
 }
+
+type stateSampler struct {
+	prefix string
+	f      func(trace.TraceState) trace.TraceState
+}
+
+func (s *stateSampler) ShouldSample(p SamplingParameters) SamplingResult {
+	decision := Drop
+	if strings.HasPrefix(p.Name, s.prefix) {
+		decision = RecordAndSample
+	}
+	return SamplingResult{Decision: decision, Tracestate: s.f(p.ParentContext.TraceState)}
+}
+
+func (s stateSampler) Description() string {
+	return "stateSampler"
+}
+
+// Check that a new span propagates the SamplerResult.TraceState
+func TestSamplerTraceState(t *testing.T) {
+	mustTS := func(t trace.TraceState, err error) trace.TraceState { return t }
+	makeInserter := func(k attribute.KeyValue, prefix string) Sampler {
+		return &stateSampler{
+			prefix: prefix,
+			f:      func(t trace.TraceState) trace.TraceState { return mustTS(t.Insert(k)) },
+		}
+	}
+	makeDeleter := func(k attribute.Key, prefix string) Sampler {
+		return &stateSampler{
+			prefix: prefix,
+			f:      func(t trace.TraceState) trace.TraceState { return mustTS(t.Delete(k)) },
+		}
+	}
+	clearer := func(prefix string) Sampler {
+		return &stateSampler{
+			prefix: prefix,
+			f:      func(t trace.TraceState) trace.TraceState { return trace.TraceState{} },
+		}
+	}
+
+	tests := []struct {
+		name       string
+		sampler    Sampler
+		spanName   string
+		input      trace.TraceState
+		want       trace.TraceState
+		exportSpan bool
+	}{
+		{
+			name:       "alwaysOn",
+			sampler:    AlwaysSample(),
+			input:      mustTS(trace.TraceStateFromKeyValues(kv1)),
+			want:       mustTS(trace.TraceStateFromKeyValues(kv1)),
+			exportSpan: true,
+		},
+		{
+			name:       "alwaysOff",
+			sampler:    NeverSample(),
+			input:      mustTS(trace.TraceStateFromKeyValues(kv1)),
+			want:       mustTS(trace.TraceStateFromKeyValues(kv1)),
+			exportSpan: false,
+		},
+		{
+			name:       "insertKeySampled",
+			sampler:    makeInserter(kv2, "span"),
+			spanName:   "span0",
+			input:      mustTS(trace.TraceStateFromKeyValues(kv1)),
+			want:       mustTS(trace.TraceStateFromKeyValues(kv2, kv1)),
+			exportSpan: true,
+		},
+		{
+			name:       "insertKeyDropped",
+			sampler:    makeInserter(kv2, "span"),
+			spanName:   "nospan0",
+			input:      mustTS(trace.TraceStateFromKeyValues(kv1)),
+			want:       mustTS(trace.TraceStateFromKeyValues(kv2, kv1)),
+			exportSpan: false,
+		},
+		{
+			name:       "deleteKeySampled",
+			sampler:    makeDeleter(k1, "span"),
+			spanName:   "span0",
+			input:      mustTS(trace.TraceStateFromKeyValues(kv1, kv2)),
+			want:       mustTS(trace.TraceStateFromKeyValues(kv2)),
+			exportSpan: true,
+		},
+		{
+			name:       "deleteKeyDropped",
+			sampler:    makeDeleter(k1, "span"),
+			spanName:   "nospan0",
+			input:      mustTS(trace.TraceStateFromKeyValues(kv1, kv2, kv3)),
+			want:       mustTS(trace.TraceStateFromKeyValues(kv2, kv3)),
+			exportSpan: false,
+		},
+		{
+			name:       "clearer",
+			sampler:    clearer("span"),
+			spanName:   "span0",
+			input:      mustTS(trace.TraceStateFromKeyValues(kv1, kv3)),
+			want:       mustTS(trace.TraceStateFromKeyValues()),
+			exportSpan: true,
+		},
+	}
+
+	for _, ts := range tests {
+		ts := ts
+		t.Run(ts.name, func(t *testing.T) {
+			te := NewTestExporter()
+			tp := NewTracerProvider(WithConfig(Config{DefaultSampler: ts.sampler}), WithSyncer(te), WithResource(resource.Empty()))
+			tr := tp.Tracer("TraceState")
+
+			sc1 := trace.SpanContext{
+				TraceID:    tid,
+				SpanID:     sid,
+				TraceFlags: trace.FlagsSampled,
+				TraceState: ts.input,
+			}
+			ctx := trace.ContextWithRemoteSpanContext(context.Background(), sc1)
+			_, span := tr.Start(ctx, ts.spanName)
+
+			// span's TraceState should be set regardless of Sampled/NonSampled state.
+			require.Equal(t, ts.want, span.SpanContext().TraceState)
+
+			span.End()
+
+			got := te.Spans()
+			if len(got) > 0 != ts.exportSpan {
+				t.Errorf("unexpected number of exported spans %d", len(got))
+			}
+			if len(got) == 0 {
+				return
+			}
+
+			receivedState := got[0].SpanContext.TraceState
+
+			if diff := cmpDiff(receivedState, ts.want); diff != "" {
+				t.Errorf("TraceState not propagated: -got +want %s", diff)
+			}
+		})
+	}
+
+}