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.

Sign up for GitHub

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

Jump to bottom

[TSVB] Adds ignore DST switch for timeseries #98484

Merged
merged 11 commits into from
May 5, 2021
Merged

[TSVB] Adds ignore DST switch for timeseries #98484

merged 11 commits into from
May 5, 2021

Conversation

dimaanj
Copy link
Contributor

@dimaanj dimaanj commented Apr 27, 2021 •
edited
Loading

Closes #33006

Summary

  • add toggler for DST
  • now formatter will be re-created only when interval changes

Steps to reproduce

  1. Create timeseries visualization which spans over the DST. In example below used Europe/Berlin time zone
  2. Include first timestamp before DST: "@timestamp": "2021-03-27T08:30:00" (UTC)
  3. And another at the same time after DST: "@timestamp": "2021-03-28T08:30:00" (UTC)

Actual behaviour: First bucked is 09:30 (CET +01:00), the second is shifted due to DST and has 10:30 (CEST +02:00)

Expected behaviour: Both should be the same 09:30

Before

22402C5F-D07A-463C-A809-20269B7BC737_1_105_c

After

05969B1E-F7D5-4F6C-AD5C-1733E7D243F7_1_105_c

598E7A47-819F-4BB1-9DCC-1567897B8CC2_1_105_c

Before

F50A982A-CBD5-4B6F-96CA-40FAC31A081D

After

8C5F9AA3-3BB8-4A56-A283-FC8CA64A9DCB

@dimaanj dimaanj added Feature:TSVB TSVB (Time Series Visual Builder) release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 v7.14.0 labels Apr 27, 2021
@dimaanj dimaanj requested review from alexwizp and a team April 27, 2021 14:33
@dimaanj dimaanj self-assigned this Apr 27, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@dimaanj dimaanj marked this pull request as draft April 27, 2021 14:34
@dimaanj dimaanj changed the title [TSVB] Ignore DST switch for timeseries [TSVB] Add ignore DST switch for timeseries Apr 27, 2021
@dimaanj
Copy link
Contributor Author

dimaanj commented Apr 28, 2021

@elasticmachine merge upstream

alexwizp
alexwizp reviewed Apr 28, 2021
View reviewed changes
src/plugins/vis_type_timeseries/public/application/components/panel_config/timeseries.tsx Outdated
@@ -227,6 +228,21 @@ export class TimeseriesPanelConfig extends Component<
/>
</EuiFormRow>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiFormLabel>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use EuiFormRow component like we already did for all similar places. See #97796

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added EuiFormRow

stratoula
stratoula reviewed Apr 29, 2021
View reviewed changes
x-pack/plugins/translations/translations/ja-JP.json Outdated
@@ -4436,6 +4436,7 @@
"visTypeTimeseries.timeseries.optionsTab.dataLabel": "データ",
"visTypeTimeseries.timeseries.optionsTab.displayGridLabel": "グリッドを表示",
"visTypeTimeseries.timeseries.optionsTab.ignoreGlobalFilterLabel": "グローバルフィルターを無視しますか？",
"visTypeTimeseries.timeseries.optionsTab.ignoreDaylightTimeLabel": "夏時間を無視しますか？",
Copy link
Contributor

Choose a reason for hiding this comment

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

You shouldn't add the new translations. There is a team that finds the new texts and translates them, so no need to change these files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

@stratoula stratoula changed the title [TSVB] Add ignore DST switch for timeseries [TSVB] Adds ignore DST switch for timeseries Apr 29, 2021
@dimaanj dimaanj requested a review from VladLasitsa April 30, 2021 12:29
VladLasitsa
VladLasitsa approved these changes Apr 30, 2021
View reviewed changes
Copy link
Contributor

@VladLasitsa VladLasitsa left a comment

Choose a reason for hiding this comment

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

LGTM!

@alexwizp alexwizp marked this pull request as ready for review May 4, 2021 08:11
@alexwizp
Copy link
Contributor

alexwizp commented May 4, 2021

@elasticmachine merge upstream

@stratoula
Copy link
Contributor

@elasticmachine merge upstream

stratoula
stratoula approved these changes May 5, 2021
View reviewed changes
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.

LGTM, I tested it locally and I can't find any regression. I think it works fine.

@kibanamachine
Copy link
Contributor

💚 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 1.7MB 1.7MB +952.0B

History

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

cc @dmitriynj

@alexwizp alexwizp added the auto-backport Deprecated - use backport:version if exact versions are needed label May 5, 2021
@dimaanj dimaanj merged commit 203c40c into elastic:master May 5, 2021
@dimaanj dimaanj deleted the ignore-dst-timeseries branch May 5, 2021 13:33
@kibanamachine kibanamachine mentioned this pull request May 5, 2021
Merged
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request May 5, 2021
@dimaanj @kibanamachine
[TSVB] Adds ignore DST switch for timeseries (elastic#98484)
5abf2c5 
* [TSVB] add ignore DST switch

* [TSVB] move YesNo component to timeseries panel, get rid of getTimezone call

* [TSVB] reduce formatter creation number

* [TSVB] add usage EuFormRow, remove redundant translations

* [TSVB] use boolean instead of number

* [TSVB] use number for YesNo

* [TSVB] resolve YesNo value type

* [TSVB] replace number with boolean

Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
✅ 7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request May 5, 2021
@kibanamachine @dimaanj
[TSVB] Adds ignore DST switch for timeseries (#98484) (#99343)
d800252 
* [TSVB] add ignore DST switch

* [TSVB] move YesNo component to timeseries panel, get rid of getTimezone call

* [TSVB] reduce formatter creation number

* [TSVB] add usage EuFormRow, remove redundant translations

* [TSVB] use boolean instead of number

* [TSVB] use number for YesNo

* [TSVB] resolve YesNo value type

* [TSVB] replace number with boolean

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Dmitry Tomashevich <[email protected]>
@alexwizp alexwizp mentioned this pull request Jun 3, 2021
Closed
@stratoula stratoula mentioned this pull request Jan 5, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexwizp alexwizp alexwizp approved these changes

@stratoula stratoula stratoula approved these changes

@VladLasitsa VladLasitsa VladLasitsa approved these changes

Assignees

@dimaanj dimaanj

Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:TSVB TSVB (Time Series Visual Builder) release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.14.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Timeseries Visual Builder - DST shift in historic data
6 participants
@dimaanj @elasticmachine @alexwizp @stratoula @kibanamachine @VladLasitsa