diff --git a/changelogs/unreleased/9414-shubham-pampattiwar b/changelogs/unreleased/9414-shubham-pampattiwar new file mode 100644 index 000000000..bf7ccbfec --- /dev/null +++ b/changelogs/unreleased/9414-shubham-pampattiwar @@ -0,0 +1 @@ +Add Prometheus metrics for maintenance jobs \ No newline at end of file diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index 15c4afd95..b5af9a65b 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -756,6 +756,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string s.config.RepoMaintenanceJobConfig, s.logLevel, s.config.LogFormat, + s.metrics, ).SetupWithManager(s.mgr); err != nil { s.logger.Fatal(err, "unable to create controller", "controller", constant.ControllerBackupRepo) } diff --git a/pkg/controller/backup_repository_controller.go b/pkg/controller/backup_repository_controller.go index d2d32d745..eb622908e 100644 --- a/pkg/controller/backup_repository_controller.go +++ b/pkg/controller/backup_repository_controller.go @@ -42,6 +42,7 @@ import ( velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/constant" "github.com/vmware-tanzu/velero/pkg/label" + "github.com/vmware-tanzu/velero/pkg/metrics" repoconfig "github.com/vmware-tanzu/velero/pkg/repository/config" "github.com/vmware-tanzu/velero/pkg/repository/maintenance" repomanager "github.com/vmware-tanzu/velero/pkg/repository/manager" @@ -66,6 +67,7 @@ type BackupRepoReconciler struct { repoMaintenanceConfig string logLevel logrus.Level logFormat *logging.FormatFlag + metrics *metrics.ServerMetrics } func NewBackupRepoReconciler( @@ -78,6 +80,7 @@ func NewBackupRepoReconciler( repoMaintenanceConfig string, logLevel logrus.Level, logFormat *logging.FormatFlag, + metrics *metrics.ServerMetrics, ) *BackupRepoReconciler { c := &BackupRepoReconciler{ client, @@ -90,6 +93,7 @@ func NewBackupRepoReconciler( repoMaintenanceConfig, logLevel, logFormat, + metrics, } return c @@ -491,6 +495,12 @@ func (r *BackupRepoReconciler) runMaintenanceIfDue(ctx context.Context, req *vel job, err := funcStartMaintenanceJob(r.Client, ctx, req, r.repoMaintenanceConfig, r.logLevel, r.logFormat, log) if err != nil { log.WithError(err).Warn("Starting repo maintenance failed") + + // Record failure metric when job fails to start + if r.metrics != nil { + r.metrics.RegisterMaintenanceJobFailure(req.Name) + } + return r.patchBackupRepository(ctx, req, func(rr *velerov1api.BackupRepository) { updateRepoMaintenanceHistory(rr, velerov1api.BackupRepositoryMaintenanceFailed, &metav1.Time{Time: startTime}, nil, fmt.Sprintf("Failed to start maintenance job, err: %v", err)) }) @@ -505,11 +515,30 @@ func (r *BackupRepoReconciler) runMaintenanceIfDue(ctx context.Context, req *vel if status.Result == velerov1api.BackupRepositoryMaintenanceFailed { log.WithError(err).Warn("Pruning repository failed") + + // Record failure metric + if r.metrics != nil { + r.metrics.RegisterMaintenanceJobFailure(req.Name) + if status.StartTimestamp != nil && status.CompleteTimestamp != nil { + duration := status.CompleteTimestamp.Sub(status.StartTimestamp.Time).Seconds() + r.metrics.ObserveMaintenanceJobDuration(req.Name, duration) + } + } + return r.patchBackupRepository(ctx, req, func(rr *velerov1api.BackupRepository) { updateRepoMaintenanceHistory(rr, velerov1api.BackupRepositoryMaintenanceFailed, status.StartTimestamp, status.CompleteTimestamp, status.Message) }) } + // Record success metric + if r.metrics != nil { + r.metrics.RegisterMaintenanceJobSuccess(req.Name) + if status.StartTimestamp != nil && status.CompleteTimestamp != nil { + duration := status.CompleteTimestamp.Sub(status.StartTimestamp.Time).Seconds() + r.metrics.ObserveMaintenanceJobDuration(req.Name, duration) + } + } + return r.patchBackupRepository(ctx, req, func(rr *velerov1api.BackupRepository) { rr.Status.LastMaintenanceTime = &metav1.Time{Time: status.CompleteTimestamp.Time} updateRepoMaintenanceHistory(rr, velerov1api.BackupRepositoryMaintenanceSucceeded, status.StartTimestamp, status.CompleteTimestamp, status.Message) diff --git a/pkg/controller/backup_repository_controller_test.go b/pkg/controller/backup_repository_controller_test.go index 0a4a04610..1fc1e9199 100644 --- a/pkg/controller/backup_repository_controller_test.go +++ b/pkg/controller/backup_repository_controller_test.go @@ -19,6 +19,8 @@ import ( "testing" "time" + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -32,6 +34,7 @@ import ( velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/builder" + "github.com/vmware-tanzu/velero/pkg/metrics" "github.com/vmware-tanzu/velero/pkg/repository" "github.com/vmware-tanzu/velero/pkg/repository/maintenance" repomaintenance "github.com/vmware-tanzu/velero/pkg/repository/maintenance" @@ -65,6 +68,7 @@ func mockBackupRepoReconciler(t *testing.T, mockOn string, arg any, ret ...any) "", logrus.InfoLevel, nil, + nil, ) } @@ -584,6 +588,7 @@ func TestGetRepositoryMaintenanceFrequency(t *testing.T) { "", logrus.InfoLevel, nil, + nil, ) freq := reconciler.getRepositoryMaintenanceFrequency(test.repo) @@ -716,6 +721,7 @@ func TestNeedInvalidBackupRepo(t *testing.T) { "", logrus.InfoLevel, nil, + nil, ) need := reconciler.needInvalidBackupRepo(test.oldBSL, test.newBSL) @@ -1581,6 +1587,7 @@ func TestDeleteOldMaintenanceJobWithConfigMap(t *testing.T) { repoMaintenanceConfigName, logrus.InfoLevel, nil, + nil, ) _, err := reconciler.Reconcile(t.Context(), ctrl.Request{NamespacedName: types.NamespacedName{Namespace: test.repo.Namespace, Name: "repo"}}) @@ -1638,6 +1645,7 @@ func TestInitializeRepoWithRepositoryTypes(t *testing.T) { "", logrus.InfoLevel, nil, + nil, ) err := reconciler.initializeRepo(t.Context(), rr, location, reconciler.logger) @@ -1689,6 +1697,7 @@ func TestInitializeRepoWithRepositoryTypes(t *testing.T) { "", logrus.InfoLevel, nil, + nil, ) err := reconciler.initializeRepo(t.Context(), rr, location, reconciler.logger) @@ -1739,6 +1748,7 @@ func TestInitializeRepoWithRepositoryTypes(t *testing.T) { "", logrus.InfoLevel, nil, + nil, ) err := reconciler.initializeRepo(t.Context(), rr, location, reconciler.logger) @@ -1750,3 +1760,189 @@ func TestInitializeRepoWithRepositoryTypes(t *testing.T) { assert.Equal(t, velerov1api.BackupRepositoryPhaseReady, rr.Status.Phase) }) } + +func TestMaintenanceJobMetricsRecording(t *testing.T) { + now := time.Now().Round(time.Second) + + tests := []struct { + name string + repo *velerov1api.BackupRepository + startJobFunc func(client.Client, context.Context, *velerov1api.BackupRepository, string, logrus.Level, *logging.FormatFlag, logrus.FieldLogger) (string, error) + waitJobFunc func(client.Client, context.Context, string, string, logrus.FieldLogger) (velerov1api.BackupRepositoryMaintenanceStatus, error) + expectSuccess bool + expectFailure bool + expectDuration bool + }{ + { + name: "metrics recorded on successful maintenance", + repo: &velerov1api.BackupRepository{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: velerov1api.DefaultNamespace, + Name: "test-repo-success", + }, + Spec: velerov1api.BackupRepositorySpec{ + MaintenanceFrequency: metav1.Duration{Duration: time.Hour}, + }, + Status: velerov1api.BackupRepositoryStatus{ + LastMaintenanceTime: &metav1.Time{Time: now.Add(-2 * time.Hour)}, + }, + }, + startJobFunc: startMaintenanceJobSucceed, + waitJobFunc: waitMaintenanceJobCompleteFunc(now, velerov1api.BackupRepositoryMaintenanceSucceeded, ""), + expectSuccess: true, + expectFailure: false, + expectDuration: true, + }, + { + name: "metrics recorded on failed maintenance", + repo: &velerov1api.BackupRepository{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: velerov1api.DefaultNamespace, + Name: "test-repo-failure", + }, + Spec: velerov1api.BackupRepositorySpec{ + MaintenanceFrequency: metav1.Duration{Duration: time.Hour}, + }, + Status: velerov1api.BackupRepositoryStatus{ + LastMaintenanceTime: &metav1.Time{Time: now.Add(-2 * time.Hour)}, + }, + }, + startJobFunc: startMaintenanceJobSucceed, + waitJobFunc: func(client.Client, context.Context, string, string, logrus.FieldLogger) (velerov1api.BackupRepositoryMaintenanceStatus, error) { + return velerov1api.BackupRepositoryMaintenanceStatus{ + StartTimestamp: &metav1.Time{Time: now}, + CompleteTimestamp: &metav1.Time{Time: now.Add(time.Minute)}, // Job ran for 1 minute then failed + Result: velerov1api.BackupRepositoryMaintenanceFailed, + Message: "test error", + }, nil + }, + expectSuccess: false, + expectFailure: true, + expectDuration: true, + }, + { + name: "metrics recorded on job start failure", + repo: &velerov1api.BackupRepository{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: velerov1api.DefaultNamespace, + Name: "test-repo-start-fail", + }, + Spec: velerov1api.BackupRepositorySpec{ + MaintenanceFrequency: metav1.Duration{Duration: time.Hour}, + }, + Status: velerov1api.BackupRepositoryStatus{ + LastMaintenanceTime: &metav1.Time{Time: now.Add(-2 * time.Hour)}, + }, + }, + startJobFunc: startMaintenanceJobFail, + expectSuccess: false, + expectFailure: true, + expectDuration: false, // No duration when job fails to start + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // Create metrics instance + m := metrics.NewServerMetrics() + + // Create reconciler with metrics + reconciler := mockBackupRepoReconciler(t, "", test.repo, nil) + reconciler.metrics = m + reconciler.clock = &fakeClock{now} + + err := reconciler.Client.Create(t.Context(), test.repo) + require.NoError(t, err) + + // Set up job functions + funcStartMaintenanceJob = test.startJobFunc + funcWaitMaintenanceJobComplete = test.waitJobFunc + + // Run maintenance + _ = reconciler.runMaintenanceIfDue(t.Context(), test.repo, velerotest.NewLogger()) + + // Verify metrics were recorded + successCount := getMaintenanceMetricValue(t, m, "maintenance_job_success_total", test.repo.Name) + failureCount := getMaintenanceMetricValue(t, m, "maintenance_job_failure_total", test.repo.Name) + durationCount := getMaintenanceDurationCount(t, m, test.repo.Name) + + if test.expectSuccess { + assert.Equal(t, float64(1), successCount, "Success metric should be recorded") + } else { + assert.Equal(t, float64(0), successCount, "Success metric should not be recorded") + } + + if test.expectFailure { + assert.Equal(t, float64(1), failureCount, "Failure metric should be recorded") + } else { + assert.Equal(t, float64(0), failureCount, "Failure metric should not be recorded") + } + + if test.expectDuration { + assert.Equal(t, uint64(1), durationCount, "Duration metric should be recorded") + } else { + assert.Equal(t, uint64(0), durationCount, "Duration metric should not be recorded") + } + }) + } +} + +// Helper to get maintenance metric value from ServerMetrics +func getMaintenanceMetricValue(t *testing.T, m *metrics.ServerMetrics, metricName, repoName string) float64 { + t.Helper() + + metricMap := m.Metrics() + collector, ok := metricMap[metricName] + if !ok { + return 0 + } + + ch := make(chan prometheus.Metric, 1) + collector.Collect(ch) + close(ch) + + for metric := range ch { + dto := &dto.Metric{} + err := metric.Write(dto) + require.NoError(t, err) + + for _, label := range dto.Label { + if *label.Name == "repository_name" && *label.Value == repoName { + if dto.Counter != nil { + return *dto.Counter.Value + } + } + } + } + return 0 +} + +// Helper to get maintenance duration histogram count +func getMaintenanceDurationCount(t *testing.T, m *metrics.ServerMetrics, repoName string) uint64 { + t.Helper() + + metricMap := m.Metrics() + collector, ok := metricMap["maintenance_job_duration_seconds"] + if !ok { + return 0 + } + + ch := make(chan prometheus.Metric, 1) + collector.Collect(ch) + close(ch) + + for metric := range ch { + dto := &dto.Metric{} + err := metric.Write(dto) + require.NoError(t, err) + + for _, label := range dto.Label { + if *label.Name == "repository_name" && *label.Value == repoName { + if dto.Histogram != nil { + return *dto.Histogram.SampleCount + } + } + } + } + return 0 +} diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 557a0a4b8..84508bed0 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -27,6 +27,11 @@ type ServerMetrics struct { metrics map[string]prometheus.Collector } +// Metrics returns the metrics map for testing purposes. +func (m *ServerMetrics) Metrics() map[string]prometheus.Collector { + return m.metrics +} + const ( metricNamespace = "velero" podVolumeMetricsNamespace = "podVolume" @@ -75,6 +80,11 @@ const ( DataDownloadFailureTotal = "data_download_failure_total" DataDownloadCancelTotal = "data_download_cancel_total" + // maintenance job metrics + maintenanceJobSuccessTotal = "maintenance_job_success_total" + maintenanceJobFailureTotal = "maintenance_job_failure_total" + maintenanceJobDurationSeconds = "maintenance_job_duration_seconds" + // Labels nodeMetricLabel = "node" podVolumeOperationLabel = "operation" @@ -82,6 +92,7 @@ const ( pvbNameLabel = "pod_volume_backup" scheduleLabel = "schedule" backupNameLabel = "backupName" + repositoryNameLabel = "repository_name" // metrics values BackupLastStatusSucc int64 = 1 @@ -333,6 +344,41 @@ func NewServerMetrics() *ServerMetrics { }, []string{scheduleLabel, backupNameLabel}, ), + maintenanceJobSuccessTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: metricNamespace, + Name: maintenanceJobSuccessTotal, + Help: "Total number of successful maintenance jobs", + }, + []string{repositoryNameLabel}, + ), + maintenanceJobFailureTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: metricNamespace, + Name: maintenanceJobFailureTotal, + Help: "Total number of failed maintenance jobs", + }, + []string{repositoryNameLabel}, + ), + maintenanceJobDurationSeconds: prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Namespace: metricNamespace, + Name: maintenanceJobDurationSeconds, + Help: "Time taken to complete maintenance jobs, in seconds", + Buckets: []float64{ + toSeconds(1 * time.Minute), + toSeconds(5 * time.Minute), + toSeconds(10 * time.Minute), + toSeconds(15 * time.Minute), + toSeconds(30 * time.Minute), + toSeconds(1 * time.Hour), + toSeconds(2 * time.Hour), + toSeconds(3 * time.Hour), + toSeconds(4 * time.Hour), + }, + }, + []string{repositoryNameLabel}, + ), }, } } @@ -912,3 +958,24 @@ func (m *ServerMetrics) RegisterBackupLocationUnavailable(backupLocationName str g.WithLabelValues(backupLocationName).Set(float64(0)) } } + +// RegisterMaintenanceJobSuccess records a successful maintenance job. +func (m *ServerMetrics) RegisterMaintenanceJobSuccess(repositoryName string) { + if c, ok := m.metrics[maintenanceJobSuccessTotal].(*prometheus.CounterVec); ok { + c.WithLabelValues(repositoryName).Inc() + } +} + +// RegisterMaintenanceJobFailure records a failed maintenance job. +func (m *ServerMetrics) RegisterMaintenanceJobFailure(repositoryName string) { + if c, ok := m.metrics[maintenanceJobFailureTotal].(*prometheus.CounterVec); ok { + c.WithLabelValues(repositoryName).Inc() + } +} + +// ObserveMaintenanceJobDuration records the number of seconds a maintenance job took. +func (m *ServerMetrics) ObserveMaintenanceJobDuration(repositoryName string, seconds float64) { + if h, ok := m.metrics[maintenanceJobDurationSeconds].(*prometheus.HistogramVec); ok { + h.WithLabelValues(repositoryName).Observe(seconds) + } +} diff --git a/pkg/metrics/metrics_test.go b/pkg/metrics/metrics_test.go index 36bf29c7c..005228417 100644 --- a/pkg/metrics/metrics_test.go +++ b/pkg/metrics/metrics_test.go @@ -372,3 +372,148 @@ func getHistogramCount(t *testing.T, vec *prometheus.HistogramVec, scheduleLabel t.Fatalf("Histogram with schedule label '%s' not found", scheduleLabel) return 0 } + +// TestMaintenanceJobMetrics verifies that maintenance job metrics are properly recorded. +func TestMaintenanceJobMetrics(t *testing.T) { + tests := []struct { + name string + repositoryName string + description string + }{ + { + name: "maintenance job metrics for repository", + repositoryName: "default-restic-abcd", + description: "Metrics should be recorded with the repository name label", + }, + { + name: "maintenance job metrics for different repository", + repositoryName: "velero-backup-repo-xyz", + description: "Metrics should be recorded with different repository name", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + m := NewServerMetrics() + + // Test maintenance job success metric + t.Run("RegisterMaintenanceJobSuccess", func(t *testing.T) { + m.RegisterMaintenanceJobSuccess(tc.repositoryName) + + metric := getMaintenanceMetricValue(t, m.metrics[maintenanceJobSuccessTotal].(*prometheus.CounterVec), tc.repositoryName) + assert.Equal(t, float64(1), metric, tc.description) + }) + + // Test maintenance job failure metric + t.Run("RegisterMaintenanceJobFailure", func(t *testing.T) { + m.RegisterMaintenanceJobFailure(tc.repositoryName) + + metric := getMaintenanceMetricValue(t, m.metrics[maintenanceJobFailureTotal].(*prometheus.CounterVec), tc.repositoryName) + assert.Equal(t, float64(1), metric, tc.description) + }) + + // Test maintenance job duration metric + t.Run("ObserveMaintenanceJobDuration", func(t *testing.T) { + m.ObserveMaintenanceJobDuration(tc.repositoryName, 300.5) + + // For histogram, we check the count + metric := getMaintenanceHistogramCount(t, m.metrics[maintenanceJobDurationSeconds].(*prometheus.HistogramVec), tc.repositoryName) + assert.Equal(t, uint64(1), metric, tc.description) + }) + }) + } +} + +// TestMultipleMaintenanceJobsAccumulate verifies that multiple maintenance jobs +// accumulate metrics under the same repository label. +func TestMultipleMaintenanceJobsAccumulate(t *testing.T) { + m := NewServerMetrics() + repoName := "default-restic-test" + + // Simulate multiple maintenance job executions + m.RegisterMaintenanceJobSuccess(repoName) + m.RegisterMaintenanceJobSuccess(repoName) + m.RegisterMaintenanceJobSuccess(repoName) + m.RegisterMaintenanceJobFailure(repoName) + m.RegisterMaintenanceJobFailure(repoName) + + // Record multiple durations + m.ObserveMaintenanceJobDuration(repoName, 120.5) + m.ObserveMaintenanceJobDuration(repoName, 180.3) + m.ObserveMaintenanceJobDuration(repoName, 90.7) + + // Verify accumulated metrics + successMetric := getMaintenanceMetricValue(t, m.metrics[maintenanceJobSuccessTotal].(*prometheus.CounterVec), repoName) + assert.Equal(t, float64(3), successMetric, "All maintenance job successes should be counted") + + failureMetric := getMaintenanceMetricValue(t, m.metrics[maintenanceJobFailureTotal].(*prometheus.CounterVec), repoName) + assert.Equal(t, float64(2), failureMetric, "All maintenance job failures should be counted") + + durationCount := getMaintenanceHistogramCount(t, m.metrics[maintenanceJobDurationSeconds].(*prometheus.HistogramVec), repoName) + assert.Equal(t, uint64(3), durationCount, "All maintenance job durations should be observed") +} + +// Helper function to get metric value from a CounterVec with repository_name label +func getMaintenanceMetricValue(t *testing.T, vec prometheus.Collector, repositoryName string) float64 { + t.Helper() + ch := make(chan prometheus.Metric, 1) + vec.Collect(ch) + close(ch) + + for metric := range ch { + dto := &dto.Metric{} + err := metric.Write(dto) + require.NoError(t, err) + + // Check if this metric has the expected repository_name label + hasCorrectLabel := false + for _, label := range dto.Label { + if *label.Name == "repository_name" && *label.Value == repositoryName { + hasCorrectLabel = true + break + } + } + + if hasCorrectLabel { + if dto.Counter != nil { + return *dto.Counter.Value + } + if dto.Gauge != nil { + return *dto.Gauge.Value + } + } + } + + t.Fatalf("Metric with repository_name label '%s' not found", repositoryName) + return 0 +} + +// Helper function to get histogram count with repository_name label +func getMaintenanceHistogramCount(t *testing.T, vec *prometheus.HistogramVec, repositoryName string) uint64 { + t.Helper() + ch := make(chan prometheus.Metric, 1) + vec.Collect(ch) + close(ch) + + for metric := range ch { + dto := &dto.Metric{} + err := metric.Write(dto) + require.NoError(t, err) + + // Check if this metric has the expected repository_name label + hasCorrectLabel := false + for _, label := range dto.Label { + if *label.Name == "repository_name" && *label.Value == repositoryName { + hasCorrectLabel = true + break + } + } + + if hasCorrectLabel && dto.Histogram != nil { + return *dto.Histogram.SampleCount + } + } + + t.Fatalf("Histogram with repository_name label '%s' not found", repositoryName) + return 0 +}