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

Add Features doc for new Time settings #3142

Merged
merged 2 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions apps/zui/docs/features/Packet-Captures.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
---
sidebar_position: 2
---
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dropped these sidebar_position directives in all these Features articles since that way they're automatically sorted alphabetically.


# Packet Captures

The video below describes Zui's features for working with packet capture
Expand Down
4 changes: 0 additions & 4 deletions apps/zui/docs/features/Preview-Load.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
---
sidebar_position: 1
---

# Preview & Load

The video below shows how you can preview and
Expand Down
6 changes: 2 additions & 4 deletions apps/zui/docs/features/README.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
---
sidebar_position: 4
---

# Features

Learn how to get the most from Zui features. Topics described here include:

* [Packet Captures](Packet-Captures.md)
* [Preview and load data](Preview-Load.md)
* [Time Display](Time-Display.md)
35 changes: 35 additions & 0 deletions apps/zui/docs/features/Time-Display.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Time Display

By default, Zui displays Zed `time` values in the Table, Inspector,
and Detail views using [ZSON format](https://zed.brimdata.io/docs/formats/zson#23-primitive-values).
This is an [RFC 3339](https://datatracker.ietf.org/doc/html/rfc3339)
date/time string in nanosecond precision ending in `Z` to indicate
[UTC](https://en.wikipedia.org/wiki/Coordinated_Universal_Time), e.g.,
`2024-08-14T19:12:51.123456789Z`.

Starting with Zui release v1.18.0, two different options in Zui **Settings**
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put in the forward-looking reference to the next Zui release so that way I can merge this right away and ping people on community Slack and/or the person that opened brimdata/brimcap#352 to get early feedback.

may be used to change the presentation of time values.

![Settings - Time](../media/Settings-Time.png)

1. If the **Time Zone** setting is changed from its default **UTC** while the
**Time Format** setting remains at its empty default, `time` values will be
rendered instead in an RFC 3339 format with a numeric offset, e.g.,
`2024-08-14T12:12:51.123-07:00` for setting `US/Pacific`. This format can be
represented using [strftime directives](https://github.com/samsonjs/strftime?tab=readme-ov-file#supported-specifiers)
as `%Y-%m-%dT%H:%M:%S.%L%:z`. Note that this format is acceptable as a
literal `time` value in a Zed program, e.g., assuming data containing a field
`ts` of the `time` type, the [search expression](https://zed.brimdata.io/docs/language/search-expressions)
`ts > 2024-08-14T12:12:51.123-07:00` is syntactically valid.

2. If the **Time Format** setting is changed from its empty default, any
[strftime directives](https://github.com/samsonjs/strftime?tab=readme-ov-file#supported-specifiers)
in the setting are used to format time values. For instance the setting
`%m/%d/%Y %Z` would produce the displayed value `08/14/2024 Pacific Daylight Time`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should use the capital %Z in this example since it's broken in the strftime library.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jameskerr: Yeah, you'd mentioned offline how you perceived %Z to be broken and I'd not spotted a problem before putting up this PR. And trying it again now it seems to be working as I'd expect.

For instance, I start by importing this file t.zson:

{ts:2024-08-14T19:12:51.123456789Z}

Here's how it looks with the default ZSON formatting:

image

Here's how it changes if I change just the Time Zone to "US/Pacific" and let the the default strftime formatting show the modified base time with the offset. So, hour 19 changes to hour 12.

image

Now I go back into Settings and also change Time Format to %Y-%m-%dT%H:%M:%S.%L %Z. The base time still shows hour 12 so that seems correct, and the written out time zone communicates what the numeric offset had been doing before.

image

So, that all seems correct to me, but maybe there's something else I'm missing. 😄 If it'd be easier to hash this out interactively on a Zoom just ping me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jameskerr and I spoke offline and I understand what's broken now. Here's some detail for the record.

I wasn't triggering the problem in my last comment above because I picked "US/Pacific" which happens to also be my local time zone. However, if I pick a different time zone like "US/Eastern", we see:

image

That shows the hour has been correctly adjusted to 15 but the time zone shown is still my local one. In other words, the library behaves as if %Z is to "tell me about the time zone I'm in" as opposed to "tell me about the time zone I asked to format in". This seems like a bug. However, I'm guessing that most users that reach for these settings will probably be seeking to display in their local time zone anyway, so they may not even notice.

For now I'll rework the example to avoid showing %Z to avoid drawing unnecessary attention.


:::tip Note
These settings do not currently change the times shown on the X axis of the
stacked bar chart that appears above query results. Addressing this is tracked
in issue [zui/3141](https://github.com/brimdata/zui/issues/3141). Please add a
comment to the issue if you find it affects your use of Zui.
:::
Binary file added apps/zui/docs/media/Settings-Time.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions apps/zui/src/electron/run-main/run-configurations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export function runConfigurations() {
properties: {
timeZone: {
name: "timeZone",
label: "Timezone",
label: "Time Zone",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I was assembling the screenshot, I realized the two-word "Time Zone" seems to be the more common use in places like RFC 3339 and ISO 8601, so I've switched to that.

type: "string",
defaultValue: "UTC",
enum: time.getZoneNames(),
Expand All @@ -50,7 +50,7 @@ export function runConfigurations() {
placeholder: "%Y-%m-%dT%H:%M:%S.%L%:z",
helpLink: {
label: "docs",
url: "https://github.com/samsonjs/strftime?tab=readme-ov-file#supported-specifiers",
url: "https://zui.brimdata.io/docs/features/Time-Display",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chicken/egg. This link will be broken until the PR merges. C'est la vie!

},
},
thousandsSeparator: {
Expand Down
Loading