[Monitoring] Ensure all charts use the configured timezone #45949

chrisronline · 2019-09-17T19:49:04Z

Resolves #29010

This PR ensures all monitoring charts respect the configured time zone.

TODO

~~Use a simpler way to fetch uiSettings -> await request.getUiSettingsService().get('dateFormat:tz')~~

Testing

Load up Kibana with monitoring enabled
Go to Kibana Management -> Advanced Settings
Change the dateFormat:tz to something other than Browser
Go to the monitoring UI and ensure all charts x-axis respects the configured timezone

elasticmachine · 2019-09-17T19:49:06Z

Pinging @elastic/stack-monitoring

elasticmachine · 2019-09-17T21:39:46Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

cachedout · 2019-09-18T08:39:40Z

I cannot get this fix to work. No matter what time zone I set Kibana to, I see the same times shown in the X-axis on Stack Monitoring charts. Happy to Zoom to show you what I'm seeing if it helps.

ycombinator · 2019-09-20T15:17:28Z

With this PR, no matter what time zone I set in Kibana Advanced Settings (browser, America/New_York, etc.), all monitoring charts always displayed times on the x-axis in UTC.

For comparison, on master, no matter what time zone I set in Kibana Advanced Settings (browser, America/New_York, etc.), all monitoring charts always displayed times on the x-axis in my browser's time zone.

chrisronline · 2019-09-20T20:08:11Z

Okay thanks for the time and feedback all!

I went back to the drawing board and think we have a better solution.

At a high level, we are fetching the data from ES in the same fashion as before, which is in UTC timestamps. Once we receive the data back, we are offsetting the UTC timestamps based on the dateFormat:tz configured timezone (which is what the flot docs recommend). Then, we are passing down the offsetted UTC timestamps back to the client and telling the client that they are UTC timestamps (so nothing on the client attempts to format or convert to the browser timezone). As a result, if you are looking some chart for a certain time period, changing dateFormat:tz will only change the numbers on the x-axis - the data will remain the same.

This should work fine, but it's a significant change that might require messaging somehow. We might also need to give support a heads up in case issues/questions arise as a result from this.

Let's start a conversation here about this and figure out our exact approach moving forward. I'm going to CC some parties that might have some thoughts, or historical reasons why this change hasn't been done in the past.

cc @pickypg @skearns64

pickypg · 2019-09-20T20:29:50Z

I can't think of any reason why we wouldn't honor the setting. For a long time, Monitoring did not touch Kibana Advanced Settings though, so historically that's probably the reason.

