[Feat][Dashboard] Add global UTC timezone button in navbar with local storage #48510

nadongjun · 2024-11-02T06:35:10Z

Why are these changes needed?

[Observability] Ability to show timestamps in UTC #47762

Changes Included

Timezone Selection Button: Added a button at the top of the UI that lets users choose their preferred timezone (UTC). This selection will update displayed dates accordingly in supported sections.

Persistent Settings: The selected timezone is saved to the browser’s local storage, ensuring that the setting remains consistent upon page reloads.

Supported Sections:

Three main areas were considered for timezone adjustments:

1. Grafana-Related Data: The selected timezone setting is now applied using the timezone parameter to set UTC in the URL. https://grafana.com/blog/2024/10/23/grafana-11.3-release-all-the-new-features/
2. Tables and Details for Jobs, Nodes, Actors, and Workers: The selected timezone setting is fully applied to these sections.
3. Log Data: Currently unable to support timezone adjustments due to technical limitations.

Scope: Given current limitations, only the Log Data section (Point 3 above) does not support timezone adjustments, while Grafana-Related Data and the Jobs, Nodes, Actors, and Workers sections are fully updated to reflect the timezone selection.

Result:

Related issue number

Closes #47762

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
- I've added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

… storage Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

nadongjun · 2024-11-06T00:17:39Z

@alanwguo Could you take a look at this when you have a moment?

alanwguo

Thanks!

Can we add a description or tooltip near the timezone selector that explains that timezone of logs may not respect this value?

alanwguo · 2024-11-06T00:21:29Z

python/ray/dashboard/client/src/pages/layout/MainNavLayout.tsx

