From 59842d5e88869d49ae28edda0ff30e07c5b74123 Mon Sep 17 00:00:00 2001 From: raojinlin Date: Wed, 18 Sep 2024 10:26:21 +0800 Subject: [PATCH] feat(alerting): custom alert support endpoint errors (#844) * feat(alerting): add support for including endpoint errors in custom alerts - Updated `buildHTTPRequest` method in `AlertProvider` to accept a `result` parameter. - Added support for including `[ENDPOINT_ERRORS]` in both the request body and URL, which will be replaced by the errors from `Result.Errors[]`. - Adjusted `CreateExternalEndpointResult` to capture and store errors from query parameters. - This allows custom alerts to include detailed error information, enhancing the flexibility of alert notifications. * feat: add ENDPOINT_ERRORS example * feat: add tests * Refactor: code review feedback * delete unsed errors * Update README.md * Apply suggestions from code review --------- Co-authored-by: raojinlin Co-authored-by: TwiN --- README.md | 6 ++-- alerting/provider/custom/custom.go | 6 ++-- alerting/provider/custom/custom_test.go | 48 +++++++++++++++++++++++++ api/external_endpoint.go | 3 ++ api/external_endpoint_test.go | 34 +++++++++++++++--- 5 files changed, 88 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 6e9de23a..bb281f66 100644 --- a/README.md +++ b/README.md @@ -307,12 +307,13 @@ external-endpoints: To push the status of an external endpoint, the request would have to look like this: ``` -POST /api/v1/endpoints/{key}/external?success={success} +POST /api/v1/endpoints/{key}/external?success={success}&error={error} ``` Where: - `{key}` has the pattern `_` in which both variables have ` `, `/`, `_`, `,` and `.` replaced by `-`. - Using the example configuration above, the key would be `core_ext-ep-test`. - `{success}` is a boolean (`true` or `false`) value indicating whether the health check was successful or not. +- `{error}`: a string describing the reason for a failed health check. If {success} is false, this should contain the error message; if the check is successful, it can be omitted or left empty. You must also pass the token as a `Bearer` token in the `Authorization` header. @@ -1357,6 +1358,7 @@ Furthermore, you may use the following placeholders in the body (`alerting.custo - `[ENDPOINT_NAME]` (resolved from `endpoints[].name`) - `[ENDPOINT_GROUP]` (resolved from `endpoints[].group`) - `[ENDPOINT_URL]` (resolved from `endpoints[].url`) +- `[RESULT_ERRORS]` (resolved from the health evaluation of a given health check) If you have an alert using the `custom` provider with `send-on-resolved` set to `true`, you can use the `[ALERT_TRIGGERED_OR_RESOLVED]` placeholder to differentiate the notifications. @@ -1371,7 +1373,7 @@ alerting: method: "POST" body: | { - "text": "[ALERT_TRIGGERED_OR_RESOLVED]: [ENDPOINT_GROUP] - [ENDPOINT_NAME] - [ALERT_DESCRIPTION]" + "text": "[ALERT_TRIGGERED_OR_RESOLVED]: [ENDPOINT_GROUP] - [ENDPOINT_NAME] - [ALERT_DESCRIPTION] - [RESULT_ERRORS]" } endpoints: - name: website diff --git a/alerting/provider/custom/custom.go b/alerting/provider/custom/custom.go index fc661edf..91555547 100644 --- a/alerting/provider/custom/custom.go +++ b/alerting/provider/custom/custom.go @@ -50,7 +50,7 @@ func (provider *AlertProvider) GetAlertStatePlaceholderValue(resolved bool) stri return status } -func (provider *AlertProvider) buildHTTPRequest(ep *endpoint.Endpoint, alert *alert.Alert, resolved bool) *http.Request { +func (provider *AlertProvider) buildHTTPRequest(ep *endpoint.Endpoint, alert *alert.Alert, result *endpoint.Result, resolved bool) *http.Request { body, url, method := provider.Body, provider.URL, provider.Method body = strings.ReplaceAll(body, "[ALERT_DESCRIPTION]", alert.GetDescription()) url = strings.ReplaceAll(url, "[ALERT_DESCRIPTION]", alert.GetDescription()) @@ -60,6 +60,8 @@ func (provider *AlertProvider) buildHTTPRequest(ep *endpoint.Endpoint, alert *al url = strings.ReplaceAll(url, "[ENDPOINT_GROUP]", ep.Group) body = strings.ReplaceAll(body, "[ENDPOINT_URL]", ep.URL) url = strings.ReplaceAll(url, "[ENDPOINT_URL]", ep.URL) + body = strings.ReplaceAll(body, "[RESULT_ERRORS]", strings.Join(result.Errors, ",")) + url = strings.ReplaceAll(url, "[RESULT_ERRORS]", strings.Join(result.Errors, ",")) if resolved { body = strings.ReplaceAll(body, "[ALERT_TRIGGERED_OR_RESOLVED]", provider.GetAlertStatePlaceholderValue(true)) url = strings.ReplaceAll(url, "[ALERT_TRIGGERED_OR_RESOLVED]", provider.GetAlertStatePlaceholderValue(true)) @@ -79,7 +81,7 @@ func (provider *AlertProvider) buildHTTPRequest(ep *endpoint.Endpoint, alert *al } func (provider *AlertProvider) Send(ep *endpoint.Endpoint, alert *alert.Alert, result *endpoint.Result, resolved bool) error { - request := provider.buildHTTPRequest(ep, alert, resolved) + request := provider.buildHTTPRequest(ep, alert, result, resolved) response, err := client.GetHTTPClient(provider.ClientConfig).Do(request) if err != nil { return err diff --git a/alerting/provider/custom/custom_test.go b/alerting/provider/custom/custom_test.go index 1678a7fe..67d94f65 100644 --- a/alerting/provider/custom/custom_test.go +++ b/alerting/provider/custom/custom_test.go @@ -140,6 +140,53 @@ func TestAlertProvider_buildHTTPRequest(t *testing.T) { request := customAlertProvider.buildHTTPRequest( &endpoint.Endpoint{Name: "endpoint-name", Group: "endpoint-group", URL: "https://example.com"}, &alert.Alert{Description: &alertDescription}, + &endpoint.Result{Errors: []string{}}, + scenario.Resolved, + ) + if request.URL.String() != scenario.ExpectedURL { + t.Error("expected URL to be", scenario.ExpectedURL, "got", request.URL.String()) + } + body, _ := io.ReadAll(request.Body) + if string(body) != scenario.ExpectedBody { + t.Error("expected body to be", scenario.ExpectedBody, "got", string(body)) + } + }) + } +} + +func TestAlertProviderWithResultErrors_buildHTTPRequest(t *testing.T) { + customAlertWithErrorsProvider := &AlertProvider{ + URL: "https://example.com/[ENDPOINT_GROUP]/[ENDPOINT_NAME]?event=[ALERT_TRIGGERED_OR_RESOLVED]&description=[ALERT_DESCRIPTION]&url=[ENDPOINT_URL]&error=[RESULT_ERRORS]", + Body: "[ENDPOINT_NAME],[ENDPOINT_GROUP],[ALERT_DESCRIPTION],[ENDPOINT_URL],[ALERT_TRIGGERED_OR_RESOLVED],[RESULT_ERRORS]", + } + alertDescription := "alert-description" + scenarios := []struct { + AlertProvider *AlertProvider + Resolved bool + ExpectedURL string + ExpectedBody string + Errors []string + }{ + { + AlertProvider: customAlertWithErrorsProvider, + Resolved: true, + ExpectedURL: "https://example.com/endpoint-group/endpoint-name?event=RESOLVED&description=alert-description&url=https://example.com&error=", + ExpectedBody: "endpoint-name,endpoint-group,alert-description,https://example.com,RESOLVED,", + }, + { + AlertProvider: customAlertWithErrorsProvider, + Resolved: false, + ExpectedURL: "https://example.com/endpoint-group/endpoint-name?event=TRIGGERED&description=alert-description&url=https://example.com&error=error1,error2", + ExpectedBody: "endpoint-name,endpoint-group,alert-description,https://example.com,TRIGGERED,error1,error2", + Errors: []string{"error1", "error2"}, + }, + } + for _, scenario := range scenarios { + t.Run(fmt.Sprintf("resolved-%v-with-default-placeholders-and-result-errors", scenario.Resolved), func(t *testing.T) { + request := customAlertWithErrorsProvider.buildHTTPRequest( + &endpoint.Endpoint{Name: "endpoint-name", Group: "endpoint-group", URL: "https://example.com"}, + &alert.Alert{Description: &alertDescription}, + &endpoint.Result{Errors: scenario.Errors}, scenario.Resolved, ) if request.URL.String() != scenario.ExpectedURL { @@ -190,6 +237,7 @@ func TestAlertProvider_buildHTTPRequestWithCustomPlaceholder(t *testing.T) { request := customAlertProvider.buildHTTPRequest( &endpoint.Endpoint{Name: "endpoint-name", Group: "endpoint-group"}, &alert.Alert{Description: &alertDescription}, + &endpoint.Result{}, scenario.Resolved, ) if request.URL.String() != scenario.ExpectedURL { diff --git a/api/external_endpoint.go b/api/external_endpoint.go index 4874d805..6cbe6db3 100644 --- a/api/external_endpoint.go +++ b/api/external_endpoint.go @@ -46,6 +46,9 @@ func CreateExternalEndpointResult(cfg *config.Config) fiber.Handler { Success: c.QueryBool("success"), Errors: []string{}, } + if !result.Success && c.Query("error") != "" { + result.Errors = append(result.Errors, c.Query("error")) + } convertedEndpoint := externalEndpoint.ToEndpoint() if err := store.Get().Insert(convertedEndpoint, result); err != nil { if errors.Is(err, common.ErrEndpointNotFound) { diff --git a/api/external_endpoint_test.go b/api/external_endpoint_test.go index 95fc432a..d63ab6c0 100644 --- a/api/external_endpoint_test.go +++ b/api/external_endpoint_test.go @@ -76,6 +76,12 @@ func TestCreateExternalEndpointResult(t *testing.T) { AuthorizationHeaderBearerToken: "Bearer token", ExpectedCode: 200, }, + { + Name: "good-token-success-true-with-ignored-error-because-success-true", + Path: "/api/v1/endpoints/g_n/external?success=true&error=failed", + AuthorizationHeaderBearerToken: "Bearer token", + ExpectedCode: 200, + }, { Name: "good-token-success-false", Path: "/api/v1/endpoints/g_n/external?success=false", @@ -88,6 +94,12 @@ func TestCreateExternalEndpointResult(t *testing.T) { AuthorizationHeaderBearerToken: "Bearer token", ExpectedCode: 200, }, + { + Name: "good-token-success-false-with-error", + Path: "/api/v1/endpoints/g_n/external?success=false&error=failed", + AuthorizationHeaderBearerToken: "Bearer token", + ExpectedCode: 200, + }, } for _, scenario := range scenarios { t.Run(scenario.Name, func(t *testing.T) { @@ -114,21 +126,33 @@ func TestCreateExternalEndpointResult(t *testing.T) { if endpointStatus.Key != "g_n" { t.Errorf("expected key to be g_n but got %s", endpointStatus.Key) } - if len(endpointStatus.Results) != 3 { + if len(endpointStatus.Results) != 5 { t.Errorf("expected 3 results but got %d", len(endpointStatus.Results)) } if !endpointStatus.Results[0].Success { t.Errorf("expected first result to be successful") } - if endpointStatus.Results[1].Success { - t.Errorf("expected second result to be unsuccessful") + if !endpointStatus.Results[1].Success { + t.Errorf("expected second result to be successful") + } + if len(endpointStatus.Results[1].Errors) > 0 { + t.Errorf("expected second result to have no errors") } if endpointStatus.Results[2].Success { t.Errorf("expected third result to be unsuccessful") } + if endpointStatus.Results[3].Success { + t.Errorf("expected fourth result to be unsuccessful") + } + if endpointStatus.Results[4].Success { + t.Errorf("expected fifth result to be unsuccessful") + } + if len(endpointStatus.Results[4].Errors) == 0 || endpointStatus.Results[4].Errors[0] != "failed" { + t.Errorf("expected fifth result to have errors: failed") + } externalEndpointFromConfig := cfg.GetExternalEndpointByKey("g_n") - if externalEndpointFromConfig.NumberOfFailuresInARow != 2 { - t.Errorf("expected 2 failures in a row but got %d", externalEndpointFromConfig.NumberOfFailuresInARow) + if externalEndpointFromConfig.NumberOfFailuresInARow != 3 { + t.Errorf("expected 3 failures in a row but got %d", externalEndpointFromConfig.NumberOfFailuresInARow) } if externalEndpointFromConfig.NumberOfSuccessesInARow != 0 { t.Errorf("expected 0 successes in a row but got %d", externalEndpointFromConfig.NumberOfSuccessesInARow)