From 3db3dc301fe241f4ccb159d4f232d6bab095054b Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Thu, 3 Mar 2022 16:01:26 -0500 Subject: [PATCH 1/5] Implemented logic for opentracing baggage propagation --- propagators/ot/ot_data_test.go | 59 ++++++++++++++++++++++++++++----- propagators/ot/ot_propagator.go | 43 +++++++++++++++++++----- 2 files changed, 85 insertions(+), 17 deletions(-) diff --git a/propagators/ot/ot_data_test.go b/propagators/ot/ot_data_test.go index a56b8649967..3c65b057540 100644 --- a/propagators/ot/ot_data_test.go +++ b/propagators/ot/ot_data_test.go @@ -16,6 +16,7 @@ package ot_test import ( "go.opentelemetry.io/otel/trace" + "strings" ) const ( @@ -38,11 +39,13 @@ var ( traceID32 = trace.TraceID{0xa1, 0xce, 0x92, 0x9d, 0x0e, 0x0e, 0x47, 0x36, 0xa3, 0xce, 0x92, 0x9d, 0x0e, 0x0e, 0x47, 0x36} spanID = trace.SpanID{0x00, 0xf0, 0x67, 0xaa, 0x0b, 0xa9, 0x02, 0xb7} emptyBaggage = map[string]string{} - // TODO: once baggage extraction is supported, re-enable this - // baggageSet = attribute.NewSet( - // attribute.String(baggageKey, baggageValue), - // attribute.String(baggageKey2, baggageValue2), - // ) + baggageSet = map[string]string{ + baggageKey: baggageValue, + } + baggageSet2 = map[string]string{ + baggageKey: baggageValue, + baggageKey2: baggageValue2, + } ) type extractTest struct { @@ -85,9 +88,22 @@ var extractHeaders = []extractTest{ SpanID: spanID, TraceFlags: trace.FlagsSampled, }, - emptyBaggage, - // TODO: once baggage extraction is supported, re-enable this - // &baggageSet, + baggageSet, + }, + { + "baggage multiple values", + map[string]string{ + traceIDHeader: traceID32Str, + spanIDHeader: spanIDStr, + sampledHeader: "0", + baggageHeader: baggageValue, + baggageHeader2: baggageValue2, + }, + trace.SpanContextConfig{ + TraceID: traceID32, + SpanID: spanID, + }, + baggageSet2, }, { "left padding 64 bit trace ID", @@ -168,6 +184,33 @@ var invalidExtractHeaders = []extractTest{ sampledHeader: "wired", }, }, + { + name: "invalid baggage key", + headers: map[string]string{ + traceIDHeader: traceID32Str, + spanIDHeader: spanIDStr, + sampledHeader: "1", + "ot-baggage-d–76": "test", + }, + }, + { + name: "invalid baggage value", + headers: map[string]string{ + traceIDHeader: traceID32Str, + spanIDHeader: spanIDStr, + sampledHeader: "1", + baggageHeader: "øtel", + }, + }, + { + name: "invalid baggage result (too large)", + headers: map[string]string{ + traceIDHeader: traceID32Str, + spanIDHeader: spanIDStr, + sampledHeader: "1", + baggageHeader: strings.Repeat("s", 8188), + }, + }, { name: "missing headers", headers: map[string]string{}, diff --git a/propagators/ot/ot_propagator.go b/propagators/ot/ot_propagator.go index facd6331bcd..66a10fe793e 100644 --- a/propagators/ot/ot_propagator.go +++ b/propagators/ot/ot_propagator.go @@ -28,9 +28,10 @@ import ( const ( // Default OT Header names. - traceIDHeader = "ot-tracer-traceid" - spanIDHeader = "ot-tracer-spanid" - sampledHeader = "ot-tracer-sampled" + traceIDHeader = "ot-tracer-traceid" + spanIDHeader = "ot-tracer-spanid" + sampledHeader = "ot-tracer-sampled" + baggageHeaderPrefix = "ot-baggage-" otTraceIDPadding = "0000000000000000" @@ -72,7 +73,7 @@ func (o OT) Inject(ctx context.Context, carrier propagation.TextMapCarrier) { } for _, m := range baggage.FromContext(ctx).Members() { - carrier.Set(fmt.Sprintf("ot-baggage-%s", m.Key()), m.Value()) + carrier.Set(fmt.Sprintf("%s%s", baggageHeaderPrefix, m.Key()), m.Value()) } } @@ -93,11 +94,12 @@ func (o OT) Extract(ctx context.Context, carrier propagation.TextMapCarrier) con if err != nil || !sc.IsValid() { return ctx } - // TODO: implement extracting baggage - // - // this currently is not achievable without an implementation of `keys` - // on the carrier, see: - // https://github.com/open-telemetry/opentelemetry-go/issues/1493 + + bags, err := extractBags(carrier) + if err != nil { + return ctx + } + ctx = baggage.ContextWithBaggage(ctx, bags) return trace.ContextWithRemoteSpanContext(ctx, sc) } @@ -105,6 +107,29 @@ func (o OT) Fields() []string { return []string{traceIDHeader, spanIDHeader, sampledHeader} } +// extractBags reconstructs the baggage information from opentracing +func extractBags(carrier propagation.TextMapCarrier) (baggage.Baggage, error) { + emptyBags, _ := baggage.New() + var members []baggage.Member + for _, key := range carrier.Keys() { + lowerKey := strings.ToLower(key) + if !strings.HasPrefix(lowerKey, baggageHeaderPrefix) { + continue + } + strippedKey := strings.TrimPrefix(lowerKey, baggageHeaderPrefix) + member, err := baggage.NewMember(strippedKey, carrier.Get(key)) + if err != nil { + return emptyBags, err + } + members = append(members, member) + } + bags, err := baggage.New(members...) + if err != nil { + return emptyBags, err + } + return bags, nil +} + // extract reconstructs a SpanContext from header values based on OT // headers. func extract(traceID, spanID, sampled string) (trace.SpanContext, error) { From 9f13bf5ff514eaa4b7e76798bf5cc62f93820f5b Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Thu, 3 Mar 2022 16:55:22 -0500 Subject: [PATCH 2/5] Linting --- propagators/ot/ot_data_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/propagators/ot/ot_data_test.go b/propagators/ot/ot_data_test.go index 3c65b057540..3665434b26d 100644 --- a/propagators/ot/ot_data_test.go +++ b/propagators/ot/ot_data_test.go @@ -15,8 +15,9 @@ package ot_test import ( - "go.opentelemetry.io/otel/trace" "strings" + + "go.opentelemetry.io/otel/trace" ) const ( From 1a0ca25fbfac2fd854ec2dae035c5ebadd51cca8 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Tue, 8 Mar 2022 10:28:01 -0500 Subject: [PATCH 3/5] Don't fail on bad baggage --- propagators/ot/ot_data_test.go | 15 +++++++++++++++ propagators/ot/ot_propagator.go | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/propagators/ot/ot_data_test.go b/propagators/ot/ot_data_test.go index 3665434b26d..00ce4412c91 100644 --- a/propagators/ot/ot_data_test.go +++ b/propagators/ot/ot_data_test.go @@ -193,6 +193,11 @@ var invalidExtractHeaders = []extractTest{ sampledHeader: "1", "ot-baggage-d–76": "test", }, + expected: trace.SpanContextConfig{ + TraceID: traceID32, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + }, }, { name: "invalid baggage value", @@ -202,6 +207,11 @@ var invalidExtractHeaders = []extractTest{ sampledHeader: "1", baggageHeader: "øtel", }, + expected: trace.SpanContextConfig{ + TraceID: traceID32, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + }, }, { name: "invalid baggage result (too large)", @@ -211,6 +221,11 @@ var invalidExtractHeaders = []extractTest{ sampledHeader: "1", baggageHeader: strings.Repeat("s", 8188), }, + expected: trace.SpanContextConfig{ + TraceID: traceID32, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + }, }, { name: "missing headers", diff --git a/propagators/ot/ot_propagator.go b/propagators/ot/ot_propagator.go index 66a10fe793e..13c8f6603ae 100644 --- a/propagators/ot/ot_propagator.go +++ b/propagators/ot/ot_propagator.go @@ -97,7 +97,7 @@ func (o OT) Extract(ctx context.Context, carrier propagation.TextMapCarrier) con bags, err := extractBags(carrier) if err != nil { - return ctx + return trace.ContextWithRemoteSpanContext(ctx, sc) } ctx = baggage.ContextWithBaggage(ctx, bags) return trace.ContextWithRemoteSpanContext(ctx, sc) From 7210e0a716e743b3cd1b451ac7142bdac54d22ce Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Tue, 8 Mar 2022 15:43:15 -0500 Subject: [PATCH 4/5] Update PR to construct a multi error --- propagators/ot/go.mod | 1 + propagators/ot/go.sum | 11 +++++++++-- propagators/ot/ot_propagator.go | 19 +++++++++++-------- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/propagators/ot/go.mod b/propagators/ot/go.mod index b38661ec494..a7781b4f2c3 100644 --- a/propagators/ot/go.mod +++ b/propagators/ot/go.mod @@ -7,4 +7,5 @@ require ( github.com/stretchr/testify v1.7.0 go.opentelemetry.io/otel v1.4.1 go.opentelemetry.io/otel/trace v1.4.1 + go.uber.org/multierr v1.8.0 ) diff --git a/propagators/ot/go.sum b/propagators/ot/go.sum index 8951479d125..b9066108f7c 100644 --- a/propagators/ot/go.sum +++ b/propagators/ot/go.sum @@ -1,5 +1,6 @@ -github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/go-logr/logr v1.2.2 h1:ahHml/yUpnlb96Rp8HCvtYVPY8ZYpxq3g7UYchIYwbs= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= @@ -9,15 +10,21 @@ github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= go.opentelemetry.io/otel v1.4.1 h1:QbINgGDDcoQUoMJa2mMaWno49lja9sHwp6aoa2n3a4g= go.opentelemetry.io/otel v1.4.1/go.mod h1:StM6F/0fSwpd8dKWDCdRr7uRvEPYdW0hBSlbdTiUde4= go.opentelemetry.io/otel/trace v1.4.1 h1:O+16qcdTrT7zxv2J6GejTPFinSwA++cYerC5iSiF8EQ= go.opentelemetry.io/otel/trace v1.4.1/go.mod h1:iYEVbroFCNut9QkwEczV9vMRPHNKSSwYZjulEtsmhFc= +go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw= +go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= +go.uber.org/multierr v1.8.0 h1:dg6GjLku4EH+249NNmoIciG9N/jURbDG+pFlTkhzIC8= +go.uber.org/multierr v1.8.0/go.mod h1:7EAYxJLBy9rStEaz58O2t4Uvip6FSURkq8/ppBp95ak= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b h1:h8qDotaEPuJATrMmW04NCwg7v22aHH28wwpauUhK9Oo= +gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/propagators/ot/ot_propagator.go b/propagators/ot/ot_propagator.go index 13c8f6603ae..b98d9a1632e 100644 --- a/propagators/ot/ot_propagator.go +++ b/propagators/ot/ot_propagator.go @@ -20,6 +20,8 @@ import ( "fmt" "strings" + "go.uber.org/multierr" + "go.opentelemetry.io/otel/baggage" "go.opentelemetry.io/otel/propagation" @@ -107,9 +109,9 @@ func (o OT) Fields() []string { return []string{traceIDHeader, spanIDHeader, sampledHeader} } -// extractBags reconstructs the baggage information from opentracing +// extractBags extracts OpenTracing baggage information from carrier. func extractBags(carrier propagation.TextMapCarrier) (baggage.Baggage, error) { - emptyBags, _ := baggage.New() + var err error var members []baggage.Member for _, key := range carrier.Keys() { lowerKey := strings.ToLower(key) @@ -117,17 +119,18 @@ func extractBags(carrier propagation.TextMapCarrier) (baggage.Baggage, error) { continue } strippedKey := strings.TrimPrefix(lowerKey, baggageHeaderPrefix) - member, err := baggage.NewMember(strippedKey, carrier.Get(key)) - if err != nil { - return emptyBags, err + member, e := baggage.NewMember(strippedKey, carrier.Get(key)) + if e != nil { + err = multierr.Append(err, e) + continue } members = append(members, member) } - bags, err := baggage.New(members...) + bags, e := baggage.New(members...) if err != nil { - return emptyBags, err + return bags, multierr.Append(err, e) } - return bags, nil + return bags, err } // extract reconstructs a SpanContext from header values based on OT From 8b1c7c085be8500bd9acb1d2c970a1000123d5e2 Mon Sep 17 00:00:00 2001 From: Anthony Mirabella Date: Wed, 9 Mar 2022 01:55:08 -0500 Subject: [PATCH 5/5] Update propagators/ot/ot_propagator.go Co-authored-by: Chester Cheung --- propagators/ot/ot_propagator.go | 1 - 1 file changed, 1 deletion(-) diff --git a/propagators/ot/ot_propagator.go b/propagators/ot/ot_propagator.go index b98d9a1632e..9d340790138 100644 --- a/propagators/ot/ot_propagator.go +++ b/propagators/ot/ot_propagator.go @@ -23,7 +23,6 @@ import ( "go.uber.org/multierr" "go.opentelemetry.io/otel/baggage" - "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/trace" )