From d174f0d311c9ef391b36b34bf3598453576a1dc7 Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Wed, 16 Mar 2016 15:45:33 -0700 Subject: [PATCH 1/4] Migrate to the simpler/revised carriers --- propagation_ot.go | 177 +++++++++++++++++++++------------------------- tracer.go | 18 ++--- 2 files changed, 88 insertions(+), 107 deletions(-) diff --git a/propagation_ot.go b/propagation_ot.go index 78f6cf05afb4..47b6f42470aa 100644 --- a/propagation_ot.go +++ b/propagation_ot.go @@ -2,10 +2,10 @@ package basictracer import ( "bytes" - "encoding/base64" "encoding/binary" "io" "net/http" + "net/url" "strconv" "strings" "time" @@ -13,14 +13,14 @@ import ( opentracing "github.com/opentracing/opentracing-go" ) -type splitTextPropagator struct { +type textMapPropagator struct { tracer *tracerImpl } -type splitBinaryPropagator struct { +type binaryPropagator struct { tracer *tracerImpl } type goHTTPPropagator struct { - *splitBinaryPropagator + *textMapPropagator } const ( @@ -33,41 +33,35 @@ const ( fieldNameSampled = prefixTracerState + "sampled" ) -func (p *splitTextPropagator) Inject( +func (p *textMapPropagator) Inject( sp opentracing.Span, - carrier interface{}, + opaqueCarrier interface{}, ) error { sc, ok := sp.(*spanImpl) if !ok { return opentracing.ErrInvalidSpan } - splitTextCarrier, ok := carrier.(*opentracing.SplitTextCarrier) + carrier, ok := opaqueCarrier.(opentracing.TextMapCarrier) if !ok { return opentracing.ErrInvalidCarrier } - if splitTextCarrier.TracerState == nil { - splitTextCarrier.TracerState = make(map[string]string, tracerStateFieldCount) - } - splitTextCarrier.TracerState[fieldNameTraceID] = strconv.FormatInt(sc.raw.TraceID, 16) - splitTextCarrier.TracerState[fieldNameSpanID] = strconv.FormatInt(sc.raw.SpanID, 16) - splitTextCarrier.TracerState[fieldNameSampled] = strconv.FormatBool(sc.raw.Sampled) + carrier[fieldNameTraceID] = strconv.FormatInt(sc.raw.TraceID, 16) + carrier[fieldNameSpanID] = strconv.FormatInt(sc.raw.SpanID, 16) + carrier[fieldNameSampled] = strconv.FormatBool(sc.raw.Sampled) sc.Lock() - if l := len(sc.raw.Baggage); l > 0 && splitTextCarrier.Baggage == nil { - splitTextCarrier.Baggage = make(map[string]string, l) - } for k, v := range sc.raw.Baggage { - splitTextCarrier.Baggage[prefixBaggage+k] = v + carrier[prefixBaggage+k] = v } sc.Unlock() return nil } -func (p *splitTextPropagator) Join( +func (p *textMapPropagator) Join( operationName string, - carrier interface{}, + opaqueCarrier interface{}, ) (opentracing.Span, error) { - splitTextCarrier, ok := carrier.(*opentracing.SplitTextCarrier) + carrier, ok := opaqueCarrier.(opentracing.TextMapCarrier) if !ok { return nil, opentracing.ErrInvalidCarrier } @@ -75,7 +69,8 @@ func (p *splitTextPropagator) Join( var traceID, propagatedSpanID int64 var sampled bool var err error - for k, v := range splitTextCarrier.TracerState { + decodedBaggage := make(map[string]string) + for k, v := range carrier { switch strings.ToLower(k) { case fieldNameTraceID: traceID, err = strconv.ParseInt(v, 16, 64) @@ -93,22 +88,17 @@ func (p *splitTextPropagator) Join( return nil, opentracing.ErrTraceCorrupted } default: - continue - } - requiredFieldCount++ - } - var decodedBaggage map[string]string - if splitTextCarrier.Baggage != nil { - decodedBaggage = make(map[string]string) - for k, v := range splitTextCarrier.Baggage { lowercaseK := strings.ToLower(k) if strings.HasPrefix(lowercaseK, prefixBaggage) { decodedBaggage[strings.TrimPrefix(lowercaseK, prefixBaggage)] = v } + // Balance off the requiredFieldCount++ just below... + requiredFieldCount-- } + requiredFieldCount++ } if requiredFieldCount < tracerStateFieldCount { - if len(splitTextCarrier.TracerState) == 0 { + if requiredFieldCount == 0 { return nil, opentracing.ErrTraceNotFound } return nil, opentracing.ErrTraceCorrupted @@ -133,18 +123,19 @@ func (p *splitTextPropagator) Join( ), nil } -func (p *splitBinaryPropagator) Inject( +func (p *binaryPropagator) Inject( sp opentracing.Span, - carrier interface{}, + opaqueCarrier interface{}, ) error { sc, ok := sp.(*spanImpl) if !ok { return opentracing.ErrInvalidSpan } - splitBinaryCarrier, ok := carrier.(*opentracing.SplitBinaryCarrier) + carrier, ok := opaqueCarrier.(opentracing.BinaryCarrier) if !ok { return opentracing.ErrInvalidCarrier } + buffer := &bytes.Buffer{} var err error var sampledByte byte if sc.raw.Sampled { @@ -152,74 +143,75 @@ func (p *splitBinaryPropagator) Inject( } // Handle the trace and span ids, and sampled status. - contextBuf := bytes.NewBuffer(splitBinaryCarrier.TracerState[:0]) - err = binary.Write(contextBuf, binary.BigEndian, sc.raw.TraceID) + err = binary.Write(buffer, binary.BigEndian, sc.raw.TraceID) if err != nil { return err } - err = binary.Write(contextBuf, binary.BigEndian, sc.raw.SpanID) + err = binary.Write(buffer, binary.BigEndian, sc.raw.SpanID) if err != nil { return err } - err = binary.Write(contextBuf, binary.BigEndian, sampledByte) + err = binary.Write(buffer, binary.BigEndian, sampledByte) if err != nil { return err } // Handle the baggage. - baggageBuf := bytes.NewBuffer(splitBinaryCarrier.Baggage[:0]) - err = binary.Write(baggageBuf, binary.BigEndian, int32(len(sc.raw.Baggage))) + err = binary.Write(buffer, binary.BigEndian, int32(len(sc.raw.Baggage))) if err != nil { return err } for k, v := range sc.raw.Baggage { - if err = binary.Write(baggageBuf, binary.BigEndian, int32(len(k))); err != nil { + if err = binary.Write(buffer, binary.BigEndian, int32(len(k))); err != nil { return err } - baggageBuf.WriteString(k) - if err = binary.Write(baggageBuf, binary.BigEndian, int32(len(v))); err != nil { + buffer.WriteString(k) + if err = binary.Write(buffer, binary.BigEndian, int32(len(v))); err != nil { return err } - baggageBuf.WriteString(v) + buffer.WriteString(v) } - splitBinaryCarrier.TracerState = contextBuf.Bytes() - splitBinaryCarrier.Baggage = baggageBuf.Bytes() + // Write out to the carrier. + if carrier == nil { + // Allocate if needed. + carrier = &([]byte{}) + } + *carrier = buffer.Bytes() return nil } -func (p *splitBinaryPropagator) Join( +func (p *binaryPropagator) Join( operationName string, - carrier interface{}, + opaqueCarrier interface{}, ) (opentracing.Span, error) { - splitBinaryCarrier, ok := carrier.(*opentracing.SplitBinaryCarrier) + carrier, ok := opaqueCarrier.(opentracing.BinaryCarrier) if !ok { return nil, opentracing.ErrInvalidCarrier } - if len(splitBinaryCarrier.TracerState) == 0 { + if len(*carrier) == 0 { return nil, opentracing.ErrTraceNotFound } // Handle the trace, span ids, and sampled status. - contextReader := bytes.NewReader(splitBinaryCarrier.TracerState) + reader := bytes.NewReader(*carrier) var traceID, propagatedSpanID int64 var sampledByte byte - if err := binary.Read(contextReader, binary.BigEndian, &traceID); err != nil { + if err := binary.Read(reader, binary.BigEndian, &traceID); err != nil { return nil, opentracing.ErrTraceCorrupted } - if err := binary.Read(contextReader, binary.BigEndian, &propagatedSpanID); err != nil { + if err := binary.Read(reader, binary.BigEndian, &propagatedSpanID); err != nil { return nil, opentracing.ErrTraceCorrupted } - if err := binary.Read(contextReader, binary.BigEndian, &sampledByte); err != nil { + if err := binary.Read(reader, binary.BigEndian, &sampledByte); err != nil { return nil, opentracing.ErrTraceCorrupted } // Handle the baggage. - baggageReader := bytes.NewReader(splitBinaryCarrier.Baggage) var numBaggage int32 - if err := binary.Read(baggageReader, binary.BigEndian, &numBaggage); err != nil { + if err := binary.Read(reader, binary.BigEndian, &numBaggage); err != nil { return nil, opentracing.ErrTraceCorrupted } iNumBaggage := int(numBaggage) @@ -229,20 +221,20 @@ func (p *splitBinaryPropagator) Join( baggageMap = make(map[string]string, iNumBaggage) var keyLen, valLen int32 for i := 0; i < iNumBaggage; i++ { - if err := binary.Read(baggageReader, binary.BigEndian, &keyLen); err != nil { + if err := binary.Read(reader, binary.BigEndian, &keyLen); err != nil { return nil, opentracing.ErrTraceCorrupted } buf.Grow(int(keyLen)) - if n, err := io.CopyN(&buf, baggageReader, int64(keyLen)); err != nil || int32(n) != keyLen { + if n, err := io.CopyN(&buf, reader, int64(keyLen)); err != nil || int32(n) != keyLen { return nil, opentracing.ErrTraceCorrupted } key := buf.String() buf.Reset() - if err := binary.Read(baggageReader, binary.BigEndian, &valLen); err != nil { + if err := binary.Read(reader, binary.BigEndian, &valLen); err != nil { return nil, opentracing.ErrTraceCorrupted } - if n, err := io.CopyN(&buf, baggageReader, int64(valLen)); err != nil || int32(n) != valLen { + if n, err := io.CopyN(&buf, reader, int64(valLen)); err != nil || int32(n) != valLen { return nil, opentracing.ErrTraceCorrupted } baggageMap[key] = buf.String() @@ -269,58 +261,47 @@ func (p *splitBinaryPropagator) Join( ), nil } -const ( - tracerStateHeaderName = "Tracer-State" - traceBaggageHeaderName = "Trace-Baggage" -) - func (p *goHTTPPropagator) Inject( sp opentracing.Span, - carrier interface{}, + opaqueCarrier interface{}, ) error { - // Defer to SplitBinary for the real work. - splitBinaryCarrier := opentracing.NewSplitBinaryCarrier() - if err := p.splitBinaryPropagator.Inject(sp, splitBinaryCarrier); err != nil { - return err + headerCarrier, ok := opaqueCarrier.(http.Header) + if !ok { + return opentracing.ErrInvalidCarrier } - // Encode into the HTTP header as two base64 strings. - header := carrier.(http.Header) - header.Add(tracerStateHeaderName, base64.StdEncoding.EncodeToString( - splitBinaryCarrier.TracerState)) - header.Add(traceBaggageHeaderName, base64.StdEncoding.EncodeToString( - splitBinaryCarrier.Baggage)) + // Defer to TextMapCarrier for the real work. + textMapCarrier := opentracing.TextMapCarrier{} + if err := p.textMapPropagator.Inject(sp, textMapCarrier); err != nil { + return err + } + // Encode as URL-escaped HTTP header vals. + for headerKey, headerVal := range textMapCarrier { + headerCarrier.Add(headerKey, url.QueryEscape(headerVal)) + } return nil } func (p *goHTTPPropagator) Join( operationName string, - carrier interface{}, + opaqueCarrier interface{}, ) (opentracing.Span, error) { - // Decode the two base64-encoded data blobs from the HTTP header. - header := carrier.(http.Header) - tracerStateBase64, found := header[http.CanonicalHeaderKey(tracerStateHeaderName)] - if !found || len(tracerStateBase64) == 0 { - return nil, opentracing.ErrTraceNotFound - } - traceBaggageBase64, found := header[http.CanonicalHeaderKey(traceBaggageHeaderName)] - if !found || len(traceBaggageBase64) == 0 { - return nil, opentracing.ErrTraceNotFound - } - tracerStateBinary, err := base64.StdEncoding.DecodeString(tracerStateBase64[0]) - if err != nil { - return nil, opentracing.ErrTraceCorrupted - } - traceBaggageBinary, err := base64.StdEncoding.DecodeString(traceBaggageBase64[0]) - if err != nil { - return nil, opentracing.ErrTraceCorrupted + headerCarrier, ok := opaqueCarrier.(http.Header) + if !ok { + return nil, opentracing.ErrInvalidCarrier } - // Defer to SplitBinary for the real work. - splitBinaryCarrier := &opentracing.SplitBinaryCarrier{ - TracerState: tracerStateBinary, - Baggage: traceBaggageBinary, + // Build a TextMapCarrier from the string->[]string http.Header map. + textCarrier := make(opentracing.TextMapCarrier, len(headerCarrier)) + for k, vals := range headerCarrier { + // We don't know what to do with anything beyond slice item v[0]: + unescaped, err := url.QueryUnescape(vals[0]) + if err != nil { + continue + } + textCarrier[k] = unescaped } - return p.splitBinaryPropagator.Join(operationName, splitBinaryCarrier) + // Defer to textMapCarrier for the rest of the work. + return p.textMapPropagator.Join(operationName, textCarrier) } diff --git a/tracer.go b/tracer.go index fc01cf26b9eb..55af86402d58 100644 --- a/tracer.go +++ b/tracer.go @@ -79,9 +79,9 @@ func DefaultOptions() Options { // NewWithOptions creates a customized Tracer. func NewWithOptions(opts Options) opentracing.Tracer { rval := &tracerImpl{Options: opts} - rval.textPropagator = &splitTextPropagator{rval} - rval.binaryPropagator = &splitBinaryPropagator{rval} - rval.goHTTPPropagator = &goHTTPPropagator{rval.binaryPropagator} + rval.textPropagator = &textMapPropagator{rval} + rval.binaryPropagator = &binaryPropagator{rval} + rval.goHTTPPropagator = &goHTTPPropagator{rval.textPropagator} rval.accessorPropagator = &accessorPropagator{rval} return rval } @@ -99,8 +99,8 @@ func New(recorder SpanRecorder) opentracing.Tracer { // Implements the `Tracer` interface. type tracerImpl struct { Options - textPropagator *splitTextPropagator - binaryPropagator *splitBinaryPropagator + textPropagator *textMapPropagator + binaryPropagator *binaryPropagator goHTTPPropagator *goHTTPPropagator accessorPropagator *accessorPropagator } @@ -189,9 +189,9 @@ var Delegator delegatorType func (t *tracerImpl) Inject(sp opentracing.Span, format interface{}, carrier interface{}) error { switch format { - case opentracing.SplitText: + case opentracing.TextMap: return t.textPropagator.Inject(sp, carrier) - case opentracing.SplitBinary: + case opentracing.Binary: return t.binaryPropagator.Inject(sp, carrier) case opentracing.GoHTTPHeader: return t.goHTTPPropagator.Inject(sp, carrier) @@ -204,9 +204,9 @@ func (t *tracerImpl) Inject(sp opentracing.Span, format interface{}, carrier int func (t *tracerImpl) Join(operationName string, format interface{}, carrier interface{}) (opentracing.Span, error) { switch format { - case opentracing.SplitText: + case opentracing.TextMap: return t.textPropagator.Join(operationName, carrier) - case opentracing.SplitBinary: + case opentracing.Binary: return t.binaryPropagator.Join(operationName, carrier) case opentracing.GoHTTPHeader: return t.goHTTPPropagator.Join(operationName, carrier) From 2182d8e8c64ca6d9b6ca7df82f529bd2266e7613 Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Wed, 16 Mar 2016 23:08:24 -0700 Subject: [PATCH 2/4] Move to interface-based carriers --- propagation_ot.go | 90 +++++++++++++++++++---------------------------- 1 file changed, 37 insertions(+), 53 deletions(-) diff --git a/propagation_ot.go b/propagation_ot.go index 47b6f42470aa..354191ab6b80 100644 --- a/propagation_ot.go +++ b/propagation_ot.go @@ -5,7 +5,6 @@ import ( "encoding/binary" "io" "net/http" - "net/url" "strconv" "strings" "time" @@ -45,13 +44,13 @@ func (p *textMapPropagator) Inject( if !ok { return opentracing.ErrInvalidCarrier } - carrier[fieldNameTraceID] = strconv.FormatInt(sc.raw.TraceID, 16) - carrier[fieldNameSpanID] = strconv.FormatInt(sc.raw.SpanID, 16) - carrier[fieldNameSampled] = strconv.FormatBool(sc.raw.Sampled) + carrier.Add(fieldNameTraceID, strconv.FormatInt(sc.raw.TraceID, 16)) + carrier.Add(fieldNameSpanID, strconv.FormatInt(sc.raw.SpanID, 16)) + carrier.Add(fieldNameSampled, strconv.FormatBool(sc.raw.Sampled)) sc.Lock() for k, v := range sc.raw.Baggage { - carrier[prefixBaggage+k] = v + carrier.Add(prefixBaggage+k, v) } sc.Unlock() return nil @@ -70,22 +69,22 @@ func (p *textMapPropagator) Join( var sampled bool var err error decodedBaggage := make(map[string]string) - for k, v := range carrier { + err = carrier.GetAll(func(k, v string) error { switch strings.ToLower(k) { case fieldNameTraceID: traceID, err = strconv.ParseInt(v, 16, 64) if err != nil { - return nil, opentracing.ErrTraceCorrupted + return opentracing.ErrTraceCorrupted } case fieldNameSpanID: propagatedSpanID, err = strconv.ParseInt(v, 16, 64) if err != nil { - return nil, opentracing.ErrTraceCorrupted + return opentracing.ErrTraceCorrupted } case fieldNameSampled: sampled, err = strconv.ParseBool(v) if err != nil { - return nil, opentracing.ErrTraceCorrupted + return opentracing.ErrTraceCorrupted } default: lowercaseK := strings.ToLower(k) @@ -96,6 +95,10 @@ func (p *textMapPropagator) Join( requiredFieldCount-- } requiredFieldCount++ + return nil + }) + if err != nil { + return nil, err } if requiredFieldCount < tracerStateFieldCount { if requiredFieldCount == 0 { @@ -135,7 +138,6 @@ func (p *binaryPropagator) Inject( if !ok { return opentracing.ErrInvalidCarrier } - buffer := &bytes.Buffer{} var err error var sampledByte byte if sc.raw.Sampled { @@ -143,43 +145,42 @@ func (p *binaryPropagator) Inject( } // Handle the trace and span ids, and sampled status. - err = binary.Write(buffer, binary.BigEndian, sc.raw.TraceID) + err = binary.Write(carrier, binary.BigEndian, sc.raw.TraceID) if err != nil { return err } - err = binary.Write(buffer, binary.BigEndian, sc.raw.SpanID) + err = binary.Write(carrier, binary.BigEndian, sc.raw.SpanID) if err != nil { return err } - err = binary.Write(buffer, binary.BigEndian, sampledByte) + err = binary.Write(carrier, binary.BigEndian, sampledByte) if err != nil { return err } // Handle the baggage. - err = binary.Write(buffer, binary.BigEndian, int32(len(sc.raw.Baggage))) + err = binary.Write(carrier, binary.BigEndian, int32(len(sc.raw.Baggage))) if err != nil { return err } - for k, v := range sc.raw.Baggage { - if err = binary.Write(buffer, binary.BigEndian, int32(len(k))); err != nil { + for key, val := range sc.raw.Baggage { + if err = binary.Write(carrier, binary.BigEndian, int32(len(key))); err != nil { return err } - buffer.WriteString(k) - if err = binary.Write(buffer, binary.BigEndian, int32(len(v))); err != nil { + if _, err = io.WriteString(carrier, key); err != nil { return err } - buffer.WriteString(v) - } - // Write out to the carrier. - if carrier == nil { - // Allocate if needed. - carrier = &([]byte{}) + if err = binary.Write(carrier, binary.BigEndian, int32(len(val))); err != nil { + return err + } + if _, err = io.WriteString(carrier, val); err != nil { + return err + } } - *carrier = buffer.Bytes() + return nil } @@ -191,27 +192,23 @@ func (p *binaryPropagator) Join( if !ok { return nil, opentracing.ErrInvalidCarrier } - if len(*carrier) == 0 { - return nil, opentracing.ErrTraceNotFound - } // Handle the trace, span ids, and sampled status. - reader := bytes.NewReader(*carrier) var traceID, propagatedSpanID int64 var sampledByte byte - if err := binary.Read(reader, binary.BigEndian, &traceID); err != nil { + if err := binary.Read(carrier, binary.BigEndian, &traceID); err != nil { return nil, opentracing.ErrTraceCorrupted } - if err := binary.Read(reader, binary.BigEndian, &propagatedSpanID); err != nil { + if err := binary.Read(carrier, binary.BigEndian, &propagatedSpanID); err != nil { return nil, opentracing.ErrTraceCorrupted } - if err := binary.Read(reader, binary.BigEndian, &sampledByte); err != nil { + if err := binary.Read(carrier, binary.BigEndian, &sampledByte); err != nil { return nil, opentracing.ErrTraceCorrupted } // Handle the baggage. var numBaggage int32 - if err := binary.Read(reader, binary.BigEndian, &numBaggage); err != nil { + if err := binary.Read(carrier, binary.BigEndian, &numBaggage); err != nil { return nil, opentracing.ErrTraceCorrupted } iNumBaggage := int(numBaggage) @@ -221,20 +218,20 @@ func (p *binaryPropagator) Join( baggageMap = make(map[string]string, iNumBaggage) var keyLen, valLen int32 for i := 0; i < iNumBaggage; i++ { - if err := binary.Read(reader, binary.BigEndian, &keyLen); err != nil { + if err := binary.Read(carrier, binary.BigEndian, &keyLen); err != nil { return nil, opentracing.ErrTraceCorrupted } buf.Grow(int(keyLen)) - if n, err := io.CopyN(&buf, reader, int64(keyLen)); err != nil || int32(n) != keyLen { + if n, err := io.CopyN(&buf, carrier, int64(keyLen)); err != nil || int32(n) != keyLen { return nil, opentracing.ErrTraceCorrupted } key := buf.String() buf.Reset() - if err := binary.Read(reader, binary.BigEndian, &valLen); err != nil { + if err := binary.Read(carrier, binary.BigEndian, &valLen); err != nil { return nil, opentracing.ErrTraceCorrupted } - if n, err := io.CopyN(&buf, reader, int64(valLen)); err != nil || int32(n) != valLen { + if n, err := io.CopyN(&buf, carrier, int64(valLen)); err != nil || int32(n) != valLen { return nil, opentracing.ErrTraceCorrupted } baggageMap[key] = buf.String() @@ -271,15 +268,10 @@ func (p *goHTTPPropagator) Inject( } // Defer to TextMapCarrier for the real work. - textMapCarrier := opentracing.TextMapCarrier{} + textMapCarrier := opentracing.HTTPHeaderTextMapCarrier{headerCarrier} if err := p.textMapPropagator.Inject(sp, textMapCarrier); err != nil { return err } - // Encode as URL-escaped HTTP header vals. - for headerKey, headerVal := range textMapCarrier { - headerCarrier.Add(headerKey, url.QueryEscape(headerVal)) - } - return nil } @@ -293,15 +285,7 @@ func (p *goHTTPPropagator) Join( } // Build a TextMapCarrier from the string->[]string http.Header map. - textCarrier := make(opentracing.TextMapCarrier, len(headerCarrier)) - for k, vals := range headerCarrier { - // We don't know what to do with anything beyond slice item v[0]: - unescaped, err := url.QueryUnescape(vals[0]) - if err != nil { - continue - } - textCarrier[k] = unescaped - } + textMapCarrier := opentracing.HTTPHeaderTextMapCarrier{headerCarrier} // Defer to textMapCarrier for the rest of the work. - return p.textMapPropagator.Join(operationName, textCarrier) + return p.textMapPropagator.Join(operationName, textMapCarrier) } From 85f1a62e4f59502a5566570f8f1ac3b643af9cdd Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Fri, 18 Mar 2016 17:18:26 -0700 Subject: [PATCH 3/4] Adapt to interface proposals in opentracing-go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Here is benchmark data vs master: ``` benchmark old ns/op new ns/op delta BenchmarkSpan_Empty-8 450 449 -0.22% BenchmarkSpan_100Events-8 34384 34367 -0.05% BenchmarkSpan_1000Events-8 34812 34973 +0.46% BenchmarkSpan_100Tags-8 43703 42846 -1.96% BenchmarkSpan_1000Tags-8 43114 43155 +0.10% BenchmarkSpan_100BaggageItems-8 142956 138515 -3.11% BenchmarkTrimmedSpan_100Events_100Tags_100BaggageItems-8 166500 161684 -2.89% BenchmarkInject_TextMap_Empty-8 418 2157 +416.03% BenchmarkInject_TextMap_100BaggageItems-8 18617 78072 +319.36% BenchmarkInject_Binary_Empty-8 991 770 -22.30% BenchmarkInject_Binary_100BaggageItems-8 41699 49336 +18.31% BenchmarkJoin_TextMap_Empty-8 1280 2696 +110.63% BenchmarkJoin_TextMap_100BaggageItems-8 50890 143159 +181.31% BenchmarkJoin_Binary_Empty-8 1223 1150 -5.97% BenchmarkJoin_Binary_100BaggageItems-8 78066 78061 -0.01% benchmark old allocs new allocs delta BenchmarkInject_TextMap_Empty-8 3 6 +100.00% BenchmarkInject_TextMap_100BaggageItems-8 103 206 +100.00% BenchmarkInject_Binary_Empty-8 11 9 -18.18% BenchmarkInject_Binary_100BaggageItems-8 411 409 -0.49% BenchmarkJoin_TextMap_Empty-8 1 14 +1300.00% BenchmarkJoin_TextMap_100BaggageItems-8 12 424 +3433.33% BenchmarkJoin_Binary_Empty-8 11 10 -9.09% BenchmarkJoin_Binary_100BaggageItems-8 619 618 -0.16% benchmark old bytes new bytes delta BenchmarkSpan_100Tags-8 10519 10520 +0.01% BenchmarkSpan_100BaggageItems-8 10502 10520 +0.17% BenchmarkInject_TextMap_Empty-8 33 378 +1045.45% BenchmarkInject_TextMap_100BaggageItems-8 3233 15666 +384.57% BenchmarkInject_Binary_Empty-8 296 147 -50.34% BenchmarkInject_Binary_100BaggageItems-8 3496 14667 +319.54% BenchmarkJoin_TextMap_Empty-8 1 320 +31900.00% BenchmarkJoin_TextMap_100BaggageItems-8 10518 23594 +124.32% BenchmarkJoin_Binary_Empty-8 168 184 +9.52% BenchmarkJoin_Binary_100BaggageItems-8 20793 20809 +0.08% ``` Some of these regressions can be improved by using a map[string]string instead of an actual http.Header object as the "backend" for the TextMap. The binary protocol at master is not worth thinking too carefully about since it's not even using protobufs/etc. But these do demonstrate that the old API – while clunky – at least didn't have many moving parts (or allocations). --- propagation_ot.go | 46 ++++++++-------------------------------------- tracer.go | 6 ------ 2 files changed, 8 insertions(+), 44 deletions(-) diff --git a/propagation_ot.go b/propagation_ot.go index 354191ab6b80..162126fc4384 100644 --- a/propagation_ot.go +++ b/propagation_ot.go @@ -4,7 +4,6 @@ import ( "bytes" "encoding/binary" "io" - "net/http" "strconv" "strings" "time" @@ -40,7 +39,7 @@ func (p *textMapPropagator) Inject( if !ok { return opentracing.ErrInvalidSpan } - carrier, ok := opaqueCarrier.(opentracing.TextMapCarrier) + carrier, ok := opaqueCarrier.(opentracing.TextMapWriter) if !ok { return opentracing.ErrInvalidCarrier } @@ -60,7 +59,7 @@ func (p *textMapPropagator) Join( operationName string, opaqueCarrier interface{}, ) (opentracing.Span, error) { - carrier, ok := opaqueCarrier.(opentracing.TextMapCarrier) + carrier, ok := opaqueCarrier.(opentracing.TextMapReader) if !ok { return nil, opentracing.ErrInvalidCarrier } @@ -69,7 +68,7 @@ func (p *textMapPropagator) Join( var sampled bool var err error decodedBaggage := make(map[string]string) - err = carrier.GetAll(func(k, v string) error { + err = carrier.ReadAll(func(k, v string) error { switch strings.ToLower(k) { case fieldNameTraceID: traceID, err = strconv.ParseInt(v, 16, 64) @@ -134,7 +133,7 @@ func (p *binaryPropagator) Inject( if !ok { return opentracing.ErrInvalidSpan } - carrier, ok := opaqueCarrier.(opentracing.BinaryCarrier) + carrier, ok := opaqueCarrier.(io.Writer) if !ok { return opentracing.ErrInvalidCarrier } @@ -188,7 +187,7 @@ func (p *binaryPropagator) Join( operationName string, opaqueCarrier interface{}, ) (opentracing.Span, error) { - carrier, ok := opaqueCarrier.(opentracing.BinaryCarrier) + carrier, ok := opaqueCarrier.(io.Reader) if !ok { return nil, opentracing.ErrInvalidCarrier } @@ -197,6 +196,9 @@ func (p *binaryPropagator) Join( var sampledByte byte if err := binary.Read(carrier, binary.BigEndian, &traceID); err != nil { + if err == io.EOF { + return nil, opentracing.ErrTraceNotFound + } return nil, opentracing.ErrTraceCorrupted } if err := binary.Read(carrier, binary.BigEndian, &propagatedSpanID); err != nil { @@ -257,35 +259,3 @@ func (p *binaryPropagator) Join( nil, ), nil } - -func (p *goHTTPPropagator) Inject( - sp opentracing.Span, - opaqueCarrier interface{}, -) error { - headerCarrier, ok := opaqueCarrier.(http.Header) - if !ok { - return opentracing.ErrInvalidCarrier - } - - // Defer to TextMapCarrier for the real work. - textMapCarrier := opentracing.HTTPHeaderTextMapCarrier{headerCarrier} - if err := p.textMapPropagator.Inject(sp, textMapCarrier); err != nil { - return err - } - return nil -} - -func (p *goHTTPPropagator) Join( - operationName string, - opaqueCarrier interface{}, -) (opentracing.Span, error) { - headerCarrier, ok := opaqueCarrier.(http.Header) - if !ok { - return nil, opentracing.ErrInvalidCarrier - } - - // Build a TextMapCarrier from the string->[]string http.Header map. - textMapCarrier := opentracing.HTTPHeaderTextMapCarrier{headerCarrier} - // Defer to textMapCarrier for the rest of the work. - return p.textMapPropagator.Join(operationName, textMapCarrier) -} diff --git a/tracer.go b/tracer.go index 55af86402d58..938957c4d055 100644 --- a/tracer.go +++ b/tracer.go @@ -81,7 +81,6 @@ func NewWithOptions(opts Options) opentracing.Tracer { rval := &tracerImpl{Options: opts} rval.textPropagator = &textMapPropagator{rval} rval.binaryPropagator = &binaryPropagator{rval} - rval.goHTTPPropagator = &goHTTPPropagator{rval.textPropagator} rval.accessorPropagator = &accessorPropagator{rval} return rval } @@ -101,7 +100,6 @@ type tracerImpl struct { Options textPropagator *textMapPropagator binaryPropagator *binaryPropagator - goHTTPPropagator *goHTTPPropagator accessorPropagator *accessorPropagator } @@ -193,8 +191,6 @@ func (t *tracerImpl) Inject(sp opentracing.Span, format interface{}, carrier int return t.textPropagator.Inject(sp, carrier) case opentracing.Binary: return t.binaryPropagator.Inject(sp, carrier) - case opentracing.GoHTTPHeader: - return t.goHTTPPropagator.Inject(sp, carrier) } if _, ok := format.(delegatorType); ok { return t.accessorPropagator.Inject(sp, carrier) @@ -208,8 +204,6 @@ func (t *tracerImpl) Join(operationName string, format interface{}, carrier inte return t.textPropagator.Join(operationName, carrier) case opentracing.Binary: return t.binaryPropagator.Join(operationName, carrier) - case opentracing.GoHTTPHeader: - return t.goHTTPPropagator.Join(operationName, carrier) } if _, ok := format.(delegatorType); ok { return t.accessorPropagator.Join(operationName, carrier) From 2bd87dddb64d917e8438834d14b8496c48e8f1b4 Mon Sep 17 00:00:00 2001 From: Ben Sigelman Date: Sun, 20 Mar 2016 21:04:32 -0700 Subject: [PATCH 4/4] Follow suit with the latest OT-go revisions --- propagation_ot.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/propagation_ot.go b/propagation_ot.go index 162126fc4384..1f94ccc8a09e 100644 --- a/propagation_ot.go +++ b/propagation_ot.go @@ -43,13 +43,13 @@ func (p *textMapPropagator) Inject( if !ok { return opentracing.ErrInvalidCarrier } - carrier.Add(fieldNameTraceID, strconv.FormatInt(sc.raw.TraceID, 16)) - carrier.Add(fieldNameSpanID, strconv.FormatInt(sc.raw.SpanID, 16)) - carrier.Add(fieldNameSampled, strconv.FormatBool(sc.raw.Sampled)) + carrier.Set(fieldNameTraceID, strconv.FormatInt(sc.raw.TraceID, 16)) + carrier.Set(fieldNameSpanID, strconv.FormatInt(sc.raw.SpanID, 16)) + carrier.Set(fieldNameSampled, strconv.FormatBool(sc.raw.Sampled)) sc.Lock() for k, v := range sc.raw.Baggage { - carrier.Add(prefixBaggage+k, v) + carrier.Set(prefixBaggage+k, v) } sc.Unlock() return nil @@ -68,7 +68,7 @@ func (p *textMapPropagator) Join( var sampled bool var err error decodedBaggage := make(map[string]string) - err = carrier.ReadAll(func(k, v string) error { + err = carrier.ForeachKey(func(k, v string) error { switch strings.ToLower(k) { case fieldNameTraceID: traceID, err = strconv.ParseInt(v, 16, 64)