Alerting: Parse App URL only once (#39855)

pkg/services/ngalert/ngalert.go

@@ -2,6 +2,7 @@ package ngalert
 
 import (
 	"context"
+	"net/url"
 	"time"
 
 	"github.com/grafana/grafana/pkg/api/routing"
@@ -125,11 +126,17 @@ func (ng *AlertNG) init() error {
 		DisabledOrgs:            ng.Cfg.UnifiedAlerting.DisabledOrgs,
 		MinRuleInterval:         ng.getRuleMinInterval(),
 	}
+
+	appUrl, err := url.Parse(ng.Cfg.AppURL)
+	if err != nil {
+		ng.Log.Error("Failed to parse application URL. Continue without it.", "error", err)
+		appUrl = nil
+	}
 	stateManager := state.NewManager(ng.Log, ng.Metrics.GetStateMetrics(), store, store)
-	schedule := schedule.NewScheduler(schedCfg, ng.DataService, ng.Cfg.AppURL, stateManager)
+	scheduler := schedule.NewScheduler(schedCfg, ng.DataService, appUrl, stateManager)
 
 	ng.stateManager = stateManager
-	ng.schedule = schedule
+	ng.schedule = scheduler
 
 	api := api.API{
 		Cfg:                  ng.Cfg,

pkg/services/ngalert/schedule/compat.go

@@ -11,22 +11,15 @@ import (
 	apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
 	"github.com/prometheus/alertmanager/api/v2/models"
 
-	"github.com/grafana/grafana/pkg/infra/log"
 	ngModels "github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/state"
 )
 
-func FromAlertStateToPostableAlerts(logger log.Logger, firingStates []*state.State, stateManager *state.Manager, appURL string) apimodels.PostableAlerts {
+func FromAlertStateToPostableAlerts(firingStates []*state.State, stateManager *state.Manager, appURL *url.URL) apimodels.PostableAlerts {
 	alerts := apimodels.PostableAlerts{PostableAlerts: make([]models.PostableAlert, 0, len(firingStates))}
 	var sentAlerts []*state.State
 	ts := time.Now()
 
-	u, err := url.Parse(appURL)
-	if err != nil {
-		logger.Debug("failed to parse URL while joining URL", "url", appURL, "err", err.Error())
-		u = nil
-	}
-
 	for _, alertState := range firingStates {
 		if !alertState.NeedsSending(stateManager.ResendDelay) {
 			continue
@@ -38,12 +31,15 @@ func FromAlertStateToPostableAlerts(logger log.Logger, firingStates []*state.Sta
 			nA["__value_string__"] = alertState.Results[0].EvaluationString
 		}
 
-		genURL := appURL
+		var urlStr string
-		if uid := nL[ngModels.RuleUIDLabel]; len(uid) > 0 && u != nil {
+		if uid := nL[ngModels.RuleUIDLabel]; len(uid) > 0 && appURL != nil {
-			oldPath := u.Path
+			u := *appURL
 			u.Path = path.Join(u.Path, fmt.Sprintf("/alerting/%s/edit", uid))
-			genURL = u.String()
+			urlStr = u.String()
-			u.Path = oldPath
+		} else if appURL != nil {
+			urlStr = appURL.String()
+		} else {
+			urlStr = ""
 		}
 
 		alerts.PostableAlerts = append(alerts.PostableAlerts, models.PostableAlert{
@@ -52,7 +48,7 @@ func FromAlertStateToPostableAlerts(logger log.Logger, firingStates []*state.Sta
 			EndsAt:      strfmt.DateTime(alertState.EndsAt),
 			Alert: models.Alert{
 				Labels:       models.LabelSet(nL),
-				GeneratorURL: strfmt.URI(genURL),
+				GeneratorURL: strfmt.URI(urlStr),
 			},
 		})
 		alertState.LastSentAt = ts

pkg/services/ngalert/schedule/schedule.go

@@ -74,7 +74,7 @@ type schedule struct {
 
 	stateManager *state.Manager
 
-	appURL string
+	appURL *url.URL
 
 	multiOrgNotifier *notifier.MultiOrgAlertmanager
 	metrics          *metrics.Scheduler
@@ -109,8 +109,9 @@ type SchedulerCfg struct {
 }
 
 // NewScheduler returns a new schedule.
-func NewScheduler(cfg SchedulerCfg, dataService *tsdb.Service, appURL string, stateManager *state.Manager) *schedule {
+func NewScheduler(cfg SchedulerCfg, dataService *tsdb.Service, appURL *url.URL, stateManager *state.Manager) *schedule {
 	ticker := alerting.NewTicker(cfg.C.Now(), time.Second*0, cfg.C, int64(cfg.BaseInterval.Seconds()))
+
 	sch := schedule{
 
 		registry:                alertRuleRegistry{alertRuleInfo: make(map[models.AlertRuleKey]alertRuleInfo)},
@@ -475,7 +476,7 @@ func (sch *schedule) ruleRoutine(grafanaCtx context.Context, key models.AlertRul
 
 				processedStates := sch.stateManager.ProcessEvalResults(alertRule, results)
 				sch.saveAlertStates(processedStates)
-				alerts := FromAlertStateToPostableAlerts(sch.log, processedStates, sch.stateManager, sch.appURL)
+				alerts := FromAlertStateToPostableAlerts(processedStates, sch.stateManager, sch.appURL)
 
 				sch.log.Debug("sending alerts to notifier", "count", len(alerts.PostableAlerts), "alerts", alerts.PostableAlerts, "org", alertRule.OrgID)
 				n, err := sch.multiOrgNotifier.AlertmanagerFor(alertRule.OrgID)

pkg/services/ngalert/schedule/schedule_test.go

@@ -3,6 +3,7 @@ package schedule_test
 import (
 	"context"
 	"fmt"
+	"net/url"
 	"runtime"
 	"strings"
 	"testing"
@@ -155,7 +156,11 @@ func TestAlertingTicker(t *testing.T) {
 		},
 	}
 	st := state.NewManager(schedCfg.Logger, testMetrics.GetStateMetrics(), dbstore, dbstore)
-	sched := schedule.NewScheduler(schedCfg, nil, "http://localhost", st)
+	appUrl := &url.URL{
+		Scheme: "http",
+		Host:   "localhost",
+	}
+	sched := schedule.NewScheduler(schedCfg, nil, appUrl, st)
 
 	ctx := context.Background()
 

pkg/services/ngalert/schedule/schedule_unit_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"math/rand"
+	"net/url"
 	"testing"
 	"time"
 
@@ -247,7 +248,11 @@ func setupScheduler(t *testing.T, rs store.RuleStore, is store.InstanceStore, ac
 		AdminConfigPollInterval: 10 * time.Minute, // do not poll in unit tests.
 	}
 	st := state.NewManager(schedCfg.Logger, m.GetStateMetrics(), rs, is)
-	return NewScheduler(schedCfg, nil, "http://localhost", st), mockedClock
+	appUrl := &url.URL{
+		Scheme: "http",
+		Host:   "localhost",
+	}
+	return NewScheduler(schedCfg, nil, appUrl, st), mockedClock
 }
 
 // createTestAlertRule creates a dummy alert definition to be used by the tests.