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

Explore: Support user timezone #16469

Merged
merged 12 commits into from
Apr 29, 2019
Merged

Explore: Support user timezone #16469

merged 12 commits into from
Apr 29, 2019

Conversation

marefr
Copy link
Member

@marefr marefr commented Apr 9, 2019

What this PR does / why we need it:
Explore now uses the timezone of the user to decide if local browser time (current) or UTC should be used.

Some tests and cleanup probably needed.

Which issue(s) this PR fixes:
Closes #12812

Special notes for your reviewer:
Would be nice with some initial feedback to know if I'm going in the right direction.

After looking into this a bit feels like timezone needs to be part of the url when going from dashboard to Explore and in that case it overrides the user timezone setting - what do you think?

@marefr marefr added this to Under review in Observability (deprecated, use Observability Squad) via automation Apr 9, 2019
Copy link
Contributor

@hugohaggmark hugohaggmark left a comment

Choose a reason for hiding this comment

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

LGTM!

As you said yourself, we should add this to the URL as well. But in a separate PR.

@steven-sheehy
Copy link
Contributor

This might be the wrong place, but is there any thought given to merging the "Timestamp" and "Local Time" columns in Log explorer? I've always found it confusing that there's multiple time columns. In my opinion there should be one that defaults to user time zone but maybe with an option to change it to UTC or other timezone.

@marefr
Copy link
Member Author

marefr commented Apr 9, 2019

@steven-sheehy yes I think your comment deserves a new issue. But good idea now when we're adding support for local browser time/utc.

@marefr marefr marked this pull request as ready for review April 9, 2019 21:57
Copy link
Contributor

@davkal davkal left a comment

Choose a reason for hiding this comment

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

Code looks great.

Small issue:
The range string is still in local time when using UTC.
To repro, set UTC, run a query, then click on the left arrow next to the timepicker.

@marefr
Copy link
Member Author

marefr commented Apr 15, 2019

Thanks @davkal will have a look at it.

@marefr marefr moved this from Under review to In progress in Observability (deprecated, use Observability Squad) Apr 15, 2019
@marefr marefr moved this from In progress to Under review in Observability (deprecated, use Observability Squad) Apr 15, 2019
Now uses TimeRange instead of RawTimeRange in explore item
state tree and only parsing actual time in a few action
handlers.
Time picker should now properly handle moving back/forward and
apply time range when both utc and non utc time zone.
URL range representation is changed from YYYY-MM-DD HH:mm:ss
to epoch ms.
@torkelo
Copy link
Member

torkelo commented Apr 18, 2019

Looks good, like how time range is now parsed on init & change time actions

Copy link
Member

@torkelo torkelo left a comment

Choose a reason for hiding this comment

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

Looks good. making onChangeTime on the graph be from/to in ms epoch sounds like a good idea

@jschill
Copy link
Contributor

jschill commented Apr 23, 2019

@davkal We're unable to reproduce that issue. Are you sure you saved after changing back to Local browser?

@marefr
Copy link
Member Author

marefr commented Apr 23, 2019

@torkelo now I added a new interface to @grafana/ui called AbsoluteTimeRange which holds from/to as numbers. In the future I'm thinking that we can add an interface RelativeTimeRange which holds from/to as strings. We can then create a new type that's either RelativeTimeRange or either AbsoluteTimeRange and this could then be passed around instead of today's RawTimeRange/TimeRange which includes moment.

@torkelo
Copy link
Member

torkelo commented Apr 23, 2019

Think that could be work, only problem will be dashboard time range and datasource plug-ins expecting a moment

public/app/features/explore/state/selectors.ts Outdated Show resolved Hide resolved
public/app/features/explore/Explore.tsx Outdated Show resolved Hide resolved
public/app/core/utils/explore.ts Show resolved Hide resolved
Also makes a copy of the time range passed to timeSrv to make sure immutability
of explore time range when for example elasticsearch test datasources uses
timeSrv and sets a time range of last 1 min
Copy link
Contributor

@hugohaggmark hugohaggmark left a comment

Choose a reason for hiding this comment

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

LGTM! (again)

public/app/features/explore/Explore.tsx Outdated Show resolved Hide resolved
@davkal
Copy link
Contributor

davkal commented Apr 26, 2019

Works now! Been changing wrong prefs. All systems 💚

@marefr
Copy link
Member Author

