Skip to content

Commit e88a091

Browse files
Aneurysm9Aneurysm9
authored
Make SpanContext Immutable (#1573)
* Make SpanContext Immutable * Adds NewSpanContext() constructor and SpanContextConfig{} struct for constructing a new SpanContext when all fields are known * Adds With<field>() methods to SpanContext for deriving a SpanContext with a single field changed. * Updates all uses of SpanContext to use the new API Signed-off-by: Anthony J Mirabella <[email protected]> * Update CHANGELOG.md Signed-off-by: Anthony J Mirabella <[email protected]> * Add tests for new SpanContext constructor and derivation Signed-off-by: Anthony J Mirabella <[email protected]> * Address PR feedback * Fix new uses of SpanContext from main
1 parent d75e268 commit e88a091

37 files changed

+554
-343
lines changed

‎CHANGELOG.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
88

99
## [Unreleased]
1010

11-
## Added
11+
### Added
1212

1313
- Added `Marshler` config option to `otlphttp` to enable otlp over json or protobufs. (#1586)
1414
- A `ForceFlush` method to the `"go.opentelemetry.io/otel/sdk/trace".TracerProvider` to flush all registered `SpanProcessor`s. (#1608)
1515

16-
1716
### Changed
1817

1918
- Update the `ForceFlush` method signature to the `"go.opentelemetry.io/otel/sdk/trace".SpanProcessor` to accept a `context.Context` and return an error. (#1608)
@@ -22,14 +21,15 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
2221
- `"go.opentelemetry.io/sdk/metric/controller.basic".WithPusher` is replaced with `WithExporter` to provide consistent naming across project. (#1656)
2322
- Added non-empty string check for trace `Attribute` keys. (#1659)
2423
- Add `description` to SpanStatus only when `StatusCode` is set to error. (#1662)
24+
- `trace.SpanContext` is now immutable and has no exported fields. (#1573)
25+
- `trace.NewSpanContext()` can be used in conjunction with the `trace.SpanContextConfig` struct to initialize a new `SpanContext` where all values are known.
2526

2627
### Removed
2728

2829
- Removed the exported `SimpleSpanProcessor` and `BatchSpanProcessor` structs.
2930
These are now returned as a SpanProcessor interface from their respective constructors. (#1638)
3031
- Removed setting status to `Error` while recording an error as a span event in `RecordError`. (#1663)
3132

32-
3333
### Fixed
3434

3535
- `SamplingResult.TraceState` is correctly propagated to a newly created

‎bridge/opencensus/bridge.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,5 +186,5 @@ func (s *span) AddLink(l octrace.Link) {
186186
}
187187

188188
func (s *span) String() string {
189-
return fmt.Sprintf("span %s", s.otSpan.SpanContext().SpanID.String())
189+
return fmt.Sprintf("span %s", s.otSpan.SpanContext().SpanID().String())
190190
}

‎bridge/opencensus/bridge_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ func TestMixedAPIs(t *testing.T) {
6666
// Reverse the order we look at the spans in, since they are listed in last-to-first order.
6767
i = len(spans) - i - 1
6868
// Verify that OpenCensus spans and opentelemetry spans have each other as parents.
69-
if spans[i].ParentSpanID() != parent.SpanContext().SpanID {
70-
t.Errorf("Span %v had parent %v. Expected %d", spans[i].Name(), spans[i].ParentSpanID(), parent.SpanContext().SpanID)
69+
if spans[i].ParentSpanID() != parent.SpanContext().SpanID() {
70+
t.Errorf("Span %v had parent %v. Expected %d", spans[i].Name(), spans[i].ParentSpanID(), parent.SpanContext().SpanID())
7171
}
7272
parent = spans[i]
7373
}
@@ -111,8 +111,8 @@ func TestStartSpanWithRemoteParent(t *testing.T) {
111111
t.Fatalf("Got %d spans, exepected %d", len(spans), 1)
112112
}
113113

114-
if spans[0].ParentSpanID() != parent.SpanContext().SpanID {
115-
t.Errorf("Span %v, had parent %v. Expected %d", spans[0].Name(), spans[0].ParentSpanID(), parent.SpanContext().SpanID)
114+
if spans[0].ParentSpanID() != parent.SpanContext().SpanID() {
115+
t.Errorf("Span %v, had parent %v. Expected %d", spans[0].Name(), spans[0].ParentSpanID(), parent.SpanContext().SpanID())
116116
}
117117
}
118118

@@ -150,8 +150,8 @@ func TestToFromContext(t *testing.T) {
150150
// Reverse the order we look at the spans in, since they are listed in last-to-first order.
151151
i = len(spans) - i - 1
152152
// Verify that OpenCensus spans and opentelemetry spans have each other as parents.
153-
if spans[i].ParentSpanID() != parent.SpanContext().SpanID {
154-
t.Errorf("Span %v had parent %v. Expected %d", spans[i].Name(), spans[i].ParentSpanID(), parent.SpanContext().SpanID)
153+
if spans[i].ParentSpanID() != parent.SpanContext().SpanID() {
154+
t.Errorf("Span %v had parent %v. Expected %d", spans[i].Name(), spans[i].ParentSpanID(), parent.SpanContext().SpanID())
155155
}
156156
parent = spans[i]
157157
}

‎bridge/opencensus/utils/utils.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,16 @@ import (
2828
// error handler.
2929
func OTelSpanContextToOC(sc trace.SpanContext) octrace.SpanContext {
3030
if sc.IsDebug() || sc.IsDeferred() {
31-
otel.Handle(fmt.Errorf("ignoring OpenTelemetry Debug or Deferred trace flags for span %q because they are not supported by OpenCensus", sc.SpanID))
31+
otel.Handle(fmt.Errorf("ignoring OpenTelemetry Debug or Deferred trace flags for span %q because they are not supported by OpenCensus", sc.SpanID()))
3232
}
3333
var to octrace.TraceOptions
3434
if sc.IsSampled() {
3535
// OpenCensus doesn't expose functions to directly set sampled
3636
to = 0x1
3737
}
3838
return octrace.SpanContext{
39-
TraceID: octrace.TraceID(sc.TraceID),
40-
SpanID: octrace.SpanID(sc.SpanID),
39+
TraceID: octrace.TraceID(sc.TraceID()),
40+
SpanID: octrace.SpanID(sc.SpanID()),
4141
TraceOptions: to,
4242
}
4343
}
@@ -49,9 +49,9 @@ func OCSpanContextToOTel(sc octrace.SpanContext) trace.SpanContext {
4949
if sc.IsSampled() {
5050
traceFlags = trace.FlagsSampled
5151
}
52-
return trace.SpanContext{
52+
return trace.NewSpanContext(trace.SpanContextConfig{
5353
TraceID: trace.TraceID(sc.TraceID),
5454
SpanID: trace.SpanID(sc.SpanID),
5555
TraceFlags: traceFlags,
56-
}
56+
})
5757
}

‎bridge/opencensus/utils/utils_test.go

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ func TestOTelSpanContextToOC(t *testing.T) {
3535
},
3636
{
3737
description: "sampled",
38-
input: trace.SpanContext{
38+
input: trace.NewSpanContext(trace.SpanContextConfig{
3939
TraceID: trace.TraceID([16]byte{1}),
4040
SpanID: trace.SpanID([8]byte{2}),
4141
TraceFlags: trace.FlagsSampled,
42-
},
42+
}),
4343
expected: octrace.SpanContext{
4444
TraceID: octrace.TraceID([16]byte{1}),
4545
SpanID: octrace.SpanID([8]byte{2}),
@@ -48,10 +48,10 @@ func TestOTelSpanContextToOC(t *testing.T) {
4848
},
4949
{
5050
description: "not sampled",
51-
input: trace.SpanContext{
51+
input: trace.NewSpanContext(trace.SpanContextConfig{
5252
TraceID: trace.TraceID([16]byte{1}),
5353
SpanID: trace.SpanID([8]byte{2}),
54-
},
54+
}),
5555
expected: octrace.SpanContext{
5656
TraceID: octrace.TraceID([16]byte{1}),
5757
SpanID: octrace.SpanID([8]byte{2}),
@@ -60,11 +60,11 @@ func TestOTelSpanContextToOC(t *testing.T) {
6060
},
6161
{
6262
description: "debug flag",
63-
input: trace.SpanContext{
63+
input: trace.NewSpanContext(trace.SpanContextConfig{
6464
TraceID: trace.TraceID([16]byte{1}),
6565
SpanID: trace.SpanID([8]byte{2}),
6666
TraceFlags: trace.FlagsDebug,
67-
},
67+
}),
6868
expected: octrace.SpanContext{
6969
TraceID: octrace.TraceID([16]byte{1}),
7070
SpanID: octrace.SpanID([8]byte{2}),
@@ -97,11 +97,11 @@ func TestOCSpanContextToOTel(t *testing.T) {
9797
SpanID: octrace.SpanID([8]byte{2}),
9898
TraceOptions: octrace.TraceOptions(0x1),
9999
},
100-
expected: trace.SpanContext{
100+
expected: trace.NewSpanContext(trace.SpanContextConfig{
101101
TraceID: trace.TraceID([16]byte{1}),
102102
SpanID: trace.SpanID([8]byte{2}),
103103
TraceFlags: trace.FlagsSampled,
104-
},
104+
}),
105105
},
106106
{
107107
description: "not sampled",
@@ -110,10 +110,10 @@ func TestOCSpanContextToOTel(t *testing.T) {
110110
SpanID: octrace.SpanID([8]byte{2}),
111111
TraceOptions: octrace.TraceOptions(0),
112112
},
113-
expected: trace.SpanContext{
113+
expected: trace.NewSpanContext(trace.SpanContextConfig{
114114
TraceID: trace.TraceID([16]byte{1}),
115115
SpanID: trace.SpanID([8]byte{2}),
116-
},
116+
}),
117117
},
118118
{
119119
description: "trace state is ignored",
@@ -122,17 +122,15 @@ func TestOCSpanContextToOTel(t *testing.T) {
122122
SpanID: octrace.SpanID([8]byte{2}),
123123
Tracestate: &tracestate.Tracestate{},
124124
},
125-
expected: trace.SpanContext{
125+
expected: trace.NewSpanContext(trace.SpanContextConfig{
126126
TraceID: trace.TraceID([16]byte{1}),
127127
SpanID: trace.SpanID([8]byte{2}),
128-
},
128+
}),
129129
},
130130
} {
131131
t.Run(tc.description, func(t *testing.T) {
132132
output := OCSpanContextToOTel(tc.input)
133-
if output.SpanID != tc.expected.SpanID ||
134-
output.TraceID != tc.expected.TraceID ||
135-
output.TraceFlags != tc.expected.TraceFlags {
133+
if !output.Equal(tc.expected) {
136134
t.Fatalf("Got %+v spancontext, exepected %+v.", output, tc.expected)
137135
}
138136
})

‎bridge/opentracing/internal/mock.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,11 @@ func (t *MockTracer) Start(ctx context.Context, name string, opts ...trace.SpanO
7676
if startTime.IsZero() {
7777
startTime = time.Now()
7878
}
79-
spanContext := trace.SpanContext{
79+
spanContext := trace.NewSpanContext(trace.SpanContextConfig{
8080
TraceID: t.getTraceID(ctx, config),
8181
SpanID: t.getSpanID(),
8282
TraceFlags: 0,
83-
}
83+
})
8484
span := &MockSpan{
8585
mockTracer: t,
8686
officialTracer: t,
@@ -117,7 +117,7 @@ func (t *MockTracer) addSpareContextValue(ctx context.Context) context.Context {
117117

118118
func (t *MockTracer) getTraceID(ctx context.Context, config *trace.SpanConfig) trace.TraceID {
119119
if parent := t.getParentSpanContext(ctx, config); parent.IsValid() {
120-
return parent.TraceID
120+
return parent.TraceID()
121121
}
122122
if len(t.SpareTraceIDs) > 0 {
123123
traceID := t.SpareTraceIDs[0]
@@ -132,7 +132,7 @@ func (t *MockTracer) getTraceID(ctx context.Context, config *trace.SpanConfig) t
132132

133133
func (t *MockTracer) getParentSpanID(ctx context.Context, config *trace.SpanConfig) trace.SpanID {
134134
if parent := t.getParentSpanContext(ctx, config); parent.IsValid() {
135-
return parent.SpanID
135+
return parent.SpanID()
136136
}
137137
return trace.SpanID{}
138138
}

‎bridge/opentracing/mix_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -229,12 +229,12 @@ func (cast *currentActiveSpanTest) runOTOtelOT(t *testing.T, ctx context.Context
229229
}
230230

231231
func (cast *currentActiveSpanTest) recordSpans(t *testing.T, ctx context.Context) context.Context {
232-
spanID := trace.SpanContextFromContext(ctx).SpanID
232+
spanID := trace.SpanContextFromContext(ctx).SpanID()
233233
cast.recordedCurrentOtelSpanIDs = append(cast.recordedCurrentOtelSpanIDs, spanID)
234234

235235
spanID = trace.SpanID{}
236236
if bridgeSpan, ok := ot.SpanFromContext(ctx).(*bridgeSpan); ok {
237-
spanID = bridgeSpan.otelSpan.SpanContext().SpanID
237+
spanID = bridgeSpan.otelSpan.SpanContext().SpanID()
238238
}
239239
cast.recordedActiveOTSpanIDs = append(cast.recordedActiveOTSpanIDs, spanID)
240240
return ctx
@@ -637,19 +637,19 @@ func checkTraceAndSpans(t *testing.T, tracer *internal.MockTracer, expectedTrace
637637
}
638638
for idx, span := range tracer.FinishedSpans {
639639
sctx := span.SpanContext()
640-
if sctx.TraceID != expectedTraceID {
641-
t.Errorf("Expected trace ID %v in span %d (%d), got %v", expectedTraceID, idx, sctx.SpanID, sctx.TraceID)
640+
if sctx.TraceID() != expectedTraceID {
641+
t.Errorf("Expected trace ID %v in span %d (%d), got %v", expectedTraceID, idx, sctx.SpanID(), sctx.TraceID())
642642
}
643643
expectedSpanID := spanIDs[idx]
644644
expectedParentSpanID := parentSpanIDs[idx]
645-
if sctx.SpanID != expectedSpanID {
646-
t.Errorf("Expected finished span %d to have span ID %d, but got %d", idx, expectedSpanID, sctx.SpanID)
645+
if sctx.SpanID() != expectedSpanID {
646+
t.Errorf("Expected finished span %d to have span ID %d, but got %d", idx, expectedSpanID, sctx.SpanID())
647647
}
648648
if span.ParentSpanID != expectedParentSpanID {
649-
t.Errorf("Expected finished span %d (span ID: %d) to have parent span ID %d, but got %d", idx, sctx.SpanID, expectedParentSpanID, span.ParentSpanID)
649+
t.Errorf("Expected finished span %d (span ID: %d) to have parent span ID %d, but got %d", idx, sctx.SpanID(), expectedParentSpanID, span.ParentSpanID)
650650
}
651-
if span.SpanKind != sks[span.SpanContext().SpanID] {
652-
t.Errorf("Expected finished span %d (span ID: %d) to have span.kind to be '%v' but was '%v'", idx, sctx.SpanID, sks[span.SpanContext().SpanID], span.SpanKind)
651+
if span.SpanKind != sks[span.SpanContext().SpanID()] {
652+
t.Errorf("Expected finished span %d (span ID: %d) to have span.kind to be '%v' but was '%v'", idx, sctx.SpanID(), sks[span.SpanContext().SpanID()], span.SpanKind)
653653
}
654654
}
655655
}

‎exporters/otlp/internal/otlptest/data.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,11 @@ func (OneRecordCheckpointSet) ForEach(kindSelector exportmetric.ExportKindSelect
8282
// may be useful for testing driver's trace export.
8383
func SingleSpanSnapshot() []*exporttrace.SpanSnapshot {
8484
sd := &exporttrace.SpanSnapshot{
85-
SpanContext: trace.SpanContext{
85+
SpanContext: trace.NewSpanContext(trace.SpanContextConfig{
8686
TraceID: trace.TraceID{2, 3, 4, 5, 6, 7, 8, 9, 2, 3, 4, 5, 6, 7, 8, 9},
8787
SpanID: trace.SpanID{3, 4, 5, 6, 7, 8, 9, 0},
8888
TraceFlags: trace.FlagsSampled,
89-
},
89+
}),
9090
ParentSpanID: trace.SpanID{1, 2, 3, 4, 5, 6, 7, 8},
9191
SpanKind: trace.SpanKindInternal,
9292
Name: "foo",

‎exporters/otlp/internal/transform/span.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,13 @@ func span(sd *export.SpanSnapshot) *tracepb.Span {
101101
return nil
102102
}
103103

104+
tid := sd.SpanContext.TraceID()
105+
sid := sd.SpanContext.SpanID()
106+
104107
s := &tracepb.Span{
105-
TraceId: sd.SpanContext.TraceID[:],
106-
SpanId: sd.SpanContext.SpanID[:],
107-
TraceState: sd.SpanContext.TraceState.String(),
108+
TraceId: tid[:],
109+
SpanId: sid[:],
110+
TraceState: sd.SpanContext.TraceState().String(),
108111
Status: status(sd.StatusCode, sd.StatusMessage),
109112
StartTimeUnixNano: uint64(sd.StartTime.UnixNano()),
110113
EndTimeUnixNano: uint64(sd.EndTime.UnixNano()),
@@ -152,9 +155,12 @@ func links(links []trace.Link) []*tracepb.Span_Link {
152155
// being reused -- in short we need a new otLink per iteration.
153156
otLink := otLink
154157

158+
tid := otLink.TraceID()
159+
sid := otLink.SpanID()
160+
155161
sl = append(sl, &tracepb.Span_Link{
156-
TraceId: otLink.TraceID[:],
157-
SpanId: otLink.SpanID[:],
162+
TraceId: tid[:],
163+
SpanId: sid[:],
158164
Attributes: Attributes(otLink.Attributes),
159165
})
160166
}

‎exporters/otlp/internal/transform/span_test.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,7 @@ func TestLinks(t *testing.T) {
148148
assert.Equal(t, expected, got[1])
149149

150150
// Changes to our links should not change the produced links.
151-
l[1].TraceID[0] = byte(0x1)
152-
l[1].SpanID[0] = byte(0x1)
151+
l[1].SpanContext = l[1].WithTraceID(trace.TraceID{})
153152
assert.Equal(t, expected, got[1])
154153
}
155154

@@ -201,11 +200,11 @@ func TestSpanData(t *testing.T) {
201200
endTime := startTime.Add(10 * time.Second)
202201
traceState, _ := trace.TraceStateFromKeyValues(attribute.String("key1", "val1"), attribute.String("key2", "val2"))
203202
spanData := &export.SpanSnapshot{
204-
SpanContext: trace.SpanContext{
203+
SpanContext: trace.NewSpanContext(trace.SpanContextConfig{
205204
TraceID: trace.TraceID{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F},
206205
SpanID: trace.SpanID{0xFF, 0xFE, 0xFD, 0xFC, 0xFB, 0xFA, 0xF9, 0xF8},
207206
TraceState: traceState,
208-
},
207+
}),
209208
SpanKind: trace.SpanKindServer,
210209
ParentSpanID: trace.SpanID{0xEF, 0xEE, 0xED, 0xEC, 0xEB, 0xEA, 0xE9, 0xE8},
211210
Name: "span data to span data",
@@ -225,21 +224,21 @@ func TestSpanData(t *testing.T) {
225224
},
226225
Links: []trace.Link{
227226
{
228-
SpanContext: trace.SpanContext{
227+
SpanContext: trace.NewSpanContext(trace.SpanContextConfig{
229228
TraceID: trace.TraceID{0xC0, 0xC1, 0xC2, 0xC3, 0xC4, 0xC5, 0xC6, 0xC7, 0xC8, 0xC9, 0xCA, 0xCB, 0xCC, 0xCD, 0xCE, 0xCF},
230229
SpanID: trace.SpanID{0xB0, 0xB1, 0xB2, 0xB3, 0xB4, 0xB5, 0xB6, 0xB7},
231230
TraceFlags: 0,
232-
},
231+
}),
233232
Attributes: []attribute.KeyValue{
234233
attribute.String("LinkType", "Parent"),
235234
},
236235
},
237236
{
238-
SpanContext: trace.SpanContext{
237+
SpanContext: trace.NewSpanContext(trace.SpanContextConfig{
239238
TraceID: trace.TraceID{0xE0, 0xE1, 0xE2, 0xE3, 0xE4, 0xE5, 0xE6, 0xE7, 0xE8, 0xE9, 0xEA, 0xEB, 0xEC, 0xED, 0xEE, 0xEF},
240239
SpanID: trace.SpanID{0xD0, 0xD1, 0xD2, 0xD3, 0xD4, 0xD5, 0xD6, 0xD7},
241240
TraceFlags: 0,
242-
},
241+
}),
243242
Attributes: []attribute.KeyValue{
244243
attribute.String("LinkType", "Child"),
245244
},

0 commit comments

Comments
 (0)