Alerting: In migration, fallback to '1s' for malformed min interval

[JacobsonMT]

What is this feature?

During legacy migration, when we encounter an alert datasource query with a min interval (interval field in the query model) that is not parseable, instead of failing the migration we fallback to a min interval of 1s and continue.

Note that this doesn't mean an intervalMs of 1s will be used, the intervalMs (actual interval of the query) will still be calculated based on ~max((to-from)/maxDataPoints, minInterval) where maxDataPoints=1500.

Why do we need this feature?

There is a bug in legacy alerting (existing for a few major versions) which allows arbitrary dashboard variables to be used as the min interval, even though those variables do not work and will cause the legacy alert to fail with interval calculation failed: time: invalid duration.

Legacy alerts with these broken intervals are likely rare, so failing the migration is not unreasonable. However, since legacy alerts with these variables used to be functional at some point, it seems low risk and an altogether benefit to fix them during migration.

Who is this feature for?

Users migrating from legacy alerting to Grafana Alerting.

Which issue(s) does this PR fix?:

Fixes #78606

Special notes for your reviewer:

Little bit of "boy scout rule" here as I chose to fix datasource not being passed to calculateInterval in adjacent code. It should have little to no functional effect, but is technically correct based on how legacy alerting works:

grafana/pkg/services/alerting/conditions/query.go

Line 261 in 025b2f3

calculatedInterval, err := calculateInterval(timeRange, queryModel, datasource)

[JohnnyQQQQ]

Is there a scenario where interval and intervalMs could be already set? interval being a variable and intervalMs being a valid duration?

[JacobsonMT]

From what I've seen in my tests, and if I can believe the comment above GetIntervalFrom():

grafana/pkg/services/ngalert/migration/cond_trans.go

Lines 343 to 347 in 82f3127

	// interval.GetIntervalFrom has two problems (but they do not affect us here):
	// - it returns the min-interval, so it should be called interval.GetMinIntervalFrom
	// - it falls back to model.intervalMs. it should not, because that one is the real final
	// interval-value calculated by the browser. but, in this specific case (old-alert),
	// that value is not set, so the fallback never happens.

Then intervalMs shouldn't be set in legacy alerting. That being said, I did go back and forth a bit on whether I should set the interval to an empty string instead of 1s.

[JohnnyQQQQ]

LGTM, one small question.