[pdata] Deprecate HexString func on SpanID and TraceID (#6530)

`[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 implement `fmt.Stringer` interface to allow `SpanID` and `TraceID` instances to be used in log statements and avoid unnecessary allocation when log is not emitted. But the `String` method is not encouraged to be used as a replacement for `HexString` because of the concerns as described above.

.chloggen/deprecate_hexstring.yaml

@@ -0,0 +1,4 @@
+change_type: 'deprecation'
+component: pdata
+note: Deprecate `pcommon.[Span|Trace]ID.HexString` methods. Call `hex.EncodeToString` explicitly instead.
+issues: [6514]

exporter/loggingexporter/internal/otlptext/databuffer.go

@@ -34,7 +34,7 @@ func (b *dataBuffer) logEntry(format string, a ...interface{}) {
 	b.buf.WriteString("\n")
 }
 
-func (b *dataBuffer) logAttr(attr string, value string) {
+func (b *dataBuffer) logAttr(attr string, value interface{}) {
 	b.logEntry("    %-15s: %s", attr, value)
 }
 
@@ -258,8 +258,8 @@ func (b *dataBuffer) logLinks(description string, sl ptrace.SpanLinkSlice) {
 	for i := 0; i < sl.Len(); i++ {
 		l := sl.At(i)
 		b.logEntry("SpanLink #%d", i)
-		b.logEntry("     -> Trace ID: %s", l.TraceID().HexString())
-		b.logEntry("     -> ID: %s", l.SpanID().HexString())
+		b.logEntry("     -> Trace ID: %s", l.TraceID())
+		b.logEntry("     -> ID: %s", l.SpanID())
 		b.logEntry("     -> TraceState: %s", l.TraceState().AsRaw())
 		b.logEntry("     -> DroppedAttributesCount: %d", l.DroppedAttributesCount())
 		if l.Attributes().Len() == 0 {

exporter/loggingexporter/internal/otlptext/logs.go

@@ -51,8 +51,8 @@ func (textLogsMarshaler) MarshalLogs(ld plog.Logs) ([]byte, error) {
 				buf.logEntry("SeverityNumber: %s(%d)", lr.SeverityNumber(), lr.SeverityNumber())
 				buf.logEntry("Body: %s", valueToString(lr.Body()))
 				buf.logAttributes("Attributes", lr.Attributes())
-				buf.logEntry("Trace ID: %s", lr.TraceID().HexString())
-				buf.logEntry("Span ID: %s", lr.SpanID().HexString())
+				buf.logEntry("Trace ID: %s", lr.TraceID())
+				buf.logEntry("Span ID: %s", lr.SpanID())
 				buf.logEntry("Flags: %d", lr.Flags())
 			}
 		}

exporter/loggingexporter/internal/otlptext/traces.go

@@ -45,9 +45,9 @@ func (textTracesMarshaler) MarshalTraces(td ptrace.Traces) ([]byte, error) {
 			for k := 0; k < spans.Len(); k++ {
 				buf.logEntry("Span #%d", k)
 				span := spans.At(k)
-				buf.logAttr("Trace ID", span.TraceID().HexString())
-				buf.logAttr("Parent ID", span.ParentSpanID().HexString())
-				buf.logAttr("ID", span.SpanID().HexString())
+				buf.logAttr("Trace ID", span.TraceID())
+				buf.logAttr("Parent ID", span.ParentSpanID())
+				buf.logAttr("ID", span.SpanID())
 				buf.logAttr("Name", span.Name())
 				buf.logAttr("Kind", span.Kind().String())
 				buf.logAttr("Start time", span.StartTimestamp().String())

exporter/otlphttpexporter/otlp_test.go

@@ -261,8 +261,10 @@ func TestIssue_4221(t *testing.T) {
 		tr := ptraceotlp.NewExportRequest()
 		require.NoError(t, tr.UnmarshalProto(unbase64Data))
 		span := tr.Traces().ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0)
-		assert.Equal(t, "4303853f086f4f8c86cf198b6551df84", span.TraceID().HexString())
-		assert.Equal(t, "e5513c32795c41b9", span.SpanID().HexString())
+		traceID := span.TraceID()
+		assert.Equal(t, "4303853f086f4f8c86cf198b6551df84", hex.EncodeToString(traceID[:]))
+		spanID := span.SpanID()
+		assert.Equal(t, "e5513c32795c41b9", hex.EncodeToString(spanID[:]))
 	}))
 
 	exp := startTracesExporter(t, "", svr.URL)
@@ -281,14 +283,16 @@ func TestIssue_4221(t *testing.T) {
 	require.NoError(t, err)
 	copy(traceIDBytes[:], traceIDBytesSlice)
 	span.SetTraceID(traceIDBytes)
-	assert.Equal(t, "4303853f086f4f8c86cf198b6551df84", span.TraceID().HexString())
+	traceID := span.TraceID()
+	assert.Equal(t, "4303853f086f4f8c86cf198b6551df84", hex.EncodeToString(traceID[:]))
 
 	var spanIDBytes [8]byte
 	spanIDBytesSlice, err := hex.DecodeString("e5513c32795c41b9")
 	require.NoError(t, err)
 	copy(spanIDBytes[:], spanIDBytesSlice)
 	span.SetSpanID(spanIDBytes)
-	assert.Equal(t, "e5513c32795c41b9", span.SpanID().HexString())
+	spanID := span.SpanID()
+	assert.Equal(t, "e5513c32795c41b9", hex.EncodeToString(spanID[:]))
 
 	span.SetEndTimestamp(1634684637873000000)
 	span.Attributes().PutInt("span_index", 3)

pdata/pcommon/spanid.go

@@ -29,7 +29,19 @@ func NewSpanIDEmpty() SpanID {
 	return emptySpanID
 }
 
-// HexString returns hex representation of the SpanID.
+// String 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 ""

pdata/pcommon/spanid_test.go

@@ -20,7 +20,7 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestNewSpanID(t *testing.T) {
+func TestSpanID(t *testing.T) {
 	sid := SpanID([8]byte{1, 2, 3, 4, 4, 3, 2, 1})
 	assert.Equal(t, [8]byte{1, 2, 3, 4, 4, 3, 2, 1}, [8]byte(sid))
 	assert.False(t, sid.IsEmpty())
@@ -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)

pdata/pcommon/traceid.go

@@ -30,7 +30,19 @@ func NewTraceIDEmpty() TraceID {
 	return emptyTraceID
 }
 
-// HexString returns hex representation of the TraceID.
+// String 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 ""

pdata/pcommon/traceid_test.go

@@ -24,7 +24,6 @@ func TestTraceID(t *testing.T) {
 	tid := TraceID([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 8, 7, 6, 5, 4, 3, 2, 1})
 	assert.Equal(t, [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 8, 7, 6, 5, 4, 3, 2, 1}, [16]byte(tid))
 	assert.False(t, tid.IsEmpty())
-	assert.Equal(t, "01020304050607080807060504030201", tid.HexString())
 }
 
 func TestNewTraceIDEmpty(t *testing.T) {
@@ -41,6 +40,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)