Just wanted to add one thing to the review: make sure shared links where the time picker is used for something other than now-1h type searches, that it honors the timezone properly (doesn't shift).

elasticmachine · 2019-09-20T21:41:42Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

igoristic · 2019-09-23T18:38:47Z

@chrisronline
I think date-picker's "from date/time" and "to date/time" should always match the chart's x values (timestamps). The only thing that should change is the data shift in the chart (based on the offset). For example if I pick a range from 1pm to 9pm and there is a single peek value at 5pm(utc) the chart should still show something like:

|          *           |
|                      |
|1pm ..... 5pm .... 9pm|

Now with a -4h timezone offset:

| *                    |
|                      |
|1pm ..... 5pm .... 9pm|

chrisronline · 2019-09-24T17:26:51Z

@igoristic

This is what I'm seeing with this PR:

Set dateFormat:tz to UTC
Set the timeframe from 1pm - 5pm in a stack monitoring page with charts
Note the url will look like: http://localhost:5601/app/monitoring#/elasticsearch?_g=(cluster_uuid:xk61MITwT2qYsjx0A4kFeQ,refreshInterval:(pause:!f,value:10000),time:(from:'Tue Sep 24 2019 13:00:00 GMT+0000',to:'Tue Sep 24 2019 21:00:00 GMT+0000'))
Note the x-axis window is 13:00 - 21:00
Change the dateFormat:tz to US/Pacific
Refresh the stack monitoring page
Observe that the url is: http://localhost:5601/app/monitoring#/elasticsearch?_g=(cluster_uuid:xk61MITwT2qYsjx0A4kFeQ,refreshInterval:(pause:!f,value:10000),time:(from:'Tue Sep 24 2019 13:00:00 GMT+0000',to:'Tue Sep 24 2019 21:00:00 GMT+0000')) (Which is unchanged)
Now the chart x-axis window is 6:00 - 14:00
The data shows up in the same place on the graph

I'm not sure what you're suggesting here. Are you suggesting this behavior isn't ideal? Maybe that in step 8, the x-axis window should remain unchanged and still show 13:00 - 21:00?

igoristic · 2019-09-24T17:39:54Z

@chrisronline

Maybe that in step 8, the x-axis window should remain unchanged and still show 13:00 - 21:00

Exactly. This way we always maintain the from and to labels and it's 1:1 with the top right date-picker. I think only the values/data should shift left/right, and fall into their respective offset buckets instead of the timestamps changing.

chrisronline · 2019-09-24T18:55:12Z

Exactly. This way we always maintain the from and to labels and it's 1:1 with the top right date-picker. I think only the values/data should shift left/right, and fall into their respective offset buckets instead of the timestamps changing.

I'm looking at other parts of Kibana and I'm seeing the behavior I described in this comment, notably, visualize. I'm assuming other parts of Kibana also follow this behavior, but happy to see examples if you know any.

Assuming this isn't deviating from the rest of Kibana, I think we should continue with how it works right now and we can open a ticket/discussion about possibly changing it and the implications that might have. WDYT?

igoristic · 2019-09-24T22:57:40Z

@chrisronline

The behavior I'm trying to convey is actually very similar to Visualize and Discover.

Notice how the chart's timespan labels are always in sync with the top-right time-range picker (no matter the timezone). So, the url should always contain the UTC while the time-range and chart x labels should both have the timezone's relative offset

chrisronline · 2019-09-25T14:42:07Z

@igoristic Are you not seeing this behavior in the PR? I'm seeing it work the way you're describing. Can you maybe include screenshots of the monitoring UI with specifics about what you're seeing?

igoristic · 2019-09-25T21:31:25Z

@chrisronline

Sorry about the confusion. I guess I only tested browser timezone and didn't reset the link for other timezone tests which skewed my results. Here is a miss-match with the Browser timezone:

As you can see the chart's labels are actually in UTC (if compared to timestamps in URL)

Also, another thing I noticed is that zooming (via scrub) is now broken since the selected range is the offset, but we're sending those same values as UTC

chrisronline · 2019-09-30T19:43:23Z

@igoristic Solid catch! Fixed up now and ready for another round.

elasticmachine · 2019-09-30T21:07:48Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-30T22:06:04Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-10-01T01:41:35Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

igoristic

Working as expected. Great job!

ycombinator

@chrisronline Functionality LGTM but I see there's a TODO in the PR description. Please let me know when you've resolved it and I'll re-review this PR. Thanks.

chrisronline · 2019-10-02T14:39:29Z

@ycombinator Thanks for the reminder. This is done in 20a9d16

ycombinator

LGTM.

elasticmachine · 2019-10-02T15:48:48Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-10-02T16:34:23Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…5949) * Consistently apply dateFormat:tz to all monitoring time-series data * Ensure browser timezone works properly * Fix tests * Fix other test * Simplfy timezone fetching * Fix tests

…47137) * Consistently apply dateFormat:tz to all monitoring time-series data * Ensure browser timezone works properly * Fix tests * Fix other test * Simplfy timezone fetching * Fix tests

…47138) * Consistently apply dateFormat:tz to all monitoring time-series data * Ensure browser timezone works properly * Fix tests * Fix other test * Simplfy timezone fetching * Fix tests

chrisronline · 2019-10-02T17:57:48Z

Backport:

7.x: 46e6424
7.4: 953e76b

chrisronline added review Team:Monitoring Stack Monitoring team v8.0.0 v7.5.0 Feature:Stack Monitoring v7.4.1 labels Sep 17, 2019

chrisronline requested a review from igoristic September 17, 2019 19:49

chrisronline self-assigned this Sep 17, 2019

chrisronline added the release_note:fix label Sep 17, 2019

chrisronline requested a review from ycombinator September 19, 2019 19:05

Consistently apply dateFormat:tz to all monitoring time-series data

4beac7d

