implement code review suggestions

elastic · Apr 11, 2018 · d47e7b1 · d47e7b1
1 parent bb711e4
commit d47e7b1
Show file tree

Hide file tree

Showing 8 changed files with 11 additions and 42 deletions.
diff --git a/processor/error/event.go b/processor/error/event.go
@@ -65,7 +65,7 @@ func DecodeEvent(input interface{}, err error) (*Event, error) {
 		Id:        decoder.StringPtr(raw, "id"),
 		Culprit:   decoder.StringPtr(raw, "culprit"),
 		Context:   decoder.MapStr(raw, "context"),
-		Timestamp: decoder.TimeRFC3339(raw, "timestamp"),
+		Timestamp: decoder.TimeRFC3339WithDefault(raw, "timestamp"),
 	}
 	transactionId := decoder.StringPtr(raw, "id", "transaction")
 	if transactionId != nil {

diff --git a/processor/error/payload.go b/processor/error/payload.go
@@ -1,8 +1,6 @@
 package error
 
 import (
-	"time"
-
 	"github.com/elastic/apm-server/config"
 	m "github.com/elastic/apm-server/model"
 	"github.com/elastic/apm-server/utility"
@@ -65,10 +63,6 @@ func (pa *payload) transform(config config.Config) []beat.Event {
 	var events []beat.Event
 	for _, event := range pa.Events {
 		context := context.Transform(event.Context)
-		var zeroTime = time.Time{}
-		if event.Timestamp == zeroTime {
-			event.Timestamp = time.Now()
-		}
 		ev := beat.Event{
 			Fields: common.MapStr{
 				"processor":  processorEntry,

diff --git a/processor/error/payload_test.go b/processor/error/payload_test.go
@@ -175,13 +175,3 @@ func TestPayloadTransform(t *testing.T) {
 		}
 	}
 }
-
-func TestSetTimestamp(t *testing.T) {
-	p := payload{
-		Service: m.Service{Name: "myservice"},
-		Events:  []Event{{}},
-	}
-	for _, event := range p.transform(config.Config{}) {
-		assert.InDelta(t, time.Now().Unix(), event.Timestamp.Unix(), time.Millisecond.Seconds()*10)
-	}
-}
diff --git a/processor/transaction/event.go b/processor/transaction/event.go
@@ -43,7 +43,7 @@ func DecodeEvent(input interface{}, err error) (*Event, error) {
 		Name:      decoder.StringPtr(raw, "name"),
 		Result:    decoder.StringPtr(raw, "result"),
 		Duration:  decoder.Float64(raw, "duration"),
-		Timestamp: decoder.TimeRFC3339(raw, "timestamp"),
+		Timestamp: decoder.TimeRFC3339WithDefault(raw, "timestamp"),
 		Context:   decoder.MapStr(raw, "context"),
 		Marks:     decoder.MapStr(raw, "marks"),
 		Sampled:   decoder.BoolPtr(raw, "sampled"),

diff --git a/processor/transaction/payload.go b/processor/transaction/payload.go
@@ -1,8 +1,6 @@
 package transaction
 
 import (
-	"time"
-
 	"github.com/elastic/apm-server/config"
 	m "github.com/elastic/apm-server/model"
 	"github.com/elastic/apm-server/utility"
@@ -67,10 +65,6 @@ func (pa *payload) transform(config config.Config) []beat.Event {
 
 	var events []beat.Event
 	for _, event := range pa.Events {
-		var zeroTime = time.Time{}
-		if event.Timestamp == zeroTime {
-			event.Timestamp = time.Now()
-		}
 		ev := beat.Event{
 			Fields: common.MapStr{
 				"processor":        processorTransEntry,

diff --git a/processor/transaction/payload_test.go b/processor/transaction/payload_test.go
@@ -239,13 +239,3 @@ func TestPayloadTransform(t *testing.T) {
 		}
 	}
 }
-
-func TestSetTimestamp(t *testing.T) {
-	p := payload{
-		Service: m.Service{Name: "myservice"},
-		Events:  []Event{{}},
-	}
-	for _, event := range p.transform(config.Config{}) {
-		assert.InDelta(t, time.Now().Unix(), event.Timestamp.Unix(), time.Millisecond.Seconds()*10)
-	}
-}
diff --git a/utility/data_fetcher.go b/utility/data_fetcher.go
@@ -143,10 +143,11 @@ func (d *ManualDecoder) MapStr(base map[string]interface{}, key string, keys ...
 	return nil
 }
 
-func (d *ManualDecoder) TimeRFC3339(base map[string]interface{}, key string, keys ...string) time.Time {
+// if the looked up value doesn't exist, it returns now
+func (d *ManualDecoder) TimeRFC3339WithDefault(base map[string]interface{}, key string, keys ...string) time.Time {
 	val := getDeep(base, keys...)[key]
 	if val == nil {
-		return time.Time{}
+		return time.Now()
 	}
 	if valStr, ok := val.(string); ok {
 		if valTime, err := time.Parse(time.RFC3339, valStr); err == nil {

diff --git a/utility/data_fetcher_test.go b/utility/data_fetcher_test.go
@@ -193,17 +193,17 @@ func TestMapStr(t *testing.T) {
 }
 
 func TestTimeRFC3339(t *testing.T) {
-	var outnil time.Time
+	var outZero time.Time
 	tp, _ := time.Parse(time.RFC3339, "2017-05-30T18:53:27.154Z")
 	for _, test := range []testStr{
 		{key: "time", keys: []string{}, out: tp, err: nil},
-		{key: "missing", keys: []string{"a", "b"}, out: outnil, err: nil},
-		{key: "str", keys: []string{"a", "b"}, out: outnil, err: fetchErr},
-		{key: "b", keys: []string{"a"}, out: outnil, err: fetchErr},
+		{key: "missing", keys: []string{"a", "b"}, out: time.Now(), err: nil},
+		{key: "str", keys: []string{"a", "b"}, out: outZero, err: fetchErr},
+		{key: "b", keys: []string{"a"}, out: outZero, err: fetchErr},
 	} {
 		decoder := ManualDecoder{}
-		out := decoder.TimeRFC3339(decoderBase, test.key, test.keys...)
-		assert.Equal(t, out, test.out)
+		out := decoder.TimeRFC3339WithDefault(decoderBase, test.key, test.keys...)
+		assert.InDelta(t, out.Unix(), test.out.(time.Time).Unix(), time.Millisecond.Seconds()*10)
 		assert.Equal(t, decoder.Err, test.err)
 	}
 }