+    window.location.reload();
+  };
+
+  const timezones = [


where did this list come from? Does this cover most common time zones?

This list was exported from GitHub's Timezone selection options.

Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

nadongjun · 2024-11-06T11:50:26Z

@alanwguo I added a tooltip with the message "The timezone of logs may not match this selection." Thanks for the review!

alanwguo · 2024-11-07T01:43:18Z

@scottsun94, plz review as well.

A couple of suggestions. When the dropdown is collapsed, can we make the contents of the dropdown slimmer. Like "GMT-8" instead of the full name.

Second. Can we detect the timezone used by the head node and set that as the default for the UI? I think in many cases, the user's time zone won't match the system's time zone which is often UTC for cloud-based instances. By setting the default to the head node, at least the logs and the UI will match timezones.

nadongjun · 2024-11-07T02:11:13Z

@alanwguo Thanks for the review,

The first suggestion sounds doable.

For the second, would it work to set the default timezone to the timezone where the dashboard server (React server) is running?

Also, is there a separate API to fetch the UTC timezone of the head node?

scottsun94 · 2024-11-07T02:38:38Z

Here is a great article about the timezone UX: https://www.linkedin.com/pulse/designers-guide-time-zone-selection-ux-vitaly-friedman/.

Grafana seems a great example to learn from:

Have suggested time zones at the top. We can consider Ray cluster time (or Dashboard server time), Browser time, UTC
Detect and show dashboard/cluster time by default
show city, country (dimmed), timezone names (dimmed) and allow users to search by them. Maybe just follow what Grafana has?

if it's easy, we can try grouping by continents & sort by alphabetically as well

nadongjun · 2024-11-07T02:54:27Z

@scottsun94 Thank you for sharing the great article. I will refer to it and incorporate the suggestions accordingly.

alanwguo · 2024-11-07T17:49:51Z

@alanwguo Thanks for the review,

The first suggestion sounds doable.

For the second, would it work to set the default timezone to the timezone where the dashboard server (React server) is running?

Also, is there a separate API to fetch the UTC timezone of the head node?

There is no API for this for now, but you can add a new API to http_server_head and call it and add the timezone to GlobalContext in App.tsx

nadongjun · 2024-11-08T00:12:57Z

@alanwguo

Thank you for the guidance! Here's how I plan to proceed with the additional changes:

Refactor the timezone select box into a separate component and add a search feature for better usability.
Add a new API to http_server_head to fetch the server-side timezone and set it as the default timezone.

Please let me know if you have any additional feedback.

scottsun94 · 2024-11-08T01:01:44Z

@nadongjun after the UI change is done, can you share a short walkthrough video or screenshots? Would love to take another look.

nadongjun · 2024-11-08T06:59:21Z

@scottsun94 Sure, I am currently working on the UI changes first. Once the updates are complete, I will share a video walkthrough and tag you.

nadongjun · 2024-11-08T16:51:25Z

@nadongjun after the UI change is done, can you share a short walkthrough video or screenshots? Would love to take another look.

I've implemented the changes using the react-select library.

Screen.Recording.2024-11-09.at.1.41.04.AM.mov

alanwguo · 2024-11-08T18:38:13Z

@nadongjun have you tried implementing with https://mui.com/material-ui/react-autocomplete/ ?

Then we can use the existing MUI library and don't have to take on a new dependency

scottsun94 · 2024-11-08T19:26:06Z

Played with the design a bit. Wondering if we should:

Show the GMT+xx:xx in the input box to make it conciser so that you can reduce the width of the box
add a divider between (browser time, coordinated universal time, etc.) and the rest of the time zones
adjust the format for each row to something like {Continent}/{City} {Country}, {timezone name (if exists)} {offset from GMT}
sort by Continent and city

nadongjun · 2024-11-09T01:24:08Z

Screen.Recording.2024-11-09.at.10.09.23.AM.mov

The timezone list content and the System category are currently being updated.

@alanwguo Thank you for the suggestion to use Autocomplete from MUI. I've updated the implementation to use the existing MUI library, so no new dependencies are added.

@scottsun94
Regarding the design changes:

I’ve added sorting and categorized time zones by continent, as I thought it would be helpful to display time zones grouped by continent.
Each row now displays in the format {Continent}/{City} {Country} {GMT Offset}.
The input box width has also been reduced for a more concise display.

Please let me know if you think any changes are needed. Thanks again for the helpful input!

scottsun94 · 2024-11-09T06:02:21Z

Thanks for the quick turnaround. Nice work!

A few small visual nits before we are good to go:

Can you align the box with the rest of the UI on the right? We should have a 24px padding to the right.
Can we change the font color to #5F6469 (secondary) when it's not hovered/selected? And when it's selected, can you use #8C9196 (tertiary)? We use these two shades else where. I forgot if we have existing variables for them. You can take a look.
Can you make sure the border color of the box is the same as other borders in the UI?

Can we align the group title and the timezones on the left? Don't see an obvious value to add indent here?
The spacing between the group title and the rest of the timezones can be smaller. Can be the same as the spacing between the time zones

Can we add dividers between different system/continents like this?
Can we use the following font colors for different texts?

group titles: continent/system: #5F6469 (secondary)
{Continent}/{city}, GMT offsets: #000 (primary)
{country}: #8C9196 (tertiary)

…ection Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

nadongjun · 2024-11-12T22:53:16Z

I can take a look at the CSS issues. I'll add a commit to this branch after you address my other comments.

I’ll address your comments and let you know once the changes are committed. Appreciate your help with the CSS issues.

…aving button state directly to local storage Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

…ndle timezone conversion Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

nadongjun · 2024-11-15T16:04:17Z

Screen.Recording.2024-11-16.at.12.52.50.AM.mov

@alanwguo

As per your review, I have made changes to avoid using external libraries. The dashboard server now maintains a single list of timezone names corresponding to each offset and returns the appropriate timezone name for a given offset via the /timezone API.

Additionally, instead of using useEffect, the data is now directly saved to localStorage, and the timezone name retrieved from the /timezone API is not stored in localStorage.

For UI adjustments, the Dashboard Server Timezone will display Asia/Irkutsk, which is the representative timezone for +09:00 on the server side, as seen in the demo video. It might be better to hide the value for the system timezone in this case.

Please feel free to share any additional feedback regarding the changes.

scottsun94 · 2024-11-15T20:05:10Z

For UI adjustments, the Dashboard Server Timezone will display Asia/Irkutsk, which is the representative timezone for +09:00 on the server side, as seen in the demo video. It might be better to hide the value for the system timezone in this case.

Yeah. agree.

Another possible small enhancement: maybe use a banner to make it more prominent that logs may not match the selection in the dropdown itself.

alanwguo

Thanks! This is looking close. Just some minor stylistic changes.

Please just address my comments, I'll work on any follow-up UI changes including the banner @scottsun94 suggested.

alanwguo · 2024-11-16T22:10:11Z

python/requirements.txt

@@ -60,4 +60,4 @@ pandas>=1.3
 pydantic!=2.0.*,!=2.1.*,!=2.2.*,!=2.3.*,!=2.4.*,<3  # Serve users can use pydantic<2
 py-spy>=0.2.0
 memray; sys_platform != "win32" # memray is not supported on Windows
-pyOpenSSL
+pyOpenSSL


revert the changes to this file plz.

alanwguo · 2024-11-16T22:36:36Z

python/ray/dashboard/client/src/components/SearchComponent.tsx

@@ -77,3 +83,910 @@ export const SearchSelect = ({
    </TextField>
  );
 };
+
+export const SearchTimezone = ({


Let's move this to a separate file SearchTimezone.tsx

alanwguo · 2024-11-16T22:39:25Z

python/ray/dashboard/client/src/App.tsx

+  /**
+   * The name of the current server-side timezone.
+   */
+  serverTimeZone: string | undefined;
+
+  serverTimeZoneLoaded: boolean | undefined;


let's combine these into a single variable.

serverTimeZone will either be undefined if it's unloaded. null, if we couldn't fetch it, or the value from the server.

alanwguo · 2024-11-16T22:41:59Z

python/ray/dashboard/http_server_head.py

+    def get_gmt_offset(self):
+        current_tz = datetime.now().astimezone().tzinfo
+        offset = current_tz.utcoffset(None)
+        hours, remainder = divmod(offset.total_seconds(), 3600)
+        minutes = remainder // 60
+        sign = "+" if hours >= 0 else "-"
+        return f"GMT{sign}{abs(int(hours)):02d}:{abs(int(minutes)):02d}"


let's move this and most of the implementation of get_timezone into a separate file. SOmething like timezone_utils.py

alanwguo · 2024-11-16T22:42:28Z

python/ray/dashboard/http_server_head.py

+
+    @routes.get("/timezone")
+    async def get_timezone(self, req) -> aiohttp.web.Response:
+        timezones = [


after moving this to a new file, let's up-level this to be a file-level const.

alanwguo · 2024-11-16T22:42:43Z

python/ray/dashboard/http_server_head.py

+    @routes.get("/timezone")
+    async def get_timezone(self, req) -> aiohttp.web.Response:
+        timezones = [
+            {"utc": "GMT-12:00", "value": "Etc/GMT+12"},


instead of utc the key should be offset

alanwguo · 2024-11-16T22:43:21Z

python/ray/dashboard/http_server_head.py

+            )
+
+            if current_timezone:
+                return aiohttp.web.Response(text=str(current_timezone))


instead of returning raw text, let's return a simple json dictionary so we may add additional fields in the future.

Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

…t, value) Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

nadongjun · 2024-11-17T02:53:53Z

@alanwguo

Thank you for your feedback. I’ve incorporated your suggestions.

Reverted and recommitted the changes to python/requirements.txt.
Updated the raw string in the dashboard API (/timezone) response to use JSON format ({offset, value}).
Refactored timezone-related logic in the dashboard API into a separate function and moved the client-side timezone list to a dedicated file.

Let me know if there’s anything else to improve!

alanwguo

a couple more comments for a bit cleaner code.

alanwguo · 2024-11-19T02:15:59Z

python/ray/dashboard/client/src/components/SearchComponent.tsx

+  const [timezone, setTimezone] = useState<string>("");
+
+  useEffect(() => {
+    if (serverTimeZoneLoaded) {


can we delete serverTimeZoneLoaded now that serverTimeZone has the null and undefined case?

alanwguo · 2024-11-19T02:16:24Z

python/ray/dashboard/client/src/pages/metrics/utils.ts

@@ -26,6 +27,11 @@ type PrometheusHealthcheckRsp = {
  msg: string;
 };

+type TimezonekRsp = {


Suggested change

type TimezonekRsp = {

type TimezoneRsp = {

alanwguo · 2024-11-19T02:17:36Z

python/ray/dashboard/client/src/pages/metrics/utils.ts

+  offset?: string | null;
+  value?: string | null;


do these have to be nullable or undefined? Seems like the entire TimeZoneInfo can be possibly undefined, but if we do get the TimezoneInfo, the fields inside should both have a value.

Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

nadongjun · 2024-11-19T10:01:26Z

a couple more comments for a bit cleaner code.

Thank you for your feedback. I've addressed the review comments and made the following changes:

Removed serverTimeZoneLoaded and modified the code to detect loading by checking if serverTimeZone is undefined.
Fixed variable typos throughout the code.
Updated the TimeZoneInfo object to have offset and value as strict string types
(Additional change) Modified the Autocomplete options to calculate the browser's UTC offset using new Date() instead of searching from a predefined list.

Move current timezone logic into the global context since its used by multiple places Signed-off-by: Alan Guo <aguo@anyscale.com>

alanwguo · 2024-11-19T21:42:32Z

Okay I pushed a commit that updates the styling. I wasn't able to get a banner to show up in the Autocomplete options list but I think i covered the other style changes.

I also refactored the code slightly to move the currentTimeZone logic into the GlobalContext since it is re-used in multiple places.

I updated the default time for the Overview page for now-1h instead of now-30m. For some really weird reason, the timezone query param doesn't work when the its 30 minutes, but works with every other value?

scottsun94 · 2024-11-19T21:51:44Z

Okay I pushed a commit that updates the styling. I wasn't able to get a banner to show up in the Autocomplete options list but I think i covered the other style changes.

So the message about the log timezone will still show up like this?

alanwguo · 2024-11-19T21:56:35Z

Okay I pushed a commit that updates the styling. I wasn't able to get a banner to show up in the Autocomplete options list but I think i covered the other style changes.

So the message about the log timezone will still show up like this?

yeah

scottsun94 · 2024-11-19T22:29:53Z

Okay I pushed a commit that updates the styling. I wasn't able to get a banner to show up in the Autocomplete options list but I think i covered the other style changes.

So the message about the log timezone will still show up like this?

yeah

Okay. SGTM

Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

nadongjun · 2024-11-22T00:20:13Z

Could anyone merge this PR?

… storage (ray-project#48510) Signed-off-by: Dongjun Na <kmu5544616@gmail.com> Signed-off-by: Connor Sanders <connor@elastiflow.com>

… storage (ray-project#48510) Signed-off-by: Dongjun Na <kmu5544616@gmail.com> Signed-off-by: hjiang <dentinyhao@gmail.com>

[Feat][Dashboard] Add global UTC timezone button in navbar with local…

33d4f80

… storage Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

nadongjun force-pushed the dashboard/tz branch from 0431e38 to 33d4f80 Compare November 4, 2024 00:15

nadongjun marked this pull request as draft November 4, 2024 04:10

nadongjun added 2 commits November 4, 2024 20:45

Add timezone parameter to Grafana d-solo URL

c3167d8

Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

lint

b9a77e6

Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

nadongjun marked this pull request as ready for review November 4, 2024 11:54

Merge branch 'master' into dashboard/tz

12fc8ab

alanwguo reviewed Nov 6, 2024

View reviewed changes

nadongjun added 2 commits November 6, 2024 20:31

Merge branch 'master' into dashboard/tz

4f5666e

Add tooltip to Box component

40b49f1

Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

Merge branch 'master' into dashboard/tz

cd709da

nadongjun requested a review from alanwguo November 7, 2024 00:02

Merge branch 'master' into dashboard/tz

3f8a46c

nadongjun added 2 commits November 9, 2024 18:10

Add SearchTimezone component with React Autocomplete for timezone sel…

73e5946

…ection Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

Add tzlocal dependency and timezone API to dashboard/http_server_head

a1b66bd

Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

nadongjun added 2 commits November 16, 2024 00:48

Refactor server-side timezone handling by removing dependencies and s…

404b876

…aving button state directly to local storage Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

Changed the timezone API to use offsets and updated formatValue to ha…

4720696

…ndle timezone conversion Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

alanwguo reviewed Nov 16, 2024

View reviewed changes

nadongjun added 4 commits November 17, 2024 09:51

Revert changes to requirements.txt

ced594e

Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

Refactor /timezone dashboard API

d8ffbdc

Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

Refactor SearchTimezone and enhance timezone handling with JSON(offse…

366112b

…t, value) Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

Add timezones data to file

d8a2d88

Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

Merge branch 'master' into dashboard/tz

3fa02b1

alanwguo reviewed Nov 19, 2024

View reviewed changes

Refactor timezone handling and offset calculation

9466973

Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

nadongjun and others added 2 commits November 19, 2024 19:01

Merge branch 'master' into dashboard/tz

e8d9f4b

update styling of dropdown

8d88192

Move current timezone logic into the global context since its used by multiple places Signed-off-by: Alan Guo <aguo@anyscale.com>

alanwguo approved these changes Nov 19, 2024

View reviewed changes

nadongjun added 3 commits November 20, 2024 18:04

Merge branch 'master' into dashboard/tz

9aadde4

lint

434ca68

Signed-off-by: Dongjun Na <kmu5544616@gmail.com>

Merge branch 'master' into dashboard/tz

493f577

jjyao added the go add ONLY when ready to merge, run all tests label Nov 22, 2024

jjyao merged commit b9cd63a into ray-project:master Nov 22, 2024
6 checks passed

dentiny pushed a commit to dentiny/ray that referenced this pull request Dec 7, 2024

[Feat][Dashboard] Add global UTC timezone button in navbar with local…

f0b0f60

… storage (ray-project#48510) Signed-off-by: Dongjun Na <kmu5544616@gmail.com> Signed-off-by: hjiang <dentinyhao@gmail.com>

[Feat][Dashboard] Add global UTC timezone button in navbar with local storage #48510

[Feat][Dashboard] Add global UTC timezone button in navbar with local storage #48510

Conversation

nadongjun commented Nov 2, 2024 • edited Loading

Why are these changes needed?

Changes Included

Supported Sections:

Result:

Related issue number

Checks

nadongjun commented Nov 6, 2024

alanwguo left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nadongjun commented Nov 6, 2024

alanwguo commented Nov 7, 2024

nadongjun commented Nov 7, 2024

scottsun94 commented Nov 7, 2024

nadongjun commented Nov 7, 2024

alanwguo commented Nov 7, 2024

nadongjun commented Nov 8, 2024

scottsun94 commented Nov 8, 2024

nadongjun commented Nov 8, 2024

nadongjun commented Nov 8, 2024

alanwguo commented Nov 8, 2024

scottsun94 commented Nov 8, 2024 • edited Loading

nadongjun commented Nov 9, 2024

scottsun94 commented Nov 9, 2024

nadongjun commented Nov 12, 2024

nadongjun commented Nov 15, 2024

scottsun94 commented Nov 15, 2024

alanwguo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nadongjun commented Nov 17, 2024

alanwguo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nadongjun commented Nov 19, 2024

alanwguo commented Nov 19, 2024

scottsun94 commented Nov 19, 2024

alanwguo commented Nov 19, 2024

scottsun94 commented Nov 19, 2024

nadongjun commented Nov 22, 2024

nadongjun commented Nov 2, 2024 •

edited

Loading

alanwguo left a comment •

edited

Loading

scottsun94 commented Nov 8, 2024 •

edited

Loading