Skip to content
This repository has been archived by the owner on May 6, 2020. It is now read-only.

logger: Fix nanosecond timestamp test #1018

Merged

Conversation

jodh-intel
Copy link
Contributor

TestNewSystemLogHook() checks to ensure a nanonsecond timestamp is
generated. However, the time.RFC3339Nano time format truncates
trailing zeros meaning a nanosecond timestamp won't always have nine
digits.

Fixes #1017.

Signed-off-by: James O. D. Hunt [email protected]

`TestNewSystemLogHook()` checks to ensure a nanonsecond timestamp is
generated. However, the `time.RFC3339Nano` time format truncates
trailing zeros meaning a nanosecond timestamp won't always have nine
digits.

Fixes clearcontainers#1017.

Signed-off-by: James O. D. Hunt <[email protected]>
Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one query, but other than that
lgtm

// nano-seconds. Note that the quantifier range is
// required because the time.RFC3339Nano format
// trunctates trailing zeros.
`\d{1,9}` +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just checking - I presume '0' comes out as '0', and hence always at least 1 digit? :-)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hehe, same question

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good question. It isn't documented afaics but observationally there is always a leading zero, yes. I've also discovered something rather interesting by looking at the parsing code:

time.RFC3339Nano truncates trailing zeros because it is defined like this:

RFC3339Nano = "2006-01-02T15:04:05.999999999Z07:00"

But, you can create a non-truncating format by switching the values after the decimal point to zeros!:

const RFC3339NanoNoTruncate = "2006-01-02T15:04:05.000000000Z07:00"

Kinda surprising someone didn't add that as a standard option given the documented sorting behaviour of time.RFC3339Nano.

/cc @markdryan.

@clearcontainersbot
Copy link

kubernetes qa-passed 👍

@devimc
Copy link

devimc commented Feb 21, 2018

lgtm

Approved with PullApprove Approved with PullApprove

@jodh-intel jodh-intel mentioned this pull request Feb 22, 2018
@jodh-intel
Copy link
Contributor Author

The metrics CI is again stopping the button from going green :-(

Force-merging as everything else is happy and this is causing the new release on #1021 to fail CI.

@jodh-intel jodh-intel merged commit 9686008 into clearcontainers:master Feb 22, 2018
@grahamwhaley
Copy link
Contributor

Just to note, the metrics CI failed due to a network/docker hub issue:

07:00:21 Get https://registry-1.docker.io/v2/library/nginx/manifests/sha256:600bff7fb36d7992512f8c07abd50aac08db8f17c94e3c83e47d53435a1a6f7c: net/http: TLS handshake timeout
07:00:21 ERROR: Failed to docker pull image nginx

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants