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 images cached on rule instead of dashboard panel signature (#58510)

Browse Source
This commit is contained in:
George Robinson 2022-11-09 17:01:48 +00:00 committed by GitHub
parent b92a0223e3
commit 7e852720e3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 19 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -132,12 +132,22 @@ func (s *ScreenshotImageService) NewImage(ctx context.Context, r *models.AlertRu
return nil, ErrNoPanel return nil, ErrNoPanel
} }
opts := screenshot.ScreenshotOptions{
DashboardUID: *r.DashboardUID,
PanelID: *r.PanelID,
Timeout: screenshotTimeout,
}
// To prevent concurrent screenshots of the same dashboard panel we use singleflight,
// deduplicated on a base64 hash of the screenshot options.
optsHash := base64.StdEncoding.EncodeToString(opts.Hash())
logger := s.logger.FromContext(ctx).New( logger := s.logger.FromContext(ctx).New(
"dashboard", *r.DashboardUID, "dashboard", opts.DashboardUID,
"panel", *r.PanelID) "panel", opts.PanelID)
// If there is an image is in the cache return it instead of taking another screenshot // If there is an image is in the cache return it instead of taking another screenshot
if image, ok := s.cache.Get(ctx, r.GetKey().String()); ok { if image, ok := s.cache.Get(ctx, optsHash); ok {
logger.Debug("Found cached image", "token", image.Token) logger.Debug("Found cached image", "token", image.Token)
return &image, nil return &image, nil
} }
@ -150,15 +160,6 @@ func (s *ScreenshotImageService) NewImage(ctx context.Context, r *models.AlertRu
ctx, cancelFunc := context.WithTimeout(ctx, screenshotTimeout) ctx, cancelFunc := context.WithTimeout(ctx, screenshotTimeout)
defer cancelFunc() defer cancelFunc()
opts := screenshot.ScreenshotOptions{
DashboardUID: *r.DashboardUID,
PanelID: *r.PanelID,
Timeout: screenshotTimeout,
}
// To prevent concurrent screenshots of the same dashboard panel we use singleflight,
// deduplicated on a base64 hash of the screenshot options.
optsHash := base64.StdEncoding.EncodeToString(opts.Hash())
logger.Debug("Requesting screenshot") logger.Debug("Requesting screenshot")
result, err, _ := s.singleflight.Do(optsHash, func() (interface{}, error) { result, err, _ := s.singleflight.Do(optsHash, func() (interface{}, error) {
@ -195,7 +196,7 @@ func (s *ScreenshotImageService) NewImage(ctx context.Context, r *models.AlertRu
} }
image := result.(models.Image) image := result.(models.Image)
if err = s.cache.Set(ctx, r.GetKey().String(), image); err != nil { if err = s.cache.Set(ctx, optsHash, image); err != nil {
s.logger.Warn("Failed to cache image", s.logger.Warn("Failed to cache image",
"token", image.Token, "token", image.Token,
"error", err) "error", err)

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

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