From b1fe7fec9ffafb2028f6ef546e26cc643997a881 Mon Sep 17 00:00:00 2001 From: Lukas Vogel Date: Sat, 12 Sep 2020 02:51:59 +0200 Subject: [PATCH] Prefix TraceID/SpanID.String() with zeroes (#533) * TraceID.String prefix with zeroes The wire encoding of the TraceID uses zero prefixes (#472). The JaegerUI also uses zero prefixes (since 1.16). So it makes sense that the Stringer of the TraceID also does this. Resolves #532 Signed-off-by: Lukas Vogel * SpanID.String prefix with zeroes Also test parsing back works. Signed-off-by: Lukas Vogel * fix tests? how do I run this thing locally? Signed-off-by: Lukas Vogel * fix tests! Signed-off-by: Lukas Vogel --- span_context.go | 6 ++-- span_context_test.go | 59 ++++++++++++++++++++++++++++++++++++-- zipkin/propagation_test.go | 12 ++++---- 3 files changed, 65 insertions(+), 12 deletions(-) diff --git a/span_context.go b/span_context.go index 03aa99a0..568764a7 100644 --- a/span_context.go +++ b/span_context.go @@ -344,9 +344,9 @@ func (c *SpanContext) isDebugIDContainerOnly() bool { func (t TraceID) String() string { if t.High == 0 { - return fmt.Sprintf("%x", t.Low) + return fmt.Sprintf("%016x", t.Low) } - return fmt.Sprintf("%x%016x", t.High, t.Low) + return fmt.Sprintf("%016x%016x", t.High, t.Low) } // TraceIDFromString creates a TraceID from a hexadecimal string @@ -379,7 +379,7 @@ func (t TraceID) IsValid() bool { // ------- SpanID ------- func (s SpanID) String() string { - return fmt.Sprintf("%x", uint64(s)) + return fmt.Sprintf("%016x", uint64(s)) } // SpanIDFromString creates a SpanID from a hexadecimal string diff --git a/span_context_test.go b/span_context_test.go index 51b97a00..cf6584b7 100644 --- a/span_context_test.go +++ b/span_context_test.go @@ -15,6 +15,7 @@ package jaeger import ( + "math" "testing" "github.com/stretchr/testify/assert" @@ -63,9 +64,9 @@ func TestContextFromString(t *testing.T) { assert.EqualValues(t, 1, ctx.spanID) assert.EqualValues(t, 1, ctx.parentID) assert.True(t, ctx.IsSampled()) - assert.Equal(t, "ff", SpanID(255).String()) - assert.Equal(t, "ff", TraceID{Low: 255}.String()) - assert.Equal(t, "ff00000000000000ff", TraceID{High: 255, Low: 255}.String()) + assert.Equal(t, "00000000000000ff", SpanID(255).String()) + assert.Equal(t, "00000000000000ff", TraceID{Low: 255}.String()) + assert.Equal(t, "00000000000000ff00000000000000ff", TraceID{High: 255, Low: 255}.String()) ctx = NewSpanContext(TraceID{High: 255, Low: 255}, SpanID(1), SpanID(1), false, nil) assert.Equal(t, "00000000000000ff00000000000000ff:0000000000000001:0000000000000001:0", ctx.String()) } @@ -166,3 +167,55 @@ func TestSpanContext_CopyFrom(t *testing.T) { assert.Equal(t, ctx, ctx2) assert.Equal(t, "y", ctx2.baggage["x"]) } + +func TestTraceIDString(t *testing.T) { + var tests = map[string]struct { + in TraceID + expected string + }{ + "Empty TraceID": { + in: TraceID{}, + expected: "0000000000000000", + }, + "TraceID low only": { + in: TraceID{Low: math.MaxUint64/16 - 405}, + expected: "0ffffffffffffe6a", + }, + "TraceID low and high": { + in: TraceID{High: math.MaxUint64 / 16, Low: math.MaxUint64/16 - 405}, + expected: "0fffffffffffffff0ffffffffffffe6a", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.expected, tc.in.String()) + parsed, err := TraceIDFromString(tc.in.String()) + assert.NoError(t, err) + assert.Equal(t, tc.in, parsed) + }) + } +} + +func TestSpanIDString(t *testing.T) { + var tests = map[string]struct { + in SpanID + expected string + }{ + "SpanID zero": { + in: 0, + expected: "0000000000000000", + }, + "SpanID non zero": { + in: math.MaxUint64/16 - 405, + expected: "0ffffffffffffe6a", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.expected, tc.in.String()) + parsed, err := SpanIDFromString(tc.in.String()) + assert.NoError(t, err) + assert.Equal(t, tc.in, parsed) + }) + } +} diff --git a/zipkin/propagation_test.go b/zipkin/propagation_test.go index 453e82d1..d7c5c6f6 100644 --- a/zipkin/propagation_test.go +++ b/zipkin/propagation_test.go @@ -32,31 +32,31 @@ var ( var ( rootSampledHeader = opentracing.TextMapCarrier{ - "x-b3-traceid": "1", + "x-b3-traceid": "0000000000000001", "x-b3-spanid": "2", "x-b3-sampled": "1", "baggage-foo": "bar", } nonRootSampledHeader = opentracing.TextMapCarrier{ - "x-b3-traceid": "1", + "x-b3-traceid": "0000000000000001", "x-b3-spanid": "2", "x-b3-parentspanid": "1", "x-b3-sampled": "1", } nonRootNonSampledHeader = opentracing.TextMapCarrier{ - "x-b3-traceid": "1", + "x-b3-traceid": "0000000000000001", "x-b3-spanid": "2", "x-b3-parentspanid": "1", "x-b3-sampled": "0", } rootSampledBooleanHeader = opentracing.TextMapCarrier{ - "x-b3-traceid": "1", + "x-b3-traceid": "0000000000000001", "x-b3-spanid": "2", "x-b3-sampled": "true", "baggage-foo": "bar", } nonRootSampledBooleanHeader = opentracing.TextMapCarrier{ - "x-b3-traceid": "1", + "x-b3-traceid": "0000000000000001", "x-b3-spanid": "2", "x-b3-parentspanid": "1", "x-b3-sampled": "true", @@ -156,7 +156,7 @@ func TestCustomBaggagePrefix(t *testing.T) { err := propag.Inject(sc, hdr) assert.Nil(t, err) m := opentracing.TextMapCarrier{ - "x-b3-traceid": "1", + "x-b3-traceid": "0000000000000001", "x-b3-spanid": "2", "x-b3-sampled": "1", "emoji:)foo": "bar",