Skip to content

Commit 401b489

Browse files
committed
handle msgpack the same way as json (safe defaults)
Unmarshal into object with timestamp handling. Add test case for unmarshaling
1 parent eaa9f61 commit 401b489

File tree

3 files changed

+152
-22
lines changed

3 files changed

+152
-22
lines changed

receiver/libhoneyreceiver/internal/libhoneyevent/libhoneyevent.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package libhoneyevent // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/libhoneyreceiver/internal/libhoneyevent"
55

66
import (
7+
"bytes"
78
"crypto/rand"
89
"encoding/binary"
910
"encoding/hex"
@@ -15,6 +16,7 @@ import (
1516
"strings"
1617
"time"
1718

19+
"github.com/vmihailenco/msgpack/v5"
1820
"go.opentelemetry.io/collector/pdata/pcommon"
1921
"go.opentelemetry.io/collector/pdata/plog"
2022
"go.opentelemetry.io/collector/pdata/ptrace"
@@ -87,6 +89,56 @@ func (l *LibhoneyEvent) UnmarshalJSON(j []byte) error {
8789
return nil
8890
}
8991

92+
// UnmarshalMsgpack overrides the unmarshall to make sure the MsgPackTimestamp is set
93+
func (l *LibhoneyEvent) UnmarshalMsgpack(data []byte) error {
94+
type _libhoneyEvent LibhoneyEvent
95+
tstr := eventtime.GetEventTimeDefaultString()
96+
tzero := time.Time{}
97+
tmp := _libhoneyEvent{Time: "none", MsgPackTimestamp: &tzero, Samplerate: 1}
98+
99+
// Use a temporary struct to avoid recursion
100+
type tempEvent struct {
101+
Samplerate int `msgpack:"samplerate"`
102+
MsgPackTimestamp *time.Time `msgpack:"time"`
103+
Time string `msgpack:"-"` // Ignore during msgpack unmarshal
104+
Data map[string]any `msgpack:"data"`
105+
}
106+
107+
var tmpEvent tempEvent
108+
// First unmarshal into the temp struct
109+
decoder := msgpack.NewDecoder(bytes.NewReader(data))
110+
decoder.UseLooseInterfaceDecoding(true)
111+
err := decoder.Decode(&tmpEvent)
112+
if err != nil {
113+
return err
114+
}
115+
116+
// Copy fields to our tmp struct
117+
tmp.Samplerate = tmpEvent.Samplerate
118+
tmp.MsgPackTimestamp = tmpEvent.MsgPackTimestamp
119+
tmp.Data = tmpEvent.Data
120+
121+
// Check if Time field exists in Data and extract it
122+
if timeStr, ok := tmpEvent.Data["time"].(string); ok {
123+
tmp.Time = timeStr
124+
}
125+
126+
// Apply the same timestamp logic as JSON
127+
if (tmp.MsgPackTimestamp == nil || tmp.MsgPackTimestamp.IsZero()) && tmp.Time == "none" {
128+
// neither timestamp was set. give it right now.
129+
tmp.Time = tstr
130+
tnow := time.Now()
131+
tmp.MsgPackTimestamp = &tnow
132+
}
133+
if tmp.MsgPackTimestamp == nil || tmp.MsgPackTimestamp.IsZero() {
134+
propertime := eventtime.GetEventTime(tmp.Time)
135+
tmp.MsgPackTimestamp = &propertime
136+
}
137+
138+
*l = LibhoneyEvent(tmp)
139+
return nil
140+
}
141+
90142
// DebugString returns a string representation of the LibhoneyEvent
91143
func (l *LibhoneyEvent) DebugString() string {
92144
return fmt.Sprintf("%#v", l)

receiver/libhoneyreceiver/internal/libhoneyevent/libhoneyevent_test.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
"github.com/stretchr/testify/assert"
1212
"github.com/stretchr/testify/require"
13+
"github.com/vmihailenco/msgpack/v5"
1314
"go.opentelemetry.io/collector/pdata/pcommon"
1415
"go.opentelemetry.io/collector/pdata/plog"
1516
"go.opentelemetry.io/collector/pdata/ptrace"
@@ -610,3 +611,99 @@ func TestGetParentID(t *testing.T) {
610611
})
611612
}
612613
}
614+
615+
func TestLibhoneyEvent_UnmarshalMsgpack(t *testing.T) {
616+
tests := []struct {
617+
name string
618+
msgpackData map[string]any
619+
expectNonNilTimestamp bool
620+
wantErr bool
621+
}{
622+
{
623+
name: "msgpack with nil timestamp",
624+
msgpackData: map[string]any{
625+
"data": map[string]any{
626+
"key": "value",
627+
},
628+
"samplerate": 1,
629+
// time field is not set (nil)
630+
},
631+
expectNonNilTimestamp: true,
632+
},
633+
{
634+
name: "msgpack with time string in data",
635+
msgpackData: map[string]any{
636+
"data": map[string]any{
637+
"key": "value",
638+
"time": "2024-01-01T00:00:00Z",
639+
},
640+
"samplerate": 2,
641+
},
642+
expectNonNilTimestamp: true,
643+
},
644+
{
645+
name: "msgpack with timestamp field",
646+
msgpackData: map[string]any{
647+
"time": time.Now(),
648+
"data": map[string]any{
649+
"key": "value",
650+
},
651+
"samplerate": 3,
652+
},
653+
expectNonNilTimestamp: true,
654+
},
655+
{
656+
name: "msgpack with zero timestamp",
657+
msgpackData: map[string]any{
658+
"time": time.Time{},
659+
"data": map[string]any{
660+
"key": "value",
661+
},
662+
"samplerate": 4,
663+
},
664+
expectNonNilTimestamp: true,
665+
},
666+
}
667+
668+
for _, tt := range tests {
669+
t.Run(tt.name, func(t *testing.T) {
670+
// Marshal the test data to msgpack
671+
msgpackBytes, err := msgpack.Marshal(tt.msgpackData)
672+
require.NoError(t, err)
673+
674+
// Unmarshal using our custom unmarshaller
675+
var event LibhoneyEvent
676+
err = event.UnmarshalMsgpack(msgpackBytes)
677+
678+
if tt.wantErr {
679+
assert.Error(t, err)
680+
return
681+
}
682+
683+
require.NoError(t, err)
684+
685+
// The key assertion: MsgPackTimestamp should never be nil after unmarshalling
686+
if tt.expectNonNilTimestamp {
687+
assert.NotNil(t, event.MsgPackTimestamp, "MsgPackTimestamp should never be nil after UnmarshalMsgpack")
688+
689+
// Additional checks
690+
if event.MsgPackTimestamp != nil && !event.MsgPackTimestamp.IsZero() {
691+
// If we have a valid timestamp, Time string should also be set
692+
assert.NotEmpty(t, event.Time, "Time string should be set when MsgPackTimestamp is valid")
693+
}
694+
}
695+
696+
// Check that data was preserved
697+
assert.Equal(t, tt.msgpackData["samplerate"], event.Samplerate)
698+
if data, ok := tt.msgpackData["data"].(map[string]any); ok {
699+
// Remove "time" from data if it was extracted
700+
delete(data, "time")
701+
if len(data) > 0 {
702+
for k, v := range data {
703+
assert.Equal(t, v, event.Data[k])
704+
}
705+
}
706+
}
707+
})
708+
}
709+
}

