From fe6c67e7e9b408d7f1e05356e4ccafadf0475b10 Mon Sep 17 00:00:00 2001 From: jianwu Date: Wed, 21 Aug 2024 08:33:56 -0700 Subject: [PATCH] OpenCensus bridge to support TraceState (#5651) # 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 --- CHANGELOG.md | 2 + .../internal/oc2otel/span_context.go | 12 +++++ .../internal/oc2otel/span_context_test.go | 45 ++++++++++++---- .../internal/otel2oc/span_context.go | 10 ++++ .../internal/otel2oc/span_context_test.go | 54 ++++++++++++++++--- trace/tracestate.go | 10 ++++ trace/tracestate_test.go | 46 ++++++++++++++++ 7 files changed, 163 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e490614e868..f6ad01d49fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/bridge/opencensus/internal/oc2otel/span_context.go b/bridge/opencensus/internal/oc2otel/span_context.go index 18c80c9b1ba..2a866f7f8bd 100644 --- a/bridge/opencensus/internal/oc2otel/span_context.go +++ b/bridge/opencensus/internal/oc2otel/span_context.go @@ -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" @@ -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, }) } diff --git a/bridge/opencensus/internal/oc2otel/span_context_test.go b/bridge/opencensus/internal/oc2otel/span_context_test.go index 1a2bc62aa98..2f04c056948 100644 --- a/bridge/opencensus/internal/oc2otel/span_context_test.go +++ b/bridge/opencensus/internal/oc2otel/span_context_test.go @@ -6,6 +6,11 @@ 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" @@ -13,10 +18,21 @@ import ( ) 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", @@ -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) }) } } diff --git a/bridge/opencensus/internal/otel2oc/span_context.go b/bridge/opencensus/internal/otel2oc/span_context.go index 74dcc90b8dc..f9e16bedbcc 100644 --- a/bridge/opencensus/internal/otel2oc/span_context.go +++ b/bridge/opencensus/internal/otel2oc/span_context.go @@ -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" ) @@ -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, } } diff --git a/bridge/opencensus/internal/otel2oc/span_context_test.go b/bridge/opencensus/internal/otel2oc/span_context_test.go index 36ae3cb2331..51e6945c9c6 100644 --- a/bridge/opencensus/internal/otel2oc/span_context_test.go +++ b/bridge/opencensus/internal/otel2oc/span_context_test.go @@ -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", @@ -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) }) } } diff --git a/trace/tracestate.go b/trace/tracestate.go index 20b5cf24332..dc5e34cad0d 100644 --- a/trace/tracestate.go +++ b/trace/tracestate.go @@ -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 diff --git a/trace/tracestate_test.go b/trace/tracestate_test.go index 46b2375da49..8ea17ebcf2f 100644 --- a/trace/tracestate_test.go +++ b/trace/tracestate_test.go @@ -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"},