Alerting: Ensure long-lived repeat alerts keep images after 24h expiry (#98993)

Ensures we retake images after expiration on long-lived repeat alerts.
Otherwise, logs would show "Image not found in database" and notifications
would cease to contain an image after 24h of continuous firing.

pkg/services/ngalert/notifier/images.go

@@ -11,6 +11,7 @@ import (
 	alertingImages "github.com/grafana/alerting/images"
 	alertingModels "github.com/grafana/alerting/models"
 	alertingNotify "github.com/grafana/alerting/notify"
+
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/store"
@@ -32,7 +33,7 @@ func (i imageProvider) GetImage(ctx context.Context, uri string) (*alertingImage
 	image, err := i.getImageFromURI(ctx, uri)
 	if err != nil {
 		if errors.Is(err, models.ErrImageNotFound) {
-			i.logger.Info("Image not found in database")
+			i.logger.Info("Image not found in database", "uri", uri)
 			return nil, alertingImages.ErrImageNotFound
 		}
 		return nil, err

pkg/services/ngalert/state/state.go

@@ -575,13 +575,12 @@ func (a *State) IsStale() bool {
 	return a.StateReason == models.StateReasonMissingSeries
 }
 
-// shouldTakeImage returns true if the state just has transitioned to alerting from another state,
-// transitioned to alerting in a previous evaluation but does not have a screenshot, or has just
-// been resolved.
+// shouldTakeImage determines whether a new image should be taken for a given transition. This should return true when
+// newly transitioning to an alerting state, when no valid image exists, or when the alert has been resolved.
 func shouldTakeImage(state, previousState eval.State, previousImage *models.Image, resolved bool) bool {
 	return resolved ||
 		state == eval.Alerting && previousState != eval.Alerting ||
-		state == eval.Alerting && previousImage == nil
+		state == eval.Alerting && (previousImage == nil || previousImage.HasExpired())
 }
 
 // takeImage takes an image for the alert rule. It returns nil if screenshots are disabled or

pkg/services/ngalert/state/state_test.go

@@ -602,10 +602,16 @@ func TestShouldTakeImage(t *testing.T) {
 		state:         eval.Pending,
 		previousState: eval.Normal,
 	}, {
-		name:          "should not take image for alerting state with image",
+		name:          "should not take image for alerting state with valid image",
 		state:         eval.Alerting,
 		previousState: eval.Alerting,
-		previousImage: &ngmodels.Image{URL: "https://example.com/foo.png"},
+		previousImage: &ngmodels.Image{URL: "https://example.com/foo.png", ExpiresAt: time.Now().Add(time.Hour)},
+	}, {
+		name:          "should take image for alerting state with expired image",
+		state:         eval.Alerting,
+		previousState: eval.Alerting,
+		previousImage: &ngmodels.Image{URL: "https://example.com/foo.png", ExpiresAt: time.Now().Add(-time.Hour)},
+		expected:      true,
 	}}
 
 	for _, test := range tests {