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

Replace read_timestamp with event.created in all remaining Filebeat modules #10139

Merged
merged 12 commits into from
Jan 18, 2019

Conversation

webmat
Copy link
Contributor

@webmat webmat commented Jan 17, 2019

Note that some of the recent module migrations have already started populating event.created instead of read_timestamp. This PR does the remaining modules all at once.

Modules/filesets affected:

  • iis.error
  • kafka
  • kibana
  • nginx.access
  • osquery
  • redis
  • traefik.access

Outside of these modules, the only occurrences I see of read_timestamp are the field definitions and test_modules.py.

So this PR will alias read_timestamp to event.created as well.

I've taken the opportunity to put in place the alias for Journalbeat as well. Didn't look like it was being used there anymore.

@webmat webmat requested review from a team as code owners January 17, 2019 01:49
@webmat webmat self-assigned this Jan 17, 2019
@ruflin ruflin mentioned this pull request Jan 17, 2019
@webmat
Copy link
Contributor Author

webmat commented Jan 17, 2019

Test failures are related:

{"type":"mapper_parsing_exception","reason":"failed to parse","caused_by":{"type":"illegal_argument_exception","reason":"Cannot write to a field alias [read_timestamp]."}}

For Kibana, Redis and Kafka

@webmat webmat requested a review from a team as a code owner January 17, 2019 03:28
@webmat webmat added the review label Jan 17, 2019
@webmat webmat requested review from ruflin and ycombinator January 17, 2019 03:31
@webmat
Copy link
Contributor Author

webmat commented Jan 17, 2019

@ycombinator In migrating read_timestamp to event.created, I noticed that the Kibana pipeline wasn't using the field correctly. This field is meant to contain the time at which the Beat first saw the event, not the timestamp contained in the event from upstream.

dev-tools/ecs-migration.yml Outdated Show resolved Hide resolved
@ycombinator
Copy link
Contributor

Thanks for fixing up the Kibana pipeline, @webmat! Would it be worth adding a special mention about this breaking change in the CHANGELOG?

Mathieu Martin added 11 commits January 17, 2019 20:40
- iis.error
- nginx.access
- osquery
- traefik.access
- read_timestamp can be aliased in 6.x for forward compatibility
- it was present twice in the file, now only present once at the beginning, with other common fields
Will modify both entries to identify the only two beats affected, instead.

This reverts commit 4ee063a3cd5755d773822fa83a4cf1fda04f1441.
@webmat
Copy link
Contributor Author

webmat commented Jan 18, 2019

@ycombinator Oh yes, good point. I've done that. Hopefully not too verbose.

@webmat webmat merged commit 31fec4f into elastic:master Jan 18, 2019
@webmat webmat deleted the ecs-read-ts branch January 18, 2019 02:06
DStape pushed a commit to DStape/beats that referenced this pull request Aug 20, 2019
…astic#10139)

Note that some of the recent module migrations have already started populating `event.created` instead of `read_timestamp`. This PR finishes this work all at once.

- Replace `read_timestamp` with `event.created` in remaining Fb modules:
  - iis.error
  - kafka
  - kibana
  - nginx.access
  - osquery
  - redis
  - traefik.access
- No longer excluding `read_timestamp` from integration test results, as it's no longer expected
- Finish equivalent migration in Journalbeat by making `read_timestamp` into an alias
- Adjust Kibana module's pipeline to use the correct semantics for `event.created`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants