Skip to content

Commit f83089c

Browse files
mterharmterhar
authored
[receiver/libhoney] fix parentID to use hex-string if possible (#40934)
1 parent b4f276a commit f83089c

File tree

3 files changed

+225
-4
lines changed

3 files changed

+225
-4
lines changed

‎.chloggen/libhoneyreceiver-fix-parentid.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
change_type: bug_fix
2+
component: libhoneyreceiver
3+
note: Fix parent id handling in libhoneyreceiver
4+
issues: [40934]
5+
change_logs: [user]

‎receiver/libhoneyreceiver/internal/libhoneyevent/libhoneyevent.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ func (l *LibhoneyEvent) ToPLogRecord(newLog *plog.LogRecord, alreadyUsedFields *
259259

260260
// GetParentID returns the parent id from the event or an error if it's not found
261261
func (l *LibhoneyEvent) GetParentID(fieldName string) (trc.SpanID, error) {
262-
if pid, ok := l.Data[fieldName]; ok {
262+
if pid, ok := l.Data[fieldName]; ok && pid != nil {
263263
pid := strings.ReplaceAll(pid.(string), "-", "")
264264
pidByteArray, err := hex.DecodeString(pid)
265265
if err == nil {
@@ -280,9 +280,11 @@ func (l *LibhoneyEvent) ToPTraceSpan(newSpan *ptrace.Span, alreadyUsedFields *[]
280280
timeNs := l.MsgPackTimestamp.UnixNano()
281281
logger.Debug("processing trace with", zap.Int64("timestamp", timeNs))
282282

283-
var parentID trc.SpanID
284283
if pid, ok := l.Data[cfg.Attributes.ParentID]; ok {
285-
parentID = spanIDFrom(pid.(string))
284+
parentID, err := l.GetParentID(cfg.Attributes.ParentID)
285+
if err != nil {
286+
parentID = spanIDFrom(pid.(string))
287+
}
286288
newSpan.SetParentSpanID(pcommon.SpanID(parentID))
287289
}
288290

@@ -293,7 +295,7 @@ func (l *LibhoneyEvent) ToPTraceSpan(newSpan *ptrace.Span, alreadyUsedFields *[]
293295
break
294296
}
295297
}
296-
endTimestamp := timeNs + (int64(durationMs) * 1000000)
298+
endTimestamp := timeNs + int64(durationMs*1000000)
297299

298300
if tid, ok := l.Data[cfg.Attributes.TraceID]; ok {
299301
tid := strings.ReplaceAll(tid.(string), "-", "")

‎receiver/libhoneyreceiver/internal/libhoneyevent/libhoneyevent_test.go‎

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"go.opentelemetry.io/collector/pdata/pcommon"
1414
"go.opentelemetry.io/collector/pdata/plog"
1515
"go.opentelemetry.io/collector/pdata/ptrace"
16+
trc "go.opentelemetry.io/otel/trace"
1617
"go.uber.org/zap"
1718
)
1819

@@ -333,6 +334,76 @@ func TestToPTraceSpan(t *testing.T) {
333334
s.Attributes().PutBool("bool_attr", true)
334335
},
335336
},
337+
{
338+
name: "valid hex parent ID is correctly parsed",
339+
event: LibhoneyEvent{
340+
Time: now.Format(time.RFC3339),
341+
MsgPackTimestamp: &now,
342+
Data: map[string]any{
343+
"name": "child-span",
344+
"trace.span_id": "abcdef1234567890",
345+
"trace.trace_id": "1234567890abcdef1234567890abcdef",
346+
"trace.parent_id": "1234567890abcdef", // Valid hex-encoded span ID
347+
"duration_ms": 50.0,
348+
},
349+
Samplerate: 1,
350+
},
351+
want: func(s ptrace.Span) {
352+
s.SetName("child-span")
353+
s.SetSpanID([8]byte{0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x90})
354+
s.SetTraceID([16]byte{0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef})
355+
// Parent ID should be the actual hex-decoded value, not a hash
356+
s.SetParentSpanID([8]byte{0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef})
357+
s.SetStartTimestamp(pcommon.NewTimestampFromTime(now))
358+
s.SetEndTimestamp(pcommon.NewTimestampFromTime(now.Add(50 * time.Millisecond)))
359+
},
360+
},
361+
{
362+
name: "invalid parent ID falls back to hash",
363+
event: LibhoneyEvent{
364+
Time: now.Format(time.RFC3339),
365+
MsgPackTimestamp: &now,
366+
Data: map[string]any{
367+
"name": "child-span",
368+
"trace.span_id": "abcdef1234567890",
369+
"trace.trace_id": "1234567890abcdef1234567890abcdef",
370+
"trace.parent_id": "invalid-hex-string", // Invalid hex, should fall back to hash
371+
"duration_ms": 50.0,
372+
},
373+
Samplerate: 1,
374+
},
375+
want: func(s ptrace.Span) {
376+
s.SetName("child-span")
377+
s.SetSpanID([8]byte{0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x90})
378+
s.SetTraceID([16]byte{0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef})
379+
// Parent ID should be the hash of "invalid-hex-string"
380+
expectedHash := spanIDFrom("invalid-hex-string")
381+
s.SetParentSpanID(pcommon.SpanID(expectedHash))
382+
s.SetStartTimestamp(pcommon.NewTimestampFromTime(now))
383+
s.SetEndTimestamp(pcommon.NewTimestampFromTime(now.Add(50 * time.Millisecond)))
384+
},
385+
},
386+
{
387+
name: "sub-1-millisecond duration",
388+
event: LibhoneyEvent{
389+
Time: now.Format(time.RFC3339),
390+
MsgPackTimestamp: &now,
391+
Data: map[string]any{
392+
"name": "fast-span",
393+
"trace.span_id": "1234567890abcdef",
394+
"trace.trace_id": "1234567890abcdef1234567890abcdef",
395+
"duration_ms": 0.5, // 500 microseconds
396+
},
397+
Samplerate: 1,
398+
},
399+
want: func(s ptrace.Span) {
400+
s.SetName("fast-span")
401+
s.SetSpanID([8]byte{0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef})
402+
s.SetTraceID([16]byte{0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef})
403+
s.SetStartTimestamp(pcommon.NewTimestampFromTime(now))
404+
s.SetEndTimestamp(pcommon.NewTimestampFromTime(now.Add(500 * time.Microsecond)))
405+
},
406+
},
336407
}
337408

338409
alreadyUsedFields := []string{"name", "trace.span_id", "trace.trace_id", "duration_ms", "status.code", "status.message", "kind"}
@@ -396,3 +467,146 @@ func TestToPTraceSpan(t *testing.T) {
396467
})
397468
}
398469
}
470+
471+
func TestParentIDHandlingDifference(t *testing.T) {
472+
validHexParentID := "1234567890abcdef"
473+
474+
oldApproachResult := spanIDFrom(validHexParentID)
475+
476+
// What the new approach produces (parsing hex)
477+
event := LibhoneyEvent{
478+
Data: map[string]any{
479+
"trace.parent_id": validHexParentID,
480+
},
481+
}
482+
newApproachResult, err := event.GetParentID("trace.parent_id")
483+
require.NoError(t, err)
484+
485+
assert.NotEqual(t, oldApproachResult, newApproachResult,
486+
"Old and new approaches should produce different results for valid hex parent ID")
487+
488+
expectedBytes := []byte{0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef}
489+
assert.Equal(t, trc.SpanID(expectedBytes), newApproachResult,
490+
"New approach should correctly decode hex parent ID")
491+
492+
// For invalid hex, both approaches should produce the same result (hash)
493+
invalidParentID := "invalid-hex-string"
494+
oldInvalidResult := spanIDFrom(invalidParentID)
495+
496+
eventInvalid := LibhoneyEvent{
497+
Data: map[string]any{
498+
"trace.parent_id": invalidParentID,
499+
},
500+
}
501+
_, err = eventInvalid.GetParentID("trace.parent_id")
502+
assert.Error(t, err, "GetParentID should return error for invalid hex")
503+
504+
// The ToPTraceSpan function should fall back to hash for invalid hex
505+
span := ptrace.NewSpan()
506+
cfg := FieldMapConfig{
507+
Attributes: AttributesConfig{
508+
ParentID: "trace.parent_id",
509+
},
510+
}
511+
512+
eventInvalid.MsgPackTimestamp = &time.Time{}
513+
err = eventInvalid.ToPTraceSpan(&span, &[]string{}, cfg, *zap.NewNop())
514+
require.NoError(t, err)
515+
516+
// Should fall back to hash for invalid hex
517+
assert.Equal(t, pcommon.SpanID(oldInvalidResult), span.ParentSpanID(),
518+
"Invalid hex should fall back to hash in new approach")
519+
}
520+
521+
func TestGetParentID(t *testing.T) {
522+
tests := []struct {
523+
name string
524+
event LibhoneyEvent
525+
fieldName string
526+
want trc.SpanID
527+
wantErr bool
528+
}{
529+
{
530+
name: "valid 16-character hex string",
531+
event: LibhoneyEvent{
532+
Data: map[string]any{
533+
"trace.parent_id": "1234567890abcdef",
534+
},
535+
},
536+
fieldName: "trace.parent_id",
537+
want: trc.SpanID{0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef},
538+
},
539+
{
540+
name: "valid 32-character hex string (trace ID format)",
541+
event: LibhoneyEvent{
542+
Data: map[string]any{
543+
"trace.parent_id": "1234567890abcdef1234567890abcdef",
544+
},
545+
},
546+
fieldName: "trace.parent_id",
547+
want: trc.SpanID{0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef},
548+
},
549+
{
550+
name: "hex string with hyphens",
551+
event: LibhoneyEvent{
552+
Data: map[string]any{
553+
"trace.parent_id": "12-34-56-78-90-ab-cd-ef",
554+
},
555+
},
556+
fieldName: "trace.parent_id",
557+
want: trc.SpanID{0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef},
558+
},
559+
{
560+
name: "invalid hex string",
561+
event: LibhoneyEvent{
562+
Data: map[string]any{
563+
"trace.parent_id": "invalid-hex",
564+
},
565+
},
566+
fieldName: "trace.parent_id",
567+
wantErr: true,
568+
},
569+
{
570+
name: "field not found",
571+
event: LibhoneyEvent{
572+
Data: map[string]any{},
573+
},
574+
fieldName: "trace.parent_id",
575+
wantErr: true,
576+
},
577+
{
578+
name: "odd length hex string",
579+
event: LibhoneyEvent{
580+
Data: map[string]any{
581+
"trace.parent_id": "1234567890abcde", // 15 characters
582+
},
583+
},
584+
fieldName: "trace.parent_id",
585+
wantErr: true,
586+
},
587+
{
588+
name: "empty string",
589+
event: LibhoneyEvent{
590+
Data: map[string]any{
591+
"trace.parent_id": "",
592+
},
593+
},
594+
fieldName: "",
595+
wantErr: true,
596+
},
597+
}
598+
599+
for _, tt := range tests {
600+
t.Run(tt.name, func(t *testing.T) {
601+
got, err := tt.event.GetParentID(tt.fieldName)
602+
603+
if tt.wantErr {
604+
assert.Error(t, err)
605+
return
606+
}
607+
608+
require.NoError(t, err)
609+
assert.Equal(t, tt.want, got)
610+
})
611+
}
612+
}

0 commit comments

Comments
 (0)