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

Conversation

Projects
None yet
3 participants
@KodrAus
Copy link
Collaborator

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

This comment has been minimized.

Copy link
Collaborator Author

KodrAus commented Nov 1, 2017

@KodrAus KodrAus merged commit a897069 into master Nov 5, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@mjkillough

This comment has been minimized.

Copy link
Contributor

mjkillough commented Nov 5, 2017

@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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Collaborator Author

KodrAus commented Nov 6, 2017

Added #34 to sort the format out.

@KodrAus KodrAus deleted the feat/rfc3339-ts branch Nov 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.