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

[TSVB] Handle ignore daylight time correctly and fix shift problem #123398

Merged
merged 4 commits into from
Jan 24, 2022

Conversation

flash1293
Copy link
Contributor

@flash1293 flash1293 commented Jan 19, 2022

Fixes #120449

This PR does two things:

  • As long as "ignore daylight time" is not set, it uses the correct time zone to do the time shift - this makes sure there are no gaps (e.g. because a day is 25 hours due to DST switch)
  • The "ignore daylight time" setting is put in place specifically to not respect the time zone correctly - this is something that doesn't work with the new time axis (as the "hack" to render dates ignoring the daylight time is done via formatter). So in case this setting is active, time shift continues to function as before and the new date axis is disabled to not keep BWC of this feature

How to test

Ingest this data (expand)
PUT test_dst/_doc/1
{
  "timestamp" : "2020-01-19T03:26:21.326Z",
  "id": 1,
  "amount" : 31
}
PUT test_dst/_doc/2
{
  "timestamp" : "2020-01-20T03:26:21.326Z",
  "id": 2,
  "amount" : 22
}
PUT test_dst/_doc/3
{
  "timestamp" : "2020-01-21T03:26:21.326Z",
  "id": 3,
  "amount" : 43
}
PUT test_dst/_doc/4
{
  "timestamp" : "2020-02-19T03:26:21.326Z",
  "id": 4,
  "amount" : 31
}
PUT test_dst/_doc/5
{
  "timestamp" : "2020-02-20T03:26:21.326Z",
  "id": 5,
  "amount" : 22
}
PUT test_dst/_doc/6
{
  "timestamp" : "2020-02-21T03:26:21.326Z",
  "id": 6,
  "amount" : 43
}
PUT test_dst/_doc/7
{
  "timestamp" : "2020-03-27T03:26:21.326Z",
  "id": 7,
  "amount" : 31
}
PUT test_dst/_doc/8
{
  "timestamp" : "2020-03-28T03:26:21.326Z",
  "id": 8,
  "amount" : 22
}
PUT test_dst/_doc/9
{
  "timestamp" : "2020-03-29T03:26:21.326Z",
  "id": 9,
  "amount" : 43
}
PUT test_dst/_doc/10
{
  "timestamp" : "2020-04-27T03:26:21.326Z",
  "id": 10,
  "amount" : 31
}
PUT test_dst/_doc/11
{
  "timestamp" : "2020-04-28T03:26:21.326Z",
  "id": 11,
  "amount" : 22
}
PUT test_dst/_doc/12
{
  "timestamp" : "2020-04-29T03:26:21.326Z",
  "id": 12,
  "amount" : 43
}
PUT test_dst/_doc/13
{
  "timestamp" : "2021-01-19T03:26:21.326Z",
  "id": 13,
  "amount" : 31
}
PUT test_dst/_doc/14
{
  "timestamp" : "2021-01-20T03:26:21.326Z",
  "id": 14,
  "amount" : 22
}
PUT test_dst/_doc/15
{
  "timestamp" : "2021-01-21T03:26:21.326Z",
  "id": 15,
  "amount" : 43
}
PUT test_dst/_doc/16
{
  "timestamp" : "2021-02-19T03:26:21.326Z",
  "id": 16,
  "amount" : 31
}
PUT test_dst/_doc/17
{
  "timestamp" : "2021-02-20T03:26:21.326Z",
  "id": 17,
  "amount" : 22
}
PUT test_dst/_doc/18
{
  "timestamp" : "2021-02-21T03:26:21.326Z",
  "id": 18,
  "amount" : 43
}
PUT test_dst/_doc/19
{
  "timestamp" : "2021-03-27T03:26:21.326Z",
  "id": 19,
  "amount" : 31
}
PUT test_dst/_doc/20
{
  "timestamp" : "2021-03-28T03:26:21.326Z",
  "id": 20,
  "amount" : 22
}
PUT test_dst/_doc/21
{
  "timestamp" : "2021-03-29T03:26:21.326Z",
  "id": 21,
  "amount" : 43
}
PUT test_dst/_doc/22
{
  "timestamp" : "2021-04-27T03:26:21.326Z",
  "id": 22,
  "amount" : 31
}
PUT test_dst/_doc/23
{
  "timestamp" : "2021-04-28T03:26:21.326Z",
  "id": 23,
  "amount" : 22
}
PUT test_dst/_doc/24
{
  "timestamp" : "2021-04-29T03:26:21.326Z",
  "id": 24,
  "amount" : 43
}
  • Make sure your configured time zone has a DST switch at this day (either you are in Europe or go to advanced settings and configure a European time zone there, like Europe/Berlin)
  • Go to TSVB and look at the test_dst index
  • Select March 25 to March 31 2021 as time range
  • Do two series with two colors, shifted by one day
  • Make sure the data makes sense

It looks like this:
Screenshot 2022-01-20 at 13 13 22

You can see the first green data point is not overlaying the first red data point - this is because they are 24 hours apart (2021-03-27T03:26:21.326Z vs 2021-03-28T03:26:21.326Z), but that's not 1d because March 27 only has 23 hours.

If you hover over the tooltip, you can see it got shifted by one day in this timezone - 4:00 stays 4:00, only the day changes:
Screenshot 2022-01-20 at 13 16 18

Screenshot 2022-01-20 at 13 16 21

If ignore daylight time is enabled, the DST switch doesn't happen, effectively the chart stays in winter time:
Screenshot 2022-01-20 at 13 17 45

This means the shift of 1d is now 24 hours in all cases.

@flash1293 flash1293 added backport:skip This commit does not require backporting Feature:TSVB TSVB (Time Series Visual Builder) release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.1.0 labels Jan 20, 2022
@flash1293 flash1293 marked this pull request as ready for review January 20, 2022 15:04
@flash1293 flash1293 requested a review from a team as a code owner January 20, 2022 15:04
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

@stratoula
Copy link
Contributor

@elasticmachine merge upstream

@stratoula
Copy link
Contributor

stratoula commented Jan 24, 2022

@flash1293 I am trying to compare Lens with TSVB.

The same chart on Lens gives me for the shifted day on the 27th 04:00
image

while TSVB gives me 2:00
image

Update: This discrepancy happens only if I have selected the Browser timezone. If I change it to Athens or Berlin both work fine.

@flash1293
Copy link
Contributor Author

Update: This discrepancy happens only if I have selected the Browser timezone. If I change it to Athens or Berlin both work fine.

Interesting, it seems like the time zone is not propagated in the same way in both cases. Looking 👀

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
visTypeTimeseries 447.7KB 447.8KB +79.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

I tested it with daylight on and off. It works fine and I get the results I am expecting.

We found with @flash1293 a discrepancy between Lens and TSVB specifically when testing on Europe/Athens timezone and setting the timezone to Browser.

This is irrelevant with this PR, we will look into it separately.

@flash1293
Copy link
Contributor Author

Opened #123579 for the edge-casey follow-up

@flash1293 flash1293 merged commit 3eded8e into elastic:main Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:TSVB TSVB (Time Series Visual Builder) release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken graph when using "Offset series time by" on TSVB with series Stacked
5 participants