receiver/libhoneyreceiver/receiver.go

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"net/http"
1616
"strings"
1717
"sync"
18-
"time"
1918

2019
"github.com/vmihailenco/msgpack/v5"
2120
"go.opentelemetry.io/collector/component"
@@ -27,7 +26,6 @@ import (
2726

2827
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/errorutil"
2928
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/libhoneyreceiver/encoder"
30-
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/libhoneyreceiver/internal/eventtime"
3129
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/libhoneyreceiver/internal/libhoneyevent"
3230
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/libhoneyreceiver/internal/parser"
3331
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/libhoneyreceiver/internal/response"
@@ -253,6 +251,7 @@ func (r *libhoneyReceiver) handleEvent(resp http.ResponseWriter, req *http.Reque
253251
libhoneyevents := make([]libhoneyevent.LibhoneyEvent, 0)
254252
switch req.Header.Get("Content-Type") {
255253
case "application/x-msgpack", "application/msgpack":
254+
// The custom UnmarshalMsgpack will handle timestamp normalization
256255
decoder := msgpack.NewDecoder(bytes.NewReader(body))
257256
decoder.UseLooseInterfaceDecoding(true)
258257
err = decoder.Decode(&libhoneyevents)
@@ -261,27 +260,9 @@ func (r *libhoneyReceiver) handleEvent(resp http.ResponseWriter, req *http.Reque
261260
writeLibhoneyError(resp, enc, "failed to unmarshal msgpack")
262261
return
263262
}
264-
// Post-process msgpack events to ensure timestamps are set
265-
for i := range libhoneyevents {
266-
if libhoneyevents[i].MsgPackTimestamp == nil {
267-
if libhoneyevents[i].Time != "" {
268-
// Parse the time string and set MsgPackTimestamp
269-
propertime := eventtime.GetEventTime(libhoneyevents[i].Time)
270-
libhoneyevents[i].MsgPackTimestamp = &propertime
271-
} else {
272-
// No time field, use current time
273-
tnow := time.Now()
274-
libhoneyevents[i].MsgPackTimestamp = &tnow
275-
libhoneyevents[i].Time = eventtime.GetEventTimeDefaultString()
276-
}
277-
}
278-
}
279263
if len(libhoneyevents) > 0 {
280-
if libhoneyevents[0].MsgPackTimestamp != nil {
281-
r.settings.Logger.Debug("Decoding with msgpack worked", zap.Time("timestamp.first.msgpacktimestamp", *libhoneyevents[0].MsgPackTimestamp), zap.String("timestamp.first.time", libhoneyevents[0].Time))
282-
} else {
283-
r.settings.Logger.Debug("Decoding with msgpack worked", zap.String("timestamp.first.time", libhoneyevents[0].Time))
284-
}
264+
// MsgPackTimestamp is guaranteed to be non-nil after UnmarshalMsgpack
265+
r.settings.Logger.Debug("Decoding with msgpack worked", zap.Time("timestamp.first.msgpacktimestamp", *libhoneyevents[0].MsgPackTimestamp), zap.String("timestamp.first.time", libhoneyevents[0].Time))
285266
r.settings.Logger.Debug("event zero", zap.String("event.data", libhoneyevents[0].DebugString()))
286267
}
287268
case encoder.JSONContentType:

0 commit comments

Comments
 (0)