Add week number to telemetry report #1266
Conversation
| @@ -21,6 +21,8 @@ interface Arguments { | |||
| } | |||
|
|
|||
| process.once('message', async function({ uuid, commandData }: Arguments) { | |||
| const unixWeek = Math.floor(Date.now() / (1000 * 60 * 60 * 24 * 7)); | |||
There was a problem hiding this comment.
- How reliable is this calculation? How much offset would we end up with in a few months time?
- How easy will it be to visualize later? Should we aim to label these as
YYYY-WWinstead, for readability? Or is it easy to generate this from theunixWeekvalue from firebase?
There was a problem hiding this comment.
I'm not sure I understand the question about reliability or offset. 🤔 Do you mean because of leap seconds, or floating point errors, or...? Leap seconds should not affect this because they're accounted for in Unix time. This should precisely represent any week.
For visualization we had to convert back to a datetime in the BigQuery SQL code: TIMESTAMP_SECONDS(unixWeek * 7 * 24 * 60 * 60). This worked fine, but it may be simpler to do if we store them as YYYY-WW or even YYYY-MM-DD. It really depends on what will be easier to do in an SQL query. I would have to read more about it.
There was a problem hiding this comment.
Now that I think of it, using Unix weeks instead of calendar weeks would be more accurate for statistical purposes (since we don't have longer and shorter weeks), so let's go with that!
|
I think this may be needing a merge due to the failing test in the CI |
|
🙌 Thanks for reviewing! |
Fixes #1260