From 27e75a7e7be240ab0319d01b6bea13ce1c8787e2 Mon Sep 17 00:00:00 2001 From: jianwu chen Date: Wed, 24 Jul 2024 23:50:14 -0700 Subject: [PATCH 01/17] Update OpencensuBridge to propagate TraceState between OTel and OC spans --- .../internal/oc2otel/span_context.go | 8 +++- .../internal/oc2otel/span_context_test.go | 17 +++++-- .../internal/otel2oc/span_context.go | 10 ++++ .../internal/otel2oc/span_context_test.go | 47 ++++++++++++++++++- trace/tracestate.go | 11 +++++ 5 files changed, 86 insertions(+), 7 deletions(-) diff --git a/bridge/opencensus/internal/oc2otel/span_context.go b/bridge/opencensus/internal/oc2otel/span_context.go index 18c80c9b1ba..ae0a0a06d53 100644 --- a/bridge/opencensus/internal/oc2otel/span_context.go +++ b/bridge/opencensus/internal/oc2otel/span_context.go @@ -5,7 +5,6 @@ package oc2otel // import "go.opentelemetry.io/otel/bridge/opencensus/internal/o import ( octrace "go.opencensus.io/trace" - "go.opentelemetry.io/otel/trace" ) @@ -14,9 +13,16 @@ func SpanContext(sc octrace.SpanContext) trace.SpanContext { if sc.IsSampled() { traceFlags = trace.FlagsSampled } + + tsOtel := trace.TraceState{} + for _, entry := range sc.Tracestate.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..267fd590862 100644 --- a/bridge/opencensus/internal/oc2otel/span_context_test.go +++ b/bridge/opencensus/internal/oc2otel/span_context_test.go @@ -13,6 +13,14 @@ import ( ) func TestSpanContextConversion(t *testing.T) { + tsOc, _ := tracestate.New(nil, + tracestate.Entry{"key1", "value1"}, + tracestate.Entry{"key2", "value2"}, + ) + tsOtel := trace.TraceState{} + tsOtel, _ = tsOtel.Insert("key1", "value1") + tsOtel, _ = tsOtel.Insert("key2", "value2") + for _, tc := range []struct { description string input octrace.SpanContext @@ -47,15 +55,16 @@ 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, }), }, } { diff --git a/bridge/opencensus/internal/otel2oc/span_context.go b/bridge/opencensus/internal/otel2oc/span_context.go index 74dcc90b8dc..c8d362f487d 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 } + + keys := sc.TraceState().Keys() + entries := make([]tracestate.Entry, 0, len(keys)) + for _, key := range keys { + entries = append(entries, tracestate.Entry{key, sc.TraceState().Get(key)}) + } + 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..f4d9404c0f3 100644 --- a/bridge/opencensus/internal/otel2oc/span_context_test.go +++ b/bridge/opencensus/internal/otel2oc/span_context_test.go @@ -4,6 +4,8 @@ package otel2oc import ( + "go.opencensus.io/trace/tracestate" + "strings" "testing" octrace "go.opencensus.io/trace" @@ -12,6 +14,14 @@ import ( ) func TestSpanContextConversion(t *testing.T) { + tsOc, _ := tracestate.New(nil, + tracestate.Entry{"key1", "value1"}, + tracestate.Entry{"key2", "value2"}, + ) + tsOtel := trace.TraceState{} + tsOtel, _ = tsOtel.Insert("key1", "value1") + tsOtel, _ = tsOtel.Insert("key2", "value2") + for _, tc := range []struct { description string input trace.SpanContext @@ -45,12 +55,45 @@ 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, + }, + }, } { 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) + if !equal(output, tc.expected) { + t.Fatalf("Got %+v spancontext, expected %+v.", toString(output.Tracestate), toString(tc.expected.Tracestate)) } }) } } + +func equal(t1, t2 octrace.SpanContext) bool { + return t1.IsSampled() == t2.IsSampled() && + t1.SpanID == t2.SpanID && + t1.TraceID == t2.TraceID && + t1.TraceOptions == t2.TraceOptions && + toString(t1.Tracestate) == toString(t2.Tracestate) +} + +func toString(t *tracestate.Tracestate) string { + result := new(strings.Builder) + for _, e := range t.Entries() { + result.WriteString(e.Key) + result.WriteString("=") + result.WriteString(e.Value) + result.WriteString(",") + } + return result.String() +} diff --git a/trace/tracestate.go b/trace/tracestate.go index 20b5cf24332..e05aca91afd 100644 --- a/trace/tracestate.go +++ b/trace/tracestate.go @@ -260,6 +260,17 @@ func (ts TraceState) Get(key string) string { return "" } +// Keys return all the keys from the TraceState in the insertion order. +func (ts TraceState) Keys() []string { + keys := make([]string, 0, len(ts.list)) + // Members are stored in reverse order. + for i := ts.Len() - 1; i >= 0; i-- { + member := ts.list[i] + keys = append(keys, member.Key) + } + return keys +} + // 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 From 0838db14c4dd5dd5604ad492c4839e71bffc3e55 Mon Sep 17 00:00:00 2001 From: jianwu chen Date: Tue, 30 Jul 2024 16:58:37 -0700 Subject: [PATCH 02/17] address comments; add CHANGELOG; remove inverse logic in TraceState.Keys() --- CHANGELOG.md | 2 ++ .../internal/otel2oc/span_context.go | 2 +- .../internal/otel2oc/span_context_test.go | 11 ++++---- trace/tracestate.go | 8 ++---- trace/tracestate_test.go | 28 +++++++++++++++++++ 5 files changed, 40 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7791db7575..a88c1620e97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm This module is unstable and breaking changes may be introduced. 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) +- Add `Keys` function to `TraceState` to return a sorted list of keys. ### Fixed @@ -29,6 +30,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Correct the `Tracer` names used in `go.opentelemetry.io/otel/example/zipkin`. (#5612) - Pass the underlying error rather than a generic retry-able failure in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`, `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp` and `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#5541) - Correct comments for the priority of the `WithEndpoint` and `WithEndpointURL` options and their corresponding environment variables in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc` and `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#5641) +- Propagate `TraceState` in converting between OpenTelemetry and OpenCensus TraceContext in OpenCensus bridge. (#5642) diff --git a/bridge/opencensus/internal/otel2oc/span_context.go b/bridge/opencensus/internal/otel2oc/span_context.go index c8d362f487d..2adad0ffd02 100644 --- a/bridge/opencensus/internal/otel2oc/span_context.go +++ b/bridge/opencensus/internal/otel2oc/span_context.go @@ -20,7 +20,7 @@ func SpanContext(sc trace.SpanContext) octrace.SpanContext { keys := sc.TraceState().Keys() entries := make([]tracestate.Entry, 0, len(keys)) for _, key := range keys { - entries = append(entries, tracestate.Entry{key, sc.TraceState().Get(key)}) + entries = append(entries, tracestate.Entry{Key: key, Value: sc.TraceState().Get(key)}) } tsOc, _ := tracestate.New(nil, entries...) diff --git a/bridge/opencensus/internal/otel2oc/span_context_test.go b/bridge/opencensus/internal/otel2oc/span_context_test.go index f4d9404c0f3..46527eb7959 100644 --- a/bridge/opencensus/internal/otel2oc/span_context_test.go +++ b/bridge/opencensus/internal/otel2oc/span_context_test.go @@ -15,8 +15,9 @@ import ( func TestSpanContextConversion(t *testing.T) { tsOc, _ := tracestate.New(nil, - tracestate.Entry{"key1", "value1"}, + // Oc has a reverse order of TraceState entries compared to OTel tracestate.Entry{"key2", "value2"}, + tracestate.Entry{"key1", "value1"}, ) tsOtel := trace.TraceState{} tsOtel, _ = tsOtel.Insert("key1", "value1") @@ -90,10 +91,10 @@ func equal(t1, t2 octrace.SpanContext) bool { func toString(t *tracestate.Tracestate) string { result := new(strings.Builder) for _, e := range t.Entries() { - result.WriteString(e.Key) - result.WriteString("=") - result.WriteString(e.Value) - result.WriteString(",") + _, _ = result.WriteString(e.Key) + _, _ = result.WriteString("=") + _, _ = result.WriteString(e.Value) + _, _ = result.WriteString(",") } return result.String() } diff --git a/trace/tracestate.go b/trace/tracestate.go index e05aca91afd..4df32b6d195 100644 --- a/trace/tracestate.go +++ b/trace/tracestate.go @@ -260,13 +260,11 @@ func (ts TraceState) Get(key string) string { return "" } -// Keys return all the keys from the TraceState in the insertion order. +// Keys return all the keys from the TraceState in the reverse of insertion order. func (ts TraceState) Keys() []string { keys := make([]string, 0, len(ts.list)) - // Members are stored in reverse order. - for i := ts.Len() - 1; i >= 0; i-- { - member := ts.list[i] - keys = append(keys, member.Key) + for _, m := range ts.list { + keys = append(keys, m.Key) } return keys } diff --git a/trace/tracestate_test.go b/trace/tracestate_test.go index 46b2375da49..2dc9713776b 100644 --- a/trace/tracestate_test.go +++ b/trace/tracestate_test.go @@ -409,6 +409,34 @@ func TestTraceStateDelete(t *testing.T) { } } +func TestTraceStateKeys(t *testing.T) { + testCases := []struct { + name string + tracestate TraceState + expected []string + }{ + { + name: "With keys", + tracestate: TraceState{list: []member{ + {Key: "key1", Value: "val1"}, + {Key: "key2", Value: "val2"}, + }}, + expected: []string{"key1", "key2"}, + }, + { + name: "Without keys", + tracestate: TraceState{list: []member{}}, + expected: []string{}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expected, tc.tracestate.Keys()) + }) + } +} + var insertTS = TraceState{list: []member{ {Key: "key1", Value: "val1"}, {Key: "key2", Value: "val2"}, From e9bc04b6a7bb30f2a9d4fa3ff96681837a1e71f6 Mon Sep 17 00:00:00 2001 From: jianwu chen Date: Tue, 30 Jul 2024 17:13:11 -0700 Subject: [PATCH 03/17] . --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a88c1620e97..f39be12fefb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Correct the `Tracer` names used in `go.opentelemetry.io/otel/example/zipkin`. (#5612) - Pass the underlying error rather than a generic retry-able failure in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`, `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp` and `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#5541) - Correct comments for the priority of the `WithEndpoint` and `WithEndpointURL` options and their corresponding environment variables in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc` and `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#5641) -- Propagate `TraceState` in converting between OpenTelemetry and OpenCensus TraceContext in OpenCensus bridge. (#5642) +- Propagate `TraceState` when converting between OpenTelemetry and OpenCensus TraceContext in OpenCensus bridge. (#5642) From 18896134a4f1ab09cb55f7d0d2f4901a0f4f9aaf Mon Sep 17 00:00:00 2001 From: jianwu chen Date: Wed, 31 Jul 2024 11:30:45 -0700 Subject: [PATCH 04/17] Add SpanState.Walk() instead of Keys() --- .../internal/oc2otel/span_context_test.go | 4 ++-- .../opencensus/internal/otel2oc/span_context.go | 10 +++++----- .../internal/otel2oc/span_context_test.go | 4 ++-- trace/tracestate.go | 8 ++++---- trace/tracestate_test.go | 15 ++++++++++----- 5 files changed, 23 insertions(+), 18 deletions(-) diff --git a/bridge/opencensus/internal/oc2otel/span_context_test.go b/bridge/opencensus/internal/oc2otel/span_context_test.go index 267fd590862..0c15634d78f 100644 --- a/bridge/opencensus/internal/oc2otel/span_context_test.go +++ b/bridge/opencensus/internal/oc2otel/span_context_test.go @@ -14,8 +14,8 @@ import ( func TestSpanContextConversion(t *testing.T) { tsOc, _ := tracestate.New(nil, - tracestate.Entry{"key1", "value1"}, - tracestate.Entry{"key2", "value2"}, + tracestate.Entry{Key: "key1", Value: "value1"}, + tracestate.Entry{Key: "key2", Value: "value2"}, ) tsOtel := trace.TraceState{} tsOtel, _ = tsOtel.Insert("key1", "value1") diff --git a/bridge/opencensus/internal/otel2oc/span_context.go b/bridge/opencensus/internal/otel2oc/span_context.go index 2adad0ffd02..f9e16bedbcc 100644 --- a/bridge/opencensus/internal/otel2oc/span_context.go +++ b/bridge/opencensus/internal/otel2oc/span_context.go @@ -17,11 +17,11 @@ func SpanContext(sc trace.SpanContext) octrace.SpanContext { to = 0x1 } - keys := sc.TraceState().Keys() - entries := make([]tracestate.Entry, 0, len(keys)) - for _, key := range keys { - entries = append(entries, tracestate.Entry{Key: key, Value: sc.TraceState().Get(key)}) - } + 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{ diff --git a/bridge/opencensus/internal/otel2oc/span_context_test.go b/bridge/opencensus/internal/otel2oc/span_context_test.go index 46527eb7959..1781cd293a9 100644 --- a/bridge/opencensus/internal/otel2oc/span_context_test.go +++ b/bridge/opencensus/internal/otel2oc/span_context_test.go @@ -16,8 +16,8 @@ import ( func TestSpanContextConversion(t *testing.T) { tsOc, _ := tracestate.New(nil, // Oc has a reverse order of TraceState entries compared to OTel - tracestate.Entry{"key2", "value2"}, - tracestate.Entry{"key1", "value1"}, + tracestate.Entry{Key: "key2", Value: "value2"}, + tracestate.Entry{Key: "key1", Value: "value1"}, ) tsOtel := trace.TraceState{} tsOtel, _ = tsOtel.Insert("key1", "value1") diff --git a/trace/tracestate.go b/trace/tracestate.go index 4df32b6d195..5a1461697c1 100644 --- a/trace/tracestate.go +++ b/trace/tracestate.go @@ -261,12 +261,12 @@ func (ts TraceState) Get(key string) string { } // Keys return all the keys from the TraceState in the reverse of insertion order. -func (ts TraceState) Keys() []string { - keys := make([]string, 0, len(ts.list)) +func (ts TraceState) Walk(f func(key, value string) bool) { for _, m := range ts.list { - keys = append(keys, m.Key) + if !f(m.Key, m.Value) { + break + } } - return keys } // Insert adds a new list-member defined by the key/value pair to the diff --git a/trace/tracestate_test.go b/trace/tracestate_test.go index 2dc9713776b..5f6328f3059 100644 --- a/trace/tracestate_test.go +++ b/trace/tracestate_test.go @@ -409,11 +409,11 @@ func TestTraceStateDelete(t *testing.T) { } } -func TestTraceStateKeys(t *testing.T) { +func TestTraceStateWalk(t *testing.T) { testCases := []struct { name string tracestate TraceState - expected []string + expected [][]string }{ { name: "With keys", @@ -421,18 +421,23 @@ func TestTraceStateKeys(t *testing.T) { {Key: "key1", Value: "val1"}, {Key: "key2", Value: "val2"}, }}, - expected: []string{"key1", "key2"}, + expected: [][]string{{"key1", "val1"}, {"key2", "val2"}}, }, { name: "Without keys", tracestate: TraceState{list: []member{}}, - expected: []string{}, + expected: [][]string{}, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.expected, tc.tracestate.Keys()) + var got = [][]string{} + tc.tracestate.Walk(func(key, value string) bool { + got = append(got, []string{key, value}) + return true + }) + assert.Equal(t, tc.expected, got) }) } } From 874e7baa6a07adb36b23ab7503f2dc81c5b51526 Mon Sep 17 00:00:00 2001 From: jianwu chen Date: Wed, 31 Jul 2024 11:32:04 -0700 Subject: [PATCH 05/17] update change log --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f39be12fefb..f574495a092 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm This module is unstable and breaking changes may be introduced. 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) -- Add `Keys` function to `TraceState` to return a sorted list of keys. +- Add `Walk` function to `TraceState` to iterate all the key-value pairs. ### Fixed From 04cf2fdab256d475db41bb97a20c30babfb0042d Mon Sep 17 00:00:00 2001 From: jianwu chen Date: Wed, 31 Jul 2024 11:37:44 -0700 Subject: [PATCH 06/17] update doc --- trace/tracestate.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/trace/tracestate.go b/trace/tracestate.go index 5a1461697c1..dc5e34cad0d 100644 --- a/trace/tracestate.go +++ b/trace/tracestate.go @@ -260,7 +260,8 @@ func (ts TraceState) Get(key string) string { return "" } -// Keys return all the keys from the TraceState in the reverse of insertion order. +// 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) { From 9c5ac8387e95e1dcc2ba15557cd2fb91f4222c5e Mon Sep 17 00:00:00 2001 From: jianwu chen Date: Wed, 31 Jul 2024 11:38:42 -0700 Subject: [PATCH 07/17] . --- trace/tracestate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trace/tracestate_test.go b/trace/tracestate_test.go index 5f6328f3059..5a25c0d7fea 100644 --- a/trace/tracestate_test.go +++ b/trace/tracestate_test.go @@ -432,7 +432,7 @@ func TestTraceStateWalk(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - var got = [][]string{} + got := [][]string{} tc.tracestate.Walk(func(key, value string) bool { got = append(got, []string{key, value}) return true From 92a0cbe5284276b091061d38551253566ca98553 Mon Sep 17 00:00:00 2001 From: jianwu chen Date: Wed, 31 Jul 2024 16:24:01 -0700 Subject: [PATCH 08/17] fmt --- bridge/opencensus/internal/oc2otel/span_context.go | 1 + bridge/opencensus/internal/otel2oc/span_context_test.go | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/bridge/opencensus/internal/oc2otel/span_context.go b/bridge/opencensus/internal/oc2otel/span_context.go index ae0a0a06d53..74cbced1e8e 100644 --- a/bridge/opencensus/internal/oc2otel/span_context.go +++ b/bridge/opencensus/internal/oc2otel/span_context.go @@ -5,6 +5,7 @@ package oc2otel // import "go.opentelemetry.io/otel/bridge/opencensus/internal/o import ( octrace "go.opencensus.io/trace" + "go.opentelemetry.io/otel/trace" ) diff --git a/bridge/opencensus/internal/otel2oc/span_context_test.go b/bridge/opencensus/internal/otel2oc/span_context_test.go index 1781cd293a9..29cc79d3ec8 100644 --- a/bridge/opencensus/internal/otel2oc/span_context_test.go +++ b/bridge/opencensus/internal/otel2oc/span_context_test.go @@ -4,10 +4,11 @@ package otel2oc import ( - "go.opencensus.io/trace/tracestate" "strings" "testing" + "go.opencensus.io/trace/tracestate" + octrace "go.opencensus.io/trace" "go.opentelemetry.io/otel/trace" From d58b6595c7eb697931fb0a69ccfbc03cb5330436 Mon Sep 17 00:00:00 2001 From: jianwu chen Date: Thu, 1 Aug 2024 17:39:03 -0700 Subject: [PATCH 09/17] Update changelog; update tests otel2oc.TestSpanContextConversion with simplified logic --- CHANGELOG.md | 4 +-- .../internal/otel2oc/span_context_test.go | 25 ++----------------- trace/tracestate_test.go | 15 ++++++++++- 3 files changed, 18 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f574495a092..a8c1ad8c2d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm This module is unstable and breaking changes may be introduced. 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) -- Add `Walk` function to `TraceState` to iterate all the key-value pairs. +- 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) ### Fixed @@ -30,7 +31,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Correct the `Tracer` names used in `go.opentelemetry.io/otel/example/zipkin`. (#5612) - Pass the underlying error rather than a generic retry-able failure in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`, `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp` and `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#5541) - Correct comments for the priority of the `WithEndpoint` and `WithEndpointURL` options and their corresponding environment variables in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc` and `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#5641) -- Propagate `TraceState` when converting between OpenTelemetry and OpenCensus TraceContext in OpenCensus bridge. (#5642) diff --git a/bridge/opencensus/internal/otel2oc/span_context_test.go b/bridge/opencensus/internal/otel2oc/span_context_test.go index 29cc79d3ec8..a724240c1bc 100644 --- a/bridge/opencensus/internal/otel2oc/span_context_test.go +++ b/bridge/opencensus/internal/otel2oc/span_context_test.go @@ -4,7 +4,7 @@ package otel2oc import ( - "strings" + "github.com/stretchr/testify/assert" "testing" "go.opencensus.io/trace/tracestate" @@ -74,28 +74,7 @@ func TestSpanContextConversion(t *testing.T) { } { t.Run(tc.description, func(t *testing.T) { output := SpanContext(tc.input) - if !equal(output, tc.expected) { - t.Fatalf("Got %+v spancontext, expected %+v.", toString(output.Tracestate), toString(tc.expected.Tracestate)) - } + assert.Equal(t, tc.expected, output) }) } } - -func equal(t1, t2 octrace.SpanContext) bool { - return t1.IsSampled() == t2.IsSampled() && - t1.SpanID == t2.SpanID && - t1.TraceID == t2.TraceID && - t1.TraceOptions == t2.TraceOptions && - toString(t1.Tracestate) == toString(t2.Tracestate) -} - -func toString(t *tracestate.Tracestate) string { - result := new(strings.Builder) - for _, e := range t.Entries() { - _, _ = result.WriteString(e.Key) - _, _ = result.WriteString("=") - _, _ = result.WriteString(e.Value) - _, _ = result.WriteString(",") - } - return result.String() -} diff --git a/trace/tracestate_test.go b/trace/tracestate_test.go index 5a25c0d7fea..8ea17ebcf2f 100644 --- a/trace/tracestate_test.go +++ b/trace/tracestate_test.go @@ -413,6 +413,7 @@ func TestTraceStateWalk(t *testing.T) { testCases := []struct { name string tracestate TraceState + num int expected [][]string }{ { @@ -421,11 +422,23 @@ func TestTraceStateWalk(t *testing.T) { {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{}, }, } @@ -435,7 +448,7 @@ func TestTraceStateWalk(t *testing.T) { got := [][]string{} tc.tracestate.Walk(func(key, value string) bool { got = append(got, []string{key, value}) - return true + return len(got) < tc.num }) assert.Equal(t, tc.expected, got) }) From 6e97577bb6e80ead07528f2c0e50264ad470e668 Mon Sep 17 00:00:00 2001 From: jianwu chen Date: Fri, 2 Aug 2024 14:50:24 -0700 Subject: [PATCH 10/17] fmt --- bridge/opencensus/internal/otel2oc/span_context_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bridge/opencensus/internal/otel2oc/span_context_test.go b/bridge/opencensus/internal/otel2oc/span_context_test.go index a724240c1bc..a1a8cf122ce 100644 --- a/bridge/opencensus/internal/otel2oc/span_context_test.go +++ b/bridge/opencensus/internal/otel2oc/span_context_test.go @@ -4,9 +4,10 @@ package otel2oc import ( - "github.com/stretchr/testify/assert" "testing" + "github.com/stretchr/testify/assert" + "go.opencensus.io/trace/tracestate" octrace "go.opencensus.io/trace" From e7d428996151fbddabf518e7ac5b02445d26bd62 Mon Sep 17 00:00:00 2001 From: jianwu Date: Fri, 9 Aug 2024 10:33:39 -0700 Subject: [PATCH 11/17] Update bridge/opencensus/internal/otel2oc/span_context_test.go Co-authored-by: Sam Xie --- bridge/opencensus/internal/otel2oc/span_context_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bridge/opencensus/internal/otel2oc/span_context_test.go b/bridge/opencensus/internal/otel2oc/span_context_test.go index a1a8cf122ce..56794502255 100644 --- a/bridge/opencensus/internal/otel2oc/span_context_test.go +++ b/bridge/opencensus/internal/otel2oc/span_context_test.go @@ -18,12 +18,14 @@ import ( func TestSpanContextConversion(t *testing.T) { tsOc, _ := tracestate.New(nil, // Oc has a reverse order of TraceState entries compared to OTel - tracestate.Entry{Key: "key2", Value: "value2"}, tracestate.Entry{Key: "key1", Value: "value1"}, + tracestate.Entry{Key: "key2", Value: "value2"}, ) tsOtel := trace.TraceState{} - tsOtel, _ = tsOtel.Insert("key1", "value1") tsOtel, _ = tsOtel.Insert("key2", "value2") + tsOtel, _ = tsOtel.Insert("key1", "value1") + + httpFormatOc := &tracecontext.HTTPFormat{} for _, tc := range []struct { description string From af61537b587384697282d376d71c63b78898edbe Mon Sep 17 00:00:00 2001 From: jianwu Date: Fri, 9 Aug 2024 10:33:46 -0700 Subject: [PATCH 12/17] Update bridge/opencensus/internal/otel2oc/span_context_test.go Co-authored-by: Sam Xie --- bridge/opencensus/internal/otel2oc/span_context_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/bridge/opencensus/internal/otel2oc/span_context_test.go b/bridge/opencensus/internal/otel2oc/span_context_test.go index 56794502255..50ac6da2a9d 100644 --- a/bridge/opencensus/internal/otel2oc/span_context_test.go +++ b/bridge/opencensus/internal/otel2oc/span_context_test.go @@ -73,6 +73,7 @@ func TestSpanContextConversion(t *testing.T) { TraceOptions: octrace.TraceOptions(0), Tracestate: tsOc, }, + expectedTracestate: "key1=value1,key2=value2", }, } { t.Run(tc.description, func(t *testing.T) { From 4d255a9ce0f0029c26edcfd1dc908c2d8cb40c3c Mon Sep 17 00:00:00 2001 From: jianwu Date: Fri, 9 Aug 2024 10:33:53 -0700 Subject: [PATCH 13/17] Update bridge/opencensus/internal/otel2oc/span_context_test.go Co-authored-by: Sam Xie --- bridge/opencensus/internal/otel2oc/span_context_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/bridge/opencensus/internal/otel2oc/span_context_test.go b/bridge/opencensus/internal/otel2oc/span_context_test.go index 50ac6da2a9d..b07aeff99f6 100644 --- a/bridge/opencensus/internal/otel2oc/span_context_test.go +++ b/bridge/opencensus/internal/otel2oc/span_context_test.go @@ -79,6 +79,15 @@ func TestSpanContextConversion(t *testing.T) { t.Run(tc.description, func(t *testing.T) { output := SpanContext(tc.input) 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) }) } } From 9de698ef17f74bd31d4adf4aefe3c15e5510fed7 Mon Sep 17 00:00:00 2001 From: jianwu Date: Fri, 9 Aug 2024 10:34:01 -0700 Subject: [PATCH 14/17] Update bridge/opencensus/internal/oc2otel/span_context.go Co-authored-by: Sam Xie --- bridge/opencensus/internal/oc2otel/span_context.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bridge/opencensus/internal/oc2otel/span_context.go b/bridge/opencensus/internal/oc2otel/span_context.go index 74cbced1e8e..9dc2b6c12f4 100644 --- a/bridge/opencensus/internal/oc2otel/span_context.go +++ b/bridge/opencensus/internal/oc2otel/span_context.go @@ -15,8 +15,11 @@ func SpanContext(sc octrace.SpanContext) trace.SpanContext { traceFlags = trace.FlagsSampled } + entries := slices.Clone(sc.Tracestate.Entries()) + slices.Reverse(entries) + tsOtel := trace.TraceState{} - for _, entry := range sc.Tracestate.Entries() { + for _, entry := range entries { tsOtel, _ = tsOtel.Insert(entry.Key, entry.Value) } From 100dd242c06c374891852476faaf911926fca462 Mon Sep 17 00:00:00 2001 From: jianwu chen Date: Fri, 9 Aug 2024 10:57:29 -0700 Subject: [PATCH 15/17] fix build and test --- bridge/opencensus/internal/oc2otel/span_context.go | 1 + bridge/opencensus/internal/oc2otel/span_context_test.go | 2 +- bridge/opencensus/internal/otel2oc/span_context_test.go | 9 ++++++--- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/bridge/opencensus/internal/oc2otel/span_context.go b/bridge/opencensus/internal/oc2otel/span_context.go index 9dc2b6c12f4..11fb65a8d2e 100644 --- a/bridge/opencensus/internal/oc2otel/span_context.go +++ b/bridge/opencensus/internal/oc2otel/span_context.go @@ -5,6 +5,7 @@ package oc2otel // import "go.opentelemetry.io/otel/bridge/opencensus/internal/o import ( octrace "go.opencensus.io/trace" + "slices" "go.opentelemetry.io/otel/trace" ) diff --git a/bridge/opencensus/internal/oc2otel/span_context_test.go b/bridge/opencensus/internal/oc2otel/span_context_test.go index 0c15634d78f..dc1f6b488af 100644 --- a/bridge/opencensus/internal/oc2otel/span_context_test.go +++ b/bridge/opencensus/internal/oc2otel/span_context_test.go @@ -18,8 +18,8 @@ func TestSpanContextConversion(t *testing.T) { tracestate.Entry{Key: "key2", Value: "value2"}, ) tsOtel := trace.TraceState{} - tsOtel, _ = tsOtel.Insert("key1", "value1") tsOtel, _ = tsOtel.Insert("key2", "value2") + tsOtel, _ = tsOtel.Insert("key1", "value1") for _, tc := range []struct { description string diff --git a/bridge/opencensus/internal/otel2oc/span_context_test.go b/bridge/opencensus/internal/otel2oc/span_context_test.go index b07aeff99f6..a9b56fdb966 100644 --- a/bridge/opencensus/internal/otel2oc/span_context_test.go +++ b/bridge/opencensus/internal/otel2oc/span_context_test.go @@ -4,6 +4,8 @@ package otel2oc import ( + "go.opencensus.io/plugin/ochttp/propagation/tracecontext" + "go.opentelemetry.io/otel/bridge/opencensus/internal/oc2otel" "testing" "github.com/stretchr/testify/assert" @@ -28,9 +30,10 @@ func TestSpanContextConversion(t *testing.T) { 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", From f5b22f916bcf243d606dd8cc04a8c4ae268717ab Mon Sep 17 00:00:00 2001 From: jianwu chen Date: Fri, 9 Aug 2024 13:38:33 -0700 Subject: [PATCH 16/17] fmt --- bridge/opencensus/internal/oc2otel/span_context.go | 3 ++- bridge/opencensus/internal/otel2oc/span_context_test.go | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/bridge/opencensus/internal/oc2otel/span_context.go b/bridge/opencensus/internal/oc2otel/span_context.go index 11fb65a8d2e..2a866f7f8bd 100644 --- a/bridge/opencensus/internal/oc2otel/span_context.go +++ b/bridge/opencensus/internal/oc2otel/span_context.go @@ -4,9 +4,10 @@ package oc2otel // import "go.opentelemetry.io/otel/bridge/opencensus/internal/oc2otel" import ( - octrace "go.opencensus.io/trace" "slices" + octrace "go.opencensus.io/trace" + "go.opentelemetry.io/otel/trace" ) diff --git a/bridge/opencensus/internal/otel2oc/span_context_test.go b/bridge/opencensus/internal/otel2oc/span_context_test.go index a9b56fdb966..51e6945c9c6 100644 --- a/bridge/opencensus/internal/otel2oc/span_context_test.go +++ b/bridge/opencensus/internal/otel2oc/span_context_test.go @@ -4,9 +4,11 @@ package otel2oc import ( + "testing" + "go.opencensus.io/plugin/ochttp/propagation/tracecontext" + "go.opentelemetry.io/otel/bridge/opencensus/internal/oc2otel" - "testing" "github.com/stretchr/testify/assert" From 1eecc6cca64f0fedb777ba42e27329649b6897be Mon Sep 17 00:00:00 2001 From: jianwu chen Date: Mon, 12 Aug 2024 12:00:06 -0700 Subject: [PATCH 17/17] add more checks for header output for oc2otel span conversion --- .../internal/oc2otel/span_context_test.go | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/bridge/opencensus/internal/oc2otel/span_context_test.go b/bridge/opencensus/internal/oc2otel/span_context_test.go index dc1f6b488af..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" @@ -21,10 +26,13 @@ func TestSpanContextConversion(t *testing.T) { 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", @@ -66,13 +74,21 @@ func TestSpanContextConversion(t *testing.T) { 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) }) } }