Skip to content

Commit

Permalink
OpenCensus bridge to support TraceState (#5651)
Browse files Browse the repository at this point in the history
# Summary 
This is to fix issue: #5642 
The original logic skips copying TraceState when convert Spans between
OTel and OC.

This PR also updated the OTel TraceState to expose the Keys function for
the propagation purpose.

---------

Co-authored-by: Sam Xie <sam@samxie.me>
  • Loading branch information
jianwu and XSAM authored Aug 21, 2024
1 parent 83ae9bd commit fe6c67e
Show file tree
Hide file tree
Showing 7 changed files with 163 additions and 16 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ The next release will require at least [Go 1.22].
See our [versioning policy](VERSIONING.md) for more information about these stability guarantees. (#5629)
- Add `InstrumentationScope` field to `SpanStub` in `go.opentelemetry.io/otel/sdk/trace/tracetest`, as a replacement for the deprecated `InstrumentationLibrary`. (#5627)
- Zero value of `SimpleProcessor` in `go.opentelemetry.io/otel/sdk/log` no longer panics. (#5665)
- Add `Walk` function to `TraceState` in `go.opentelemetry.io/otel/trace` to iterate all the key-value pairs. (#5651)
- Bridge the trace state in `go.opentelemetry.io/otel/bridge/opencensus`. (#5651)
- Support [Go 1.23]. (#5720)

### Changed
Expand Down
12 changes: 12 additions & 0 deletions bridge/opencensus/internal/oc2otel/span_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package oc2otel // import "go.opentelemetry.io/otel/bridge/opencensus/internal/oc2otel"

import (
"slices"

octrace "go.opencensus.io/trace"

"go.opentelemetry.io/otel/trace"
Expand All @@ -14,9 +16,19 @@ func SpanContext(sc octrace.SpanContext) trace.SpanContext {
if sc.IsSampled() {
traceFlags = trace.FlagsSampled
}

entries := slices.Clone(sc.Tracestate.Entries())
slices.Reverse(entries)

tsOtel := trace.TraceState{}
for _, entry := range entries {
tsOtel, _ = tsOtel.Insert(entry.Key, entry.Value)
}

return trace.NewSpanContext(trace.SpanContextConfig{
TraceID: trace.TraceID(sc.TraceID),
SpanID: trace.SpanID(sc.SpanID),
TraceFlags: traceFlags,
TraceState: tsOtel,
})
}
45 changes: 35 additions & 10 deletions bridge/opencensus/internal/oc2otel/span_context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,33 @@ package oc2otel
import (
"testing"

"github.com/stretchr/testify/assert"
"go.opencensus.io/plugin/ochttp/propagation/tracecontext"

"go.opentelemetry.io/otel/bridge/opencensus/internal/otel2oc"

octrace "go.opencensus.io/trace"
"go.opencensus.io/trace/tracestate"

"go.opentelemetry.io/otel/trace"
)

func TestSpanContextConversion(t *testing.T) {
tsOc, _ := tracestate.New(nil,
tracestate.Entry{Key: "key1", Value: "value1"},
tracestate.Entry{Key: "key2", Value: "value2"},
)
tsOtel := trace.TraceState{}
tsOtel, _ = tsOtel.Insert("key2", "value2")
tsOtel, _ = tsOtel.Insert("key1", "value1")

httpFormatOc := &tracecontext.HTTPFormat{}

for _, tc := range []struct {
description string
input octrace.SpanContext
expected trace.SpanContext
description string
input octrace.SpanContext
expected trace.SpanContext
expectedTracestate string
}{
{
description: "empty",
Expand Down Expand Up @@ -47,23 +63,32 @@ func TestSpanContextConversion(t *testing.T) {
}),
},
{
description: "trace state is ignored",
description: "trace state should be propagated",
input: octrace.SpanContext{
TraceID: octrace.TraceID([16]byte{1}),
SpanID: octrace.SpanID([8]byte{2}),
Tracestate: &tracestate.Tracestate{},
Tracestate: tsOc,
},
expected: trace.NewSpanContext(trace.SpanContextConfig{
TraceID: trace.TraceID([16]byte{1}),
SpanID: trace.SpanID([8]byte{2}),
TraceID: trace.TraceID([16]byte{1}),
SpanID: trace.SpanID([8]byte{2}),
TraceState: tsOtel,
}),
expectedTracestate: "key1=value1,key2=value2",
},
} {
t.Run(tc.description, func(t *testing.T) {
output := SpanContext(tc.input)
if !output.Equal(tc.expected) {
t.Fatalf("Got %+v spancontext, expected %+v.", output, tc.expected)
}
assert.Equal(t, tc.expected, output)

// Ensure the otel tracestate and oc tracestate has the same header output
_, ts := httpFormatOc.SpanContextToHeaders(tc.input)
assert.Equal(t, tc.expectedTracestate, ts)
assert.Equal(t, tc.expectedTracestate, tc.expected.TraceState().String())

// The reverse conversion should yield the original input
input := otel2oc.SpanContext(output)
assert.Equal(t, tc.input, input)
})
}
}
10 changes: 10 additions & 0 deletions bridge/opencensus/internal/otel2oc/span_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package otel2oc // import "go.opentelemetry.io/otel/bridge/opencensus/internal/o

import (
octrace "go.opencensus.io/trace"
"go.opencensus.io/trace/tracestate"

"go.opentelemetry.io/otel/trace"
)
Expand All @@ -15,9 +16,18 @@ func SpanContext(sc trace.SpanContext) octrace.SpanContext {
// OpenCensus doesn't expose functions to directly set sampled
to = 0x1
}

entries := make([]tracestate.Entry, 0, sc.TraceState().Len())
sc.TraceState().Walk(func(key, value string) bool {
entries = append(entries, tracestate.Entry{Key: key, Value: value})
return true
})
tsOc, _ := tracestate.New(nil, entries...)

return octrace.SpanContext{
TraceID: octrace.TraceID(sc.TraceID()),
SpanID: octrace.SpanID(sc.SpanID()),
TraceOptions: to,
Tracestate: tsOc,
}
}
54 changes: 48 additions & 6 deletions bridge/opencensus/internal/otel2oc/span_context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,36 @@ package otel2oc
import (
"testing"

"go.opencensus.io/plugin/ochttp/propagation/tracecontext"

"go.opentelemetry.io/otel/bridge/opencensus/internal/oc2otel"

"github.com/stretchr/testify/assert"

"go.opencensus.io/trace/tracestate"

octrace "go.opencensus.io/trace"

"go.opentelemetry.io/otel/trace"
)

func TestSpanContextConversion(t *testing.T) {
tsOc, _ := tracestate.New(nil,
// Oc has a reverse order of TraceState entries compared to OTel
tracestate.Entry{Key: "key1", Value: "value1"},
tracestate.Entry{Key: "key2", Value: "value2"},
)
tsOtel := trace.TraceState{}
tsOtel, _ = tsOtel.Insert("key2", "value2")
tsOtel, _ = tsOtel.Insert("key1", "value1")

httpFormatOc := &tracecontext.HTTPFormat{}

for _, tc := range []struct {
description string
input trace.SpanContext
expected octrace.SpanContext
description string
input trace.SpanContext
expected octrace.SpanContext
expectedTracestate string
}{
{
description: "empty",
Expand Down Expand Up @@ -45,12 +65,34 @@ func TestSpanContextConversion(t *testing.T) {
TraceOptions: octrace.TraceOptions(0),
},
},
{
description: "trace state should be propagated",
input: trace.NewSpanContext(trace.SpanContextConfig{
TraceID: trace.TraceID([16]byte{1}),
SpanID: trace.SpanID([8]byte{2}),
TraceState: tsOtel,
}),
expected: octrace.SpanContext{
TraceID: octrace.TraceID([16]byte{1}),
SpanID: octrace.SpanID([8]byte{2}),
TraceOptions: octrace.TraceOptions(0),
Tracestate: tsOc,
},
expectedTracestate: "key1=value1,key2=value2",
},
} {
t.Run(tc.description, func(t *testing.T) {
output := SpanContext(tc.input)
if output != tc.expected {
t.Fatalf("Got %+v spancontext, expected %+v.", output, tc.expected)
}
assert.Equal(t, tc.expected, output)

// Ensure the otel tracestate and oc tracestate has the same header output
_, ts := httpFormatOc.SpanContextToHeaders(tc.expected)
assert.Equal(t, tc.expectedTracestate, ts)
assert.Equal(t, tc.expectedTracestate, tc.input.TraceState().String())

// The reverse conversion should yield the original input
input := oc2otel.SpanContext(output)
assert.Equal(t, tc.input, input)
})
}
}
10 changes: 10 additions & 0 deletions trace/tracestate.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,16 @@ func (ts TraceState) Get(key string) string {
return ""
}

// Walk walks all key value pairs in the TraceState by calling f
// Iteration stops if f returns false.
func (ts TraceState) Walk(f func(key, value string) bool) {
for _, m := range ts.list {
if !f(m.Key, m.Value) {
break
}
}
}

// Insert adds a new list-member defined by the key/value pair to the
// TraceState. If a list-member already exists for the given key, that
// list-member's value is updated. The new or updated list-member is always
Expand Down
46 changes: 46 additions & 0 deletions trace/tracestate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,52 @@ func TestTraceStateDelete(t *testing.T) {
}
}

func TestTraceStateWalk(t *testing.T) {
testCases := []struct {
name string
tracestate TraceState
num int
expected [][]string
}{
{
name: "With keys",
tracestate: TraceState{list: []member{
{Key: "key1", Value: "val1"},
{Key: "key2", Value: "val2"},
}},
num: 3,
expected: [][]string{{"key1", "val1"}, {"key2", "val2"}},
},
{
name: "With keys walk partially",
tracestate: TraceState{list: []member{
{Key: "key1", Value: "val1"},
{Key: "key2", Value: "val2"},
}},
num: 1,
expected: [][]string{{"key1", "val1"}},
},

{
name: "Without keys",
tracestate: TraceState{list: []member{}},
num: 2,
expected: [][]string{},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
got := [][]string{}
tc.tracestate.Walk(func(key, value string) bool {
got = append(got, []string{key, value})
return len(got) < tc.num
})
assert.Equal(t, tc.expected, got)
})
}
}

var insertTS = TraceState{list: []member{
{Key: "key1", Value: "val1"},
{Key: "key2", Value: "val2"},
Expand Down

0 comments on commit fe6c67e

Please sign in to comment.