From c5ae1bcfe06449fafe6cc4abb8d4a4c17a8c94e4 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Thu, 10 Nov 2022 09:58:38 +0000 Subject: [PATCH] Alerting: Fix logging pointer address of DashboardUID and PanelID variables (#58539) --- pkg/services/ngalert/image/service.go | 14 ++++++++------ pkg/services/ngalert/models/alert_rule.go | 16 ++++++++++++++++ pkg/services/ngalert/state/manager.go | 8 ++++---- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/pkg/services/ngalert/image/service.go b/pkg/services/ngalert/image/service.go index 051019def28..385375537cd 100644 --- a/pkg/services/ngalert/image/service.go +++ b/pkg/services/ngalert/image/service.go @@ -115,24 +115,26 @@ func NewScreenshotImageServiceFromCfg(cfg *setting.Cfg, db *store.DBstore, ds da func (s *ScreenshotImageService) NewImage(ctx context.Context, r *models.AlertRule) (*models.Image, error) { logger := s.logger.FromContext(ctx) - if r.DashboardUID == nil || *r.DashboardUID == "" { + dashboardUID := r.GetDashboardUID() + if dashboardUID == "" { logger.Debug("Cannot take screenshot for alert rule as it is not associated with a dashboard") return nil, models.ErrNoDashboard } - if r.PanelID == nil || *r.PanelID == 0 { + panelID := r.GetPanelID() + if panelID <= 0 { logger.Debug("Cannot take screenshot for alert rule as it is not associated with a panel") return nil, models.ErrNoPanel } + logger = logger.New("dashboard", dashboardUID, "panel", panelID) + opts := screenshot.ScreenshotOptions{ - DashboardUID: *r.DashboardUID, - PanelID: *r.PanelID, + DashboardUID: dashboardUID, + PanelID: panelID, Timeout: screenshotTimeout, } - logger = logger.New("dashboard", opts.DashboardUID, "panel", opts.PanelID) - // 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()) diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index ee13175dfb7..c2573b57bd3 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -166,6 +166,22 @@ type AlertRule struct { Labels map[string]string } +// GetDashboardUID returns the DashboardUID or "". +func (alertRule *AlertRule) GetDashboardUID() string { + if alertRule.DashboardUID != nil { + return *alertRule.DashboardUID + } + return "" +} + +// GetPanelID returns the Panel ID or -1. +func (alertRule *AlertRule) GetPanelID() int64 { + if alertRule.PanelID != nil { + return *alertRule.PanelID + } + return -1 +} + type LabelOption func(map[string]string) func WithoutInternalLabels() LabelOption { diff --git a/pkg/services/ngalert/state/manager.go b/pkg/services/ngalert/state/manager.go index f8087b590be..912fc05f314 100644 --- a/pkg/services/ngalert/state/manager.go +++ b/pkg/services/ngalert/state/manager.go @@ -246,8 +246,8 @@ func (st *Manager) setNextState(ctx context.Context, alertRule *ngModels.AlertRu image, err := takeImage(ctx, st.images, alertRule) if err != nil { logger.Warn("Failed to take an image", - "dashboard", alertRule.DashboardUID, - "panel", alertRule.PanelID, + "dashboard", alertRule.GetDashboardUID(), + "panel", alertRule.GetPanelID(), "error", err) } else if image != nil { currentState.Image = image @@ -373,8 +373,8 @@ func (st *Manager) staleResultsHandler(ctx context.Context, logger log.Logger, r image, err := takeImage(ctx, st.images, r) if err != nil { logger.Warn("Failed to take an image", - "dashboard", r.DashboardUID, - "panel", r.PanelID, + "dashboard", r.GetDashboardUID(), + "panel", r.GetPanelID(), "error", err) } else if image != nil { resolvedImage = image