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
open-telemetry · Apr 16, 2020 · 669d4b3 · 669d4b3
1 parent ebc245b
commit 669d4b3
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 71 deletions.
diff --git 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
@@ -159,91 +159,90 @@ func TestHasSpanID(t *testing.T) {
 	}
 }
 
-func TestSpanIDString(t *testing.T) {
+func TestSpanContextIsSampled(t *testing.T) {
 	for _, testcase := range []struct {
 		name string
 		sc   core.SpanContext
-		want string
+		want bool
 	}{
 		{
-			name: "SpanContext.SpanIDString returns string representation of self.TraceID values > 0",
-			sc:   core.SpanContext{SpanID: [8]byte{42}},
-			want: `2a00000000000000`,
+			name: "sampled",
+			sc: core.SpanContext{
+				TraceID:    core.TraceID([16]byte{1}),
+				TraceFlags: core.TraceFlagsSampled,
+			},
+			want: true,
 		}, {
-			name: "SpanContext.SpanIDString returns string representation of self.TraceID values == 0",
-			sc:   core.SpanContext{},
-			want: `0000000000000000`,
+			name: "sampled plus unused",
+			sc: core.SpanContext{
+				TraceID:    core.TraceID([16]byte{1}),
+				TraceFlags: core.TraceFlagsSampled | core.TraceFlagsUnused,
+			},
+			want: true,
+		}, {
+			name: "not sampled/default",
+			sc:   core.SpanContext{TraceID: core.TraceID{}},
+			want: false,
 		},
 	} {
 		t.Run(testcase.name, func(t *testing.T) {
-			//proto: func (sc SpanContext) SpanIDString() string {}
-			have := testcase.sc.SpanIDString()
+			have := testcase.sc.IsSampled()
 			if have != testcase.want {
-				t.Errorf("Want: %s, but have: %s", testcase.want, have)
+				t.Errorf("Want: %v, but have: %v", testcase.want, have)
 			}
 		})
 	}
 }
 
-func TestTraceIDString(t *testing.T) {
+func TestStringTraceID(t *testing.T) {
 	for _, testcase := range []struct {
 		name string
-		sc   core.SpanContext
+		tid  core.TraceID
 		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`,
+			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 (sc SpanContext) TraceIDString() string {}
-			have := testcase.sc.TraceIDString()
+			//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 TestSpanContextIsSampled(t *testing.T) {
+func TestStringSpanID(t *testing.T) {
 	for _, testcase := range []struct {
 		name string
-		sc   core.SpanContext
-		want bool
+		sid  core.SpanID
+		want string
 	}{
 		{
-			name: "sampled",
-			sc: core.SpanContext{
-				TraceID:    core.TraceID([16]byte{1}),
-				TraceFlags: core.TraceFlagsSampled,
-			},
-			want: true,
-		}, {
-			name: "sampled plus unused",
-			sc: core.SpanContext{
-				TraceID:    core.TraceID([16]byte{1}),
-				TraceFlags: core.TraceFlagsSampled | core.TraceFlagsUnused,
-			},
-			want: true,
-		}, {
-			name: "not sampled/default",
-			sc:   core.SpanContext{TraceID: core.TraceID{}},
-			want: false,
+			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 (sc SpanContext) TraceIDString() string {}
-			have := testcase.sc.IsSampled()
+			//proto: func (t TraceID) String() string {}
+			have := testcase.sid.String()
 			if have != testcase.want {
-				t.Errorf("Want: %v, but have: %v", testcase.want, have)
+				t.Errorf("Want: %s, but have: %s", testcase.want, have)
 			}
 		})
 	}

diff --git 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
@@ -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
@@ -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
@@ -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 {