Live: broadcast events when dashboard is saved #27583

ryantxu · 2020-09-14T23:42:48Z

When the "live" feature flag is enabled and a dashboard is saved, an event is broadcast to everyone looking at that dashboard.

If the viewer is in edit mode, or has local edits, we show a popup asking what to do. Otherwise the dashboard is updated automatically.

torkelo

Left some comments, but before I read your description in detail. so missed your own comment on the router thing :)

this subscribes to a channel for each dashboard -- this would let use out-of-the-box centrifuge presence features for dashboards. Alternatively we could have a single channel and manage the notifications internally (not too hard and may be better)

Any pros / cons? I assume a centrifuge channel is just an internal thing for routing events? (so not a new websocket connection). And don't we have to use that in order route events efficiently? (So we do not need implement our own event routing so only browsers who are interested in specific events get's them). For example we only want browsers who have a specific dashboard open to get events related to it, not get events for all open dashboards. So given that I think we need to use a channel per dashboard right?

public/app/features/live/pageTracker.ts

packages/grafana-runtime/src/services/live.ts

pkg/services/live/events.go

bergquist · 2020-09-15T15:28:24Z

🕺

ryantxu · 2020-09-15T22:23:26Z

Any pros / cons?

Yes -- i went in circles on this and yes one channel for each dashboard is the right approach.

@torkelo -- I made a bunch of improvements since you looked, I would love feedback :)

bergquist · 2020-09-17T10:18:37Z

I certainly find this an interesting feature. Hard to say exactly what the default behavior should be for the viewer who gets the new version pushed.

torkelo · 2020-09-17T11:27:32Z

Sorry for no follow-up review, 7.2 release and fixes have been pressing. And after that, the annotation/exemplar architecture is higher prio

torkelo · 2020-09-30T07:16:05Z

pkg/services/live/features/dashboard.go

+type dashboardEvent struct {
+	UID       string `json:"uid"`
+	Action    string `json:"action"` // saved, editing
+	UserID    int64  `json:"userId,omitempty"`


Working with event systems before, it's always nice if the events are information-rich and more usable standalone.

It would mean including more dashboard & user info here like dashboard name, (maybe a user object, that includes id, login, email, avatarUrl) makes consuming and using these events much easier without requiring further API calls.

We could add the whole json model... But I think we would still need to do a full refresh in the current system regardless. If only dashboard were an observable 🙂

I was more thinking of other uses for this event (like an admin viewing a list of activity), and for the modal, we might want to show the user name & profile image of the user who updated the same dashboard you have.

So ash much meta info about the entities in the event is usually good. Could also be used by a search or cache subsystem for example to update index state and for that we do not want to issue further db/api calls.

I'm happy to add more info now -- but since this is an internal event, I think we should wait and add more info when we have a real use for it.

I was more thinking of other uses for this event

That is info that should be attached on the 'presense' side -- that still needs to be explored some more, but I fear this PR is getting too big as is :)

torkelo · 2020-09-30T16:29:23Z

public/app/features/live/dashboard/dashboardWatcher.ts

+        if (event.message.sessionId === sessionId) {
+          return; // skip internal messages
+        }


is there away to filter this out in liveSrv?

torkelo

Nice! left a few nits. But yea the typing looks a bit messy.

torkelo · 2020-09-30T16:31:16Z

public/app/features/live/dashboard/dashboardWatcher.ts

+  const dashboardConfig: LiveChannelConfig = {
+    path: '${uid}',
+    description: 'Dashboard change events',
+    variables: [{ value: 'uid', label: '${uid}', description: 'unique id for a dashboard' }],


how are dashboardConfig.variables used?

idea here is that this is mostly documentation for what variables exist and what they need to be. Looking now, we should likely remove this until it has something concrete, but I want to be careful that the path based routing has some way to be more constrained

public/app/features/live/dashboard/DashboardChangedModal.tsx

torkelo · 2020-09-30T16:35:10Z

public/app/features/live/dashboard/DashboardChangedModal.tsx

+
+    return (
+      <Modal
+        isOpen={!!!dismiss}


is this not the same as !dismiss ?

fixed -- looks like ! and !!! are identical for a boolean value, but different if the value can be undefined of falsey

You could use !Boolean(dismiss)

torkelo · 2020-09-30T16:43:17Z

public/app/features/live/dashboard/DashboardChangedModal.tsx

+    const radioClass = css`
+      cursor: pointer;
+      width: 100%;
+      padding: 20px;
+      background: ${config.theme.colors.dropdownBg};
+
+      &:hover {
+        background: ${config.theme.colors.formCheckboxBgCheckedHover};
+      }
+    `;


this feels like a new design system component. Not sure what to do for this PR if there is no current component that fits.

For a new component & we should use useTheme & useStyles

Co-authored-by: Torkel Ödegaard <torkel@grafana.org>

torkelo · 2020-09-30T18:37:17Z

public/app/features/live/dashboard/DashboardChangedModal.tsx

@@ -104,3 +96,17 @@ export class DashboardChangedModal extends PureComponent<Props, State> {
    );
  }
 }
