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

Use UTC time for server stats history, localize times on the client #1851

Merged
merged 3 commits into from
Aug 17, 2021

Conversation

obradovichv
Copy link
Contributor

Server-side stats history time sourcing changed from server time(time zone dependent) to UTC, aligning it with other server-side time values sourced from time.time(). This enables the same frame of reference for all clients by knowing it is UTC. Localization is now performed client-side on server-rendered dates and times.

Localization of the report.html start and end datetimes intentionally formats the value according to the client's default locale to keep with the localization theme of this change.

The date strings are manually parsed instead of using Date.parse() as recommended by MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/parsed
"It is not recommended to use Date.parse as until ES5, parsing of strings was entirely implementation dependent. There are still many differences in how different hosts parse date strings, therefore date strings should be manually parsed (a library can help if many different formats are to be accommodated)."

Fixes #1835

Locust uses server time to store the time of a task in stats history.
When fetching the initial stats history for the client (such on on page
refresh) it is written by the server using using this stored time.
While rendering a chart on the client, locust uses the localized client
time for the time the stats history was fetched. This would lead to a
jump in the x-axis time series when server and client time zones
differed.

Stats history (stats.py) now derives time from the time component of
the current datetime in UTC instead of server time zone.
This aligns it with other time stores used throughout locust
- particularly the start_time and end_time of reports - that make use
of time.time(), which is in UTC.

The client UI report (report.html) now localizes the server-rendered
times from history (formerly in server TZ, now in UTC).
This aligns it with the client-side report timekeeping of locust.js
updateStats() used for the x-axis of charts.

Issue: locustio#1835
The "during" start and end datetimes of "Download Report" (/stats/report)
were using server-rendered times, i.e.: strftime("%Y-%m-%d %H:%M:%S").
When server and client time zones differed, the times of the report
would not match the client time zone.
These datetimes are now localized similarly to the rest of the report.
@cyberw
Copy link
Collaborator

cyberw commented Aug 16, 2021

I think supporting pre ES5 browsers doesnt really make it to our priority list (10 year old browsers are likely to have all kinds of other issues anyway), and I would prioritize keeping the code simple. Other than that it looks good!

Simplify date parsing. The date value is converted to ISO8601-like
format in UTC (Z) and parsed using built-in Date.parse().
@obradovichv
Copy link
Contributor Author

Agreed, the spread operator would likely invalidate any 10 year old browser anyway 😆
Updated PR accordingly to use Date.parse() to parsing the datetime instead of manually parsing the value.

I have left stats_data.html manual parsing unchanged because it uses only the time component and would need an ISO8601-like date format to work with Date.parse(), perhaps the {current date} or {unix epoch} would be appropriate "fake" date components to pass into Date.parse() but it does not feel like a simplification of "HH:MM".split(":") by comparison. I can update to this if preferred.

If a future feature stores stat history with the full datetime for the time series, then the client-side localization can be updated to Date.parse() without any complicated date injections.

@cyberw cyberw merged commit d7efc71 into locustio:master Aug 17, 2021
@cyberw
Copy link
Collaborator

cyberw commented Aug 17, 2021

Thanks!

Do you by any chance have time to look into another thing related to graphing? I'd like to not have a line for response times at all until requests have actually been reported (the first part of the line in this screen shot)

image

@obradovichv
Copy link
Contributor Author

Sure, I'll take a look.
Created new issue to track it: #1852

@cyberw
Copy link
Collaborator

cyberw commented Aug 17, 2021

Awesome, thanks!

@obradovichv obradovichv deleted the fix-time-l10n branch August 17, 2021 17:46
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 this pull request may close these issues.

time display in live charts switches to the local time upon refresh
2 participants