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

Jump to bottom

Add syslog support for ISO8601 format timestamp #10736

Merged
merged 3 commits into from
Feb 15, 2019
Merged

Add syslog support for ISO8601 format timestamp #10736

merged 3 commits into from
Feb 15, 2019

Conversation

kaiyan-sheng
Copy link
Contributor

This PR is to add support for syslog which has ISO8601 format timestamps. For example:
Suse Format:

2018-08-14T14:30:02.203151+02:00 linux-sqrz systemd[4179]: Stopped target Basic System.
2018-08-14T14:30:02.203251+02:00 linux-sqrz systemd[4179]: Stopped target Paths.

closes #8716

@kaiyan-sheng kaiyan-sheng requested a review from a team as a code owner February 13, 2019 21:01
@kaiyan-sheng kaiyan-sheng self-assigned this Feb 13, 2019
@kaiyan-sheng kaiyan-sheng added Filebeat Filebeat Team:Integrations Label for the Integrations team labels Feb 13, 2019
@kaiyan-sheng kaiyan-sheng added needs_backport PR is waiting to be backported to other branches. v7.0.0 labels Feb 13, 2019
sayden
sayden approved these changes Feb 14, 2019
View reviewed changes
Copy link
Contributor

@sayden sayden left a comment

Choose a reason for hiding this comment

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

Looks good! Tested locally. Error seems unrelated

ruflin
ruflin suggested changes Feb 14, 2019
View reviewed changes
filebeat/module/system/syslog/test/suse-syslog.log-expected.json
@@ -0,0 +1,28 @@
[
{
"ecs.version": "1.0.0-beta2",
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see an @timestamp in the output here. Do I miss something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ruflin unfortunately all the timestamps got removed in https://github.com/elastic/beats/pull/10736/files/5c8b9d3205efc146891117823f724935600ed372#diff-7f560f729ae36bb40472cb039fedc34fL42

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, regarding to the @timestamp field: https://github.com/elastic/beats/blob/master/filebeat/tests/system/test_modules.py#L220 because timestamp got removed from system.syslog metricset

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. I now understand why it happens. The problem now is that the log file above actually has timestamps with the year inside but we don't test it. This becomes especially important as we just added a pattern to support year but the tests we have to confirm it works, don't do that.

Is there an easy way to fix this?

ruflin
ruflin approved these changes Feb 15, 2019
View reviewed changes
Copy link
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

I had a chat with @kaiyan-sheng about the missing timestamp. We decided to merge it as is but follow up with a convention or something similar to make it possible in the future to have here the timestamps generated.

@kaiyan-sheng kaiyan-sheng merged commit cb241f3 into elastic:master Feb 15, 2019
@kaiyan-sheng kaiyan-sheng deleted the suse_timestamp branch February 15, 2019 16:17
kaiyan-sheng added a commit that referenced this pull request Feb 18, 2019
@kaiyan-sheng
Cherry-pick to 7.0: Add syslog support for ISO8601 format timestamp (#…
b1fd1f4 
…10736) (#10776)

* Add syslog support for ISO8601 format timestamp (#10736)

* Add syslog support for ISO8601 format timestamp

* Add changelog

* Change timestamp pattern

(cherry picked from commit cb241f3)

* only cherrypick the correct changelog
@kaiyan-sheng kaiyan-sheng removed needs_backport PR is waiting to be backported to other branches. v7.0.0 labels Feb 18, 2019
@kaiyan-sheng kaiyan-sheng mentioned this pull request Feb 18, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruflin ruflin ruflin approved these changes

@sayden sayden sayden approved these changes

Assignees

@kaiyan-sheng kaiyan-sheng

Labels
Filebeat Filebeat Team:Integrations Label for the Integrations team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Parse ISO8601 timestamps by default
3 participants
@kaiyan-sheng @ruflin @sayden