Skip to content

Commit

Permalink
[pdata] Deprecate HexString func on SpanID and TraceID
Browse files Browse the repository at this point in the history
`[SpanID|TraceID].HexString` methods currently return an empty string for empty SpanID and TraceID instances. This behavior is not defined in the OTel spec and contradicts with w3c recommendations. We don't want to promote a behavior which possibly can be changed in future.

Additionally we introduce `[SpanID|TraceID].String` methods that have the same logic as HexString and can be used in log statement as Stringer instance to avoid unnecessary allocation when log is not emitted. But the `String` method is not encouraged to be used as a replacement for HexString because due to the concerns as described above.
  • Loading branch information
dmitryax committed Nov 11, 2022
1 parent 65152ee commit e5cbe08
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 2 deletions.
4 changes: 4 additions & 0 deletions .chloggen/deprecate_hexstring.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
change_type: 'deprecation'
component: pdata
note: Deprecate `pcommon.[Span|Trace]ID.HexString` methods. Call `hex.EncodeToString` explicitly instead.
issues: [6514]
14 changes: 13 additions & 1 deletion pdata/pcommon/spanid.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,19 @@ func NewSpanIDEmpty() SpanID {
return emptySpanID
}

// HexString returns hex representation of the SpanID.
// HexString returns string representation of the SpanID.
//
// Important: Don't rely on this method to get a string identifier of SpanID,
// Use hex.EncodeToString explicitly instead.
// This method meant to implement Stringer interface for display purposes only.
func (ms SpanID) String() string {
if ms.IsEmpty() {
return ""
}
return hex.EncodeToString(ms[:])
}

// Deprecated: [0.65.0] Call hex.EncodeToString explicitly instead.
func (ms SpanID) HexString() string {
if ms.IsEmpty() {
return ""
Expand Down
8 changes: 8 additions & 0 deletions pdata/pcommon/spanid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ func TestSpanIDHexString(t *testing.T) {
assert.Equal(t, "1223ad1223ad1223", sid.HexString())
}

func TestSpanIDString(t *testing.T) {
sid := SpanID([8]byte{})
assert.Equal(t, "", sid.String())

sid = SpanID([8]byte{0x12, 0x23, 0xAD, 0x12, 0x23, 0xAD, 0x12, 0x23})
assert.Equal(t, "1223ad1223ad1223", sid.String())
}

func TestSpanIDImmutable(t *testing.T) {
initialBytes := [8]byte{0x12, 0x23, 0xAD, 0x12, 0x23, 0xAD, 0x12, 0x23}
sid := SpanID(initialBytes)
Expand Down
14 changes: 13 additions & 1 deletion pdata/pcommon/traceid.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,19 @@ func NewTraceIDEmpty() TraceID {
return emptyTraceID
}

// HexString returns hex representation of the TraceID.
// HexString returns string representation of the TraceID.
//
// Important: Don't rely on this method to get a string identifier of TraceID.
// Use hex.EncodeToString explicitly instead.
// This method meant to implement Stringer interface for display purposes only.
func (ms TraceID) String() string {
if ms.IsEmpty() {
return ""
}
return hex.EncodeToString(ms[:])
}

// Deprecated: [0.65.0] Call hex.EncodeToString explicitly instead.
func (ms TraceID) HexString() string {
if ms.IsEmpty() {
return ""
Expand Down
8 changes: 8 additions & 0 deletions pdata/pcommon/traceid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ func TestTraceIDHexString(t *testing.T) {
assert.Equal(t, "12345678123456781234567812345678", tid.HexString())
}

func TestTraceIDString(t *testing.T) {
tid := TraceID([16]byte{})
assert.Equal(t, "", tid.String())

tid = [16]byte{0x12, 0x34, 0x56, 0x78, 0x12, 0x34, 0x56, 0x78, 0x12, 0x34, 0x56, 0x78, 0x12, 0x34, 0x56, 0x78}
assert.Equal(t, "12345678123456781234567812345678", tid.String())
}

func TestTraceIDImmutable(t *testing.T) {
initialBytes := [16]byte{0x12, 0x34, 0x56, 0x78, 0x12, 0x34, 0x56, 0x78, 0x12, 0x34, 0x56, 0x78, 0x12, 0x34, 0x56, 0x78}
tid := TraceID(initialBytes)
Expand Down

0 comments on commit e5cbe08

Please sign in to comment.