From 7c5bc60a525b385f15b15c14c0cee689447dec9f Mon Sep 17 00:00:00 2001 From: Joseph Kavanagh Date: Wed, 19 Jul 2023 01:13:27 +0100 Subject: [PATCH] refactor(metrics): merge `ack_waiting` into `latest_version_is_deployed` 0=no, 1=yes, 2=approved, 3=skipped --- commands/init.go | 10 -- commands/init_test.go | 8 +- service/handlers.go | 4 - service/init.go | 1 + service/status/status.go | 42 +++++++- service/status/status_test.go | 164 ++++++++++++++++++++++--------- web/api/v1/http-api-edit_test.go | 6 +- web/metrics/prometheus.go | 16 +-- web/metrics/prometheus_test.go | 4 +- webhook/init.go | 10 -- 10 files changed, 176 insertions(+), 89 deletions(-) diff --git a/commands/init.go b/commands/init.go index 33f7739e..8807fd23 100644 --- a/commands/init.go +++ b/commands/init.go @@ -80,13 +80,6 @@ func (c *Controller) InitMetrics() { "", "FAIL") } - - // ########## - // # Gauges # - // ########## - metric.SetPrometheusGauge(metric.AckWaiting, - *c.ServiceStatus.ServiceID, - float64(0)) } // DeleteMetrics for this Controller. @@ -108,9 +101,6 @@ func (c *Controller) DeleteMetrics() { "", "FAIL") } - - metric.DeletePrometheusGauge(metric.AckWaiting, - *c.ServiceStatus.ServiceID) } // FormattedString will convert Command to a string in the format of '[ "arg0", "arg1" ]' diff --git a/commands/init_test.go b/commands/init_test.go index a85df7a3..16a25967 100644 --- a/commands/init_test.go +++ b/commands/init_test.go @@ -318,7 +318,7 @@ func TestController_Metrics(t *testing.T) { // WHEN the Prometheus metrics are initialised with initMetrics hadC := testutil.CollectAndCount(metric.CommandMetric) - hadG := testutil.CollectAndCount(metric.AckWaiting) + hadG := testutil.CollectAndCount(metric.LatestVersionIsDeployed) controller.InitMetrics() // THEN it can be collected @@ -330,8 +330,8 @@ func TestController_Metrics(t *testing.T) { (gotC - hadC), wantC) } // gauges - gotG := testutil.CollectAndCount(metric.AckWaiting) - wantG := 1 + gotG := testutil.CollectAndCount(metric.LatestVersionIsDeployed) + wantG := 0 if (gotG - hadG) != wantG { t.Errorf("%d Gauge metrics's were initialised, expecting %d", (gotG - hadG), wantG) @@ -346,7 +346,7 @@ func TestController_Metrics(t *testing.T) { gotC, hadC) } // gauges - gotG = testutil.CollectAndCount(metric.AckWaiting) + gotG = testutil.CollectAndCount(metric.LatestVersionIsDeployed) if gotG != hadG { t.Errorf("Gauge metrics's were deleted, got %d. expecting %d", gotG, hadG) diff --git a/service/handlers.go b/service/handlers.go index 89fde774..f2edff83 100644 --- a/service/handlers.go +++ b/service/handlers.go @@ -19,7 +19,6 @@ import ( "time" "github.com/release-argus/Argus/util" - metric "github.com/release-argus/Argus/web/metrics" ) // UpdatedVersion will register the version change, setting `s.Status.DeployedVersion` @@ -100,9 +99,6 @@ func (s *Service) HandleUpdateActions(writeToDB bool) { } else { jLog.Info("Waiting for approval on the Web UI", util.LogFrom{Primary: s.ID}, true) - metric.SetPrometheusGauge(metric.AckWaiting, - s.ID, - 1) s.Status.AnnounceQueryNewVersion() } } else { diff --git a/service/init.go b/service/init.go index cc85ecb3..e7b750e9 100644 --- a/service/init.go +++ b/service/init.go @@ -168,6 +168,7 @@ func (s *Service) DeleteMetrics() { s.Notify.DeleteMetrics() s.CommandController.DeleteMetrics() s.WebHook.DeleteMetrics() + s.Status.DeleteMetrics() } // ResetMetrics of the Service. diff --git a/service/status/status.go b/service/status/status.go index d588be9a..eb2645e7 100644 --- a/service/status/status.go +++ b/service/status/status.go @@ -186,7 +186,22 @@ func (s *Status) ApprovedVersion() string { // SetApprovedVersion. func (s *Status) SetApprovedVersion(version string, writeToDB bool) { s.mutex.Lock() - s.approvedVersion = version + { + s.approvedVersion = version + + // Update metrics if we're acting on the latest version + if strings.HasSuffix(s.approvedVersion, s.latestVersion) { + value := float64(3) // Skipping latest version + if s.approvedVersion == s.latestVersion { + value = 2 // Approving latest version + } + if s.ServiceID != nil { + metric.SetPrometheusGauge(metric.LatestVersionIsDeployed, + *s.ServiceID, + value) + } + } + } s.mutex.Unlock() if writeToDB { @@ -402,9 +417,32 @@ func (s *Status) GetWebURL() string { // setLatestVersionIsDeployedMetric will set the metric for whether the latest version is currently deployed. func (s *Status) setLatestVersionIsDeployedMetric() { + if s.ServiceID == nil { + return + } + value := float64(0) // Not deployed if s.latestVersion == s.deployedVersion { value = 1 // Is deployed + + // Latest version isn't deployed, but has been approved/skipped, so carry that over + } else if strings.HasSuffix(s.approvedVersion, s.latestVersion) { + value = 3 // Latest version was skipped + if s.approvedVersion == s.latestVersion { + value = 2 // Latest version was approved + } + } + metric.SetPrometheusGauge(metric.LatestVersionIsDeployed, + *s.ServiceID, + value) +} + +// DeleteMetrics of the Status. +func (s *Status) DeleteMetrics() { + if s == nil || s.ServiceID == nil { + return } - metric.SetPrometheusGauge(metric.LatestVersionIsDeployed, *s.ServiceID, value) + + metric.DeletePrometheusGauge(metric.LatestVersionIsDeployed, + *s.ServiceID) } diff --git a/service/status/status_test.go b/service/status/status_test.go index 7bddfdd2..d0bc1224 100644 --- a/service/status/status_test.go +++ b/service/status/status_test.go @@ -182,53 +182,83 @@ func TestStatus_SetLastQueried(t *testing.T) { } func TestStatus_ApprovedVersion(t *testing.T) { - // GIVEN a Status - approvedVersion := "0.0.2" deployedVersion := "0.0.1" latestVersion := "0.0.3" - announceChannel := make(chan []byte, 4) - databaseChannel := make(chan dbtype.Message, 4) - status := New( - &announceChannel, &databaseChannel, nil, - "", "", "", "", "", "") - status.Init( - 0, 0, 0, - stringPtr("TestStatus_SetApprovedVersion"), - stringPtr("https://example.com")) - status.SetLatestVersion(latestVersion, false) - status.SetDeployedVersion(deployedVersion, false) - - // WHEN SetApprovedVersion is called - status.SetApprovedVersion(approvedVersion, true) - - // THEN the Status is as expected - // ApprovedVersion - got := status.ApprovedVersion() - if got != approvedVersion { - t.Errorf("ApprovedVersion not set to %s. Got %s", - approvedVersion, got) - } - // LatestVersion - got = status.LatestVersion() - if got != latestVersion { - t.Errorf("LatestVersion not set to %s. Got %s", - latestVersion, got) - } - // DeployedVersion - got = status.DeployedVersion() - if got != deployedVersion { - t.Errorf("DeployedVersion not set to %s. Got %s", - deployedVersion, got) - } - // AnnounceChannel - if len(*status.AnnounceChannel) != 1 { - t.Errorf("AnnounceChannel should have 1 message, but has %d", - len(*status.AnnounceChannel)) - } - // DatabaseChannel - if len(*status.DatabaseChannel) != 1 { - t.Errorf("DatabaseChannel should have 1 message, but has %d", - len(*status.DatabaseChannel)) + // GIVEN a Status + tests := map[string]struct { + approving string + latestVersionIsDeployedMetric float64 + }{ + "Approving LatestVersion": { + approving: latestVersion, + latestVersionIsDeployedMetric: 2, + }, + "Skipping LatestVersion": { + approving: "SKIP_" + latestVersion, + latestVersionIsDeployedMetric: 3, + }, + "Approving non-latest version": { + approving: "0.0.2a", + latestVersionIsDeployedMetric: 0, + }, + } + + for name, tc := range tests { + name, tc := name, tc + t.Run(name, func(t *testing.T) { + t.Parallel() + + announceChannel := make(chan []byte, 4) + databaseChannel := make(chan dbtype.Message, 4) + status := New( + &announceChannel, &databaseChannel, nil, + "", "", "", "", "", "") + status.Init( + 0, 0, 0, + stringPtr("TestStatus_SetApprovedVersion_"+name), + stringPtr("https://example.com")) + status.SetLatestVersion(latestVersion, false) + status.SetDeployedVersion(deployedVersion, false) + + // WHEN SetApprovedVersion is called + status.SetApprovedVersion(tc.approving, true) + + // THEN the Status is as expected + // ApprovedVersion + got := status.ApprovedVersion() + if got != tc.approving { + t.Errorf("ApprovedVersion not set to %s. Got %s", + tc.approving, got) + } + // LatestVersion + got = status.LatestVersion() + if got != latestVersion { + t.Errorf("LatestVersion not set to %s. Got %s", + latestVersion, got) + } + // DeployedVersion + got = status.DeployedVersion() + if got != deployedVersion { + t.Errorf("DeployedVersion not set to %s. Got %s", + deployedVersion, got) + } + // AnnounceChannel + if len(*status.AnnounceChannel) != 1 { + t.Errorf("AnnounceChannel should have 1 message, but has %d", + len(*status.AnnounceChannel)) + } + // DatabaseChannel + if len(*status.DatabaseChannel) != 1 { + t.Errorf("DatabaseChannel should have 1 message, but has %d", + len(*status.DatabaseChannel)) + } + // AND LatestVersionIsDeployedVersion metric is updated + gotMetric := testutil.ToFloat64(metric.LatestVersionIsDeployed.WithLabelValues(*status.ServiceID)) + if gotMetric != tc.latestVersionIsDeployedMetric { + t.Errorf("LatestVersionIsDeployedVersion metric should be %f, not %f", + tc.latestVersionIsDeployedMetric, gotMetric) + } + }) } } @@ -799,7 +829,7 @@ shoutrrr: {bash: false, bish: nil, bosh: true}, for name, tc := range tests { name, tc := name, tc t.Run(name, func(t *testing.T) { - // t.Parallel() + t.Parallel() tc.status.SetApprovedVersion(tc.approvedVersion, false) tc.status.SetDeployedVersion(tc.deployedVersion, false) @@ -848,7 +878,7 @@ shoutrrr: {bash: false, bish: nil, bosh: true}, } } -func TestSetLatestVersionIsDeployedMetric(t *testing.T) { +func TestStatus_SetLatestVersionIsDeployedMetric(t *testing.T) { // GIVEN a Status tests := map[string]struct { latestVersion string @@ -892,3 +922,43 @@ func TestSetLatestVersionIsDeployedMetric(t *testing.T) { }) } } + +func TestStatus_DeleteMetrics(t *testing.T) { + // GIVEN a Status + tests := map[string]struct { + serviceID *string + }{ + "nil serviceID": { + serviceID: nil, + }, + "non-nil serviceID": { + serviceID: stringPtr("TestStatus_DeleteMetrics"), + }, + } + + for name, tc := range tests { + name, tc := name, tc + t.Run(name, func(t *testing.T) { + t.Parallel() + + status := Status{} + status.Init( + 0, 0, 0, + tc.serviceID, + stringPtr("http://example.com")) + + // WHEN DeleteMetrics is called on it + status.DeleteMetrics() + + // THEN the metrics are deleted + got := float64(0) + if tc.serviceID != nil { + got = testutil.ToFloat64(metric.LatestVersionIsDeployed.WithLabelValues(*status.ServiceID)) + } + if got != 0 { + t.Errorf("Expected LatestVersionIsDeployed to be 0, not %f", + got) + } + }) + } +} diff --git a/web/api/v1/http-api-edit_test.go b/web/api/v1/http-api-edit_test.go index d83275ad..d67bab77 100644 --- a/web/api/v1/http-api-edit_test.go +++ b/web/api/v1/http-api-edit_test.go @@ -94,7 +94,7 @@ func TestHTTP_VersionRefreshUncreated(t *testing.T) { "url": "https://valid.release-argus.io/plain", "regex": `stable version: "v?([0-9.+)"`, }, - wantBody: `"error":"values failed validity check:.*Invalid RegEx`, + wantBody: `"error":"values failed validity check:.*regex: .*invalid`, wantStatusCode: http.StatusBadRequest, }, } @@ -191,7 +191,7 @@ func TestHTTP_VersionRefresh(t *testing.T) { params: map[string]string{ "url_commands": `[{"type":"regex","regex":"beta: \"v?([0-9.+-beta)\""}]`, "semantic_versioning": "false"}, - wantBody: `{.*"error":".*Invalid RegEx`, + wantBody: `{.*"error":".*regex: .*invalid`, wantStatusCode: http.StatusOK, wantLatestVersion: "", }, @@ -237,7 +237,7 @@ func TestHTTP_VersionRefresh(t *testing.T) { deployedVersion: true, params: map[string]string{ "regex": "v?([0-9.+)"}, - wantBody: `\{.*"error":".*Invalid RegEx`, + wantBody: `\{.*"error":".*regex: .*invalid`, wantStatusCode: http.StatusOK, wantLatestVersion: "", }, diff --git a/web/metrics/prometheus.go b/web/metrics/prometheus.go index ef97efc2..51cc8344 100644 --- a/web/metrics/prometheus.go +++ b/web/metrics/prometheus.go @@ -21,6 +21,7 @@ import ( // Prometheus metric. var ( + // Count of the number of times each latest version query has passed/failed LatestVersionQueryMetric = promauto.NewCounterVec(prometheus.CounterOpts{ Name: "latest_version_query_result_total", Help: "Number of times the latest version check has passed/failed."}, @@ -28,6 +29,7 @@ var ( "id", "result", }) + // Count of the number of times each deployed version query has passed/failed DeployedVersionQueryMetric = promauto.NewCounterVec(prometheus.CounterOpts{ Name: "deployed_version_query_result_total", Help: "Number of times the deployed version check has passed/failed."}, @@ -35,6 +37,7 @@ var ( "id", "result", }) + // Count of the number of times each Command has passed/failed CommandMetric = promauto.NewCounterVec(prometheus.CounterOpts{ Name: "command_result_total", Help: "Number of times a Command has passed/failed."}, @@ -43,6 +46,7 @@ var ( "result", "service_id", }) + // Count of the number of times each Notify has passed/failed NotifyMetric = promauto.NewCounterVec(prometheus.CounterOpts{ Name: "notify_result_total", Help: "Number of times a Notify message has passed/failed."}, @@ -52,6 +56,7 @@ var ( "service_id", "type", }) + // Count of the number of times each WebHook has passed/failed WebHookMetric = promauto.NewCounterVec(prometheus.CounterOpts{ Name: "webhook_result_total", Help: "Number of times a WebHook has passed/failed."}, @@ -60,27 +65,24 @@ var ( "result", "service_id", }) + // Latest version query successful - 0=no, 1=yes, 2=no_regex_match, 3=semantic_version_fail, 4=progressive_version_fail LatestVersionQueryLiveness = promauto.NewGaugeVec(prometheus.GaugeOpts{ Name: "latest_version_query_result_last", Help: "Whether this service's last latest version query was successful (0=no, 1=yes, 2=no_regex_match, 3=semantic_version_fail, 4=progressive_version_fail)."}, []string{ "id", }) + // Lateest deployed version query successful - 0=no, 1=yes DeployedVersionQueryLiveness = promauto.NewGaugeVec(prometheus.GaugeOpts{ Name: "deployed_version_query_result_last", Help: "Whether this service's last deployed version query was successful (0=no, 1=yes)."}, []string{ "id", }) + // Latest version is deployed - 0=no, 1=yes, 2=approved, 3=skipped LatestVersionIsDeployed = promauto.NewGaugeVec(prometheus.GaugeOpts{ Name: "latest_version_is_deployed", - Help: "Whether this service's latest version is the same as its deployed version (0=no, 1=yes)."}, - []string{ - "id", - }) - AckWaiting = promauto.NewGaugeVec(prometheus.GaugeOpts{ - Name: "ack_waiting", - Help: "Whether a new release is waiting to be acknowledged (skipped/approved; 0=no, 1=yes)."}, + Help: "Whether this service's latest version is the same as its deployed version (0=no, 1=yes, 2=approved, 3=skipped)."}, []string{ "id", }) diff --git a/web/metrics/prometheus_test.go b/web/metrics/prometheus_test.go index 7c8cc798..c982c1a4 100644 --- a/web/metrics/prometheus_test.go +++ b/web/metrics/prometheus_test.go @@ -183,8 +183,8 @@ func TestPrometheusGaugeVec(t *testing.T) { "DeployedVersionQueryLiveness": { metric: DeployedVersionQueryLiveness, args: []string{"SERVICE_ID"}}, - "AckWaiting": { - metric: AckWaiting, + "LatestVersionIsDeployed": { + metric: LatestVersionIsDeployed, args: []string{"SERVICE_ID"}}, } diff --git a/webhook/init.go b/webhook/init.go index 218d85ea..bff593e0 100644 --- a/webhook/init.go +++ b/webhook/init.go @@ -123,13 +123,6 @@ func (w *WebHook) initMetrics() { *w.ServiceStatus.ServiceID, "", "FAIL") - - // ########## - // # Gauges # - // ########## - metric.SetPrometheusGauge(metric.AckWaiting, - *w.ServiceStatus.ServiceID, - float64(0)) } // DeleteMetrics of the Slice. @@ -159,7 +152,4 @@ func (w *WebHook) deleteMetrics() { *w.ServiceStatus.ServiceID, "", "FAIL") - - metric.DeletePrometheusGauge(metric.AckWaiting, - *w.ServiceStatus.ServiceID) }