+
+const getStyles = (theme: GrafanaTheme) => {


use stylesFactory here so we only build the styles once

pkg/services/live/features/testdata.go

kaydelaney · 2020-09-30T23:24:37Z

pkg/services/live/features/testdata.go

+		g.running = true
+
+		// Run in the background
+		go g.runRandomCSV()


Is this threadsafe? For example if nothing is subscribed to yet, but then two or more clients subscribe at around the same time is there any guarantee that multiple threads running runRandomCSV won't be started?

Co-authored-by: kay delaney <45561153+kaydelaney@users.noreply.github.com>

ryantxu · 2020-10-01T02:42:46Z

pkg/services/live/live.go

-func (b *GrafanaLive) Publish(channel string, data []byte) bool {
-	_, err := b.node.Publish(channel, data)
+// GetChannelHandler gives threadsafe access to the channel
+func (g *GrafanaLive) GetChannelHandler(channel string) (models.ChannelHandler, error) {


@marefr -- can you audit this function? It is called frequently and potentially from multiple threads.

public/app/features/live/live.ts

kaydelaney · 2020-10-01T13:50:04Z

Looking good! Just left some small comments.
One last thing: Is "Presense" intentional or is that a typo that should be changed to "Presence"

Co-authored-by: kay delaney <45561153+kaydelaney@users.noreply.github.com>

ryantxu · 2020-10-01T17:36:54Z

Is "Presense" intentional or is that a typo that should be changed to "Presence"

dooh -- just dyslexia and poor spelling. Thanks!

torkelo

Yay, so awesome to get to nice things like this 🚀

Co-authored-by: kay delaney <45561153+kaydelaney@users.noreply.github.com> Co-authored-by: Torkel Ödegaard <torkel@grafana.org>

ryantxu added 3 commits September 14, 2020 16:27

edit callback

075c1dc

fix lint

36cc105

merge master

09c0b67

torkelo reviewed Sep 15, 2020

View reviewed changes

public/app/features/live/pageTracker.ts Outdated Show resolved Hide resolved

packages/grafana-runtime/src/services/live.ts Outdated Show resolved Hide resolved

pkg/services/live/events.go Outdated Show resolved Hide resolved

ryantxu added 3 commits September 15, 2020 08:40

Merge remote-tracking branch 'origin/master' into live-dashboard-update

0819a52

better event detection

e0891e8

dashboard edit mode

2110347

ryantxu added 2 commits September 15, 2020 15:42

Merge remote-tracking branch 'origin/master' into live-dashboard-update

33730cb

fix tests

63b879b

ryantxu requested a review from torkelo September 15, 2020 23:02

ryantxu added this to the 7.3 milestone Sep 15, 2020

ryantxu requested a review from kaydelaney September 16, 2020 15:26

ryantxu mentioned this pull request Sep 17, 2020

Live: scope channels to plugins #27630

Closed

2 tasks

ryantxu added 12 commits September 22, 2020 13:33

update dashboard settings

c79c52f

merge master

d2ebc3f

fix issues

9e33d01

use changeTracker

092ce8a

dashboard editing

eb6d3ae

Merge remote-tracking branch 'origin/master' into live-dashboard-update

12e223f

add backend with presense

f2c7022

now with backend support

665fa60

Merge remote-tracking branch 'origin/master' into live-dashboard-update

7bc21fb

fix spelling

7fdb34d

support read/write on all plugins

70a0c49

support read/write on all plugins

4db9a3a

ryantxu mentioned this pull request Sep 25, 2020

WIP/Live: manage backend streams #27828

Closed

torkelo reviewed Sep 30, 2020

View reviewed changes

ryantxu and others added 2 commits September 30, 2020 10:47

Update public/app/features/live/dashboard/DashboardChangedModal.tsx

5bbcb03

Co-authored-by: Torkel Ödegaard <torkel@grafana.org>

add modal

add6d06

torkelo reviewed Sep 30, 2020

View reviewed changes

ryantxu added 2 commits September 30, 2020 12:46

update typings

d3e175f

use styleFactory

5e619eb

kaydelaney reviewed Sep 30, 2020

View reviewed changes

ryantxu and others added 3 commits September 30, 2020 16:26

replace chat with broadcast... make it work with telegraf

9daaae8

Merge remote-tracking branch 'origin/master' into live-dashboard-update

6306133

Update pkg/services/live/features/testdata.go

24e0c59

Co-authored-by: kay delaney <45561153+kaydelaney@users.noreply.github.com>

ryantxu commented Oct 1, 2020

View reviewed changes

kaydelaney reviewed Oct 1, 2020

View reviewed changes

public/app/features/live/live.ts Outdated Show resolved Hide resolved

kaydelaney reviewed Oct 1, 2020

View reviewed changes

public/app/features/live/live.ts Outdated Show resolved Hide resolved

kaydelaney reviewed Oct 1, 2020

View reviewed changes

public/app/features/live/live.ts Outdated Show resolved Hide resolved

ryantxu and others added 5 commits October 1, 2020 08:19

Update public/app/features/live/live.ts

a6411f3

Co-authored-by: kay delaney <45561153+kaydelaney@users.noreply.github.com>

Update public/app/features/live/live.ts

96aa2d7

Co-authored-by: kay delaney <45561153+kaydelaney@users.noreply.github.com>

Update public/app/features/live/live.ts

c1f5fa9

Co-authored-by: kay delaney <45561153+kaydelaney@users.noreply.github.com>

Presense >> Presence

c0e98aa

Merge remote-tracking branch 'origin/master' into live-dashboard-update

3572eb3

torkelo approved these changes Oct 1, 2020

View reviewed changes

ryantxu merged commit 8a5fc00 into master Oct 1, 2020

ryantxu deleted the live-dashboard-update branch October 1, 2020 17:46

This was referenced Nov 9, 2020

Feature Request: Support Optional Dashboard Reload #1169

Closed

[Feature request] Mark dashboard as outdated when another user changes it #7369

Closed

[Feature] Custom flash messages #13022

Closed

ryantxu added a commit that referenced this pull request Nov 18, 2020

Live: broadcast events when dashboard is saved (#27583)

faf1bdb

Co-authored-by: kay delaney <45561153+kaydelaney@users.noreply.github.com> Co-authored-by: Torkel Ödegaard <torkel@grafana.org>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Live: broadcast events when dashboard is saved #27583

Live: broadcast events when dashboard is saved #27583

ryantxu commented Sep 14, 2020 •

edited

Loading

torkelo left a comment

bergquist commented Sep 15, 2020

ryantxu commented Sep 15, 2020

bergquist commented Sep 17, 2020

torkelo commented Sep 17, 2020

torkelo Sep 30, 2020

ryantxu Sep 30, 2020

torkelo Sep 30, 2020

ryantxu Sep 30, 2020

ryantxu Sep 30, 2020

torkelo Sep 30, 2020

torkelo left a comment

torkelo Sep 30, 2020

ryantxu Sep 30, 2020

torkelo Sep 30, 2020

ryantxu Sep 30, 2020

hugohaggmark Oct 1, 2020

torkelo Sep 30, 2020

torkelo Sep 30, 2020

kaydelaney Sep 30, 2020

ryantxu Oct 1, 2020

kaydelaney commented Oct 1, 2020 •

edited

Loading

ryantxu commented Oct 1, 2020

torkelo left a comment

Live: broadcast events when dashboard is saved #27583

Live: broadcast events when dashboard is saved #27583

Conversation

ryantxu commented Sep 14, 2020 • edited Loading

torkelo left a comment

Choose a reason for hiding this comment

bergquist commented Sep 15, 2020

ryantxu commented Sep 15, 2020

bergquist commented Sep 17, 2020

torkelo commented Sep 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

torkelo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaydelaney commented Oct 1, 2020 • edited Loading

ryantxu commented Oct 1, 2020

torkelo left a comment

Choose a reason for hiding this comment

ryantxu commented Sep 14, 2020 •

edited

Loading

kaydelaney commented Oct 1, 2020 •

edited

Loading