IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
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.
This commit is contained in:
George Robinson 2023-02-09 20:23:01 +00:00 committed by GitHub
parent 204818e793
commit 1f984409a2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 28 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
pkg/services/ngalert/image/service.go
View File

@ -135,6 +135,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: s.screenshotTimeout,

15
pkg/services/ngalert/image/service_test.go
View File

@ -38,10 +38,11 @@ func TestScreenshotImageService(t *testing.T) {
t.Run("image is taken, uploaded, saved to database and cached", func(t *testing.T) {
// 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: 5 * time.Second,
@ -62,7 +63,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,
@ -75,10 +76,11 @@ func TestScreenshotImageService(t *testing.T) {
t.Run("image is taken, upload return error, saved to database without URL and cached", func(t *testing.T) {
// 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: 5 * time.Second,
@ -98,7 +100,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,
@ -113,7 +115,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,
@ -126,10 +128,11 @@ func TestScreenshotImageService(t *testing.T) {
t.Run("error is returned when timeout is exceeded", func(t *testing.T) {
// assert that the cache is checked for an existing image
cache.EXPECT().Get(gomock.Any(), "jtThkFaZLA4=").Return(models.Image{}, false)
cache.EXPECT().Get(gomock.Any(), "TTHub8HUe2U=").Return(models.Image{}, false)
// assert that when the timeout is exceeded an error is returned
screenshots.EXPECT().Take(gomock.Any(), screenshot.ScreenshotOptions{
OrgID: 1,
DashboardUID: "qux",
PanelID: 1,
Timeout: 5 * time.Second,

2
pkg/services/screenshot/option.go
View File

@ -19,6 +19,7 @@ var (
// ScreenshotOptions are the options for taking a screenshot.
type ScreenshotOptions struct {
OrgID int64
DashboardUID string
PanelID int64
From string
@ -56,6 +57,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(s.From))

22
pkg/services/screenshot/option_test.go
View File

@ -88,30 +88,30 @@ 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{0x3, 0x52, 0x33, 0x5f, 0xed, 0x96, 0x47, 0xb5}, o.Hash())
assert.Equal(t, []byte{0x13, 0x59, 0xa3, 0x88, 0x6a, 0xdd, 0x26, 0x3}, o.Hash())
o.OrgID = 1
assert.Equal(t, []byte{0xd8, 0xfb, 0xf2, 0x7, 0x87, 0x92, 0x57, 0x28}, o.Hash())
o.From = "now-6h"
assert.Equal(t, []byte{0x1b, 0x61, 0xc4, 0x40, 0x1f, 0xae, 0xa0, 0xe0}, o.Hash())
assert.Equal(t, []byte{0x52, 0x3f, 0x86, 0xe3, 0x59, 0x1a, 0x7f, 0xfd}, o.Hash())
o.To = "now-2h"
assert.Equal(t, []byte{0x2, 0x4f, 0xfd, 0xd5, 0x68, 0xdd, 0xd1, 0x99}, o.Hash())
assert.Equal(t, []byte{0x8b, 0x78, 0xf4, 0xe7, 0xaa, 0x6, 0xba, 0xde}, o.Hash())
o.Width = 100
o = o.SetDefaults()
assert.Equal(t, []byte{0x16, 0x6c, 0xd, 0xad, 0x9f, 0x32, 0x9d, 0x1}, o.Hash())
assert.Equal(t, []byte{0xa8, 0x75, 0x43, 0xc1, 0xb5, 0xd5, 0xd2, 0x3c}, o.Hash())
o.Height = 100
o = o.SetDefaults()
assert.Equal(t, []byte{0x2, 0x82, 0x96, 0x68, 0x2b, 0x57, 0x7d, 0xdd}, o.Hash())
assert.Equal(t, []byte{0x3d, 0xf3, 0x37, 0x3d, 0x9a, 0xfd, 0x71, 0x88}, o.Hash())
o.Theme = "Not a theme"
o = o.SetDefaults()
assert.Equal(t, []byte{0x2, 0x82, 0x96, 0x68, 0x2b, 0x57, 0x7d, 0xdd}, o.Hash())
assert.Equal(t, []byte{0x3b, 0xd1, 0xfb, 0x3f, 0x3, 0x64, 0xba, 0xad}, o.Hash())
// the timeout should not change the sum
o.Timeout = DefaultTimeout + 1
assert.Equal(t, []byte{0x2, 0x82, 0x96, 0x68, 0x2b, 0x57, 0x7d, 0xdd}, o.Hash())
assert.Equal(t, []byte{0x3b, 0xd1, 0xfb, 0x3f, 0x3, 0x64, 0xba, 0xad}, o.Hash())
}

10
pkg/services/screenshot/screenshot.go
View File

@ -85,8 +85,8 @@ func (s *HeadlessScreenshotService) Take(ctx context.Context, opts ScreenshotOpt
start := time.Now()
defer func() { s.duration.Observe(time.Since(start).Seconds()) }()
q := dashboards.GetDashboardQuery{UID: opts.DashboardUID}
qResult, err := s.ds.GetDashboard(ctx, &q)
q := dashboards.GetDashboardQuery{OrgID: opts.OrgID, UID: opts.DashboardUID}
dashboard, err := s.ds.GetDashboard(ctx, &q)
if err != nil {
s.instrumentError(err)
return nil, err
@ -95,9 +95,9 @@ func (s *HeadlessScreenshotService) Take(ctx context.Context, opts ScreenshotOpt
opts = opts.SetDefaults()
u := url.URL{}
u.Path = path.Join("d-solo", qResult.UID, qResult.Slug)
u.Path = path.Join("d-solo", dashboard.UID, dashboard.Slug)
p := u.Query()
p.Add("orgId", strconv.FormatInt(qResult.OrgID, 10))
p.Add("orgId", strconv.FormatInt(dashboard.OrgID, 10))
p.Add("panelId", strconv.FormatInt(opts.PanelID, 10))
p.Add("from", opts.From)
p.Add("to", opts.To)
@ -105,7 +105,7 @@ func (s *HeadlessScreenshotService) Take(ctx context.Context, opts ScreenshotOpt
renderOpts := rendering.Opts{
AuthOpts: rendering.AuthOpts{
OrgID: qResult.OrgID,
OrgID: dashboard.OrgID,
OrgRole: org.RoleAdmin,
},
ErrorOpts: rendering.ErrorOpts{