marefr commented Apr 29, 2019

Think that could be work, only problem will be dashboard time range and datasource plug-ins expecting a moment

@torkelo have added an epic to fix this long term #16806

@marefr marefr merged commit 02cb7ff into master Apr 29, 2019
Observability (deprecated, use Observability Squad) automation moved this from Under review to Done Apr 29, 2019
@marefr marefr deleted the 12812_explore_timezone branch April 29, 2019 16:28
@marefr marefr added this to the 6.2 milestone Apr 29, 2019
ryantxu added a commit to ryantxu/grafana that referenced this pull request Apr 29, 2019
…naui

* grafana/master:
  Explore: Support user timezone (grafana#16469)
  Plugins: rename vizPlugin to panelPlugin (grafana#16802)
  Plugins: move app/feature/plugin properties into PluginMeta (grafana#16809)
  Plugins: move PanelPluginMeta to grafana/ui (grafana#16804)
ryantxu added a commit to ryantxu/grafana that referenced this pull request Apr 29, 2019
* grafana/master:
  Explore: Support user timezone (grafana#16469)
  Plugins: rename vizPlugin to panelPlugin (grafana#16802)
  Plugins: move app/feature/plugin properties into PluginMeta (grafana#16809)
  Plugins: move PanelPluginMeta to grafana/ui (grafana#16804)
  Plugins: move datasource specific meta out of the main meta type (grafana#16803)
  updates changelog for 6.1.6
  Fix: Fetch histogram series from other api route (grafana#16768)
  phantomjs: set web-security to true
  Chore: Lowered implicit anys limit to 5668
bergquist added a commit to vivtony00/grafana that referenced this pull request Apr 30, 2019
* master: (470 commits)
  removes gopkg.lock from root folder
  Explore: Support user timezone (grafana#16469)
  Plugins: rename vizPlugin to panelPlugin (grafana#16802)
  Plugins: move app/feature/plugin properties into PluginMeta (grafana#16809)
  Plugins: move PanelPluginMeta to grafana/ui (grafana#16804)
  Plugins: move datasource specific meta out of the main meta type (grafana#16803)
  updates changelog for 6.1.6
  Fix: Fetch histogram series from other api route (grafana#16768)
  phantomjs: set web-security to true
  Chore: Lowered implicit anys limit to 5668
  build: restore postgres integration tests (grafana#16801)
  docs: explain correct access control model of GCS buckets (grafana#16792)
  Chore: Fixed no implicit any Typescript errors (grafana#16799)
  Feature: introduce LdapActiveSyncEnabled setting (grafana#16787)
  Plugins: ReactPanelPlugin to VizPanelPlugin (grafana#16779)
  UX: Improve Grafana usage for smaller screens  (grafana#16783)
  ThresholdEditor: Minor style fix for smaller screens (grafana#16791)
  Build: Use isolated modules for ts-jest (grafana#16786)
  LDAP Refactoring to support syncronizing more than one user at a time. (grafana#16705)
  build: removes unused vendored files
  ...
ryantxu added a commit to ryantxu/grafana that referenced this pull request Apr 30, 2019
* grafana/master:
  Auth: Enable retries and transaction for some db calls for auth tokens  (grafana#16785)
  Provisioning: Show file path of provisioning file in save/delete dialogs (grafana#16706)
  Add tracing headers for prometheus datasource (grafana#16724)
  Config: Fixes bug where timeouts for alerting was not parsed correctly (grafana#16784)
  build: removes gopkg files from dev docker file (grafana#16817)
  Provisioning: Trying to fix failing test (grafana#16800)
  Table: React table fix rotate support in storybook (grafana#16816)
  TestData: add log level in dummy message (grafana#16815)
  removes gopkg.lock from root folder
  Explore: Support user timezone (grafana#16469)
  Plugins: rename vizPlugin to panelPlugin (grafana#16802)
  Plugins: move app/feature/plugin properties into PluginMeta (grafana#16809)
  Plugins: move PanelPluginMeta to grafana/ui (grafana#16804)
  Plugins: move datasource specific meta out of the main meta type (grafana#16803)
  updates changelog for 6.1.6
  Fix: Fetch histogram series from other api route (grafana#16768)
  phantomjs: set web-security to true
  Chore: Lowered implicit anys limit to 5668
ryantxu added a commit to ryantxu/grafana that referenced this pull request Apr 30, 2019
…MetricPanelCtrl

* grafana/master:
  Explore: Use SeriesData format for loki/logs (grafana#16793)
  Refactor: move NavModel to @grafana/ui (grafana#16813)
  Auth: Enable retries and transaction for some db calls for auth tokens  (grafana#16785)
  Provisioning: Show file path of provisioning file in save/delete dialogs (grafana#16706)
  Add tracing headers for prometheus datasource (grafana#16724)
  Config: Fixes bug where timeouts for alerting was not parsed correctly (grafana#16784)
  build: removes gopkg files from dev docker file (grafana#16817)
  Provisioning: Trying to fix failing test (grafana#16800)
  Table: React table fix rotate support in storybook (grafana#16816)
  TestData: add log level in dummy message (grafana#16815)
  removes gopkg.lock from root folder
  Explore: Support user timezone (grafana#16469)
  Plugins: rename vizPlugin to panelPlugin (grafana#16802)
  Plugins: move app/feature/plugin properties into PluginMeta (grafana#16809)
  Plugins: move PanelPluginMeta to grafana/ui (grafana#16804)
  Plugins: move datasource specific meta out of the main meta type (grafana#16803)
  updates changelog for 6.1.6
  Fix: Fetch histogram series from other api route (grafana#16768)
  phantomjs: set web-security to true
  Chore: Lowered implicit anys limit to 5668
ryantxu added a commit to ryantxu/grafana that referenced this pull request Apr 30, 2019
* grafana/master:
  Explore: Use SeriesData format for loki/logs (grafana#16793)
  Refactor: move NavModel to @grafana/ui (grafana#16813)
  Auth: Enable retries and transaction for some db calls for auth tokens  (grafana#16785)
  Provisioning: Show file path of provisioning file in save/delete dialogs (grafana#16706)
  Add tracing headers for prometheus datasource (grafana#16724)
  Config: Fixes bug where timeouts for alerting was not parsed correctly (grafana#16784)
  build: removes gopkg files from dev docker file (grafana#16817)
  Provisioning: Trying to fix failing test (grafana#16800)
  Table: React table fix rotate support in storybook (grafana#16816)
  TestData: add log level in dummy message (grafana#16815)
  removes gopkg.lock from root folder
  Explore: Support user timezone (grafana#16469)
  Plugins: rename vizPlugin to panelPlugin (grafana#16802)
  Plugins: move app/feature/plugin properties into PluginMeta (grafana#16809)
  Plugins: move PanelPluginMeta to grafana/ui (grafana#16804)
  Plugins: move datasource specific meta out of the main meta type (grafana#16803)
  updates changelog for 6.1.6
  Fix: Fetch histogram series from other api route (grafana#16768)
  phantomjs: set web-security to true
  Chore: Lowered implicit anys limit to 5668
ryantxu added a commit to ryantxu/grafana that referenced this pull request May 2, 2019
* grafana/master: (27 commits)
  FormLabel: allow any rather than just a string for tooltip (grafana#16841)
  prometheus: fix regression after adding support for tracing headers (grafana#16829)
  area/circleci: Speed up circleci build process for branches and pr (grafana#16778)
  DataProxy: Restore Set-Cookie header after proxy request (grafana#16838)
  docs: clarify page parameter version support for folder/dashboard search (grafana#16836)
  Chore: revise some of the gosec rules (grafana#16713)
  Refactor: consistant plugin/meta usage (grafana#16834)
  Explore: Use SeriesData format for loki/logs (grafana#16793)
  Refactor: move NavModel to @grafana/ui (grafana#16813)
  Auth: Enable retries and transaction for some db calls for auth tokens  (grafana#16785)
  Provisioning: Show file path of provisioning file in save/delete dialogs (grafana#16706)
  Add tracing headers for prometheus datasource (grafana#16724)
  Config: Fixes bug where timeouts for alerting was not parsed correctly (grafana#16784)
  build: removes gopkg files from dev docker file (grafana#16817)
  Provisioning: Trying to fix failing test (grafana#16800)
  Table: React table fix rotate support in storybook (grafana#16816)
  TestData: add log level in dummy message (grafana#16815)
  removes gopkg.lock from root folder
  Explore: Support user timezone (grafana#16469)
  Plugins: rename vizPlugin to panelPlugin (grafana#16802)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Explore: should use grafana timezone
7 participants