chrisronline force-pushed the monitoring/timezones branch from 8f606ba to 4beac7d Compare September 20, 2019 20:03

chrisronline added 2 commits September 30, 2019 14:22

Merge remote-tracking branch 'elastic/master' into monitoring/timezones

83bf542

Ensure browser timezone works properly

e0ecea5

Fix tests

8e0c509

Fix other test

97f8dc5

igoristic approved these changes Oct 2, 2019

View reviewed changes

ycombinator reviewed Oct 2, 2019

View reviewed changes

Simplfy timezone fetching

20a9d16

ycombinator approved these changes Oct 2, 2019

View reviewed changes

Fix tests

7649984

chrisronline merged commit 3f7c3e0 into elastic:master Oct 2, 2019

chrisronline deleted the monitoring/timezones branch October 2, 2019 16:43

This was referenced Oct 2, 2019

[7.x] [Monitoring] Ensure all charts use the configured timezone (#45949) #47137

Merged

[7.4] [Monitoring] Ensure all charts use the configured timezone (#45949) #47138

Merged

igoristic mentioned this pull request Oct 4, 2019

Scrub zooming broken on charts #47380

Closed

chrisronline mentioned this pull request Oct 4, 2019

[Monitoring] Fix broken scrubbing on monitoring charts #47382

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Monitoring] Ensure all charts use the configured timezone #45949

[Monitoring] Ensure all charts use the configured timezone #45949

chrisronline commented Sep 17, 2019 •

edited

Loading

elasticmachine commented Sep 17, 2019

elasticmachine commented Sep 17, 2019

cachedout commented Sep 18, 2019

ycombinator commented Sep 20, 2019

chrisronline commented Sep 20, 2019

pickypg commented Sep 20, 2019

elasticmachine commented Sep 20, 2019

igoristic commented Sep 23, 2019 •

edited

Loading

chrisronline commented Sep 24, 2019

igoristic commented Sep 24, 2019

chrisronline commented Sep 24, 2019

igoristic commented Sep 24, 2019

chrisronline commented Sep 25, 2019

igoristic commented Sep 25, 2019

chrisronline commented Sep 30, 2019

elasticmachine commented Sep 30, 2019

elasticmachine commented Sep 30, 2019

elasticmachine commented Oct 1, 2019

igoristic left a comment

ycombinator left a comment

chrisronline commented Oct 2, 2019

ycombinator left a comment

elasticmachine commented Oct 2, 2019

elasticmachine commented Oct 2, 2019

chrisronline commented Oct 2, 2019

[Monitoring] Ensure all charts use the configured timezone #45949

[Monitoring] Ensure all charts use the configured timezone #45949

Conversation

chrisronline commented Sep 17, 2019 • edited Loading

TODO

Testing

elasticmachine commented Sep 17, 2019

elasticmachine commented Sep 17, 2019

💚 Build Succeeded

cachedout commented Sep 18, 2019

ycombinator commented Sep 20, 2019

chrisronline commented Sep 20, 2019

pickypg commented Sep 20, 2019

elasticmachine commented Sep 20, 2019

💔 Build Failed

igoristic commented Sep 23, 2019 • edited Loading

chrisronline commented Sep 24, 2019

igoristic commented Sep 24, 2019

chrisronline commented Sep 24, 2019

igoristic commented Sep 24, 2019

chrisronline commented Sep 25, 2019

igoristic commented Sep 25, 2019

chrisronline commented Sep 30, 2019

elasticmachine commented Sep 30, 2019

💔 Build Failed

elasticmachine commented Sep 30, 2019

💔 Build Failed

elasticmachine commented Oct 1, 2019

💚 Build Succeeded

igoristic left a comment

Choose a reason for hiding this comment

ycombinator left a comment

Choose a reason for hiding this comment

chrisronline commented Oct 2, 2019

ycombinator left a comment

Choose a reason for hiding this comment

elasticmachine commented Oct 2, 2019

💔 Build Failed

elasticmachine commented Oct 2, 2019

💚 Build Succeeded

chrisronline commented Oct 2, 2019

chrisronline commented Sep 17, 2019 •

edited

Loading

igoristic commented Sep 23, 2019 •

edited

Loading