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

Add rfc3339 timestamps to default format #33

Merged
merged 1 commit into from
Nov 5, 2017
Merged

Conversation

KodrAus
Copy link
Collaborator

@KodrAus KodrAus commented Nov 1, 2017

Closes #30

Adds RFC3339 formatted timestamps to the default log format:

timestamps

I've made an effort to avoid allocating temporary strings or needing to parse a format for each log, so there's a currently private Timestamp struct that can be pulled off a Formatter that does this.

@KodrAus
Copy link
Collaborator Author

KodrAus commented Nov 1, 2017

r? @mjkillough

@KodrAus KodrAus merged commit a897069 into master Nov 5, 2017
@mjkillough
Copy link
Contributor

@KodrAus - Apologies for the late review, I was on holiday until this afternoon.

This looks great! :) I didn't realise how great the formatting in chrono was.

@tailhook
Copy link
Contributor

tailhook commented Nov 5, 2017

Well, since the UTC time is used anyway (which is a good thing), I think +00:00 is excessive, Z is usually used in that case and is clear enough and shorter (so easier to observe in console, since this is the primary target of env_logger). Also, I think that nanosecond precision, while can be occasionally useful is also excessive. I'm usually okay with second granularity. May be millisecond if possible.

In my opinion time::now_utc().rfc3339() is good enough and produces value that looks like: 2017-01-01T00:00:00Z, and also avoids few more dependencies.

@KodrAus
Copy link
Collaborator Author

KodrAus commented Nov 6, 2017

Thanks @mjkillough and @tailhook!

The time crate is effectively deprecated in favour of chrono so I don't think we should depend on it directly. I think the points about the format are good, we might end up having to reconstruct a simpler rfc3339 format manually though. But that's not the end of the world.

@KodrAus
Copy link
Collaborator Author

KodrAus commented Nov 6, 2017

Added #34 to sort the format out.

@KodrAus KodrAus deleted the feat/rfc3339-ts branch November 6, 2017 00:53
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.

None yet

3 participants