fix: Ensure USQL tile processing always produces an indicator (#710)

* Parse tile title first Signed-off-by: Arthur Pitman <[email protected]> * Move TileResult to own file Signed-off-by: Arthur Pitman <[email protected]> * Renaming file Signed-off-by: Arthur Pitman <[email protected]> * Introduce helper functions Signed-off-by: Arthur Pitman <[email protected]> * Use helper functions Signed-off-by: Arthur Pitman <[email protected]> * Use helper functions Signed-off-by: Arthur Pitman <[email protected]> * Use helper functions Signed-off-by: Arthur Pitman <[email protected]> * Use helper functions Signed-off-by: Arthur Pitman <[email protected]> * Remove redundant code Signed-off-by: Arthur Pitman <[email protected]> * Produce tile result on query error Signed-off-by: Arthur Pitman <[email protected]> * Remove redundant comments Signed-off-by: Arthur Pitman <[email protected]> * Create tile result in own function Signed-off-by: Arthur Pitman <[email protected]> * Process visualization types separately Signed-off-by: Arthur Pitman <[email protected]> * Introduce dashboard tile type constants Signed-off-by: Arthur Pitman <[email protected]> * Introduce visualization type constants Signed-off-by: Arthur Pitman <[email protected]> * Clean indicator name Signed-off-by: Arthur Pitman <[email protected]> * Add test Signed-off-by: Arthur Pitman <[email protected]> * Add test Signed-off-by: Arthur Pitman <[email protected]> * Add test Signed-off-by: Arthur Pitman <[email protected]> * Add test Signed-off-by: Arthur Pitman <[email protected]> * Add test Signed-off-by: Arthur Pitman <[email protected]> * Add test Signed-off-by: Arthur Pitman <[email protected]> * Add test Signed-off-by: Arthur Pitman <[email protected]> * Add test Signed-off-by: Arthur Pitman <[email protected]> * Add test Signed-off-by: Arthur Pitman <[email protected]> * Add test Signed-off-by: Arthur Pitman <[email protected]> * Add test Signed-off-by: Arthur Pitman <[email protected]> * Add tests Signed-off-by: Arthur Pitman <[email protected]> * Add comment to exported type Signed-off-by: Arthur Pitman <[email protected]>
keptn-contrib · Feb 18, 2022 · b3ca3d1 · b3ca3d1
1 parent d8c29cf
commit b3ca3d1
Show file tree

Hide file tree

Showing 37 changed files with 1,187 additions and 157 deletions.
diff --git a/internal/dynatrace/dashboard.go b/internal/dynatrace/dashboard.go
@@ -1,5 +1,39 @@
 package dynatrace
 
+const (
+	// CustomChartingTileType is the tile type for custom charting dashboard tiles
+	CustomChartingTileType = "CUSTOM_CHARTING"
+
+	// DataExplorerTileType is the tile type for data explorer dashboard tiles
+	DataExplorerTileType = "DATA_EXPLORER"
+
+	// MarkdownTileType is the tile type for markdown dashboard tiles
+	MarkdownTileType = "MARKDOWN"
+
+	// OpenProblemsTileType is the tile type for open problems dashboard tiles
+	OpenProblemsTileType = "OPEN_PROBLEMS"
+
+	// SLOTileType is the tile type for SLO dashboard tiles
+	SLOTileType = "SLO"
+
+	// USQLTileType is the tile type for USQL dashboard tiles
+	USQLTileType = "DTAQL"
+)
+
+const (
+	// ColumnChartVisualizationType is the column chart visualization type for USQL tiles
+	ColumnChartVisualizationType = "COLUMN_CHART"
+
+	// PieChartVisualizationType is the pie chart visualization type for USQL tiles
+	PieChartVisualizationType = "PIE_CHART"
+
+	// SingleValueVisualizationType is the single value visualization type for USQL tiles
+	SingleValueVisualizationType = "SINGLE_VALUE"
+
+	// TableVisualizationType is the table visualization type for USQL tiles
+	TableVisualizationType = "TABLE"
+)
+
 type Dashboard struct {
 	Metadata          *Metadata         `json:"metadata,omitempty"`
 	ID                string            `json:"id,omitempty"`

diff --git a/internal/sli/dashboard/custom_charting_tile_processing.go b/internal/sli/dashboard/custom_charting_tile_processing.go
@@ -55,11 +55,13 @@ func (p *CustomChartingTileProcessing) Process(tile *dynatrace.Tile, dashboardFi
 	tileManagementZoneFilter := NewManagementZoneFilter(dashboardFilter, tile.TileFilter.ManagementZone)
 
 	if tile.FilterConfig == nil {
-		return createFailedTileResultFromSLODefinition(sloDefinition, "Custom charting tile is missing a filterConfig element")
+		unsuccessfulTileResult := newUnsuccessfulTileResultFromSLODefinition(sloDefinition, "Custom charting tile is missing a filterConfig element")
+		return []*TileResult{&unsuccessfulTileResult}
 	}
 
 	if len(tile.FilterConfig.ChartConfig.Series) != 1 {
-		return createFailedTileResultFromSLODefinition(sloDefinition, "Custom charting tile must have exactly one series")
+		unsuccessfulTileResult := newUnsuccessfulTileResultFromSLODefinition(sloDefinition, "Custom charting tile must have exactly one series")
+		return []*TileResult{&unsuccessfulTileResult}
 	}
 
 	return p.processSeries(sloDefinition, &tile.FilterConfig.ChartConfig.Series[0], tileManagementZoneFilter, tile.FilterConfig.FiltersPerEntityType)
@@ -71,7 +73,8 @@ func (p *CustomChartingTileProcessing) processSeries(sloDefinition *keptnapi.SLO
 
 	if err != nil {
 		log.WithError(err).Warn("generateMetricQueryFromChart returned an error, SLI will not be used")
-		return createFailedTileResultFromSLODefinition(sloDefinition, "Custom charting tile could not be converted to a metric query: "+err.Error())
+		unsuccessfulTileResult := newUnsuccessfulTileResultFromSLODefinition(sloDefinition, "Custom charting tile could not be converted to a metric query: "+err.Error())
+		return []*TileResult{&unsuccessfulTileResult}
 	}
 
 	return NewMetricsQueryProcessing(p.client).Process(len(series.Dimensions), sloDefinition, metricQuery)

diff --git a/internal/sli/dashboard/dashboard_processing.go b/internal/sli/dashboard/dashboard_processing.go
@@ -76,30 +76,30 @@ func (p *Processing) Process(dashboard *dynatrace.Dashboard) *QueryResult {
 	// now lets iterate through the dashboard to find our SLIs
 	for _, tile := range dashboard.Tiles {
 		switch tile.TileType {
-		case "MARKDOWN":
+		case dynatrace.MarkdownTileType:
 			score, comparison := NewMarkdownTileProcessing().Process(&tile, createDefaultSLOScore(), createDefaultSLOComparison())
 			if score != nil && comparison != nil {
 				result.slo.TotalScore = score
 				result.slo.Comparison = comparison
 			}
-		case "SLO":
+		case dynatrace.SLOTileType:
 			tileResults := NewSLOTileProcessing(p.client, p.startUnix, p.endUnix).Process(&tile)
 			result.addTileResults(tileResults)
-		case "OPEN_PROBLEMS":
+		case dynatrace.OpenProblemsTileType:
 			tileResult := NewProblemTileProcessing(p.client, p.startUnix, p.endUnix).Process(&tile, dashboard.GetFilter())
 			result.addTileResult(tileResult)
 
 			// current logic also does security tile processing for open problem tiles
 			tileResult = NewSecurityProblemTileProcessing(p.client, p.startUnix, p.endUnix).Process(&tile, dashboard.GetFilter())
 			result.addTileResult(tileResult)
-		case "DATA_EXPLORER":
+		case dynatrace.DataExplorerTileType:
 			// here we handle the new Metric Data Explorer Tile
 			tileResults := NewDataExplorerTileProcessing(p.client, p.eventData, p.customFilters, p.startUnix, p.endUnix).Process(&tile, dashboard.GetFilter())
 			result.addTileResults(tileResults)
-		case "CUSTOM_CHARTING":
+		case dynatrace.CustomChartingTileType:
 			tileResults := NewCustomChartingTileProcessing(p.client, p.eventData, p.customFilters, p.startUnix, p.endUnix).Process(&tile, dashboard.GetFilter())
 			result.addTileResults(tileResults)
-		case "DTAQL":
+		case dynatrace.USQLTileType:
 			tileResults := NewUSQLTileProcessing(p.client, p.eventData, p.customFilters, p.startUnix, p.endUnix).Process(&tile)
 			result.addTileResults(tileResults)
 		default:

diff --git a/internal/sli/dashboard/data_explorer_tile_processing.go b/internal/sli/dashboard/data_explorer_tile_processing.go
@@ -41,7 +41,8 @@ func (p *DataExplorerTileProcessing) Process(tile *dynatrace.Tile, dashboardFilt
 	}
 
 	if len(tile.Queries) != 1 {
-		return createFailedTileResultFromSLODefinition(sloDefinition, "Data Explorer tile must have exactly one query")
+		unsuccessfulTileResult := newUnsuccessfulTileResultFromSLODefinition(sloDefinition, "Data Explorer tile must have exactly one query")
+		return []*TileResult{&unsuccessfulTileResult}
 	}
 
 	// get the tile specific management zone filter that might be needed by different tile processors
@@ -57,7 +58,8 @@ func (p *DataExplorerTileProcessing) processQuery(sloDefinition *keptnapi.SLO, d
 	metricQuery, err := p.generateMetricQueryFromDataExplorerQuery(dataQuery, managementZoneFilter)
 	if err != nil {
 		log.WithError(err).Warn("generateMetricQueryFromDataExplorerQuery returned an error, SLI will not be used")
-		return createFailedTileResultFromSLODefinition(sloDefinition, "Data Explorer tile could not be converted to a metric query: "+err.Error())
+		unsuccessfulTileResult := newUnsuccessfulTileResultFromSLODefinition(sloDefinition, "Data Explorer tile could not be converted to a metric query: "+err.Error())
+		return []*TileResult{&unsuccessfulTileResult}
 	}
 
 	return NewMetricsQueryProcessing(p.client).Process(len(dataQuery.SplitBy), sloDefinition, metricQuery)
@@ -231,5 +233,4 @@ func getSpaceAggregationTransformation(spaceAggregation string) (string, error)
 	default:
 		return "", fmt.Errorf("unknown space aggregation: %s", spaceAggregation)
 	}
-
 }
diff --git a/internal/sli/dashboard/metrics_query_processing.go b/internal/sli/dashboard/metrics_query_processing.go
@@ -34,16 +34,19 @@ func (r *MetricsQueryProcessing) Process(noOfDimensionsInChart int, sloDefinitio
 	// ERROR-CASE: Metric API return no values or an error
 	// we could not query data - so - we return the error back as part of our SLIResults
 	if err != nil {
-		return createFailedTileResultFromSLODefinitionAndMetricsQuery(sloDefinition, metricQueryComponents.metricsQuery, "Error querying Metrics API: "+err.Error())
+		unsuccessfulTileResult := newUnsuccessfulTileResultFromSLODefinitionAndSLIQuery(sloDefinition, v1metrics.NewQueryProducer(metricQueryComponents.metricsQuery).Produce(), "Error querying Metrics API: "+err.Error())
+		return []*TileResult{&unsuccessfulTileResult}
 	}
 
 	// TODO 2021-10-12: Check if having a query result with zero results is even plausable
 	if len(queryResult.Result) == 0 {
-		return createFailedTileResultFromSLODefinitionAndMetricsQuery(sloDefinition, metricQueryComponents.metricsQuery, "Expected a single result but got no result for metric ID")
+		unsuccessfulTileResult := newUnsuccessfulTileResultFromSLODefinitionAndSLIQuery(sloDefinition, v1metrics.NewQueryProducer(metricQueryComponents.metricsQuery).Produce(), "Expected a single result but got no result for metric ID")
+		return []*TileResult{&unsuccessfulTileResult}
 	}
 
 	if len(queryResult.Result) > 1 {
-		return createFailedTileResultFromSLODefinitionAndMetricsQuery(sloDefinition, metricQueryComponents.metricsQuery, "Expected a result only for a single metric ID but got multiple results")
+		unsuccessfulTileResult := newUnsuccessfulTileResultFromSLODefinitionAndSLIQuery(sloDefinition, v1metrics.NewQueryProducer(metricQueryComponents.metricsQuery).Produce(), "Expected a result only for a single metric ID but got multiple results")
+		return []*TileResult{&unsuccessfulTileResult}
 	}
 
 	var tileResults []*TileResult
@@ -59,7 +62,8 @@ func (r *MetricsQueryProcessing) Process(noOfDimensionsInChart int, sloDefinitio
 
 	dataResultCount := len(singleResult.Data)
 	if dataResultCount == 0 {
-		return createFailedTileResultFromSLODefinitionAndMetricsQuery(sloDefinition, metricQueryComponents.metricsQuery, "Metrics query result has no data")
+		unsuccessfulTileResult := newUnsuccessfulTileResultFromSLODefinitionAndSLIQuery(sloDefinition, v1metrics.NewQueryProducer(metricQueryComponents.metricsQuery).Produce(), "Metrics query result has no data")
+		return []*TileResult{&unsuccessfulTileResult}
 	}
 
 	for _, singleDataEntry := range singleResult.Data {
@@ -104,7 +108,8 @@ func (r *MetricsQueryProcessing) Process(noOfDimensionsInChart int, sloDefinitio
 
 		metricQueryForSLI, err := metrics.NewQuery(metricSelectorForSLI, entitySelectorForSLI)
 		if err != nil {
-			return createFailedTileResultFromSLODefinitionAndMetricsQuery(sloDefinition, metricQueryComponents.metricsQuery, "Could not create metrics query for SLI")
+			unsuccessfulTileResult := newUnsuccessfulTileResultFromSLODefinitionAndSLIQuery(sloDefinition, v1metrics.NewQueryProducer(metricQueryComponents.metricsQuery).Produce(), "Could not create metrics query for SLI")
+			return []*TileResult{&unsuccessfulTileResult}
 		}
 
 		// make sure we have a valid indicator name by getting rid of special characters
@@ -161,35 +166,3 @@ func getMetricsQueryString(unit string, query metrics.Query) string {
 
 	return v1metrics.NewQueryProducer(query).Produce()
 }
-
-func createFailedTileResultFromSLODefinition(sloDefinition *keptncommon.SLO, message string) []*TileResult {
-	return []*TileResult{
-		{
-			sliResult: &keptnv2.SLIResult{
-				Metric:  sloDefinition.SLI,
-				Value:   0,
-				Success: false,
-				Message: message,
-			},
-			objective: sloDefinition,
-			sliName:   sloDefinition.SLI,
-		},
-	}
-}
-
-func createFailedTileResultFromSLODefinitionAndMetricsQuery(sloDefinition *keptncommon.SLO, metricsQuery metrics.Query, message string) []*TileResult {
-	metricsQueryString := v1metrics.NewQueryProducer(metricsQuery).Produce()
-	return []*TileResult{
-		{
-			sliResult: &keptnv2.SLIResult{
-				Metric:  sloDefinition.SLI,
-				Value:   0,
-				Success: false,
-				Message: message,
-			},
-			objective: sloDefinition,
-			sliName:   sloDefinition.SLI,
-			sliQuery:  metricsQueryString,
-		},
-	}
-}
diff --git a/internal/sli/dashboard/dashboard_query.go → internal/sli/dashboard/query_result.go b/internal/sli/dashboard/dashboard_query.go → internal/sli/dashboard/query_result.go
@@ -7,13 +7,6 @@ import (
 	"github.com/keptn-contrib/dynatrace-service/internal/dynatrace"
 )
 
-type TileResult struct {
-	sliResult *keptnv2.SLIResult
-	objective *keptnapi.SLO
-	sliName   string
-	sliQuery  string
-}
-
 // QueryResult is the object returned by querying a Dynatrace dashboard for SLIs
 type QueryResult struct {
 	dashboardLink *DashboardLink

diff --git a/internal/sli/dashboard/slo_tile_processing.go b/internal/sli/dashboard/slo_tile_processing.go
@@ -28,15 +28,8 @@ func NewSLOTileProcessing(client dynatrace.ClientInterface, startUnix time.Time,
 
 func (p *SLOTileProcessing) Process(tile *dynatrace.Tile) []*TileResult {
 	if len(tile.AssignedEntities) == 0 {
-		indicatorName := "slo_tile_without_slo"
-		return []*TileResult{&TileResult{
-			sliResult: &keptnv2.SLIResult{
-				Metric:  indicatorName,
-				Success: false,
-				Message: "SLO tile contains no SLO IDs",
-			},
-			sliName: indicatorName,
-		}}
+		unsuccessfulTileResult := newUnsuccessfulTileResult("slo_tile_without_slo", "SLO tile contains no SLO IDs")
+		return []*TileResult{&unsuccessfulTileResult}
 	}
 
 	var results []*TileResult
@@ -53,29 +46,15 @@ func (p *SLOTileProcessing) processSLO(sloID string, startUnix time.Time, endUni
 	query, err := slo.NewQuery(sloID)
 	if err != nil {
 		// TODO: 2021-02-14: Check that this indicator name still aligns with all possible errors.
-		indicatorName := "slo_without_id"
-		return &TileResult{
-			sliResult: &keptnv2.SLIResult{
-				Metric:  indicatorName,
-				Success: false,
-				Message: err.Error(),
-			},
-			sliName: indicatorName,
-		}
+		unsuccessfulTileResult := newUnsuccessfulTileResult("slo_without_id", err.Error())
+		return &unsuccessfulTileResult
 	}
 
 	// Step 1: Query the Dynatrace API to get the actual value for this sloID
 	sloResult, err := dynatrace.NewSLOClient(p.client).Get(dynatrace.NewSLOClientGetParameters(query.GetSLOID(), startUnix, endUnix))
 	if err != nil {
-		indicatorName := common.CleanIndicatorName("slo_" + sloID)
-		return &TileResult{
-			sliResult: &keptnv2.SLIResult{
-				Metric:  indicatorName,
-				Success: false,
-				Message: err.Error(),
-			},
-			sliName: indicatorName,
-		}
+		unsuccessfulTileResult := newUnsuccessfulTileResult(common.CleanIndicatorName("slo_"+sloID), err.Error())
+		return &unsuccessfulTileResult
 	}
 
 	// Step 2: Transform the SLO result into an SLI result and SLO definition

diff --git a/internal/sli/dashboard/tile_result.go b/internal/sli/dashboard/tile_result.go
@@ -0,0 +1,52 @@
+package dashboard
+
+import (
+	keptnapi "github.com/keptn/go-utils/pkg/lib"
+	keptnv2 "github.com/keptn/go-utils/pkg/lib/v0_2_0"
+)
+
+// TileResult stores the result of processing a dashboard tile and retrieving the SLIResult.
+type TileResult struct {
+	sliResult *keptnv2.SLIResult
+	objective *keptnapi.SLO
+	sliName   string
+	sliQuery  string
+}
+
+func newUnsuccessfulTileResult(indicatorName string, message string) TileResult {
+	return TileResult{
+		sliResult: &keptnv2.SLIResult{
+			Metric:  indicatorName,
+			Success: false,
+			Message: message,
+		},
+		sliName: indicatorName,
+	}
+}
+
+func newUnsuccessfulTileResultFromSLODefinition(sloDefinition *keptnapi.SLO, message string) TileResult {
+	return TileResult{
+		sliResult: &keptnv2.SLIResult{
+			Metric:  sloDefinition.SLI,
+			Value:   0,
+			Success: false,
+			Message: message,
+		},
+		objective: sloDefinition,
+		sliName:   sloDefinition.SLI,
+	}
+}
+
+func newUnsuccessfulTileResultFromSLODefinitionAndSLIQuery(sloDefinition *keptnapi.SLO, sliQuery string, message string) TileResult {
+	return TileResult{
+		sliResult: &keptnv2.SLIResult{
+			Metric:  sloDefinition.SLI,
+			Value:   0,
+			Success: false,
+			Message: message,
+		},
+		objective: sloDefinition,
+		sliName:   sloDefinition.SLI,
+		sliQuery:  sliQuery,
+	}
+}