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

Fix TimeStamp #1102

Merged
merged 1 commit into from
Apr 26, 2017
Merged

Fix TimeStamp #1102

merged 1 commit into from
Apr 26, 2017

Conversation

dvas0004
Copy link
Contributor

@dvas0004 dvas0004 commented Apr 5, 2017

The previous method (using ctime) has a number of issues that make it a bit weird to integrate with popular logging tools like ELK:

  1. It appends a \n at the end of the timestamp which messes up date formatting for the unwary
  2. It outputs time in the local timezone, without actually outputting the timezone... so logging tools like ELK will assume the date is in UTC, when actually it's GMT+2 or whatever... causing silly things like display logs in the future

Easier method would be as proposed - simply output the timestamp in UNIX/EPOCH format. In this case it's millisecond epoch format - this is far more unambiguous for log parsers


This change is Reviewable

The previous method (using ctime) has a number of issues that make it a bit weird to integrate with popular logging tools like ELK:

1. It appends a \n at the end of the timestamp which messes up date formatting for the unwary
2. It outputs time in the local timezone, without actually outputting the timezone... so logging tools like ELK will assume the date is in UTC, when actually it's GMT+2 or whatever... causing silly things like display logs in the future 

Easier method would be as proposed - simply output the timestamp in UNIX/EPOCH format. In this case it's millisecond epoch format - this is far more unambiguous for log parsers
@ddpbsd
Copy link
Member

ddpbsd commented Apr 26, 2017

I don't like changing things like this, but restricting it to the json log makes it a bit better.
Thanks for the PR!

@ddpbsd ddpbsd merged commit f42f679 into ossec:master Apr 26, 2017
@dvas0004
Copy link
Contributor Author

dvas0004 commented Apr 27, 2017 via email

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.

2 participants