[v9.3.x] Alerting: Fix a bug taking screenshots with Dashboard UID (#63226)

Alerting: Fix a bug taking screenshots with Dashboard UID (#63220)

This commit fixes a bug where Grafana would fail to take a screenshot if
the same Dashboard UID was present across two or more different orgs.

(cherry picked from commit 1f984409a2)

pkg/services/ngalert/image/service.go

@@ -130,6 +130,7 @@ func (s *ScreenshotImageService) NewImage(ctx context.Context, r *models.AlertRu
 	logger = logger.New("dashboard", dashboardUID, "panel", panelID)
 
 	opts := screenshot.ScreenshotOptions{
+		OrgID:        r.OrgID,
 		DashboardUID: dashboardUID,
 		PanelID:      panelID,
 		Timeout:      screenshotTimeout,

pkg/services/ngalert/image/service_test.go

@@ -36,10 +36,11 @@ func TestScreenshotImageService(t *testing.T) {
 	ctx := context.Background()
 
 	// assert that the cache is checked for an existing image
-	cache.EXPECT().Get(gomock.Any(), "M2DGZaRLXtg=").Return(models.Image{}, false)
+	cache.EXPECT().Get(gomock.Any(), "oyh1kYgaJwM=").Return(models.Image{}, false)
 
 	// assert that a screenshot is taken
 	screenshots.EXPECT().Take(gomock.Any(), screenshot.ScreenshotOptions{
+		OrgID:        1,
 		DashboardUID: "foo",
 		PanelID:      1,
 		Timeout:      screenshotTimeout,
@@ -60,7 +61,7 @@ func TestScreenshotImageService(t *testing.T) {
 	}
 
 	// assert that the image is saved into the cache
-	cache.EXPECT().Set(gomock.Any(), "M2DGZaRLXtg=", expected).Return(nil)
+	cache.EXPECT().Set(gomock.Any(), "oyh1kYgaJwM=", expected).Return(nil)
 
 	image, err := s.NewImage(ctx, &models.AlertRule{
 		OrgID:        1,
@@ -71,10 +72,11 @@ func TestScreenshotImageService(t *testing.T) {
 	assert.Equal(t, expected, *image)
 
 	// assert that the cache is checked for an existing image
-	cache.EXPECT().Get(gomock.Any(), "rTOWVcbRidk=").Return(models.Image{}, false)
+	cache.EXPECT().Get(gomock.Any(), "yszV9tgmKAo=").Return(models.Image{}, false)
 
 	// assert that a screenshot is taken
 	screenshots.EXPECT().Take(gomock.Any(), screenshot.ScreenshotOptions{
+		OrgID:        1,
 		DashboardUID: "bar",
 		PanelID:      1,
 		Timeout:      screenshotTimeout,
@@ -94,7 +96,7 @@ func TestScreenshotImageService(t *testing.T) {
 	}
 
 	// assert that the image is saved into the cache, but without a URL
-	cache.EXPECT().Set(gomock.Any(), "rTOWVcbRidk=", expected).Return(nil)
+	cache.EXPECT().Set(gomock.Any(), "yszV9tgmKAo=", expected).Return(nil)
 
 	image, err = s.NewImage(ctx, &models.AlertRule{
 		OrgID:        1,
@@ -107,7 +109,7 @@ func TestScreenshotImageService(t *testing.T) {
 	expected = models.Image{Path: "baz.png", URL: "https://example.com/baz.png"}
 
 	// assert that the cache is checked for an existing image and it is returned
-	cache.EXPECT().Get(gomock.Any(), "8hJuVe20rVE=").Return(expected, true)
+	cache.EXPECT().Get(gomock.Any(), "he399rFDBPI=").Return(expected, true)
 
 	image, err = s.NewImage(ctx, &models.AlertRule{
 		OrgID:        1,

pkg/services/screenshot/option.go

@@ -17,6 +17,7 @@ var (
 
 // ScreenshotOptions are the options for taking a screenshot.
 type ScreenshotOptions struct {
+	OrgID        int64
 	DashboardUID string
 	PanelID      int64
 	Width        int
@@ -46,6 +47,7 @@ func (s ScreenshotOptions) SetDefaults() ScreenshotOptions {
 
 func (s ScreenshotOptions) Hash() []byte {
 	h := fnv.New64()
+	_, _ = h.Write([]byte(strconv.FormatInt(s.OrgID, 10)))
 	_, _ = h.Write([]byte(s.DashboardUID))
 	_, _ = h.Write([]byte(strconv.FormatInt(s.PanelID, 10)))
 	_, _ = h.Write([]byte(strconv.FormatInt(int64(s.Width), 10)))

pkg/services/screenshot/option_test.go

@@ -56,24 +56,24 @@ func TestScreenshotOptions(t *testing.T) {
 
 func TestScreenshotOptions_Hash(t *testing.T) {
 	o := ScreenshotOptions{}
-	assert.Equal(t, []byte{0xd9, 0x83, 0x82, 0x18, 0x6c, 0x3d, 0x7d, 0x47}, o.Hash())
+	assert.Equal(t, []byte{0xd7, 0xf3, 0x56, 0x7f, 0xec, 0x7b, 0xdf, 0x95}, o.Hash())
 
 	o = o.SetDefaults()
-	assert.Equal(t, []byte{0x6, 0x7, 0x97, 0x6, 0x53, 0xf, 0x8b, 0xf1}, o.Hash())
+	assert.Equal(t, []byte{0x71, 0x54, 0xe8, 0xea, 0x2, 0xf4, 0xd6, 0xd3}, o.Hash())
+
+	o.OrgID = 1
+	assert.Equal(t, []byte{0x8, 0x75, 0x48, 0x46, 0x88, 0xa9, 0xf3, 0xe6}, o.Hash())
 
 	o.Width = 100
-	o = o.SetDefaults()
-	assert.Equal(t, []byte{0x25, 0x50, 0xb4, 0x4b, 0x43, 0xcd, 0x3, 0x49}, o.Hash())
+	assert.Equal(t, []byte{0xff, 0xf3, 0xb1, 0x1, 0x1e, 0x20, 0x46, 0xd4}, o.Hash())
 
 	o.Height = 100
-	o = o.SetDefaults()
-	assert.Equal(t, []byte{0x51, 0xe2, 0x6f, 0x2c, 0x62, 0x7b, 0x3b, 0xc5}, o.Hash())
+	assert.Equal(t, []byte{0x57, 0xdb, 0xa7, 0xb8, 0x5a, 0x7f, 0x26, 0xe0}, o.Hash())
 
 	o.Theme = "Not a theme"
-	o = o.SetDefaults()
-	assert.Equal(t, []byte{0x51, 0xe2, 0x6f, 0x2c, 0x62, 0x7b, 0x3b, 0xc5}, o.Hash())
+	assert.Equal(t, []byte{0xc2, 0x7e, 0xed, 0x19, 0x95, 0x32, 0x55, 0x85}, o.Hash())
 
 	// the timeout should not change the sum
 	o.Timeout = DefaultTimeout + 1
-	assert.Equal(t, []byte{0x51, 0xe2, 0x6f, 0x2c, 0x62, 0x7b, 0x3b, 0xc5}, o.Hash())
+	assert.Equal(t, []byte{0xc2, 0x7e, 0xed, 0x19, 0x95, 0x32, 0x55, 0x85}, o.Hash())
 }

pkg/services/screenshot/screenshot.go

@@ -86,7 +86,7 @@ func (s *HeadlessScreenshotService) Take(ctx context.Context, opts ScreenshotOpt
 	start := time.Now()
 	defer func() { s.duration.Observe(time.Since(start).Seconds()) }()
 
-	q := models.GetDashboardQuery{Uid: opts.DashboardUID}
+	q := models.GetDashboardQuery{OrgId: opts.OrgID, Uid: opts.DashboardUID}
 	if err := s.ds.GetDashboard(ctx, &q); err != nil {
 		s.instrumentError(err)
 		return nil, err