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 doesnt get printed when specfied in message codec #3721

Merged
merged 1 commit into from
Mar 28, 2017

Conversation

vjsamuel
Copy link
Contributor

@vjsamuel vjsamuel commented Mar 3, 2017

For the following output config:

output.console:
  codec.format:
    string: '%{[@timestamp]} %{[message]}'

the output is always empty because there is no switch case in the formatevents to handle common.Time

This PR fixes that.

@elasticmachine
Copy link
Collaborator

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.

1 similar comment
@elasticmachine
Copy link
Collaborator

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.

@vjsamuel
Copy link
Contributor Author

@urso can you please provide review comments?

Copy link
Member

@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.

LGTM. Waiting for @urso to be back from vacation to also have a look.

@urso
Copy link

urso commented Mar 28, 2017

Some notes about @timestamp, not to be solved in this PR:

  • MarshalJSON normalizes the @timestamp to UTC, but String() and the EventFormatter does not. That is, time zones are not used consistently in beats.
  • unfortunately dtfmt does not support timezone yet, but I'd prefer formatting (in MarshalJSON and String()) done via dtfmt package. Difference is, dtfmt pre-builds the formatter, but stdlib formatting needs to parse the format string and builds the formatter every single time (well, minor detail)

Current PR seems to be most consistent solution as of now.

@tsg tsg merged commit 715593c into elastic:master Mar 28, 2017
urso pushed a commit to urso/beats that referenced this pull request Apr 26, 2017
ruflin pushed a commit that referenced this pull request Apr 26, 2017
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