Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

makes timestamp optional #819

Merged
merged 4 commits into from
Apr 11, 2018
Merged

makes timestamp optional #819

merged 4 commits into from
Apr 11, 2018

Conversation

jalvz
Copy link
Contributor

@jalvz jalvz commented Apr 9, 2018

fixes #723

@@ -65,6 +67,10 @@ 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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this IsZero method could be useful here: https://golang.org/pkg/time/#Time.IsZero

@@ -145,6 +145,9 @@ func (d *ManualDecoder) MapStr(base map[string]interface{}, key string, keys ...

func (d *ManualDecoder) TimeRFC3339(base map[string]interface{}, key string, keys ...string) time.Time {
val := getDeep(base, keys...)[key]
if val == nil {
return time.Time{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about introducing a TimeRFC3339WithDefault method that returns a time.Now() instead of a zero time in case it is empty? The decoding happens before the transformation, so the timestamp would be more accurate, and no further handling in the transform function would be needed.

@jalvz jalvz force-pushed the optional-timestamp branch from d47e7b1 to 8564f95 Compare April 11, 2018 07:13
@jalvz jalvz merged commit 558480c into elastic:master Apr 11, 2018
@roncohen
Copy link
Contributor

this is great. Good work @jalvz @simitt

jalvz added a commit to jalvz/apm-server that referenced this pull request Apr 12, 2018
jalvz added a commit that referenced this pull request Apr 12, 2018
@jalvz jalvz deleted the optional-timestamp branch November 5, 2018 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Force a timestamp on RUM data
3 participants