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

Teach elasticsearch/audit fileset to handle timestamps correctly #15942

Merged
merged 6 commits into from
Jan 30, 2020

Conversation

ycombinator
Copy link
Contributor

What does this PR do?

This PR teaches the elasticsearch/audit fileset to correctly parse the subtle variations of timestamp fields that can be found in the JSON-formatted Elasticsearch audit logs across multiple versions of Elasticsearch.

Why is it important?

Regardless of the version of Elasticsearch generating audit logs, the elasticsearch/audit fileset will be able to parse them, specifically the timestamps within them, correctly.

Checklist

  • My code follows the style guidelines of this project
    - [ ] I have commented my code, particularly in hard-to-understand areas
    - [ ] I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works

Related issues

@elasticmachine
Copy link
Collaborator

Pinging @elastic/stack-monitoring (Stack monitoring)

@@ -6,5 +6,4 @@ paths:
exclude_files: [".gz$"]

processors:
# Locale for timezone is only needed in non-json logs
- add_locale.when.not.regexp.message: "^{"
- add_locale: ~
Copy link
Contributor Author

@ycombinator ycombinator Jan 29, 2020

Choose a reason for hiding this comment

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

We add this processor for JSON logs as well because some versions of ES (e.g. 7.1.1) generate JSON audit logs with the @timestamp field which does not contain a time zone.

Later versions of ES will generate JSON audit logs with a timestamp field which does contain the time zone. In such cases, we will remove the event.timezone field in the ingest pipeline to avoid confusion about which timezone is correct.

Related: #13918.

CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM if tests pass without changes in the expected output for 7.3.0 (JSON file that includes timestamp)

@ycombinator ycombinator merged commit 558072d into elastic:master Jan 30, 2020
@ycombinator ycombinator deleted the fb-es-audit-ts branch January 30, 2020 17:13
@lucabelluccini
Copy link
Contributor

Thank you @ycombinator to follow up on this

ycombinator added a commit that referenced this pull request Feb 5, 2020
…15942) (#15963)

* Add samples from ES 7.1.1 audit log

* When JSON logs contain @timestamp and TZ is detected, use detected TZ

* Add CHANGELOG entry

* Remove redundant guard

* Revert unrelated change

* s/teach/improve/ in changelog entry
@ycombinator ycombinator added v7.6.0 and removed v7.6.1 labels Feb 5, 2020
ycombinator added a commit that referenced this pull request Feb 8, 2020
…15942) (#15962)

* Add samples from ES 7.1.1 audit log

* When JSON logs contain @timestamp and TZ is detected, use detected TZ

* Add CHANGELOG entry

* Remove redundant guard

* Revert unrelated change

* s/teach/improve/ in changelog entry
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.

6 participants