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

logging: waku timestamp precision #251

Closed
mkobetic opened this issue Jun 1, 2022 · 2 comments · Fixed by #253
Closed

logging: waku timestamp precision #251

mkobetic opened this issue Jun 1, 2022 · 2 comments · Fixed by #253

Comments

@mkobetic
Copy link
Contributor

mkobetic commented Jun 1, 2022

In #242 I have probably incorrectly concluded that "waku timestamp is a microsecond unix time". In reality it seems that it may have been true with Waku but Waku2 seems to be moving to nanosecond precision.

I made my conclusion based on what I observed in our test suite originally where we were using an older version of js-waku running against a go-waku node. It seems js-waku has since implemented store 2.0.0 beta4 which changed the precision of the sent timestamp that I was logging to nano-seconds.

So, the logging helper is likely wrong and should change to nanoseconds going forward, although it's not entirely clear how much of a residual mismatch should we expect while the various parts of the waku ecosystem are being upgraded to waku2. We can certainly make the helper more defensive and have it check whether the value looks like a nanosecond value or microsecond value and do the right thing. That would however hide the discrepancy, so maybe we want to see the invalid timestamps in the logs so that they can be hunted down and fixed.

I'm happy to make the PR, just need some guidance as to how to approach this.

@richard-ramos
Copy link
Member

Both go-waku (#189) and nwaku (waku-org/nwaku#842) currently use nanoseconds precision. It's currently not expected to receive microseconds. The production fleet of nwaku has this change deployed. I'd say it's safe to assume that time will be in nanoseconds.

Regarding waku1->waku2, i think only the status app is currently using waku1, and the bridge from 1 to 2 does the transformation from milliseconds to nanoseconds correctly, so I think the risk of residual mismatch should be very low

@mkobetic
Copy link
Contributor Author

mkobetic commented Jun 1, 2022

Sounds good, I'll just fix the helpers to assume nanoseconds then. Thank you!

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 a pull request may close this issue.

2 participants