fix: maintenance ignores recurrence when fixed times also set

pkg/modules/metricsexplorer/implmetricsexplorer/module.go

@@ -138,14 +138,7 @@ func (m *module) listMeterMetrics(ctx context.Context, params *metricsexplorerty
 
 func (m *module) listMetrics(ctx context.Context, orgID valuer.UUID, params *metricsexplorertypes.ListMetricsParams) (*metricsexplorertypes.ListMetricsResponse, error) {
 	sb := sqlbuilder.NewSelectBuilder()
-	sb.Select(
-		"metric_name",
-		"anyLast(description) AS description",
-		"anyLast(type) AS metric_type",
-		"argMax(unit, unix_milli) AS metric_unit",
-		"anyLast(temporality) AS temporality",
-		"anyLast(is_monotonic) AS is_monotonic",
-	)
+	sb.Select("DISTINCT metric_name")
 
 	if params.Start != nil && params.End != nil {
 		start, end, distributedTsTable, _ := telemetrymetrics.WhichTSTableToUse(uint64(*params.Start), uint64(*params.End), nil)
@@ -164,7 +157,6 @@ func (m *module) listMetrics(ctx context.Context, orgID valuer.UUID, params *met
 		sb.Where(sb.Like("lower(metric_name)", fmt.Sprintf("%%%s%%", searchLower)))
 	}
 
-	sb.GroupBy("metric_name")
 	sb.OrderBy("metric_name ASC")
 	sb.Limit(params.Limit)
 
@@ -178,47 +170,43 @@ func (m *module) listMetrics(ctx context.Context, orgID valuer.UUID, params *met
 	}
 	defer rows.Close()
 
-	var metrics []metricsexplorertypes.ListMetric
-	var metricNames []string
+	metricNames := make([]string, 0)
 	for rows.Next() {
-		var metric metricsexplorertypes.ListMetric
-		if err := rows.Scan(
-			&metric.MetricName,
-			&metric.Description,
-			&metric.MetricType,
-			&metric.MetricUnit,
-			&metric.Temporality,
-			&metric.IsMonotonic,
-		); err != nil {
-			return nil, errors.WrapInternalf(err, errors.CodeInternal, "failed to scan metric")
+		var name string
+		if err := rows.Scan(&name); err != nil {
+			return nil, errors.WrapInternalf(err, errors.CodeInternal, "failed to scan metric name")
 		}
-		metrics = append(metrics, metric)
-		metricNames = append(metricNames, metric.MetricName)
+		metricNames = append(metricNames, name)
 	}
 
 	if err := rows.Err(); err != nil {
-		return nil, errors.WrapInternalf(err, errors.CodeInternal, "error iterating metrics")
+		return nil, errors.WrapInternalf(err, errors.CodeInternal, "error iterating metric names")
 	}
 
-	if len(metrics) == 0 {
+	if len(metricNames) == 0 {
 		return &metricsexplorertypes.ListMetricsResponse{
 			Metrics: []metricsexplorertypes.ListMetric{},
 		}, nil
 	}
 
-	// Overlay any user-updated metadata on top of the timeseries metadata.
-	updatedMetadata, err := m.fetchUpdatedMetadata(ctx, orgID, metricNames)
+	metadata, err := m.GetMetricMetadataMulti(ctx, orgID, metricNames)
 	if err != nil {
 		return nil, err
 	}
-	for i := range metrics {
-		if meta, ok := updatedMetadata[metrics[i].MetricName]; ok && meta != nil {
-			metrics[i].Description = meta.Description
-			metrics[i].MetricType = meta.MetricType
-			metrics[i].MetricUnit = meta.MetricUnit
-			metrics[i].Temporality = meta.Temporality
-			metrics[i].IsMonotonic = meta.IsMonotonic
+
+	metrics := make([]metricsexplorertypes.ListMetric, 0, len(metricNames))
+	for _, name := range metricNames {
+		metric := metricsexplorertypes.ListMetric{
+			MetricName: name,
 		}
+		if meta, ok := metadata[name]; ok && meta != nil {
+			metric.Description = meta.Description
+			metric.MetricType = meta.MetricType
+			metric.MetricUnit = meta.MetricUnit
+			metric.Temporality = meta.Temporality
+			metric.IsMonotonic = meta.IsMonotonic
+		}
+		metrics = append(metrics, metric)
 	}
 
 	return &metricsexplorertypes.ListMetricsResponse{
@@ -255,18 +243,19 @@ func (m *module) GetStats(ctx context.Context, orgID valuer.UUID, req *metricsex
 		}, nil
 	}
 
-	// Overlay any user-updated metadata on top of the timeseries metadata
-	// that was already fetched in the combined query.
+	// Get metadata for all metrics
 	metricNames := make([]string, len(metricStats))
 	for i := range metricStats {
 		metricNames[i] = metricStats[i].MetricName
 	}
 
-	updatedMetadata, err := m.fetchUpdatedMetadata(ctx, orgID, metricNames)
+	metadata, err := m.GetMetricMetadataMulti(ctx, orgID, metricNames)
 	if err != nil {
 		return nil, err
 	}
-	enrichStatsWithMetadata(metricStats, updatedMetadata)
+
+	// Enrich stats with metadata
+	enrichStatsWithMetadata(metricStats, metadata)
 
 	return &metricsexplorertypes.StatsResponse{
 		Metrics: metricStats,
@@ -995,14 +984,11 @@ func (m *module) fetchMetricsStatsWithSamples(
 	samplesTable := telemetrymetrics.WhichSamplesTableToUse(uint64(req.Start), uint64(req.End), metrictypes.UnspecifiedType, metrictypes.TimeAggregationUnspecified, nil)
 	countExp := telemetrymetrics.CountExpressionForSamplesTable(samplesTable)
 
-	// Timeseries counts and metadata per metric.
+	// Timeseries counts per metric
 	tsSB := sqlbuilder.NewSelectBuilder()
 	tsSB.Select(
 		"metric_name",
 		"uniq(fingerprint) AS timeseries",
-		"anyLast(description) AS description",
-		"anyLast(type) AS metric_type",
-		"argMax(unit, unix_milli) AS metric_unit",
 	)
 	tsSB.From(fmt.Sprintf("%s.%s", telemetrymetrics.DBName, distributedTsTable))
 	tsSB.Where(tsSB.Between("unix_milli", start, end))
@@ -1050,9 +1036,6 @@ func (m *module) fetchMetricsStatsWithSamples(
 		"COALESCE(ts.timeseries, 0) AS timeseries",
 		"COALESCE(s.samples, 0) AS samples",
 		"COUNT(*) OVER() AS total",
-		"ts.description AS description",
-		"ts.metric_type AS metric_type",
-		"ts.metric_unit AS metric_unit",
 	)
 	finalSB.From("__time_series_counts ts")
 	finalSB.JoinWithOption(sqlbuilder.FullOuterJoin, "__sample_counts s", "ts.metric_name = s.metric_name")
@@ -1088,7 +1071,7 @@ func (m *module) fetchMetricsStatsWithSamples(
 			metricStat metricsexplorertypes.Stat
 			rowTotal   uint64
 		)
-		if err := rows.Scan(&metricStat.MetricName, &metricStat.TimeSeries, &metricStat.Samples, &rowTotal, &metricStat.Description, &metricStat.MetricType, &metricStat.MetricUnit); err != nil {
+		if err := rows.Scan(&metricStat.MetricName, &metricStat.TimeSeries, &metricStat.Samples, &rowTotal); err != nil {
 			return nil, 0, errors.WrapInternalf(err, errors.CodeInternal, "failed to scan metrics stats row")
 		}
 		metricStats = append(metricStats, metricStat)

pkg/types/ruletypes/maintenance.go

@@ -11,9 +11,7 @@ import (
 	"github.com/uptrace/bun"
 )
 
-var (
-	ErrCodeInvalidPlannedMaintenancePayload = errors.MustNewCode("invalid_planned_maintenance_payload")
-)
+var ErrCodeInvalidPlannedMaintenancePayload = errors.MustNewCode("invalid_planned_maintenance_payload")
 
 type MaintenanceStatus struct {
 	valuer.String
@@ -63,15 +61,15 @@ type StorablePlannedMaintenance struct {
 }
 
 type PlannedMaintenance struct {
-	ID          valuer.UUID `json:"id" required:"true"`
-	Name        string      `json:"name" required:"true"`
-	Description string      `json:"description"`
-	Schedule    *Schedule   `json:"schedule" required:"true"`
-	RuleIDs     []string    `json:"alertIds"`
-	CreatedAt   time.Time   `json:"createdAt"`
-	CreatedBy   string      `json:"createdBy"`
-	UpdatedAt   time.Time   `json:"updatedAt"`
-	UpdatedBy   string      `json:"updatedBy"`
+	ID          valuer.UUID       `json:"id" required:"true"`
+	Name        string            `json:"name" required:"true"`
+	Description string            `json:"description"`
+	Schedule    *Schedule         `json:"schedule" required:"true"`
+	RuleIDs     []string          `json:"alertIds"`
+	CreatedAt   time.Time         `json:"createdAt"`
+	CreatedBy   string            `json:"createdBy"`
+	UpdatedAt   time.Time         `json:"updatedAt"`
+	UpdatedBy   string            `json:"updatedBy"`
 	Status      MaintenanceStatus `json:"status" required:"true"`
 	Kind        MaintenanceKind   `json:"kind" required:"true"`
 }
@@ -161,8 +159,11 @@ func (m *PlannedMaintenance) ShouldSkip(ruleID string, now time.Time) bool {
 
 	currentTime := now.In(loc)
 
-	// fixed schedule
-	if !m.Schedule.StartTime.IsZero() && !m.Schedule.EndTime.IsZero() {
+	// fixed schedule — only when no recurrence is configured.
+	// When recurrence is set, the recurring check below handles everything;
+	// falling through here would cause the window to match the absolute
+	// StartTime–EndTime range instead of the daily/weekly/monthly pattern.
+	if m.Schedule.Recurrence == nil && !m.Schedule.StartTime.IsZero() && !m.Schedule.EndTime.IsZero() {
 		startTime := m.Schedule.StartTime.In(loc)
 		endTime := m.Schedule.EndTime.In(loc)
 		if currentTime.Equal(startTime) || currentTime.Equal(endTime) ||
@@ -333,6 +334,11 @@ func (m *PlannedMaintenance) Validate() error {
 	}
 
 	if m.Schedule.Recurrence != nil {
+		// Fixed range and recurrence are mutually exclusive.
+		if !m.Schedule.StartTime.IsZero() && !m.Schedule.EndTime.IsZero() {
+			return errors.Newf(errors.TypeInvalidInput, ErrCodeInvalidPlannedMaintenancePayload,
+				"cannot set both a fixed time range (startTime/endTime) and a recurrence; use one or the other")
+		}
 		if m.Schedule.Recurrence.RepeatType.IsZero() {
 			return errors.Newf(errors.TypeInvalidInput, ErrCodeInvalidPlannedMaintenancePayload, "missing repeat type in the payload")
 		}

pkg/types/ruletypes/maintenance_test.go

@@ -1,6 +1,7 @@
 package ruletypes
 
 import (
+	"strings"
 	"testing"
 	"time"
 
@@ -13,7 +14,6 @@ func timePtr(t time.Time) *time.Time {
 }
 
 func TestShouldSkipMaintenance(t *testing.T) {
-
 	cases := []struct {
 		name        string
 		maintenance *PlannedMaintenance
@@ -499,7 +499,7 @@ func TestShouldSkipMaintenance(t *testing.T) {
 					},
 				},
 			},
-			ts:   time.Date(2024, 04, 1, 12, 10, 0, 0, time.UTC),
+			ts:   time.Date(2024, 4, 1, 12, 10, 0, 0, time.UTC),
 			skip: true,
 		},
 		{
@@ -508,14 +508,14 @@ func TestShouldSkipMaintenance(t *testing.T) {
 				Schedule: &Schedule{
 					Timezone: "UTC",
 					Recurrence: &Recurrence{
-						StartTime:  time.Date(2024, 04, 01, 12, 0, 0, 0, time.UTC),
+						StartTime:  time.Date(2024, 4, 1, 12, 0, 0, 0, time.UTC),
 						Duration:   valuer.MustParseTextDuration("2h"),
 						RepeatType: RepeatTypeWeekly,
 						RepeatOn:   []RepeatOn{RepeatOnMonday},
 					},
 				},
 			},
-			ts:   time.Date(2024, 04, 15, 12, 10, 0, 0, time.UTC),
+			ts:   time.Date(2024, 4, 15, 12, 10, 0, 0, time.UTC),
 			skip: true,
 		},
 		{
@@ -524,14 +524,14 @@ func TestShouldSkipMaintenance(t *testing.T) {
 				Schedule: &Schedule{
 					Timezone: "UTC",
 					Recurrence: &Recurrence{
-						StartTime:  time.Date(2024, 04, 01, 12, 0, 0, 0, time.UTC),
+						StartTime:  time.Date(2024, 4, 1, 12, 0, 0, 0, time.UTC),
 						Duration:   valuer.MustParseTextDuration("2h"),
 						RepeatType: RepeatTypeWeekly,
 						RepeatOn:   []RepeatOn{RepeatOnMonday},
 					},
 				},
 			},
-			ts:   time.Date(2024, 04, 14, 12, 10, 0, 0, time.UTC), // 14th 04 is sunday
+			ts:   time.Date(2024, 4, 14, 12, 10, 0, 0, time.UTC), // 14th 04 is sunday
 			skip: false,
 		},
 		{
@@ -540,14 +540,14 @@ func TestShouldSkipMaintenance(t *testing.T) {
 				Schedule: &Schedule{
 					Timezone: "UTC",
 					Recurrence: &Recurrence{
-						StartTime:  time.Date(2024, 04, 01, 12, 0, 0, 0, time.UTC),
+						StartTime:  time.Date(2024, 4, 1, 12, 0, 0, 0, time.UTC),
 						Duration:   valuer.MustParseTextDuration("2h"),
 						RepeatType: RepeatTypeWeekly,
 						RepeatOn:   []RepeatOn{RepeatOnMonday},
 					},
 				},
 			},
-			ts:   time.Date(2024, 04, 16, 12, 10, 0, 0, time.UTC), // 16th 04 is tuesday
+			ts:   time.Date(2024, 4, 16, 12, 10, 0, 0, time.UTC), // 16th 04 is tuesday
 			skip: false,
 		},
 		{
@@ -556,14 +556,14 @@ func TestShouldSkipMaintenance(t *testing.T) {
 				Schedule: &Schedule{
 					Timezone: "UTC",
 					Recurrence: &Recurrence{
-						StartTime:  time.Date(2024, 04, 01, 12, 0, 0, 0, time.UTC),
+						StartTime:  time.Date(2024, 4, 1, 12, 0, 0, 0, time.UTC),
 						Duration:   valuer.MustParseTextDuration("2h"),
 						RepeatType: RepeatTypeWeekly,
 						RepeatOn:   []RepeatOn{RepeatOnMonday},
 					},
 				},
 			},
-			ts:   time.Date(2024, 05, 06, 12, 10, 0, 0, time.UTC),
+			ts:   time.Date(2024, 5, 6, 12, 10, 0, 0, time.UTC),
 			skip: true,
 		},
 		{
@@ -572,14 +572,14 @@ func TestShouldSkipMaintenance(t *testing.T) {
 				Schedule: &Schedule{
 					Timezone: "UTC",
 					Recurrence: &Recurrence{
-						StartTime:  time.Date(2024, 04, 01, 12, 0, 0, 0, time.UTC),
+						StartTime:  time.Date(2024, 4, 1, 12, 0, 0, 0, time.UTC),
 						Duration:   valuer.MustParseTextDuration("2h"),
 						RepeatType: RepeatTypeWeekly,
 						RepeatOn:   []RepeatOn{RepeatOnMonday},
 					},
 				},
 			},
-			ts:   time.Date(2024, 05, 06, 14, 00, 0, 0, time.UTC),
+			ts:   time.Date(2024, 5, 6, 14, 0, 0, 0, time.UTC),
 			skip: true,
 		},
 		{
@@ -588,13 +588,13 @@ func TestShouldSkipMaintenance(t *testing.T) {
 				Schedule: &Schedule{
 					Timezone: "UTC",
 					Recurrence: &Recurrence{
-						StartTime:  time.Date(2024, 04, 04, 12, 0, 0, 0, time.UTC),
+						StartTime:  time.Date(2024, 4, 4, 12, 0, 0, 0, time.UTC),
 						Duration:   valuer.MustParseTextDuration("2h"),
 						RepeatType: RepeatTypeMonthly,
 					},
 				},
 			},
-			ts:   time.Date(2024, 04, 04, 12, 10, 0, 0, time.UTC),
+			ts:   time.Date(2024, 4, 4, 12, 10, 0, 0, time.UTC),
 			skip: true,
 		},
 		{
@@ -603,13 +603,13 @@ func TestShouldSkipMaintenance(t *testing.T) {
 				Schedule: &Schedule{
 					Timezone: "UTC",
 					Recurrence: &Recurrence{
-						StartTime:  time.Date(2024, 04, 04, 12, 0, 0, 0, time.UTC),
+						StartTime:  time.Date(2024, 4, 4, 12, 0, 0, 0, time.UTC),
 						Duration:   valuer.MustParseTextDuration("2h"),
 						RepeatType: RepeatTypeMonthly,
 					},
 				},
 			},
-			ts:   time.Date(2024, 04, 04, 14, 10, 0, 0, time.UTC),
+			ts:   time.Date(2024, 4, 4, 14, 10, 0, 0, time.UTC),
 			skip: false,
 		},
 		{
@@ -618,13 +618,53 @@ func TestShouldSkipMaintenance(t *testing.T) {
 				Schedule: &Schedule{
 					Timezone: "UTC",
 					Recurrence: &Recurrence{
-						StartTime:  time.Date(2024, 04, 04, 12, 0, 0, 0, time.UTC),
+						StartTime:  time.Date(2024, 4, 4, 12, 0, 0, 0, time.UTC),
 						Duration:   valuer.MustParseTextDuration("2h"),
 						RepeatType: RepeatTypeMonthly,
 					},
 				},
 			},
-			ts:   time.Date(2024, 05, 04, 12, 10, 0, 0, time.UTC),
+			ts:   time.Date(2024, 5, 4, 12, 10, 0, 0, time.UTC),
+			skip: true,
+		},
+		// #7548: recurring daily with Schedule.StartTime/EndTime also set.
+		// The recurrence should govern — not the fixed range.
+		{
+			name: "recurring-daily-with-fixed-times-outside-daily-window",
+			maintenance: &PlannedMaintenance{
+				Schedule: &Schedule{
+					Timezone: "UTC",
+					// These fixed fields should be ignored when Recurrence is set.
+					StartTime: time.Date(2024, 6, 1, 10, 0, 0, 0, time.UTC),
+					EndTime:   time.Date(2024, 6, 30, 18, 0, 0, 0, time.UTC),
+					Recurrence: &Recurrence{
+						StartTime:  time.Date(2024, 6, 1, 14, 0, 0, 0, time.UTC), // daily at 14:00
+						Duration:   valuer.MustParseTextDuration("2h"),           // until 16:00
+						RepeatType: RepeatTypeDaily,
+					},
+				},
+			},
+			// 11:00 is inside the fixed range but outside the daily 14:00-16:00 window.
+			// Before the fix this returned true (bug); after fix it returns false.
+			ts:   time.Date(2024, 6, 15, 11, 0, 0, 0, time.UTC),
+			skip: false,
+		},
+		{
+			name: "recurring-daily-with-fixed-times-inside-daily-window",
+			maintenance: &PlannedMaintenance{
+				Schedule: &Schedule{
+					Timezone:  "UTC",
+					StartTime: time.Date(2024, 6, 1, 10, 0, 0, 0, time.UTC),
+					EndTime:   time.Date(2024, 6, 30, 18, 0, 0, 0, time.UTC),
+					Recurrence: &Recurrence{
+						StartTime:  time.Date(2024, 6, 1, 14, 0, 0, 0, time.UTC),
+						Duration:   valuer.MustParseTextDuration("2h"),
+						RepeatType: RepeatTypeDaily,
+					},
+				},
+			},
+			// 15:00 is inside the daily 14:00-16:00 window — should skip.
+			ts:   time.Date(2024, 6, 15, 15, 0, 0, 0, time.UTC),
 			skip: true,
 		},
 	}
@@ -636,3 +676,27 @@ func TestShouldSkipMaintenance(t *testing.T) {
 		}
 	}
 }
+
+// #7548: Validate rejects payloads that set both fixed range and recurrence.
+func TestValidate_FixedAndRecurrenceMutuallyExclusive(t *testing.T) {
+	m := PlannedMaintenance{
+		Name: "test",
+		Schedule: &Schedule{
+			Timezone:  "UTC",
+			StartTime: time.Now().Add(-time.Hour),
+			EndTime:   time.Now().Add(time.Hour),
+			Recurrence: &Recurrence{
+				StartTime:  time.Now().Add(-time.Hour),
+				Duration:   valuer.MustParseTextDuration("2h"),
+				RepeatType: RepeatTypeDaily,
+			},
+		},
+	}
+	err := m.Validate()
+	if err == nil {
+		t.Fatal("expected validation error when both fixed range and recurrence are set")
+	}
+	if !strings.Contains(err.Error(), "cannot set both") {
+		t.Errorf("unexpected error message: %v", err)
+	}
+}