Clarify how often usage reports are sent #928
Conversation
| const getLatestAudit = () => ({ maybeOne }) => maybeOne(sql`select * from audits | ||
| where action='analytics' and "loggedAt" >= current_date - cast(${AUDIT_SCHEDULE} as int) | ||
| where action='analytics' and current_date - "loggedAt"::date < ${ANALYTICS_SCHEDULE} |
There was a problem hiding this comment.
Instead of this change, I could have done:
"loggedAt" >= current_date - cast(${AUDIT_SCHEDULE} as int) + 1However, I think that would have been less clear overall.
I was initially confused about dates vs. timestamps here. Previously, current_date - cast(${AUDIT_SCHEDULE} as int) was implicitly coerced to a timestamp with time part equal to midnight. I started wondering about the time part of loggedAt and whether it mattered that it was 3 a.m. instead of midnight. Now I'm converting loggedAt to a date and calculating the number of days between that date and current_date. That way, the time part of loggedAt doesn't figure into things: it doesn't matter when the usage report started or how long it took to complete.
This approach also lets us remove the cast of ANALYTICS_SCHEDULE.
There was a problem hiding this comment.
I like this new approach a lot of making both values dates (without timestamps) and doing the subtraction there.
|
@ktuite, I'm thinking this can be reviewed async. |
Closes #921. This PR doesn't change the current behavior, but it tries to make that behavior clear. For example, I used to think that usage reports were sent every 30 days. After this PR, usage reports will continue to be sent every 31 days — but I think that fact will be clearer.
What has been done to verify that this works as intended?
Updated and new tests.
Why is this the best possible solution? Were any other approaches considered?
I considered not making any changes at all. I realized that I could send daily usage reports by setting the
default.taskSchedule.analyticsconfig to 0 instead of 1. However, I think this PR makes things a little clearer.I'll also leave a comment about an individual line.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
The current behavior is intended to stay the same. I've updated tests to help verify that there hasn't been a change in behavior.
Does this change require updates to the API documentation? If so, please update docs/api.md as part of this PR.
I don't think so. We say in the API docs that usage data is sent monthly, which is consistent with the current interval of 31 days.
Before submitting this PR, please make sure you have:
make test-fulland confirmed all checks still pass OR confirm CircleCI build passes