diff --git a/internal/sli/dashboard/dashboard_error.go b/internal/sli/dashboard/dashboard_error.go new file mode 100644 index 000000000..879ca9763 --- /dev/null +++ b/internal/sli/dashboard/dashboard_error.go @@ -0,0 +1,61 @@ +package dashboard + +import "fmt" + +// ProcessingError represents a base error that happened while processing a dashboard +type ProcessingError struct { + cause error +} + +func (pe *ProcessingError) Error() string { + return pe.cause.Error() +} + +func (pe *ProcessingError) Unwrap() error { + return pe.cause +} + +// QueryError represents an error that happened while querying a dashboard +type QueryError struct { + err *ProcessingError +} + +// NewQueryError will create a new QueryError +func NewQueryError(cause error) *QueryError { + return &QueryError{ + err: &ProcessingError{ + cause: cause, + }, + } +} + +func (e *QueryError) Error() string { + return fmt.Sprintf("could not query Dynatrace dashboard for SLIs: %v", e.err.Error()) +} + +func (e *QueryError) Unwrap() error { + return e.err +} + +type UploadFileError struct { + context string + err *ProcessingError +} + +// NewUploadFileError will create a new UploadFileError +func NewUploadFileError(context string, cause error) *UploadFileError { + return &UploadFileError{ + context: context, + err: &ProcessingError{ + cause: cause, + }, + } +} + +func (e *UploadFileError) Error() string { + return fmt.Sprintf("could not upload %s file: %v", e.context, e.err.Error()) +} + +func (e *UploadFileError) Unwrap() error { + return e.err +} diff --git a/internal/sli/get_sli_triggered_event_handler.go b/internal/sli/get_sli_triggered_event_handler.go index 96dce5460..66f8dc428 100644 --- a/internal/sli/get_sli_triggered_event_handler.go +++ b/internal/sli/get_sli_triggered_event_handler.go @@ -25,6 +25,7 @@ import ( ) const ProblemOpenSLI = "problem_open" +const NoMetricIndicator = "no metric" type GetSLIEventHandler struct { event GetSLITriggeredAdapterInterface @@ -115,9 +116,7 @@ func ensureRightTimestamps(start string, end string) (time.Time, time.Time, erro return startUnix, endUnix, nil } -/** - * Adds an SLO Entry to the SLO.yaml - */ +// addSLO adds an SLO Entry to the SLO.yaml func (eh GetSLIEventHandler) addSLO(newSLO *keptncommon.SLO) error { // first - lets load the SLO.yaml from the config repo @@ -159,45 +158,35 @@ func (eh GetSLIEventHandler) addSLO(newSLO *keptncommon.SLO) error { return nil } -// Tries to find a dynatrace dashboard that matches our project. If so - returns the SLI, SLO and SLIResults +// getDataFromDynatraceDashboard will process dynatrace dashboard (if found) and return the SLI, SLO and SLIResults func (eh *GetSLIEventHandler) getDataFromDynatraceDashboard(startUnix time.Time, endUnix time.Time) (*dashboard.DashboardLink, []*keptnv2.SLIResult, error) { - // creating Dynatrace Retrieval which allows us to call the Dynatrace API sliQuerying := dashboard.NewQuerying(eh.event, eh.event.GetCustomSLIFilters(), eh.dtClient) - - // - // Option 1: We query the data from a dashboard instead of the uploaded SLI.yaml - // ============================================================================== - // Lets see if we have a Dashboard in Dynatrace that we should parse result, err := sliQuerying.GetSLIValues(eh.dashboard, startUnix, endUnix) if err != nil { - return nil, nil, fmt.Errorf("could not query Dynatrace dashboard for SLIs: %v", err) + return nil, nil, dashboard.NewQueryError(err) } - // lets write the SLI to the config repo + // let's write the SLI to the config repo if result.HasSLIs() { err = eh.resourceClient.UploadSLI(eh.event.GetProject(), eh.event.GetStage(), eh.event.GetService(), result.SLI()) if err != nil { - return nil, nil, err + return nil, nil, dashboard.NewUploadFileError("SLI", err) } } - // lets write the SLO to the config repo + // let's write the SLO to the config repo if result.HasSLOs() { err = eh.resourceClient.UploadSLOs(eh.event.GetProject(), eh.event.GetStage(), eh.event.GetService(), result.SLO()) if err != nil { - return nil, nil, err + return nil, nil, dashboard.NewUploadFileError("SLO", err) } } return result.DashboardLink(), result.SLIResults(), nil } -/** - * getDynatraceProblemContext - * - * Will evaluate the event and - if it finds a dynatrace problem ID - will return this - otherwise it will return 0 - */ +//getDynatraceProblemContext will evaluate the event and - returns dynatrace problem ID if found, 0 otherwise func getDynatraceProblemContext(eventData GetSLITriggeredAdapterInterface) string { // iterate through the labels and find Problem URL @@ -220,7 +209,6 @@ func getDynatraceProblemContext(eventData GetSLITriggeredAdapterInterface) strin return "" } -// func (eh *GetSLIEventHandler) getSLIResultsFromCustomQueries(startUnix time.Time, endUnix time.Time) ([]*keptnv2.SLIResult, error) { // get custom metrics for project if they exist projectCustomQueries, err := eh.kClient.GetCustomQueries(eh.event.GetProject(), eh.event.GetStage(), eh.event.GetService()) @@ -275,7 +263,7 @@ func (eh *GetSLIEventHandler) getSLIResultsFromProblemContext(problemID string) success := false message := "" - // lets query the status of this problem and add it to the SLI Result + // let's query the status of this problem and add it to the SLI Result status, err := dynatrace.NewProblemsV2Client(eh.dtClient).GetStatusByID(problemID) if err != nil { message = err.Error() @@ -288,7 +276,7 @@ func (eh *GetSLIEventHandler) getSLIResultsFromProblemContext(problemID string) } } - // lets add this to the sliResults + // let's add this to the sliResults sliResult := &keptnv2.SLIResult{ Metric: problemIndicator, Value: openProblemValue, @@ -296,7 +284,8 @@ func (eh *GetSLIEventHandler) getSLIResultsFromProblemContext(problemID string) Message: message, } - // lets add this to the SLO in case this indicator is not yet in SLO.yaml. Becuase if it doesnt get added the lighthouse wont evaluate the SLI values + // let's add this to the SLO in case this indicator is not yet in SLO.yaml. + // Because if it does not get added the lighthouse will not evaluate the SLI values // we default it to open_problems<=0 sloString := fmt.Sprintf("sli=%s;pass=<=0;key=true", problemIndicator) sloDefinition := common.ParsePassAndWarningWithoutDefaultsFrom(sloString) @@ -310,10 +299,7 @@ func (eh *GetSLIEventHandler) getSLIResultsFromProblemContext(problemID string) return sliResult } -// retrieveMetrics Handles keptn.InternalGetSLIEventType -// -// First tries to find a Dynatrace dashboard and then parses it for SLIs and SLOs -// Second will go to parse the SLI.yaml and returns the SLI as passed in by the event +// retrieveMetrics will retrieve metrics either from a dashboard or from an SLI file func (eh *GetSLIEventHandler) retrieveMetrics() error { // send get-sli.started event if err := eh.sendGetSLIStartedEvent(); err != nil { @@ -379,51 +365,49 @@ func (eh *GetSLIEventHandler) retrieveMetrics() error { return eh.sendGetSLIFinishedEvent(sliResults, err) } -/** - * Sends the SLI Done Event. If err != nil it will send an error message - */ -func (eh *GetSLIEventHandler) sendGetSLIFinishedEvent(indicatorValues []*keptnv2.SLIResult, err error) error { - // if an error was set - the indicators will be set to failed and error message is set to each - indicatorValues = resetIndicatorsInCaseOfError(err, eh.event, indicatorValues) +// sendGetSLIFinishedEvent sends the SLI finished event. If err != nil it will send an error message +func (eh *GetSLIEventHandler) sendGetSLIFinishedEvent(sliResults []*keptnv2.SLIResult, err error) error { + // if an error was set - the SLI results will be set to failed and an error message is set to each + sliResults = resetSLIResultsInCaseOfError(err, eh.event, sliResults) - return eh.sendEvent(NewSucceededGetSLIFinishedEventFactory(eh.event, indicatorValues, err)) + return eh.sendEvent(NewSucceededGetSLIFinishedEventFactory(eh.event, sliResults, err)) } -func resetIndicatorsInCaseOfError(err error, eventData GetSLITriggeredAdapterInterface, indicatorValues []*keptnv2.SLIResult) []*keptnv2.SLIResult { - if err != nil { - indicators := eventData.GetIndicators() - if (indicatorValues == nil) || (len(indicatorValues) == 0) { - if indicators == nil || len(indicators) == 0 { - indicators = []string{"no metric"} - } +func resetSLIResultsInCaseOfError(err error, eventData GetSLITriggeredAdapterInterface, sliResults []*keptnv2.SLIResult) []*keptnv2.SLIResult { + if err == nil { + return sliResults + } - for _, indicatorName := range indicators { - indicatorValues = []*keptnv2.SLIResult{ - { - Metric: indicatorName, - Value: 0.0, - }, - } - } + indicators := eventData.GetIndicators() + if len(sliResults) == 0 { + var errType *dashboard.QueryError + if len(indicators) == 0 || errors.As(err, &errType) { + indicators = []string{NoMetricIndicator} } - errMessage := err.Error() - for _, indicator := range indicatorValues { - indicator.Success = false - indicator.Message = errMessage + for _, indicatorName := range indicators { + sliResults = []*keptnv2.SLIResult{ + { + Metric: indicatorName, + Value: 0.0, + }, + } } } - return indicatorValues + errMessage := err.Error() + for _, sliResult := range sliResults { + sliResult.Success = false + sliResult.Message = errMessage + } + + return sliResults } func (eh *GetSLIEventHandler) sendGetSLIStartedEvent() error { return eh.sendEvent(NewGetSliStartedEventFactory(eh.event)) } -/** - * sends cloud event back to keptn - */ func (eh *GetSLIEventHandler) sendEvent(factory adapter.CloudEventFactoryInterface) error { err := eh.kClient.SendCloudEvent(factory) if err != nil { diff --git a/internal/sli/get_sli_triggered_event_handler_retrieve_metrics_from_dashboard_fails_test.go b/internal/sli/get_sli_triggered_event_handler_retrieve_metrics_from_dashboard_fails_test.go new file mode 100644 index 000000000..d32a7ec19 --- /dev/null +++ b/internal/sli/get_sli_triggered_event_handler_retrieve_metrics_from_dashboard_fails_test.go @@ -0,0 +1,122 @@ +package sli + +import ( + "net/http" + "strconv" + "testing" + + keptnv2 "github.com/keptn/go-utils/pkg/lib/v0_2_0" + "github.com/stretchr/testify/assert" + + "github.com/keptn-contrib/dynatrace-service/internal/dynatrace" + "github.com/keptn-contrib/dynatrace-service/internal/keptn" + "github.com/keptn-contrib/dynatrace-service/internal/test" +) + +// Retrieving a dashboard by an invalid ID returns an error +// +// prerequisites: +// * we use +// * an invalid dashboard ID and Dynatrace API returns a 400 error, or +// * a valid, but not found dashboard ID and Dynatrace API returns a 404 +// * the event can have multiple indicators or none. (There is an SLO file in Keptn and the SLO files may contain indicators) +// +// We do not want to see the error attached to any indicator coming from SLO files, but attached to a "no metric" indicator +func TestThatInvalidDashboardIDProducesErrorMessageInNoMetricIndicatorEvenIfThereAreIndicators(t *testing.T) { + + type definition struct { + errorCode int + errorMessage string + dashboardID string + payload string + } + + invalidID := definition{ + errorCode: 400, + errorMessage: "Constraints violated", + dashboardID: "some-invalid-dashboard-id", + payload: "./testdata/sli_via_dashboard_test/dashboard_invalid_uuid_400.json", + } + idNotFound := definition{ + errorCode: 404, + errorMessage: "not found", + dashboardID: testDashboardID, + payload: "./testdata/sli_via_dashboard_test/dashboard_id_not_found_404.json", + } + + testConfigs := []struct { + name string + eventIndicators []string + def definition + }{ + { + name: "no indicators defined in event (SLO file) will produce single SLI result with name 'no metric'", + eventIndicators: []string{}, + def: invalidID, + }, + { + name: "one indicator defined in event (SLO file) will produce single SLI result with name 'no metric'", + eventIndicators: []string{"single-indicator-from-slo-file"}, + def: invalidID, + }, + { + name: "multiple indicators defined in event (SLO file) will produce single SLI result with name 'no metric'", + eventIndicators: []string{"first-indicator-from-slo-file", "second-indicator-from-slo-file", "third-indicator-from-slo-file"}, + def: invalidID, + }, + { + name: "no indicators defined in event (SLO file) will produce single SLI result with name 'no metric'", + eventIndicators: []string{}, + def: idNotFound, + }, + { + name: "one indicator defined in event (SLO file) will produce single SLI result with name 'no metric'", + eventIndicators: []string{"single-indicator-from-slo-file"}, + def: idNotFound, + }, + { + name: "multiple indicators defined in event (SLO file) will produce single SLI result with name 'no metric'", + eventIndicators: []string{"first-indicator-from-slo-file", "second-indicator-from-slo-file", "third-indicator-from-slo-file"}, + def: idNotFound, + }, + } + + for _, testConfig := range testConfigs { + tc := testConfig + t.Run(tc.name, func(t *testing.T) { + testEvent := &getSLIEventData{ + project: "sockshop", + stage: "staging", + service: "carts", + indicators: tc.eventIndicators, + sliStart: "", // use defaults here + sliEnd: "", // use defaults here + } + + handler := test.NewFileBasedURLHandler(t) + handler.AddExactError(dynatrace.DashboardsPath+"/"+tc.def.dashboardID, tc.def.errorCode, tc.def.payload) + + // sli and slo upload works + rClient := &uploadErrorResourceClientMock{t: t} + + getSLIFinishedEventAssertionsFunc := func(t *testing.T, actual *keptnv2.GetSLIFinishedEventData) { + assert.EqualValues(t, keptnv2.ResultFailed, actual.Result) + assert.Contains(t, actual.Message, tc.def.dashboardID) + assert.Contains(t, actual.Message, strconv.Itoa(tc.def.errorCode)) + assert.Contains(t, actual.Message, tc.def.errorMessage) + } + + runAndAssertDashboardTest(t, testEvent, handler, rClient, tc.def.dashboardID, getSLIFinishedEventAssertionsFunc, createFailedSLIResultAssertionsFunc(NoMetricIndicator)) + }) + } +} + +func runAndAssertDashboardTest(t *testing.T, getSLIEventData *getSLIEventData, handler http.Handler, rClient keptn.ResourceClientInterface, dashboardID string, getSLIFinishedEventAssertionsFunc func(t *testing.T, actual *keptnv2.GetSLIFinishedEventData), sliResultAssertionsFuncs ...func(t *testing.T, actual *keptnv2.SLIResult)) { + + // we do not need custom queries, as we are using the dashboard + kClient := &keptnClientMock{} + + runTestAndAssertNoError(t, getSLIEventData, handler, kClient, rClient, dashboardID) + + assertCorrectGetSLIEvents(t, kClient.eventSink, getSLIFinishedEventAssertionsFunc, sliResultAssertionsFuncs...) +} diff --git a/internal/sli/testdata/sli_via_dashboard_test/dashboard_id_not_found_404.json b/internal/sli/testdata/sli_via_dashboard_test/dashboard_id_not_found_404.json new file mode 100644 index 000000000..c97aec142 --- /dev/null +++ b/internal/sli/testdata/sli_via_dashboard_test/dashboard_id_not_found_404.json @@ -0,0 +1,6 @@ +{ + "error": { + "code": 404, + "message": "Dashboard e03f4be0-4712-4f12-96ee-8c486d001e9c not found" + } +} \ No newline at end of file diff --git a/internal/sli/testdata/sli_via_dashboard_test/dashboard_invalid_uuid_400.json b/internal/sli/testdata/sli_via_dashboard_test/dashboard_invalid_uuid_400.json new file mode 100644 index 000000000..387cd1823 --- /dev/null +++ b/internal/sli/testdata/sli_via_dashboard_test/dashboard_invalid_uuid_400.json @@ -0,0 +1,14 @@ +{ + "error": { + "code": 400, + "message": "Constraints violated.", + "constraintViolations": [ + { + "path": "id", + "message": "Not a valid UUID.", + "parameterLocation": "PATH", + "location": null + } + ] + } +} \ No newline at end of file