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

Timestamp override #7

Closed
wants to merge 2 commits into from

Conversation

lingster
Copy link

Added option to specify timestamp from field in event.
use the timestamp_override field to define the field to use when specifying a timestamp instead of using the timestamp of the event. Useful if parsing historical log files.

Added option to specify timestamp from field in event.
use the timestamp_override field
@lingster
Copy link
Author

Apologises I noticed in the diff that my editor appears to have changed the quotes from ` to ' I can revert this change if it's an annoyance.

@electrical
Copy link

Would it make more sense to have a config option called timestamp_field and default it to the @timestamp field?
This avoid any extra conditionals like you have now.

@lingster
Copy link
Author

ah yes - that's a splendid idea....I'll look into it!

Amend timestamp_override to be named timestamp_field with default as
@timestamp
@purbon
Copy link

purbon commented Sep 4, 2015

Thanks a lot for your contribution @lingster , in order to move forward with your PR is going to be necessary to sign the CLA agreement, you can find more information at https://www.elastic.co/contributor-agreement.

On the other side, it would be super nice if you can add test for this change. We try to enforce that all PR introduce some kind of testing, so we're able to increase the overall quality. Don't hesitate to ask any question regarding you might have, more than looking forward to help.

Looking forward to get this in,

  • purbon

@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

@purbon purbon mentioned this pull request Nov 2, 2015
@purbon
Copy link

purbon commented Nov 2, 2015

Opened #19 because of the long stalled CLA request not fulfilled, more than welcome to reopen and get this merged if the CLA gets signed.
Thanks a lot for your time.

@purbon purbon closed this Nov 2, 2015
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.

4 participants