From 669d4b3a6c8d01821a65026f49df1b85d2972c14 Mon Sep 17 00:00:00 2001 From: Shouri Piratla Date: Fri, 17 Apr 2020 04:12:48 +0530 Subject: [PATCH] TraceID and SpanID implementations for Stringer Interface (#642) * TraceID and SpanID implementations for Stringer Interface * Hex encode while stringifying * Modify format specifiers wherever SpanID is used * comment changes * Remove TraceIdString() and SpanIdString() * Comments Fixes --- api/core/span_context.go | 26 +++--- api/core/span_context_test.go | 109 +++++++++++++------------- api/trace/b3_propagator.go | 7 +- api/trace/trace_context_propagator.go | 4 +- sdk/trace/benchmark_test.go | 4 +- sdk/trace/trace_test.go | 4 +- 6 files changed, 75 insertions(+), 79 deletions(-) diff --git a/api/core/span_context.go b/api/core/span_context.go index 6f6f2d694..b97b600ed 100644 --- a/api/core/span_context.go +++ b/api/core/span_context.go @@ -59,7 +59,12 @@ func (t TraceID) IsValid() bool { // MarshalJSON implements a custom marshal function to encode TraceID // as a hex string. func (t TraceID) MarshalJSON() ([]byte, error) { - return json.Marshal(hex.EncodeToString(t[:])) + return json.Marshal(t.String()) +} + +// String returns the hex string representation form of a TraceID +func (t TraceID) String() string { + return hex.EncodeToString(t[:]) } // SpanID is a unique identify of a span in a trace. @@ -77,7 +82,12 @@ func (s SpanID) IsValid() bool { // MarshalJSON implements a custom marshal function to encode SpanID // as a hex string. func (s SpanID) MarshalJSON() ([]byte, error) { - return json.Marshal(hex.EncodeToString(s[:])) + return json.Marshal(s.String()) +} + +// String returns the hex string representation form of a SpanID +func (s SpanID) String() string { + return hex.EncodeToString(s[:]) } // TraceIDFromHex returns a TraceID from a hex string if it is compliant @@ -169,18 +179,6 @@ func (sc SpanContext) HasSpanID() bool { return sc.SpanID.IsValid() } -// SpanIDString returns a hex string representation of the span ID in -// the span context. -func (sc SpanContext) SpanIDString() string { - return hex.EncodeToString(sc.SpanID[:]) -} - -// TraceIDString returns a hex string representation of the trace ID -// in the span context. -func (sc SpanContext) TraceIDString() string { - return hex.EncodeToString(sc.TraceID[:]) -} - // IsSampled check if the sampling bit in trace flags is set. func (sc SpanContext) IsSampled() bool { return sc.TraceFlags&traceFlagsBitMaskSampled == traceFlagsBitMaskSampled diff --git a/api/core/span_context_test.go b/api/core/span_context_test.go index e3e2ac40d..a4e7f9212 100644 --- a/api/core/span_context_test.go +++ b/api/core/span_context_test.go @@ -159,60 +159,6 @@ func TestHasSpanID(t *testing.T) { } } -func TestSpanIDString(t *testing.T) { - for _, testcase := range []struct { - name string - sc core.SpanContext - want string - }{ - { - name: "SpanContext.SpanIDString returns string representation of self.TraceID values > 0", - sc: core.SpanContext{SpanID: [8]byte{42}}, - want: `2a00000000000000`, - }, { - name: "SpanContext.SpanIDString returns string representation of self.TraceID values == 0", - sc: core.SpanContext{}, - want: `0000000000000000`, - }, - } { - t.Run(testcase.name, func(t *testing.T) { - //proto: func (sc SpanContext) SpanIDString() string {} - have := testcase.sc.SpanIDString() - if have != testcase.want { - t.Errorf("Want: %s, but have: %s", testcase.want, have) - } - }) - } -} - -func TestTraceIDString(t *testing.T) { - for _, testcase := range []struct { - name string - sc core.SpanContext - want string - }{ - { - name: "SpanContext.TraceIDString returns string representation of self.TraceID values > 0", - sc: core.SpanContext{ - TraceID: core.TraceID([16]byte{255}), - }, - want: `ff000000000000000000000000000000`, - }, { - name: "SpanContext.TraceIDString returns string representation of self.TraceID values == 0", - sc: core.SpanContext{TraceID: core.TraceID{}}, - want: `00000000000000000000000000000000`, - }, - } { - t.Run(testcase.name, func(t *testing.T) { - //proto: func (sc SpanContext) TraceIDString() string {} - have := testcase.sc.TraceIDString() - if have != testcase.want { - t.Errorf("Want: %s, but have: %s", testcase.want, have) - } - }) - } -} - func TestSpanContextIsSampled(t *testing.T) { for _, testcase := range []struct { name string @@ -240,7 +186,6 @@ func TestSpanContextIsSampled(t *testing.T) { }, } { t.Run(testcase.name, func(t *testing.T) { - //proto: func (sc SpanContext) TraceIDString() string {} have := testcase.sc.IsSampled() if have != testcase.want { t.Errorf("Want: %v, but have: %v", testcase.want, have) @@ -248,3 +193,57 @@ func TestSpanContextIsSampled(t *testing.T) { }) } } + +func TestStringTraceID(t *testing.T) { + for _, testcase := range []struct { + name string + tid core.TraceID + want string + }{ + { + name: "TraceID.String returns string representation of self.TraceID values > 0", + tid: core.TraceID([16]byte{255}), + want: "ff000000000000000000000000000000", + }, + { + name: "TraceID.String returns string representation of self.TraceID values == 0", + tid: core.TraceID([16]byte{}), + want: "00000000000000000000000000000000", + }, + } { + t.Run(testcase.name, func(t *testing.T) { + //proto: func (t TraceID) String() string {} + have := testcase.tid.String() + if have != testcase.want { + t.Errorf("Want: %s, but have: %s", testcase.want, have) + } + }) + } +} + +func TestStringSpanID(t *testing.T) { + for _, testcase := range []struct { + name string + sid core.SpanID + want string + }{ + { + name: "SpanID.String returns string representation of self.SpanID values > 0", + sid: core.SpanID([8]byte{255}), + want: "ff00000000000000", + }, + { + name: "SpanID.String returns string representation of self.SpanID values == 0", + sid: core.SpanID([8]byte{}), + want: "0000000000000000", + }, + } { + t.Run(testcase.name, func(t *testing.T) { + //proto: func (t TraceID) String() string {} + have := testcase.sid.String() + if have != testcase.want { + t.Errorf("Want: %s, but have: %s", testcase.want, have) + } + }) + } +} diff --git a/api/trace/b3_propagator.go b/api/trace/b3_propagator.go index 203298e23..df2e17ff3 100644 --- a/api/trace/b3_propagator.go +++ b/api/trace/b3_propagator.go @@ -59,11 +59,10 @@ func (b3 B3) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { if b3.SingleHeader { sampled := sc.TraceFlags & core.TraceFlagsSampled supplier.Set(B3SingleHeader, - fmt.Sprintf("%s-%.16x-%.1d", sc.TraceIDString(), sc.SpanID, sampled)) + fmt.Sprintf("%s-%s-%.1d", sc.TraceID, sc.SpanID, sampled)) } else { - supplier.Set(B3TraceIDHeader, sc.TraceIDString()) - supplier.Set(B3SpanIDHeader, - fmt.Sprintf("%.16x", sc.SpanID)) + supplier.Set(B3TraceIDHeader, sc.TraceID.String()) + supplier.Set(B3SpanIDHeader, sc.SpanID.String()) var sampled string if sc.IsSampled() { diff --git a/api/trace/trace_context_propagator.go b/api/trace/trace_context_propagator.go index 75c2af08a..bd91bfc54 100644 --- a/api/trace/trace_context_propagator.go +++ b/api/trace/trace_context_propagator.go @@ -48,9 +48,9 @@ func (TraceContext) Inject(ctx context.Context, supplier propagation.HTTPSupplie if !sc.IsValid() { return } - h := fmt.Sprintf("%.2x-%s-%.16x-%.2x", + h := fmt.Sprintf("%.2x-%s-%s-%.2x", supportedVersion, - sc.TraceIDString(), + sc.TraceID, sc.SpanID, sc.TraceFlags&core.TraceFlagsSampled) supplier.Set(traceparentHeader, h) diff --git a/sdk/trace/benchmark_test.go b/sdk/trace/benchmark_test.go index 94602257f..dad9a7531 100644 --- a/sdk/trace/benchmark_test.go +++ b/sdk/trace/benchmark_test.go @@ -139,7 +139,7 @@ func BenchmarkTraceID_DotString(b *testing.B) { want := "0000000000000001000000000000002a" for i := 0; i < b.N; i++ { - if got := sc.TraceIDString(); got != want { + if got := sc.TraceID.String(); got != want { b.Fatalf("got = %q want = %q", got, want) } } @@ -149,7 +149,7 @@ func BenchmarkSpanID_DotString(b *testing.B) { sc := core.SpanContext{SpanID: core.SpanID{1}} want := "0100000000000000" for i := 0; i < b.N; i++ { - if got := sc.SpanIDString(); got != want { + if got := sc.SpanID.String(); got != want { b.Fatalf("got = %q want = %q", got, want) } } diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 9e6a3ab5d..7d61da09e 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -620,10 +620,10 @@ func checkChild(p core.SpanContext, apiSpan apitrace.Span) error { if s == nil { return fmt.Errorf("got nil child span, want non-nil") } - if got, want := s.spanContext.TraceIDString(), p.TraceIDString(); got != want { + if got, want := s.spanContext.TraceID.String(), p.TraceID.String(); got != want { return fmt.Errorf("got child trace ID %s, want %s", got, want) } - if childID, parentID := s.spanContext.SpanIDString(), p.SpanIDString(); childID == parentID { + if childID, parentID := s.spanContext.SpanID.String(), p.SpanID.String(); childID == parentID { return fmt.Errorf("got child span ID %s, parent span ID %s; want unequal IDs", childID, parentID) } if got, want := s.spanContext.TraceFlags, p.TraceFlags; got != want {