Skip to content

Commit

Permalink
[chore] [internal/splunk] Don't use pointer for Event.Time field value (
Browse files Browse the repository at this point in the history
open-telemetry#22030)

The pointer was added to avoid sending zero timestamp by Splunk HEC exporter. But using pointer is not required for that `json:omitempty` tag does the same for float64 zero values.

After this change, we cannot differentiate `nil`/`0` values of `Time` field in the internal `splunk.Event` struct, but it doesn't matter because both versions produce the same JSON string without `time` field and translates to/from the same OTLP `0` values.
  • Loading branch information
dmitryax committed May 17, 2023
1 parent 60f5708 commit 721ca5e
Show file tree
Hide file tree
Showing 11 changed files with 42 additions and 70 deletions.
14 changes: 2 additions & 12 deletions exporter/splunkhecexporter/logdata_to_splunk.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,18 +106,8 @@ func mapLogRecordToSplunkEvent(res pcommon.Resource, lr plog.LogRecord, config *
}

// nanoTimestampToEpochMilliseconds transforms nanoseconds into <sec>.<ms>. For example, 1433188255.500 indicates 1433188255 seconds and 500 milliseconds after epoch.
func nanoTimestampToEpochMilliseconds(ts pcommon.Timestamp) *float64 {
duration := time.Duration(ts)
if duration == 0 {
// some telemetry sources send data with timestamps set to 0 by design, as their original target destinations
// (i.e. before Open Telemetry) are setup with the know-how on how to consume them. In this case,
// we want to omit the time field when sending data to the Splunk HEC so that the HEC adds a timestamp
// at indexing time, which will be much more useful than a 0-epoch-time value.
return nil
}

val := duration.Round(time.Millisecond).Seconds()
return &val
func nanoTimestampToEpochMilliseconds(ts pcommon.Timestamp) float64 {
return time.Duration(ts).Round(time.Millisecond).Seconds()
}

func mergeValue(dst map[string]any, k string, v any) {
Expand Down
8 changes: 4 additions & 4 deletions exporter/splunkhecexporter/logdata_to_splunk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ func commonLogSplunkEvent(

func Test_emptyLogRecord(t *testing.T) {
event := mapLogRecordToSplunkEvent(pcommon.NewResource(), plog.NewLogRecord(), &Config{})
assert.Nil(t, event.Time)
assert.Zero(t, event.Time)
assert.Equal(t, event.Host, "unknown")
assert.Zero(t, event.Source)
assert.Zero(t, event.SourceType)
Expand All @@ -471,11 +471,11 @@ func Test_emptyLogRecord(t *testing.T) {

func Test_nanoTimestampToEpochMilliseconds(t *testing.T) {
splunkTs := nanoTimestampToEpochMilliseconds(1001000000)
assert.Equal(t, 1.001, *splunkTs)
assert.Equal(t, 1.001, splunkTs)
splunkTs = nanoTimestampToEpochMilliseconds(1001990000)
assert.Equal(t, 1.002, *splunkTs)
assert.Equal(t, 1.002, splunkTs)
splunkTs = nanoTimestampToEpochMilliseconds(0)
assert.True(t, nil == splunkTs)
assert.Zero(t, splunkTs)
}

func Test_mergeValue(t *testing.T) {
Expand Down
14 changes: 2 additions & 12 deletions exporter/splunkhecexporter/metricdata_to_splunk.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,18 +276,8 @@ func cloneMapWithSelector(fields map[string]interface{}, selector func(string) b
return newFields
}

func timestampToSecondsWithMillisecondPrecision(ts pcommon.Timestamp) *float64 {
if ts == 0 {
// some telemetry sources send data with timestamps set to 0 by design, as their original target destinations
// (i.e. before Open Telemetry) are setup with the know-how on how to consume them. In this case,
// we want to omit the time field when sending data to the Splunk HEC so that the HEC adds a timestamp
// at indexing time, which will be much more useful than a 0-epoch-time value.
return nil
}

val := math.Round(float64(ts)/1e6) / 1e3

return &val
func timestampToSecondsWithMillisecondPrecision(ts pcommon.Timestamp) float64 {
return math.Round(float64(ts)/1e6) / 1e3
}

func float64ToDimValue(f float64) string {
Expand Down
10 changes: 5 additions & 5 deletions exporter/splunkhecexporter/metricdata_to_splunk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ func Test_mergeEventsToMultiMetricFormat(t *testing.T) {

func commonSplunkMetric(
metricName string,
ts *float64,
ts float64,
keys []string,
values []interface{},
val interface{},
Expand Down Expand Up @@ -695,22 +695,22 @@ func commonSplunkMetric(

func TestTimestampFormat(t *testing.T) {
ts := pcommon.Timestamp(32001000345)
assert.Equal(t, 32.001, *timestampToSecondsWithMillisecondPrecision(ts))
assert.Equal(t, 32.001, timestampToSecondsWithMillisecondPrecision(ts))
}

func TestTimestampFormatRounding(t *testing.T) {
ts := pcommon.Timestamp(32001999345)
assert.Equal(t, 32.002, *timestampToSecondsWithMillisecondPrecision(ts))
assert.Equal(t, 32.002, timestampToSecondsWithMillisecondPrecision(ts))
}

func TestTimestampFormatRoundingWithNanos(t *testing.T) {
ts := pcommon.Timestamp(9999999999991500001)
assert.Equal(t, 9999999999.992, *timestampToSecondsWithMillisecondPrecision(ts))
assert.Equal(t, 9999999999.992, timestampToSecondsWithMillisecondPrecision(ts))
}

func TestNilTimeWhenTimestampIsZero(t *testing.T) {
ts := pcommon.Timestamp(0)
assert.Nil(t, timestampToSecondsWithMillisecondPrecision(ts))
assert.Zero(t, timestampToSecondsWithMillisecondPrecision(ts))
}

func TestMergeEvents(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions internal/splunk/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ type AccessTokenPassthroughConfig struct {

// Event represents a metric in Splunk HEC format
type Event struct {
Time *float64 `json:"time,omitempty"` // optional epoch time - set to nil if the event timestamp is missing or unknown
Time float64 `json:"time,omitempty"` // optional epoch time - set to zero if the event timestamp is missing or unknown (will be added at indexing time)
Host string `json:"host"` // hostname
Source string `json:"source,omitempty"` // optional description of the source of the event; typically the app's name
SourceType string `json:"sourcetype,omitempty"` // optional name of a Splunk parsing configuration; this is usually inferred by Splunk
Expand Down Expand Up @@ -99,14 +99,14 @@ func (e *Event) UnmarshalJSON(b []byte) error {
}
switch t := rawEvent.Time.(type) {
case float64:
e.Time = &t
e.Time = t
case string:
{
time, err := strconv.ParseFloat(t, 64)
if err != nil {
return err
}
e.Time = &time
e.Time = time
}
}
return nil
Expand Down
6 changes: 3 additions & 3 deletions internal/splunk/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func TestDecodeJsonWithNoTime(t *testing.T) {
var msg Event
err := dec.Decode(&msg)
assert.NoError(t, err)
assert.Nil(t, msg.Time)
assert.Zero(t, msg.Time)
}

func TestDecodeJsonWithNumberTime(t *testing.T) {
Expand All @@ -86,7 +86,7 @@ func TestDecodeJsonWithNumberTime(t *testing.T) {
var msg Event
err := dec.Decode(&msg)
assert.NoError(t, err)
assert.Equal(t, 1610760752.606, *msg.Time)
assert.Equal(t, 1610760752.606, msg.Time)
}

func TestDecodeJsonWithStringTime(t *testing.T) {
Expand All @@ -96,7 +96,7 @@ func TestDecodeJsonWithStringTime(t *testing.T) {
var msg Event
err := dec.Decode(&msg)
assert.NoError(t, err)
assert.Equal(t, 1610760752.606, *msg.Time)
assert.Equal(t, 1610760752.606, msg.Time)
}

func TestDecodeJsonWithInvalidStringTime(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions receiver/splunkhecreceiver/receiver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,7 @@ func Test_Metrics_splunkhecReceiver_IndexSourceTypePassthrough(t *testing.T) {

func buildSplunkHecMetricsMsg(time float64, value int64, dimensions uint) *splunk.Event {
ev := &splunk.Event{
Time: &time,
Time: time,
Event: "metric",
Fields: map[string]interface{}{
"metric_name:foo": value,
Expand All @@ -842,7 +842,7 @@ func buildSplunkHecMetricsMsg(time float64, value int64, dimensions uint) *splun

func buildSplunkHecMsg(time float64, dimensions uint) *splunk.Event {
ev := &splunk.Event{
Time: &time,
Time: time,
Event: "foo",
Fields: map[string]interface{}{},
Index: "myindex",
Expand Down
4 changes: 1 addition & 3 deletions receiver/splunkhecreceiver/splunk_to_logdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@ func splunkHecToLogData(logger *zap.Logger, events []*splunk.Event, resourceCust

// Splunk timestamps are in seconds so convert to nanos by multiplying
// by 1 billion.
if event.Time != nil {
logRecord.SetTimestamp(pcommon.Timestamp(*event.Time * 1e9))
}
logRecord.SetTimestamp(pcommon.Timestamp(event.Time * 1e9))

// Set event fields first, so the specialized attributes overwrite them if needed.
keys := make([]string, 0, len(event.Fields))
Expand Down
22 changes: 10 additions & 12 deletions receiver/splunkhecreceiver/splunk_to_logdata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func Test_SplunkHecToLogData(t *testing.T) {
name: "happy_path",
events: []*splunk.Event{
{
Time: &time,
Time: time,
Host: "localhost",
Source: "mysource",
SourceType: "mysourcetype",
Expand All @@ -76,7 +76,7 @@ func Test_SplunkHecToLogData(t *testing.T) {
name: "double",
events: []*splunk.Event{
{
Time: &time,
Time: time,
Host: "localhost",
Source: "mysource",
SourceType: "mysourcetype",
Expand All @@ -99,7 +99,7 @@ func Test_SplunkHecToLogData(t *testing.T) {
name: "array",
events: []*splunk.Event{
{
Time: &time,
Time: time,
Host: "localhost",
Source: "mysource",
SourceType: "mysourcetype",
Expand All @@ -126,7 +126,7 @@ func Test_SplunkHecToLogData(t *testing.T) {
name: "complex_structure",
events: []*splunk.Event{
{
Time: &time,
Time: time,
Host: "localhost",
Source: "mysource",
SourceType: "mysourcetype",
Expand Down Expand Up @@ -158,7 +158,6 @@ func Test_SplunkHecToLogData(t *testing.T) {
name: "nil_timestamp",
events: []*splunk.Event{
{
Time: new(float64),
Host: "localhost",
Source: "mysource",
SourceType: "mysourcetype",
Expand All @@ -179,7 +178,6 @@ func Test_SplunkHecToLogData(t *testing.T) {
name: "custom_config_mapping",
events: []*splunk.Event{
{
Time: new(float64),
Host: "localhost",
Source: "mysource",
SourceType: "mysourcetype",
Expand Down Expand Up @@ -220,7 +218,7 @@ func Test_SplunkHecToLogData(t *testing.T) {
name: "group_events_by_resource_attributes",
events: []*splunk.Event{
{
Time: &time,
Time: time,
Host: "1",
Source: "1",
SourceType: "1",
Expand All @@ -231,7 +229,7 @@ func Test_SplunkHecToLogData(t *testing.T) {
},
},
{
Time: &time,
Time: time,
Host: "2",
Source: "2",
SourceType: "2",
Expand All @@ -242,7 +240,7 @@ func Test_SplunkHecToLogData(t *testing.T) {
},
},
{
Time: &time,
Time: time,
Host: "1",
Source: "1",
SourceType: "1",
Expand All @@ -253,7 +251,7 @@ func Test_SplunkHecToLogData(t *testing.T) {
},
},
{
Time: &time,
Time: time,
Host: "2",
Source: "2",
SourceType: "2",
Expand All @@ -264,7 +262,7 @@ func Test_SplunkHecToLogData(t *testing.T) {
},
},
{
Time: &time,
Time: time,
Host: "1",
Source: "2",
SourceType: "1",
Expand All @@ -275,7 +273,7 @@ func Test_SplunkHecToLogData(t *testing.T) {
},
},
{
Time: &time,
Time: time,
Host: "2",
Source: "1",
SourceType: "2",
Expand Down
8 changes: 2 additions & 6 deletions receiver/splunkhecreceiver/splunkhec_to_metricdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,8 @@ func addDoubleGauge(metrics pmetric.MetricSlice, metricName string, value float6
attributes.CopyTo(doublePt.Attributes())
}

func convertTimestamp(sec *float64) pcommon.Timestamp {
if sec == nil {
return 0
}

return pcommon.Timestamp(*sec * 1e9)
func convertTimestamp(sec float64) pcommon.Timestamp {
return pcommon.Timestamp(sec * 1e9)
}

// Extract dimensions from the Splunk event fields to populate metric data point attributes.
Expand Down
16 changes: 8 additions & 8 deletions receiver/splunkhecreceiver/splunkhec_to_metricdata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func Test_splunkV2ToMetricsData(t *testing.T) {

buildDefaultSplunkDataPt := func() *splunk.Event {
return &splunk.Event{
Time: &sec,
Time: sec,
Host: "localhost",
Source: "source",
SourceType: "sourcetype",
Expand Down Expand Up @@ -243,7 +243,7 @@ func Test_splunkV2ToMetricsData(t *testing.T) {
name: "zero_timestamp",
splunkDataPoint: func() *splunk.Event {
pt := buildDefaultSplunkDataPt()
pt.Time = new(float64)
pt.Time = 0
return pt
}(),
wantMetricsData: func() pmetric.Metrics {
Expand Down Expand Up @@ -319,7 +319,7 @@ func TestGroupMetricsByResource(t *testing.T) {
nanoseconds := int64(sec * 1e9)
events := []*splunk.Event{
{
Time: &sec,
Time: sec,
Host: "1",
Source: "1",
SourceType: "1",
Expand All @@ -330,7 +330,7 @@ func TestGroupMetricsByResource(t *testing.T) {
},
},
{
Time: &sec,
Time: sec,
Host: "2",
Source: "2",
SourceType: "2",
Expand All @@ -341,7 +341,7 @@ func TestGroupMetricsByResource(t *testing.T) {
},
},
{
Time: &sec,
Time: sec,
Host: "1",
Source: "1",
SourceType: "1",
Expand All @@ -352,7 +352,7 @@ func TestGroupMetricsByResource(t *testing.T) {
},
},
{
Time: &sec,
Time: sec,
Host: "2",
Source: "2",
SourceType: "2",
Expand All @@ -364,7 +364,7 @@ func TestGroupMetricsByResource(t *testing.T) {
},
},
{
Time: &sec,
Time: sec,
Host: "1",
Source: "2",
SourceType: "1",
Expand All @@ -375,7 +375,7 @@ func TestGroupMetricsByResource(t *testing.T) {
},
},
{
Time: &sec,
Time: sec,
Host: "2",
Source: "1",
SourceType: "2",
Expand Down

0 comments on commit 721ca5e

Please sign in to comment.