diff --git a/pkg/tests/api/graphite/graphite_test.go b/pkg/tests/api/graphite/graphite_test.go index 29949eb535b..9dd44d47f72 100644 --- a/pkg/tests/api/graphite/graphite_test.go +++ b/pkg/tests/api/graphite/graphite_test.go @@ -85,7 +85,7 @@ func TestIntegrationGraphite(t *testing.T) { // nolint:gosec resp, err := http.Post(u, "application/json", buf1) require.NoError(t, err) - require.Equal(t, http.StatusInternalServerError, resp.StatusCode) + require.Equal(t, http.StatusBadRequest, resp.StatusCode) t.Cleanup(func() { err := resp.Body.Close() require.NoError(t, err) diff --git a/pkg/tsdb/graphite/graphite.go b/pkg/tsdb/graphite/graphite.go index 512598da1b7..2a7ffb13a91 100644 --- a/pkg/tsdb/graphite/graphite.go +++ b/pkg/tsdb/graphite/graphite.go @@ -100,7 +100,7 @@ func (s *Service) CallResource(ctx context.Context, req *backend.CallResourceReq func (s *Service) createRequest(ctx context.Context, dsInfo *datasourceInfo, params URLParams) (*http.Request, error) { u, err := url.Parse(dsInfo.URL) if err != nil { - return nil, err + return nil, backend.DownstreamError(err) } if params.SubPath != "" { @@ -125,7 +125,7 @@ func (s *Service) createRequest(ctx context.Context, dsInfo *datasourceInfo, par req, err := http.NewRequestWithContext(ctx, method, u.String(), params.Body) if err != nil { s.logger.Info("Failed to create request", "error", err) - return nil, fmt.Errorf("failed to create request: %w", err) + return nil, backend.PluginError(fmt.Errorf("failed to create request: %w", err)) } for k, v := range params.Headers { diff --git a/pkg/tsdb/graphite/healthcheck_test.go b/pkg/tsdb/graphite/healthcheck_test.go index 8c19d809ea0..98df2453b59 100644 --- a/pkg/tsdb/graphite/healthcheck_test.go +++ b/pkg/tsdb/graphite/healthcheck_test.go @@ -42,7 +42,7 @@ func (rt *healthCheckFailRoundTripper) RoundTrip(req *http.Request) (*http.Respo Status: "400", StatusCode: 400, Header: nil, - Body: nil, + Body: io.NopCloser(strings.NewReader("this is a failed healthcheck")), ContentLength: 0, Request: req, }, nil @@ -107,7 +107,7 @@ func Test_CheckHealth(t *testing.T) { assert.NoError(t, err) assert.Equal(t, backend.HealthStatusError, res.Status) assert.Equal(t, "Graphite health check failed. See details below", res.Message) - assert.Equal(t, []byte("{\"verboseMessage\": \"request failed, status: 400\" }"), res.JSONDetails) + assert.Equal(t, []byte("{\"verboseMessage\": \"request failed with error: this is a failed healthcheck\" }"), res.JSONDetails) }) } diff --git a/pkg/tsdb/graphite/query.go b/pkg/tsdb/graphite/query.go index 460ac2cfba5..6f4d2bca083 100644 --- a/pkg/tsdb/graphite/query.go +++ b/pkg/tsdb/graphite/query.go @@ -16,8 +16,10 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/backend/tracing" "github.com/grafana/grafana-plugin-sdk-go/data" + "github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" + "golang.org/x/net/html" ) func (s *Service) RunQuery(ctx context.Context, req *backend.QueryDataRequest, dsInfo *datasourceInfo) (*backend.QueryDataResponse, error) { @@ -26,10 +28,13 @@ func (s *Service) RunQuery(ctx context.Context, req *backend.QueryDataRequest, d req *http.Request formData url.Values }{} + result := backend.NewQueryDataResponse() + for _, query := range req.Queries { graphiteReq, formData, emptyQuery, err := s.createGraphiteRequest(ctx, query, dsInfo) if err != nil { - return nil, err + result.Responses[query.RefID] = backend.ErrorResponseWithErrorSource(err) + return result, nil } if emptyQuery != nil { @@ -46,7 +51,6 @@ func (s *Service) RunQuery(ctx context.Context, req *backend.QueryDataRequest, d } } - var result = backend.QueryDataResponse{} if len(emptyQueries) != 0 { s.logger.Warn("Found query models without targets", "models without targets", strings.Join(emptyQueries, "\n")) // If no queries had a valid target, return an error; otherwise, attempt with the targets we have @@ -56,9 +60,9 @@ func (s *Service) RunQuery(ctx context.Context, req *backend.QueryDataRequest, d } // marking this downstream error as it is a user error, but arguably this is a plugin error // since the plugin should have frontend validation that prevents us from getting into this state - missingQueryResponse := backend.ErrDataResponseWithSource(400, backend.ErrorSourceDownstream, "no query target found for the alert rule") + missingQueryResponse := backend.ErrDataResponseWithSource(400, backend.ErrorSourceDownstream, "no query target found") result.Responses["A"] = missingQueryResponse - return &result, nil + return result, nil } } @@ -83,7 +87,8 @@ func (s *Service) RunQuery(ctx context.Context, req *backend.QueryDataRequest, d if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) - return &result, err + result.Responses[refId] = backend.ErrorResponseWithErrorSource(backend.DownstreamError(err)) + return result, nil } defer func() { @@ -97,16 +102,13 @@ func (s *Service) RunQuery(ctx context.Context, req *backend.QueryDataRequest, d if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) - return &result, err + result.Responses[refId] = backend.ErrorResponseWithErrorSource(err) + return result, nil } frames = append(frames, queryFrames...) } - result = backend.QueryDataResponse{ - Responses: make(backend.Responses), - } - for _, f := range frames { if resp, ok := result.Responses[f.Name]; ok { resp.Frames = append(resp.Frames, f) @@ -118,7 +120,7 @@ func (s *Service) RunQuery(ctx context.Context, req *backend.QueryDataRequest, d } } - return &result, nil + return result, nil } // processQuery converts a Graphite data source query to a Graphite query target. It returns the target, @@ -127,7 +129,7 @@ func (s *Service) processQuery(query backend.DataQuery) (string, *GraphiteQuery, queryJSON := GraphiteQuery{} err := json.Unmarshal(query.JSON, &queryJSON) if err != nil { - return "", &queryJSON, false, fmt.Errorf("failed to decode the Graphite query: %w", err) + return "", &queryJSON, false, backend.PluginError(fmt.Errorf("failed to decode the Graphite query: %w", err)) } s.logger.Debug("Graphite", "query", queryJSON) currTarget := queryJSON.TargetFull @@ -237,7 +239,7 @@ func (s *Service) toDataFrames(response *http.Response, refId string) (frames da func (s *Service) parseResponse(res *http.Response) ([]TargetResponseDTO, error) { body, err := io.ReadAll(res.Body) if err != nil { - return nil, err + return nil, backend.DownstreamError(err) } defer func() { if err := res.Body.Close(); err != nil { @@ -246,20 +248,50 @@ func (s *Service) parseResponse(res *http.Response) ([]TargetResponseDTO, error) }() if res.StatusCode/100 != 2 { - s.logger.Info("Request failed", "status", res.Status, "body", string(body)) - return nil, fmt.Errorf("request failed, status: %s", res.Status) + graphiteError := parseGraphiteError(res.StatusCode, string(body)) + s.logger.Info("Request failed", "status", res.Status, "error", graphiteError, "body", string(body)) + return nil, errorsource.SourceError(backend.ErrorSourceFromHTTPStatus(res.StatusCode), fmt.Errorf("request failed with error: %s", graphiteError), false) } var data []TargetResponseDTO err = json.Unmarshal(body, &data) if err != nil { s.logger.Info("Failed to unmarshal graphite response", "error", err, "status", res.Status, "body", string(body)) - return nil, err + return nil, backend.DownstreamError(err) } return data, nil } +/** + * Duplicated from the frontend. + * Graphite-web before v1.6 returns HTTP 500 with full stack traces in an HTML page + * when a query fails. It results in massive error alerts with HTML tags in the UI. + * This function removes all HTML tags and keeps only the last line from the stack + * trace which should be the most meaningful. + */ +func parseGraphiteError(status int, body string) (errorMsg string) { + errorMsg = body + if status == http.StatusInternalServerError { + if strings.HasPrefix(body, "
+The server encountered an unexpected condition that prevented it from fulfilling the request.
+Target not found
`, + expected: "Internal Server Error\nTarget not found", + }, + { + name: "complex HTML error", + status: 500, + body: ` +The server encountered an unexpected condition that prevented it from fulfilling the request.
+Error: Invalid path 'test' and "value"
`, + expected: "Error: Invalid path 'test' and \"value\"", + }, + { + name: "HTML with whitespace and newlines", + status: 500, + body: ` + +Something went wrong
+ + Critical failure occurred + + `, + expected: "Error\nSomething went wrong\nCritical failure occurred", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := parseGraphiteError(tt.status, tt.body) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/pkg/tsdb/graphite/testdata/interval_format_transformation-RefID-A.golden.jsonc b/pkg/tsdb/graphite/testdata/interval_format_transformation-RefID-A.golden.jsonc new file mode 100644 index 00000000000..39bda73cd9d --- /dev/null +++ b/pkg/tsdb/graphite/testdata/interval_format_transformation-RefID-A.golden.jsonc @@ -0,0 +1,72 @@ +// 🌟 This was machine generated. Do not edit. 🌟 +// +// Frame[0] { +// "type": "timeseries-multi", +// "typeVersion": [ +// 0, +// 0 +// ] +// } +// Name: A +// Dimensions: 2 Fields by 2 Rows +// +-------------------------------+------------------+ +// | Name: time | Name: value | +// | Labels: | Labels: | +// | Type: []time.Time | Type: []*float64 | +// +-------------------------------+------------------+ +// | 2021-01-01 00:00:00 +0000 UTC | 100 | +// | 2021-01-01 00:01:00 +0000 UTC | 150 | +// +-------------------------------+------------------+ +// +// +// 🌟 This was machine generated. Do not edit. 🌟 +{ + "status": 200, + "frames": [ + { + "schema": { + "name": "A", + "meta": { + "type": "timeseries-multi", + "typeVersion": [ + 0, + 0 + ] + }, + "fields": [ + { + "name": "time", + "type": "time", + "typeInfo": { + "frame": "time.Time" + } + }, + { + "name": "value", + "type": "number", + "typeInfo": { + "frame": "float64", + "nullable": true + }, + "labels": {}, + "config": { + "displayNameFromDS": "hitcount(stats.counters.web.hits, '1min')" + } + } + ] + }, + "data": { + "values": [ + [ + 1609459200000, + 1609459260000 + ], + [ + 100, + 150 + ] + ] + } + } + ] +} \ No newline at end of file diff --git a/pkg/tsdb/graphite/testdata/mixed_queries_-_some_empty,_some_valid-RefID-B.golden.jsonc b/pkg/tsdb/graphite/testdata/mixed_queries_-_some_empty,_some_valid-RefID-B.golden.jsonc new file mode 100644 index 00000000000..455803dddc2 --- /dev/null +++ b/pkg/tsdb/graphite/testdata/mixed_queries_-_some_empty,_some_valid-RefID-B.golden.jsonc @@ -0,0 +1,72 @@ +// 🌟 This was machine generated. Do not edit. 🌟 +// +// Frame[0] { +// "type": "timeseries-multi", +// "typeVersion": [ +// 0, +// 0 +// ] +// } +// Name: B +// Dimensions: 2 Fields by 2 Rows +// +-------------------------------+------------------+ +// | Name: time | Name: value | +// | Labels: | Labels: | +// | Type: []time.Time | Type: []*float64 | +// +-------------------------------+------------------+ +// | 2021-01-01 00:00:00 +0000 UTC | 100 | +// | 2021-01-01 00:01:00 +0000 UTC | 150 | +// +-------------------------------+------------------+ +// +// +// 🌟 This was machine generated. Do not edit. 🌟 +{ + "status": 200, + "frames": [ + { + "schema": { + "name": "B", + "meta": { + "type": "timeseries-multi", + "typeVersion": [ + 0, + 0 + ] + }, + "fields": [ + { + "name": "time", + "type": "time", + "typeInfo": { + "frame": "time.Time" + } + }, + { + "name": "value", + "type": "number", + "typeInfo": { + "frame": "float64", + "nullable": true + }, + "labels": {}, + "config": { + "displayNameFromDS": "stats.counters.web.hits" + } + } + ] + }, + "data": { + "values": [ + [ + 1609459200000, + 1609459260000 + ], + [ + 100, + 150 + ] + ] + } + } + ] +} \ No newline at end of file diff --git a/pkg/tsdb/graphite/testdata/successful_multiple_queries-RefID-A.golden.jsonc b/pkg/tsdb/graphite/testdata/successful_multiple_queries-RefID-A.golden.jsonc new file mode 100644 index 00000000000..b8ef382179e --- /dev/null +++ b/pkg/tsdb/graphite/testdata/successful_multiple_queries-RefID-A.golden.jsonc @@ -0,0 +1,72 @@ +// 🌟 This was machine generated. Do not edit. 🌟 +// +// Frame[0] { +// "type": "timeseries-multi", +// "typeVersion": [ +// 0, +// 0 +// ] +// } +// Name: A +// Dimensions: 2 Fields by 2 Rows +// +-------------------------------+------------------+ +// | Name: time | Name: value | +// | Labels: | Labels: | +// | Type: []time.Time | Type: []*float64 | +// +-------------------------------+------------------+ +// | 2021-01-01 00:00:00 +0000 UTC | 100 | +// | 2021-01-01 00:01:00 +0000 UTC | 150 | +// +-------------------------------+------------------+ +// +// +// 🌟 This was machine generated. Do not edit. 🌟 +{ + "status": 200, + "frames": [ + { + "schema": { + "name": "A", + "meta": { + "type": "timeseries-multi", + "typeVersion": [ + 0, + 0 + ] + }, + "fields": [ + { + "name": "time", + "type": "time", + "typeInfo": { + "frame": "time.Time" + } + }, + { + "name": "value", + "type": "number", + "typeInfo": { + "frame": "float64", + "nullable": true + }, + "labels": {}, + "config": { + "displayNameFromDS": "stats.counters.web.hits" + } + } + ] + }, + "data": { + "values": [ + [ + 1609459200000, + 1609459260000 + ], + [ + 100, + 150 + ] + ] + } + } + ] +} \ No newline at end of file diff --git a/pkg/tsdb/graphite/testdata/successful_multiple_queries-RefID-B.golden.jsonc b/pkg/tsdb/graphite/testdata/successful_multiple_queries-RefID-B.golden.jsonc new file mode 100644 index 00000000000..3d208e886ee --- /dev/null +++ b/pkg/tsdb/graphite/testdata/successful_multiple_queries-RefID-B.golden.jsonc @@ -0,0 +1,72 @@ +// 🌟 This was machine generated. Do not edit. 🌟 +// +// Frame[0] { +// "type": "timeseries-multi", +// "typeVersion": [ +// 0, +// 0 +// ] +// } +// Name: B +// Dimensions: 2 Fields by 2 Rows +// +-------------------------------+------------------+ +// | Name: time | Name: value | +// | Labels: | Labels: | +// | Type: []time.Time | Type: []*float64 | +// +-------------------------------+------------------+ +// | 2021-01-01 00:00:00 +0000 UTC | 50 | +// | 2021-01-01 00:01:00 +0000 UTC | 75 | +// +-------------------------------+------------------+ +// +// +// 🌟 This was machine generated. Do not edit. 🌟 +{ + "status": 200, + "frames": [ + { + "schema": { + "name": "B", + "meta": { + "type": "timeseries-multi", + "typeVersion": [ + 0, + 0 + ] + }, + "fields": [ + { + "name": "time", + "type": "time", + "typeInfo": { + "frame": "time.Time" + } + }, + { + "name": "value", + "type": "number", + "typeInfo": { + "frame": "float64", + "nullable": true + }, + "labels": {}, + "config": { + "displayNameFromDS": "stats.counters.api.calls" + } + } + ] + }, + "data": { + "values": [ + [ + 1609459200000, + 1609459260000 + ], + [ + 50, + 75 + ] + ] + } + } + ] +} \ No newline at end of file diff --git a/pkg/tsdb/graphite/testdata/successful_single_query_with_data-RefID-A.golden.jsonc b/pkg/tsdb/graphite/testdata/successful_single_query_with_data-RefID-A.golden.jsonc new file mode 100644 index 00000000000..5c200551f5b --- /dev/null +++ b/pkg/tsdb/graphite/testdata/successful_single_query_with_data-RefID-A.golden.jsonc @@ -0,0 +1,75 @@ +// 🌟 This was machine generated. Do not edit. 🌟 +// +// Frame[0] { +// "type": "timeseries-multi", +// "typeVersion": [ +// 0, +// 0 +// ] +// } +// Name: A +// Dimensions: 2 Fields by 3 Rows +// +-------------------------------+------------------+ +// | Name: time | Name: value | +// | Labels: | Labels: | +// | Type: []time.Time | Type: []*float64 | +// +-------------------------------+------------------+ +// | 2021-01-01 00:00:00 +0000 UTC | 100 | +// | 2021-01-01 00:01:00 +0000 UTC | 150 | +// | 2021-01-01 00:02:00 +0000 UTC | 120 | +// +-------------------------------+------------------+ +// +// +// 🌟 This was machine generated. Do not edit. 🌟 +{ + "status": 200, + "frames": [ + { + "schema": { + "name": "A", + "meta": { + "type": "timeseries-multi", + "typeVersion": [ + 0, + 0 + ] + }, + "fields": [ + { + "name": "time", + "type": "time", + "typeInfo": { + "frame": "time.Time" + } + }, + { + "name": "value", + "type": "number", + "typeInfo": { + "frame": "float64", + "nullable": true + }, + "labels": {}, + "config": { + "displayNameFromDS": "stats.counters.web.hits" + } + } + ] + }, + "data": { + "values": [ + [ + 1609459200000, + 1609459260000, + 1609459320000 + ], + [ + 100, + 150, + 120 + ] + ] + } + } + ] +} \ No newline at end of file diff --git a/pkg/tsdb/graphite/testdata/successful_single_query_with_null_values-RefID-A.golden.jsonc b/pkg/tsdb/graphite/testdata/successful_single_query_with_null_values-RefID-A.golden.jsonc new file mode 100644 index 00000000000..fc00359a0ca --- /dev/null +++ b/pkg/tsdb/graphite/testdata/successful_single_query_with_null_values-RefID-A.golden.jsonc @@ -0,0 +1,75 @@ +// 🌟 This was machine generated. Do not edit. 🌟 +// +// Frame[0] { +// "type": "timeseries-multi", +// "typeVersion": [ +// 0, +// 0 +// ] +// } +// Name: A +// Dimensions: 2 Fields by 3 Rows +// +-------------------------------+------------------+ +// | Name: time | Name: value | +// | Labels: | Labels: | +// | Type: []time.Time | Type: []*float64 | +// +-------------------------------+------------------+ +// | 2021-01-01 00:00:00 +0000 UTC | 100 | +// | 2021-01-01 00:01:00 +0000 UTC | null | +// | 2021-01-01 00:02:00 +0000 UTC | 120 | +// +-------------------------------+------------------+ +// +// +// 🌟 This was machine generated. Do not edit. 🌟 +{ + "status": 200, + "frames": [ + { + "schema": { + "name": "A", + "meta": { + "type": "timeseries-multi", + "typeVersion": [ + 0, + 0 + ] + }, + "fields": [ + { + "name": "time", + "type": "time", + "typeInfo": { + "frame": "time.Time" + } + }, + { + "name": "value", + "type": "number", + "typeInfo": { + "frame": "float64", + "nullable": true + }, + "labels": {}, + "config": { + "displayNameFromDS": "stats.counters.web.hits" + } + } + ] + }, + "data": { + "values": [ + [ + 1609459200000, + 1609459260000, + 1609459320000 + ], + [ + 100, + null, + 120 + ] + ] + } + } + ] +} \ No newline at end of file diff --git a/pkg/tsdb/graphite/testdata/successful_single_query_with_tags-RefID-A.golden.jsonc b/pkg/tsdb/graphite/testdata/successful_single_query_with_tags-RefID-A.golden.jsonc new file mode 100644 index 00000000000..31fba0dcea5 --- /dev/null +++ b/pkg/tsdb/graphite/testdata/successful_single_query_with_tags-RefID-A.golden.jsonc @@ -0,0 +1,77 @@ +// 🌟 This was machine generated. Do not edit. 🌟 +// +// Frame[0] { +// "type": "timeseries-multi", +// "typeVersion": [ +// 0, +// 0 +// ] +// } +// Name: A +// Dimensions: 2 Fields by 2 Rows +// +-------------------------------+--------------------------------------------------------------------+ +// | Name: time | Name: value | +// | Labels: | Labels: environment=production, host=server1, port=8080, rate=99.5 | +// | Type: []time.Time | Type: []*float64 | +// +-------------------------------+--------------------------------------------------------------------+ +// | 2021-01-01 00:00:00 +0000 UTC | 100 | +// | 2021-01-01 00:01:00 +0000 UTC | 150 | +// +-------------------------------+--------------------------------------------------------------------+ +// +// +// 🌟 This was machine generated. Do not edit. 🌟 +{ + "status": 200, + "frames": [ + { + "schema": { + "name": "A", + "meta": { + "type": "timeseries-multi", + "typeVersion": [ + 0, + 0 + ] + }, + "fields": [ + { + "name": "time", + "type": "time", + "typeInfo": { + "frame": "time.Time" + } + }, + { + "name": "value", + "type": "number", + "typeInfo": { + "frame": "float64", + "nullable": true + }, + "labels": { + "environment": "production", + "host": "server1", + "port": "8080", + "rate": "99.5" + }, + "config": { + "displayNameFromDS": "stats.counters.web.hits" + } + } + ] + }, + "data": { + "values": [ + [ + 1609459200000, + 1609459260000 + ], + [ + 100, + 150 + ] + ] + } + } + ] +} \ No newline at end of file