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.

Sign up for GitHub

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

Rename Redis input read_timestamp to event.created #9924

Merged
merged 3 commits into from
Jan 15, 2019
Merged

Rename Redis input read_timestamp to event.created #9924

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aa4c29d
Rename Redis input read_timestamp to event.created
ruflin Jan 7, 2019
65310b1
remove common time and utc conversion
ruflin Jan 10, 2019
e03f299
Merge branch 'master' into rename-redis-timestamp
ruflin Jan 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Modify apache/error dataset to follow ECS. {pull}8963[8963]
- Rename many `traefik.access.*` fields to map to ECS. {pull}9005[9005]
- Fix parsing of GC entries in elasticsearch server log. {issue}9513[9513] {pull}9810[9810]
- Rename `read_timestamp` to `event.created` for Redis input. {pull}9924[9924]

*Heartbeat*

Expand Down
5 changes: 5 additions & 0 deletions dev-tools/ecs-migration.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,11 @@
to: http.version
alias: true

# Filebeat Redis Input

- from: read_timestamp
to: event.created
alias: false
Copy link
Contributor

@webmat webmat Jan 7, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Why alias false? I think we use read_timestamp in a few places. That could be a 1:1 mapping that catches most cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now it's false as there are still a few modules which use read_timestamp as a field. As soon as we completed the migration, we can probably make it an alias. Will add a checkbox to our migration issue.


# Auditbeat

Expand Down
4 changes: 3 additions & 1 deletion filebeat/input/redis/harvester.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,9 @@ func (h *Harvester) Run() error {
"redis": common.MapStr{
"slowlog": subEvent,
},
"read_timestamp": common.Time(time.Now().UTC()),
"event": common.MapStr{
"created": common.Time(time.Now().UTC()),
Copy link

Choose a reason for hiding this comment

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

I think we don't need common.Time anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean just using time.Now().UTC() instead? Did something change recently that we don't need it anymore? We use common.Time still quite a lot in the code base.

Copy link

Choose a reason for hiding this comment

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

I think we used to use common.Time for JSON formatting only. But with the introduction of go-structform for JSON encoding (in 6.0) we started to encode time.Time and common.Time the very same way in all our outputs.
These are the timestamp encoders for time.Time and common.Time used in all outputs: https://github.com/elastic/beats/blob/master/libbeat/outputs/codec/common.go#L28

The encoders always call UTC() + format string: yyyy-MM-dd'T'HH:mm:ss.SSS'Z'

We don't care which type you use and often check for both time types. common.Time still exists for legacy reasons (we should start to remove it at some point :) ).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the details. +1 on removing all the current usage.

I removed common.Time and UTC conversion in the most recent commit.

},
},
}

Expand Down