From c81d86efaeb7113de8595e3e6f9b3287d543f800 Mon Sep 17 00:00:00 2001 From: Adin Hodovic Date: Wed, 10 Jun 2020 18:39:16 +0200 Subject: [PATCH 1/7] Refactor Prometheus metrics to use a Count vector --- user/api.go | 203 +++-------------------------------------------- user/api_test.go | 19 +++++ 2 files changed, 28 insertions(+), 194 deletions(-) diff --git a/user/api.go b/user/api.go index 06bb8f95..24ca963c 100644 --- a/user/api.go +++ b/user/api.go @@ -25,118 +25,14 @@ import ( ) var ( - failedMarketoUploadCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "failedMarketoUploadCounter", + failedMarketoUploadCount = promauto.NewCounter(prometheus.CounterOpts{ + Name: "tidepool_shore_failed_marketo_upload_count", Help: "The total number of failures to connect to marketo due to errors", }) - statusNoUsrDetailsCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusNoUsrDetailsCounter", - Help: "The total number of STATUS_NO_USR_DETAILS errors", - }) - statusInvalidUserDetailsCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusInvalidUserDetailsCounter", - Help: "The total number of STATUS_INVALID_USER_DETAILS errors", - }) - statusUserNotFoundCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusUserNotFoundCounter", - Help: "The total number of STATUS_USER_NOT_FOUND errors", - }) - statusErrFindingUsrCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusErrFindingUsrCounter", - Help: "The total number of STATUS_ERR_FINDING_USR errors", - }) - statusErrCreatingUsrCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusErrCreatingUsrCounter", - Help: "The total number of STATUS_ERR_CREATING_USR errors", - }) - statusErrUpdatingUsrCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusErrUpdatingUsrCounter", - Help: "The total number of STATUS_ERR_UPDATING_USR errors", - }) - statusUsrAlreadyExistsCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusUsrAlreadyExistsCounter", - Help: "The total number of STATUS_USR_ALREADY_EXISTS errors", - }) - statusErrGeneratingTokenCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusErrGeneratingTokenCounter", - Help: "The total number of STATUS_ERR_GENERATING_TOKEN errors", - }) - statusErrUpdatingTokenCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusErrUpdatingTokenCounter", - Help: "The total number of STATUS_ERR_UPDATING_TOKEN errors", - }) - statusMissingUsrDetailsCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusMissingUsrDetailsCounter", - Help: "The total number of STATUS_MISSING_USR_DETAILS errors", - }) - statusErrorUpdatingPwCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusErrorUpdatingPwCounter", - Help: "The total number of STATUS_ERROR_UPDATING_PW errors", - }) - statusMissingIdPwCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusMissingIdPwCounter", - Help: "The total number of STATUS_MISSING_ID_PW errors", - }) - statusNoMatchCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusNoMatchCounter", - Help: "The total number of STATUS_NO_MATCH errors", - }) - statusNotVerifiedCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusNotVerifiedCounter", - Help: "The total number of STATUS_NOT_VERIFIED errors", - }) - statusNoTokenMatchCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusNoTokenMatchCounter", - Help: "The total number of STATUS_NO_TOKEN_MATCH errors", - }) - statusPwWrongCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusPwWrongCounter", - Help: "The total number of STATUS_PW_WRONG errors", - }) - statusErrSendingEmailCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusErrSendingEmailCounter", - Help: "The total number of STATUS_ERR_SENDING_EMAIL errors", - }) - statusNoTokenCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusNoTokenCounter", - Help: "The total number of STATUS_NO_TOKEN errors", - }) - statusServerTokenRequiredCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusServerTokenRequiredCounter", - Help: "The total number of STATUS_SERVER_TOKEN_REQUIRED errors", - }) - statusAuthHeaderRequiredCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusAuthHeaderRequiredCounter", - Help: "The total number of STATUS_AUTH_HEADER_REQUIRED errors", - }) - statusAuthHeaderInvlaidCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusAuthHeaderInvlaidCounter", - Help: "The total number of STATUS_AUTH_HEADER_INVLAID errors", - }) - statusGetstatusErrCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusGetstatusErrCounter", - Help: "The total number of STATUS_GETSTATUS_ERR errors", - }) - statusUnauthorizedCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusUnauthorizedCounter", - Help: "The total number of STATUS_UNAUTHORIZED errors", - }) - statusNoQueryCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusNoQueryCounter", - Help: "The total number of STATUS_NO_QUERY errors", - }) - statusParameterUnknownCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusParameterUnknownCounter", - Help: "The total number of STATUS_PARAMETER_UNKNOWN errors", - }) - statusOneQueryParamCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusOneQueryParamCounter", - Help: "The total number of STATUS_ONE_QUERY_PARAM errors", - }) - statusInvalidRoleCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "statusInvalidRoleCounter", - Help: "The total number of STATUS_INVALID_ROLE errors", - }) + statusCount = promauto.NewCounterVec(prometheus.CounterOpts{ + Name: "tidepool_shoreline_status_count", + Help: "The total number of errors per status", + }, []string{"status_reason", "status_code"}) ) type ( @@ -189,7 +85,7 @@ const ( STATUS_NO_TOKEN = "No x-tidepool-session-token was found" STATUS_SERVER_TOKEN_REQUIRED = "A server token is required" STATUS_AUTH_HEADER_REQUIRED = "Authorization header is required" - STATUS_AUTH_HEADER_INVLAID = "Authorization header is invalid" + STATUS_AUTH_HEADER_INVALID = "Authorization header is invalid" STATUS_GETSTATUS_ERR = "Error checking service status" STATUS_UNAUTHORIZED = "Not authorized for requested operation" STATUS_NO_QUERY = "A query must be specified" @@ -482,7 +378,7 @@ func (a *Api) UpdateUser(res http.ResponseWriter, req *http.Request, vars map[st a.marketoManager.UpdateListMembershipForUser(originalUser, updatedUser) } } else { - failedMarketoUploadCounter.Inc() + failedMarketoUploadCount.Inc() } } a.logMetricForUser(updatedUser.Id, "userupdated", sessionToken, map[string]string{"server": strconv.FormatBool(tokenData.IsServer)}) @@ -773,88 +669,7 @@ func (a *Api) sendError(res http.ResponseWriter, statusCode int, reason string, messages[index] = fmt.Sprintf("%v", extra) } - switch reason { - case STATUS_NO_USR_DETAILS: - statusNoUsrDetailsCounter.Inc() - - case STATUS_INVALID_USER_DETAILS: - statusInvalidUserDetailsCounter.Inc() - - case STATUS_USER_NOT_FOUND: - statusUserNotFoundCounter.Inc() - - case STATUS_ERR_FINDING_USR: - statusErrFindingUsrCounter.Inc() - - case STATUS_ERR_CREATING_USR: - statusErrCreatingUsrCounter.Inc() - - case STATUS_ERR_UPDATING_USR: - statusErrUpdatingUsrCounter.Inc() - - case STATUS_USR_ALREADY_EXISTS: - statusUsrAlreadyExistsCounter.Inc() - - case STATUS_ERR_GENERATING_TOKEN: - statusErrGeneratingTokenCounter.Inc() - - case STATUS_ERR_UPDATING_TOKEN: - statusErrUpdatingTokenCounter.Inc() - - case STATUS_MISSING_USR_DETAILS: - statusMissingUsrDetailsCounter.Inc() - - case STATUS_ERROR_UPDATING_PW: - statusErrorUpdatingPwCounter.Inc() - - case STATUS_MISSING_ID_PW: - statusMissingIdPwCounter.Inc() - - case STATUS_NO_MATCH: - statusNoMatchCounter.Inc() - - case STATUS_NOT_VERIFIED: - statusNotVerifiedCounter.Inc() - - case STATUS_NO_TOKEN_MATCH: - statusNoTokenMatchCounter.Inc() - - case STATUS_PW_WRONG: - statusPwWrongCounter.Inc() - - case STATUS_ERR_SENDING_EMAIL: - statusErrSendingEmailCounter.Inc() - - case STATUS_NO_TOKEN: - statusNoTokenCounter.Inc() - - case STATUS_SERVER_TOKEN_REQUIRED: - statusServerTokenRequiredCounter.Inc() - - case STATUS_AUTH_HEADER_REQUIRED: - statusAuthHeaderRequiredCounter.Inc() - - case STATUS_AUTH_HEADER_INVLAID: - statusAuthHeaderInvlaidCounter.Inc() - - case STATUS_GETSTATUS_ERR: - statusGetstatusErrCounter.Inc() - - case STATUS_UNAUTHORIZED: - statusUnauthorizedCounter.Inc() - - case STATUS_NO_QUERY: - statusNoQueryCounter.Inc() - - case STATUS_PARAMETER_UNKNOWN: - statusParameterUnknownCounter.Inc() - - case STATUS_ONE_QUERY_PARAM: - statusOneQueryParamCounter.Inc() - - case STATUS_INVALID_ROLE: - statusInvalidRoleCounter.Inc() - } + statusCount.WithLabelValues(reason, strconv.Itoa(statusCode)).Inc() a.logger.Printf("%s:%d RESPONSE ERROR: [%d %s] %s", file, line, statusCode, reason, strings.Join(messages, "; ")) sendModelAsResWithStatus(res, status.NewStatus(statusCode, reason), statusCode) diff --git a/user/api_test.go b/user/api_test.go index ece5907b..8c356079 100644 --- a/user/api_test.go +++ b/user/api_test.go @@ -385,6 +385,25 @@ func TestGetStatus_StatusInternalServerError(t *testing.T) { } +func TestGetMetrics_StatusCount(t *testing.T) { + + performRequest(t, "GET", "/users?role=clinic") + + response := performRequest(t, "GET", "/metrics") + + if response.Code != http.StatusOK { + t.Fatalf("Resp given [%d] expected [%d] ", response.Code, http.StatusOK) + } + if p, err := ioutil.ReadAll(response.Body); err != nil { + t.Fail() + } else { + metric := fmt.Sprintf("tidepool_shoreline_status_count{status_code=\"%d\",status_reason=\"%s\"}", 401, STATUS_UNAUTHORIZED) + if !strings.Contains(string(p), metric) { + t.Errorf("Expected %s in response: \n%s", metric, p) + } + } +} + //////////////////////////////////////////////////////////////////////////////// func Test_GetUsers_Error_MissingSessionToken(t *testing.T) { From 667c7498f0c6e67f9708ca0b0ae31e251fb97d74 Mon Sep 17 00:00:00 2001 From: Adin Hodovic Date: Wed, 10 Jun 2020 18:40:20 +0200 Subject: [PATCH 2/7] WIP --- user/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user/api.go b/user/api.go index 24ca963c..83bfcdbb 100644 --- a/user/api.go +++ b/user/api.go @@ -26,7 +26,7 @@ import ( var ( failedMarketoUploadCount = promauto.NewCounter(prometheus.CounterOpts{ - Name: "tidepool_shore_failed_marketo_upload_count", + Name: "tidepool_shoreline_failed_marketo_upload_count", Help: "The total number of failures to connect to marketo due to errors", }) statusCount = promauto.NewCounterVec(prometheus.CounterOpts{ From 1f302e4a8f2950bed45d6cee68b79718f018c134 Mon Sep 17 00:00:00 2001 From: Adin Hodovic Date: Wed, 10 Jun 2020 23:49:21 +0200 Subject: [PATCH 3/7] Rename to failed --- user/api.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/user/api.go b/user/api.go index 83bfcdbb..d999cf14 100644 --- a/user/api.go +++ b/user/api.go @@ -26,12 +26,12 @@ import ( var ( failedMarketoUploadCount = promauto.NewCounter(prometheus.CounterOpts{ - Name: "tidepool_shoreline_failed_marketo_upload_count", + Name: "tidepool_shoreline_failed_marketo_upload_total", Help: "The total number of failures to connect to marketo due to errors", }) statusCount = promauto.NewCounterVec(prometheus.CounterOpts{ - Name: "tidepool_shoreline_status_count", - Help: "The total number of errors per status", + Name: "tidepool_shoreline_failed_status_count", + Help: "The number of errors for each status code and status reason.", }, []string{"status_reason", "status_code"}) ) From 030923f4953c96bdc28e39efa08136e32592ca9f Mon Sep 17 00:00:00 2001 From: Adin Hodovic Date: Wed, 10 Jun 2020 23:55:37 +0200 Subject: [PATCH 4/7] Fix test --- user/api_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user/api_test.go b/user/api_test.go index 8c356079..ac50fd74 100644 --- a/user/api_test.go +++ b/user/api_test.go @@ -397,7 +397,7 @@ func TestGetMetrics_StatusCount(t *testing.T) { if p, err := ioutil.ReadAll(response.Body); err != nil { t.Fail() } else { - metric := fmt.Sprintf("tidepool_shoreline_status_count{status_code=\"%d\",status_reason=\"%s\"}", 401, STATUS_UNAUTHORIZED) + metric := fmt.Sprintf("tidepool_shoreline_failed_status_count{status_code=\"%d\",status_reason=\"%s\"}", 401, STATUS_UNAUTHORIZED) if !strings.Contains(string(p), metric) { t.Errorf("Expected %s in response: \n%s", metric, p) } From de73ea5eb2915c721c734d778ef4575f20e6e535 Mon Sep 17 00:00:00 2001 From: Adin Hodovic Date: Thu, 11 Jun 2020 18:24:51 +0200 Subject: [PATCH 5/7] Rename shoreline counter --- shoreline.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shoreline.go b/shoreline.go index 2845b9a6..4aad39df 100644 --- a/shoreline.go +++ b/shoreline.go @@ -25,7 +25,7 @@ import ( var ( failedMarketoKeyConfigurationCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "failedMarketoKeyConfigurationCounter", + Name: "tidepool_shoreline_failed_marketo_key_configuration_counter", Help: "The total number of failures to connect to marketo due to key configuration issues. Can not be resolved via retry", }) ) From c75d7d302193312c543ad13569979de57e970a83 Mon Sep 17 00:00:00 2001 From: Adin Hodovic Date: Thu, 11 Jun 2020 18:50:50 +0200 Subject: [PATCH 6/7] Fixup metric shoreline --- shoreline.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shoreline.go b/shoreline.go index 4aad39df..a9660ba7 100644 --- a/shoreline.go +++ b/shoreline.go @@ -24,9 +24,9 @@ import ( ) var ( - failedMarketoKeyConfigurationCounter = promauto.NewCounter(prometheus.CounterOpts{ - Name: "tidepool_shoreline_failed_marketo_key_configuration_counter", - Help: "The total number of failures to connect to marketo due to key configuration issues. Can not be resolved via retry", + marketoConfig = promauto.NewGauge(prometheus.GaugeOpts{ + Name: "tidepool_shoreline_marketo_config_valid", + Help: "Indicates if the latest shoreline marketo configuration is valid.", }) ) @@ -163,13 +163,13 @@ func main() { var marketoManager marketo.Manager if err := config.User.Marketo.Validate(); err != nil { logger.Println("WARNING: Marketo config is invalid", err) - failedMarketoKeyConfigurationCounter.Inc() } else { logger.Print("initializing marketo manager") marketoManager, err = marketo.NewManager(logger, config.User.Marketo) if err != nil { logger.Println("WARNING: Marketo Manager not configured;", err) } + marketoConfig.Set(1) } clientStore := user.NewMongoStoreClient(&config.Mongo) From 555fd81663e1324a25a1c5de2a3357ee420fd3db Mon Sep 17 00:00:00 2001 From: Adin Hodovic Date: Thu, 11 Jun 2020 19:42:26 +0200 Subject: [PATCH 7/7] Fixup --- shoreline.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shoreline.go b/shoreline.go index a9660ba7..108ca4e8 100644 --- a/shoreline.go +++ b/shoreline.go @@ -168,8 +168,9 @@ func main() { marketoManager, err = marketo.NewManager(logger, config.User.Marketo) if err != nil { logger.Println("WARNING: Marketo Manager not configured;", err) + } else { + marketoConfig.Set(1) } - marketoConfig.Set(1) } clientStore := user.NewMongoStoreClient(&config.Mongo)