From a1718aafce1599e2211a0a03d6301adab2d0e64f Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Mon, 6 Nov 2023 11:36:39 +0100 Subject: [PATCH] Elasticsearch: Add error source for DataQuery (#77386) * WIP * Refactor, plus update source of error in response_parser * Adjust test * Use methods and httpclient from errorsource * Update pkg/tsdb/elasticsearch/data_query.go Co-authored-by: Scott Lepper * Return nil error * Fix test * Fix integration test --------- Co-authored-by: Scott Lepper --- go.sum | 3 +++ pkg/tests/api/elasticsearch/elasticsearch_test.go | 2 +- pkg/tsdb/elasticsearch/data_query.go | 11 +++++++---- pkg/tsdb/elasticsearch/data_query_test.go | 7 +++++-- pkg/tsdb/elasticsearch/elasticsearch.go | 4 +++- pkg/tsdb/elasticsearch/error_handling_test.go | 5 +++-- pkg/tsdb/elasticsearch/response_parser.go | 5 ++--- public/dashboards/home.json | 4 ++-- 8 files changed, 26 insertions(+), 15 deletions(-) diff --git a/go.sum b/go.sum index 94188a7f9f9..1df0223f53b 100644 --- a/go.sum +++ b/go.sum @@ -1754,6 +1754,7 @@ github.com/google/pprof v0.0.0-20230228050547-1710fef4ab10/go.mod h1:79YE0hCXdHa github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/google/s2a-go v0.1.7 h1:60BLSyTrOV4/haCDW4zb1guZItoSq8foHCXrAnjBo/o= github.com/google/s2a-go v0.1.7/go.mod h1:50CgR4k1jNlWBu4UfS4AcfhVe1r6pdZPygJ3R8F0Qdw= +github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= github.com/google/subcommands v1.0.1/go.mod h1:ZjhPrFU+Olkh9WazFPsl27BQ4UPiG37m3yTrtFlrHVk= github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.1.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= @@ -1833,6 +1834,8 @@ github.com/grafana/gofpdf v0.0.0-20231002120153-857cc45be447 h1:jxJJ5z0GxqhWFbQU github.com/grafana/gofpdf v0.0.0-20231002120153-857cc45be447/go.mod h1:IxsY6mns6Q5sAnWcrptrgUrSglTZJXH/kXr9nbpb/9I= github.com/grafana/grafana-aws-sdk v0.19.1 h1:5GBiOv2AgdyjwlgAX+dtgPtXU4FgMTD9rfQUPQseEpQ= github.com/grafana/grafana-aws-sdk v0.19.1/go.mod h1:ntq2NDH12Y2Fkbc6fozpF8kYsJM9k6KNr+Xfo5w3/iM= +github.com/grafana/grafana-aws-sdk v0.19.2 h1:GCLdo3oz7gp/ZJvbgFktMP5LKdNLnhxh/nLGzuxnJPA= +github.com/grafana/grafana-aws-sdk v0.19.2/go.mod h1:IDhwY+LF6jD1zute5UvbZ5DY8aI4DQ+LjU8RjfayD20= github.com/grafana/grafana-azure-sdk-go v1.9.0 h1:4JRwlqgUtPRAQSoiV4DFZDQ3lbNsauHqj9kC6SMR9Ak= github.com/grafana/grafana-azure-sdk-go v1.9.0/go.mod h1:1vBa0KOl+/Kcm7V888OyMXDSFncmek14q7XhEkrcSaA= github.com/grafana/grafana-google-sdk-go v0.1.0 h1:LKGY8z2DSxKjYfr2flZsWgTRTZ6HGQbTqewE3JvRaNA= diff --git a/pkg/tests/api/elasticsearch/elasticsearch_test.go b/pkg/tests/api/elasticsearch/elasticsearch_test.go index 21eb70c8c16..1bfc4ab9f26 100644 --- a/pkg/tests/api/elasticsearch/elasticsearch_test.go +++ b/pkg/tests/api/elasticsearch/elasticsearch_test.go @@ -95,7 +95,7 @@ func TestIntegrationElasticsearch(t *testing.T) { 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/elasticsearch/data_query.go b/pkg/tsdb/elasticsearch/data_query.go index 35fc092d8b7..f737f746cc1 100644 --- a/pkg/tsdb/elasticsearch/data_query.go +++ b/pkg/tsdb/elasticsearch/data_query.go @@ -10,6 +10,7 @@ import ( "time" "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/log" @@ -41,12 +42,13 @@ var newElasticsearchDataQuery = func(ctx context.Context, client es.Client, data func (e *elasticsearchDataQuery) execute() (*backend.QueryDataResponse, error) { start := time.Now() + response := backend.NewQueryDataResponse() e.logger.Debug("Parsing queries", "queriesLength", len(e.dataQueries)) queries, err := parseQuery(e.dataQueries, e.logger) if err != nil { mq, _ := json.Marshal(e.dataQueries) e.logger.Error("Failed to parse queries", "error", err, "queries", string(mq), "queriesLength", len(queries), "duration", time.Since(start), "stage", es.StagePrepareRequest) - return &backend.QueryDataResponse{}, err + return errorsource.AddPluginErrorToResponse(e.dataQueries[0].RefID, response, err), nil } ms := e.client.MultiSearch() @@ -57,7 +59,7 @@ func (e *elasticsearchDataQuery) execute() (*backend.QueryDataResponse, error) { if err := e.processQuery(q, ms, from, to); err != nil { mq, _ := json.Marshal(q) e.logger.Error("Failed to process query to multisearch request builder", "error", err, "query", string(mq), "queriesLength", len(queries), "duration", time.Since(start), "stage", es.StagePrepareRequest) - return &backend.QueryDataResponse{}, err + return errorsource.AddPluginErrorToResponse(q.RefID, response, err), nil } } @@ -65,13 +67,14 @@ func (e *elasticsearchDataQuery) execute() (*backend.QueryDataResponse, error) { if err != nil { mqs, _ := json.Marshal(e.dataQueries) e.logger.Error("Failed to build multisearch request", "error", err, "queriesLength", len(queries), "queries", string(mqs), "duration", time.Since(start), "stage", es.StagePrepareRequest) - return &backend.QueryDataResponse{}, err + return errorsource.AddPluginErrorToResponse(e.dataQueries[0].RefID, response, err), nil } e.logger.Info("Prepared request", "queriesLength", len(queries), "duration", time.Since(start), "stage", es.StagePrepareRequest) res, err := e.client.ExecuteMultisearch(req) if err != nil { - return &backend.QueryDataResponse{}, err + // We are returning error containing the source that was added trough errorsource.Middleware + return errorsource.AddErrorToResponse(e.dataQueries[0].RefID, response, err), nil } return parseResponse(e.ctx, res.Responses, queries, e.client.GetConfiguredFields(), e.logger, e.tracer) diff --git a/pkg/tsdb/elasticsearch/data_query_test.go b/pkg/tsdb/elasticsearch/data_query_test.go index 337dbe42fd0..bded5d6071d 100644 --- a/pkg/tsdb/elasticsearch/data_query_test.go +++ b/pkg/tsdb/elasticsearch/data_query_test.go @@ -1428,10 +1428,12 @@ func TestExecuteElasticsearchDataQuery(t *testing.T) { t.Run("With invalid query should return error", (func(t *testing.T) { c := newFakeClient() - _, err := executeElasticsearchDataQuery(c, `{ + res, err := executeElasticsearchDataQuery(c, `{ "query": "foo", }`, from, to) - require.Error(t, err) + require.NoError(t, err) + require.Equal(t, res.Responses["A"].ErrorSource, backend.ErrorSourcePlugin) + require.Equal(t, res.Responses["A"].Error.Error(), "invalid character '}' looking for beginning of object key string") })) }) } @@ -1856,6 +1858,7 @@ func executeElasticsearchDataQuery(c es.Client, body string, from, to time.Time) { JSON: json.RawMessage(body), TimeRange: timeRange, + RefID: "A", }, }, } diff --git a/pkg/tsdb/elasticsearch/elasticsearch.go b/pkg/tsdb/elasticsearch/elasticsearch.go index cd25200b52d..e6deb0c525b 100644 --- a/pkg/tsdb/elasticsearch/elasticsearch.go +++ b/pkg/tsdb/elasticsearch/elasticsearch.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/backend/datasource" "github.com/grafana/grafana-plugin-sdk-go/backend/instancemgmt" + exphttpclient "github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource/httpclient" "github.com/grafana/grafana/pkg/infra/httpclient" "github.com/grafana/grafana/pkg/infra/log" @@ -87,7 +88,8 @@ func newInstanceSettings(httpClientProvider httpclient.Provider) datasource.Inst httpCliOpts.SigV4.Service = "es" } - httpCli, err := httpClientProvider.New(httpCliOpts) + // enable experimental http client to support errors with source + httpCli, err := exphttpclient.New(httpCliOpts) if err != nil { return nil, err } diff --git a/pkg/tsdb/elasticsearch/error_handling_test.go b/pkg/tsdb/elasticsearch/error_handling_test.go index 20dddacda4c..353c1e2409d 100644 --- a/pkg/tsdb/elasticsearch/error_handling_test.go +++ b/pkg/tsdb/elasticsearch/error_handling_test.go @@ -139,11 +139,12 @@ func TestNonElasticError(t *testing.T) { // to access the database for some reason. response := []byte(`Access to the database is forbidden`) - _, err := queryDataTestWithResponseCode(query, 403, response) + res, err := queryDataTestWithResponseCode(query, 403, response) // FIXME: we should return something better. // currently it returns the error-message about being unable to decode JSON // it is not 100% clear what we should return to the browser // (and what to debug-log for example), we could return // at least something like "unknown response, http status code 403" - require.ErrorContains(t, err, "invalid character") + require.NoError(t, err) + require.Contains(t, res.response.Responses["A"].Error.Error(), "invalid character") } diff --git a/pkg/tsdb/elasticsearch/response_parser.go b/pkg/tsdb/elasticsearch/response_parser.go index b95eb98de1d..3b332872a25 100644 --- a/pkg/tsdb/elasticsearch/response_parser.go +++ b/pkg/tsdb/elasticsearch/response_parser.go @@ -13,6 +13,7 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend" "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" "go.opentelemetry.io/otel/trace" @@ -73,9 +74,7 @@ func parseResponse(ctx context.Context, responses []*es.SearchResponse, targets resSpan.End() logger.Error("Processing error response from Elasticsearch", "error", string(me), "query", string(mt)) errResult := getErrorFromElasticResponse(res) - result.Responses[target.RefID] = backend.DataResponse{ - Error: errors.New(errResult), - } + result.Responses[target.RefID] = errorsource.Response(errorsource.PluginError(errors.New(errResult), false)) continue } diff --git a/public/dashboards/home.json b/public/dashboards/home.json index d9e24324df7..718b6b52079 100644 --- a/public/dashboards/home.json +++ b/public/dashboards/home.json @@ -32,9 +32,9 @@ "showHeadings": true, "folderId": 0, "maxItems": 30, - "tags":[], + "tags": [], "query": "" - }, + }, "title": "Dashboards", "type": "dashlist" },