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

Timeseries: Time regions migration #66998

Merged
merged 22 commits into from
Apr 27, 2023
Merged

Conversation

adela-almasan
Copy link
Contributor

@adela-almasan adela-almasan commented Apr 20, 2023

Updated migrations to include time regions. Time regions PR needs to be merged first.

Check: http://localhost:3000/d/XMjIZPmik/panel-tests-graph-time-regions?orgId=1&from=now-30d&to=now&__feature.autoMigrateOldPanels=true

Fixes #66997

image

TO DO:

  • auto refresh dashboard?

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@adela-almasan adela-almasan added area/panel/timeseries The main time series Graph panel no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes blocked Issue awaiting a resolution or consencus before moving on labels Apr 20, 2023
@adela-almasan adela-almasan added this to the 10.0.0 milestone Apr 20, 2023
@adela-almasan adela-almasan self-assigned this Apr 20, 2023
@adela-almasan adela-almasan requested a review from a team as a code owner April 20, 2023 18:13
@adela-almasan adela-almasan marked this pull request as draft April 20, 2023 18:14
Copy link
Contributor

@nmarrs nmarrs left a comment

Choose a reason for hiding this comment

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

code changes LGTM - will want to test again after blocking PR is merged

console.log('scheduling a refresh');
dashboardRefresher = setTimeout(() => {
console.log('trigger refresh for annotations');
dashboard.startRefresh({ refreshAll: true, panelIds: [] });
Copy link
Member

Choose a reason for hiding this comment

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

@dprokop @torkelo -- any suggestions on how to trigger the annotations to rerun. This was my hack, but not working consistently

Copy link
Member

Choose a reason for hiding this comment

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

@ryantxu the problem here is that this will be triggered multiple times if you have many panels with time regions on dashboard. So maybe need some kind of debounce function for this. But ideally it should only retrigger annotations. Think it could be better if this was something like (But still need to figure out the debounce), the other problem I guess is that when loading a dashboard this is kind of too late. The annotation queries might already be running. Is there a way to do these migrations in the DahboardMigrator instead so they happen before dashboard is initialized and annotation queries are issued?

getDashboardQueryRunner().run({dashboard, range: getTimeSrv().getTimeRange()}

Copy link
Member

Choose a reason for hiding this comment

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

thanks! that works -- we only trigger one refresh 1/4 sec after load

@ryantxu ryantxu marked this pull request as ready for review April 27, 2023 00:15
@ryantxu ryantxu requested review from grafanabot and a team as code owners April 27, 2023 00:15
@ryantxu ryantxu requested review from tskarhed, ashharrison90 and academo and removed request for a team April 27, 2023 00:15
@adela-almasan adela-almasan removed the blocked Issue awaiting a resolution or consencus before moving on label Apr 27, 2023
@adela-almasan adela-almasan enabled auto-merge (squash) April 27, 2023 02:04
@adela-almasan adela-almasan merged commit 2beee35 into main Apr 27, 2023
8 checks passed
@adela-almasan adela-almasan deleted the 66997_time_regions_migrations branch April 27, 2023 02:29
@zerok zerok modified the milestones: 10.0.0, 10.0.0-preview May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend area/panel/timeseries The main time series Graph panel no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timeseries: Time regions migration
7 participants