diff --git a/backend/data_adapter.go b/backend/data_adapter.go index dfca1b6b0..a34aa4087 100644 --- a/backend/data_adapter.go +++ b/backend/data_adapter.go @@ -51,34 +51,29 @@ func (a *dataSDKAdapter) QueryData(ctx context.Context, req *pluginv2.QueryDataR } resp, innerErr = a.queryDataHandler.QueryData(ctx, parsedReq) - if resp == nil || len(resp.Responses) == 0 { - return RequestStatusFromError(innerErr), innerErr - } - - if isCancelledError(innerErr) { - return RequestStatusCancelled, nil - } - - if isHTTPTimeoutError(innerErr) { - return RequestStatusError, nil + status := RequestStatusFromQueryDataResponse(resp, innerErr) + if innerErr != nil { + return status, innerErr + } else if resp == nil { + return RequestStatusError, errors.New("both response and error are nil, but one must be provided") } + ctxLogger := Logger.FromContext(ctx) // Set downstream status source in the context if there's at least one response with downstream status source, // and if there's no plugin error - var hasPluginError bool - var hasDownstreamError bool - var hasCancelledError bool - var hasHTTPTimeoutError bool - for _, r := range resp.Responses { + var hasPluginError, hasDownstreamError bool + for refID, r := range resp.Responses { if r.Error == nil { continue } - if isCancelledError(r.Error) { - hasCancelledError = true + // if error source not set and the error is a downstream error, set error source to downstream. + if !r.ErrorSource.IsValid() && IsDownstreamError(r.Error) { + r.ErrorSource = ErrorSourceDownstream } - if isHTTPTimeoutError(r.Error) { - hasHTTPTimeoutError = true + + if !r.Status.IsValid() { + r.Status = statusFromError(r.Error) } if r.ErrorSource == ErrorSourceDownstream { @@ -86,43 +81,29 @@ func (a *dataSDKAdapter) QueryData(ctx context.Context, req *pluginv2.QueryDataR } else { hasPluginError = true } - } - if hasCancelledError { - if err := WithDownstreamErrorSource(ctx); err != nil { - return RequestStatusError, fmt.Errorf("failed to set downstream status source: %w", errors.Join(innerErr, err)) + logParams := []any{ + "refID", refID, + "status", int(r.Status), + "error", r.Error, + "statusSource", string(r.ErrorSource), } - return RequestStatusCancelled, nil - } - - if hasHTTPTimeoutError { - if err := WithDownstreamErrorSource(ctx); err != nil { - return RequestStatusError, fmt.Errorf("failed to set downstream status source: %w", errors.Join(innerErr, err)) - } - return RequestStatusError, nil + ctxLogger.Error("Partial data response error", logParams...) } // A plugin error has higher priority than a downstream error, // so set to downstream only if there's no plugin error - if hasDownstreamError && !hasPluginError { - if err := WithDownstreamErrorSource(ctx); err != nil { - return RequestStatusError, fmt.Errorf("failed to set downstream status source: %w", errors.Join(innerErr, err)) - } - return RequestStatusError, nil - } - if hasPluginError { if err := WithErrorSource(ctx, ErrorSourcePlugin); err != nil { return RequestStatusError, fmt.Errorf("failed to set plugin status source: %w", errors.Join(innerErr, err)) } - return RequestStatusError, nil - } - - if innerErr != nil { - return RequestStatusFromError(innerErr), innerErr + } else if hasDownstreamError { + if err := WithDownstreamErrorSource(ctx); err != nil { + return RequestStatusError, fmt.Errorf("failed to set downstream status source: %w", errors.Join(innerErr, err)) + } } - return RequestStatusOK, nil + return status, nil }) if err != nil { return nil, err diff --git a/backend/data_adapter_test.go b/backend/data_adapter_test.go index e38eab218..0a5e540ce 100644 --- a/backend/data_adapter_test.go +++ b/backend/data_adapter_test.go @@ -140,6 +140,7 @@ func TestQueryData(t *testing.T) { name string queryDataResponse *QueryDataResponse expErrorSource ErrorSource + expError bool }{ { name: `single downstream error should be "downstream" error source`, @@ -180,6 +181,32 @@ func TestQueryData(t *testing.T) { }, expErrorSource: ErrorSourcePlugin, }, + { + name: `single downstream error without error source should be "downstream" error source`, + queryDataResponse: &QueryDataResponse{ + Responses: map[string]DataResponse{ + "A": {Error: DownstreamErrorf("boom")}, + }, + }, + expErrorSource: ErrorSourceDownstream, + }, + { + name: `multiple downstream error without error source and single plugin error should be "plugin" error source`, + queryDataResponse: &QueryDataResponse{ + Responses: map[string]DataResponse{ + "A": {Error: DownstreamErrorf("boom")}, + "B": {Error: someErr}, + "C": {Error: DownstreamErrorf("boom")}, + }, + }, + expErrorSource: ErrorSourcePlugin, + }, + { + name: "nil queryDataResponse and nil error should throw error", + queryDataResponse: nil, + expErrorSource: ErrorSourcePlugin, + expError: true, + }, } { t.Run(tc.name, func(t *testing.T) { var actualCtx context.Context @@ -190,7 +217,13 @@ func TestQueryData(t *testing.T) { _, err := a.QueryData(context.Background(), &pluginv2.QueryDataRequest{ PluginContext: &pluginv2.PluginContext{}, }) - require.NoError(t, err) + + if tc.expError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + ss := errorSourceFromContext(actualCtx) require.Equal(t, tc.expErrorSource, ss) }) diff --git a/backend/error_source.go b/backend/error_source.go index 1df6a0dd2..8c157cf30 100644 --- a/backend/error_source.go +++ b/backend/error_source.go @@ -21,6 +21,10 @@ const ( DefaultErrorSource ErrorSource = ErrorSourcePlugin ) +func (es ErrorSource) IsValid() bool { + return es == ErrorSourceDownstream || es == ErrorSourcePlugin +} + // ErrorSourceFromStatus returns an [ErrorSource] based on provided HTTP status code. func ErrorSourceFromHTTPStatus(statusCode int) ErrorSource { switch statusCode { @@ -62,7 +66,7 @@ func IsDownstreamError(err error) bool { return true } - if isHTTPTimeoutError(err) { + if isHTTPTimeoutError(err) || isCancelledError(err) { return true } diff --git a/backend/error_source_test.go b/backend/error_source_test.go index 8724b5c33..94ebe1354 100644 --- a/backend/error_source_test.go +++ b/backend/error_source_test.go @@ -1,6 +1,7 @@ package backend import ( + "context" "errors" "fmt" "net" @@ -8,8 +9,18 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) +func TestErrorSource(t *testing.T) { + var es ErrorSource + require.False(t, es.IsValid()) + require.True(t, ErrorSourceDownstream.IsValid()) + require.True(t, ErrorSourcePlugin.IsValid()) +} + func TestIsDownstreamError(t *testing.T) { tcs := []struct { name string @@ -66,6 +77,36 @@ func TestIsDownstreamError(t *testing.T) { err: fmt.Errorf("other error"), expected: false, }, + { + name: "context.Canceled", + err: context.Canceled, + expected: true, + }, + { + name: "wrapped context.Canceled", + err: fmt.Errorf("error: %w", context.Canceled), + expected: true, + }, + { + name: "joined context.Canceled", + err: errors.Join(fmt.Errorf("oh no"), context.Canceled), + expected: true, + }, + { + name: "gRPC canceled error", + err: status.Error(codes.Canceled, "canceled"), + expected: true, + }, + { + name: "wrapped gRPC canceled error", + err: fmt.Errorf("error: %w", status.Error(codes.Canceled, "canceled")), + expected: true, + }, + { + name: "joined gRPC canceled error", + err: errors.Join(fmt.Errorf("oh no"), status.Error(codes.Canceled, "canceled")), + expected: true, + }, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { diff --git a/experimental/apis/data/v0alpha1/openapi_test.go b/experimental/apis/data/v0alpha1/openapi_test.go index f219be0ea..39bb46088 100644 --- a/experimental/apis/data/v0alpha1/openapi_test.go +++ b/experimental/apis/data/v0alpha1/openapi_test.go @@ -42,5 +42,5 @@ func TestOpenAPI(t *testing.T) { // Ensure DataSourceRef exists and has three properties def, ok = defs["github.com/grafana/grafana-plugin-sdk-go/experimental/apis/data/v0alpha1.DataSourceRef"] require.True(t, ok) - require.Equal(t, []string{"type", "uid", "apiVersion"}, maps.Keys(def.Schema.Properties)) + require.ElementsMatch(t, []string{"type", "uid", "apiVersion"}, maps.Keys(def.Schema.Properties)) } diff --git a/go.mod b/go.mod index d852347c4..bace95adb 100644 --- a/go.mod +++ b/go.mod @@ -30,13 +30,13 @@ require ( github.com/urfave/cli v1.22.15 go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.53.0 go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace v0.53.0 - go.opentelemetry.io/contrib/propagators/jaeger v1.28.0 - go.opentelemetry.io/contrib/samplers/jaegerremote v0.22.0 - go.opentelemetry.io/otel v1.28.0 - go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.28.0 + go.opentelemetry.io/contrib/propagators/jaeger v1.29.0 + go.opentelemetry.io/contrib/samplers/jaegerremote v0.23.0 + go.opentelemetry.io/otel v1.29.0 + go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.29.0 go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.28.0 - go.opentelemetry.io/otel/sdk v1.28.0 - go.opentelemetry.io/otel/trace v1.28.0 + go.opentelemetry.io/otel/sdk v1.29.0 + go.opentelemetry.io/otel/trace v1.29.0 golang.org/x/exp v0.0.0-20231006140011-7918f672742d golang.org/x/net v0.28.0 golang.org/x/oauth2 v0.22.0 @@ -99,13 +99,13 @@ require ( github.com/unknwon/log v0.0.0-20150304194804-e617c87089d3 // indirect github.com/wk8/go-ordered-map/v2 v2.1.8 // indirect github.com/zeebo/xxh3 v1.0.2 // indirect - go.opentelemetry.io/otel/metric v1.28.0 // indirect + go.opentelemetry.io/otel/metric v1.29.0 // indirect go.opentelemetry.io/proto/otlp v1.3.1 // indirect golang.org/x/mod v0.17.0 // indirect golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect - google.golang.org/genproto/googleapis/api v0.0.0-20240701130421-f6361c86f094 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20240822170219-fc7c04adadcd // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20240814211410-ddb44dafa142 // indirect gopkg.in/fsnotify/fsnotify.v1 v1.4.7 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect diff --git a/go.sum b/go.sum index 86915a63f..aca7e0182 100644 --- a/go.sum +++ b/go.sum @@ -223,26 +223,26 @@ go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace v0. go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace v0.53.0/go.mod h1:ImRBLMJv177/pwiLZ7tU7HDGNdBv7rS0HQ99eN/zBl8= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.53.0 h1:4K4tsIXefpVJtvA/8srF4V4y0akAoPHkIslgAkjixJA= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.53.0/go.mod h1:jjdQuTGVsXV4vSs+CJ2qYDeDPf9yIJV23qlIzBm73Vg= -go.opentelemetry.io/contrib/propagators/jaeger v1.28.0 h1:xQ3ktSVS128JWIaN1DiPGIjcH+GsvkibIAVRWFjS9eM= -go.opentelemetry.io/contrib/propagators/jaeger v1.28.0/go.mod h1:O9HIyI2kVBrFoEwQZ0IN6PHXykGoit4mZV2aEjkTRH4= -go.opentelemetry.io/contrib/samplers/jaegerremote v0.22.0 h1:OYxqumWcd1yaV/qvCt1B7Sru9OeUNGjeXq/oldx3AGk= -go.opentelemetry.io/contrib/samplers/jaegerremote v0.22.0/go.mod h1:2tZTRqCbvx7nG57wUwd5NQpNVujOWnR84iPLllIH0Ok= +go.opentelemetry.io/contrib/propagators/jaeger v1.29.0 h1:+YPiqF5rR6PqHBlmEFLPumbSP0gY0WmCGFayXRcCLvs= +go.opentelemetry.io/contrib/propagators/jaeger v1.29.0/go.mod h1:6PD7q7qquWSp3Z4HeM3e/2ipRubaY1rXZO8NIHVDZjs= +go.opentelemetry.io/contrib/samplers/jaegerremote v0.23.0 h1:qKi9ntCcronqWqfuKxqrxZlZd82jXJEgGiAWH1+phxo= +go.opentelemetry.io/contrib/samplers/jaegerremote v0.23.0/go.mod h1:1kbAgQa5lgYC3rC6cE3jSxQ/Q13l33wv/WI8U+htwag= go.opentelemetry.io/otel v1.21.0/go.mod h1:QZzNPQPm1zLX4gZK4cMi+71eaorMSGT3A4znnUvNNEo= -go.opentelemetry.io/otel v1.28.0 h1:/SqNcYk+idO0CxKEUOtKQClMK/MimZihKYMruSMViUo= -go.opentelemetry.io/otel v1.28.0/go.mod h1:q68ijF8Fc8CnMHKyzqL6akLO46ePnjkgfIMIjUIX9z4= -go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.28.0 h1:3Q/xZUyC1BBkualc9ROb4G8qkH90LXEIICcs5zv1OYY= -go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.28.0/go.mod h1:s75jGIWA9OfCMzF0xr+ZgfrB5FEbbV7UuYo32ahUiFI= +go.opentelemetry.io/otel v1.29.0 h1:PdomN/Al4q/lN6iBJEN3AwPvUiHPMlt93c8bqTG5Llw= +go.opentelemetry.io/otel v1.29.0/go.mod h1:N/WtXPs1CNCUEx+Agz5uouwCba+i+bJGFicT8SR4NP8= +go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.29.0 h1:dIIDULZJpgdiHz5tXrTgKIMLkus6jEFa7x5SOKcyR7E= +go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.29.0/go.mod h1:jlRVBe7+Z1wyxFSUs48L6OBQZ5JwH2Hg/Vbl+t9rAgI= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.28.0 h1:R3X6ZXmNPRR8ul6i3WgFURCHzaXjHdm0karRG/+dj3s= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.28.0/go.mod h1:QWFXnDavXWwMx2EEcZsf3yxgEKAqsxQ+Syjp+seyInw= go.opentelemetry.io/otel/metric v1.21.0/go.mod h1:o1p3CA8nNHW8j5yuQLdc1eeqEaPfzug24uvsyIEJRWM= -go.opentelemetry.io/otel/metric v1.28.0 h1:f0HGvSl1KRAU1DLgLGFjrwVyismPlnuU6JD6bOeuA5Q= -go.opentelemetry.io/otel/metric v1.28.0/go.mod h1:Fb1eVBFZmLVTMb6PPohq3TO9IIhUisDsbJoL/+uQW4s= +go.opentelemetry.io/otel/metric v1.29.0 h1:vPf/HFWTNkPu1aYeIsc98l4ktOQaL6LeSoeV2g+8YLc= +go.opentelemetry.io/otel/metric v1.29.0/go.mod h1:auu/QWieFVWx+DmQOUMgj0F8LHWdgalxXqvp7BII/W8= go.opentelemetry.io/otel/sdk v1.21.0/go.mod h1:Nna6Yv7PWTdgJHVRD9hIYywQBRx7pbox6nwBnZIxl/E= -go.opentelemetry.io/otel/sdk v1.28.0 h1:b9d7hIry8yZsgtbmM0DKyPWMMUMlK9NEKuIG4aBqWyE= -go.opentelemetry.io/otel/sdk v1.28.0/go.mod h1:oYj7ClPUA7Iw3m+r7GeEjz0qckQRJK2B8zjcZEfu7Pg= +go.opentelemetry.io/otel/sdk v1.29.0 h1:vkqKjk7gwhS8VaWb0POZKmIEDimRCMsopNYnriHyryo= +go.opentelemetry.io/otel/sdk v1.29.0/go.mod h1:pM8Dx5WKnvxLCb+8lG1PRNIDxu9g9b9g59Qr7hfAAok= go.opentelemetry.io/otel/trace v1.21.0/go.mod h1:LGbsEB0f9LGjN+OZaQQ26sohbOmiMR+BaslueVtS/qQ= -go.opentelemetry.io/otel/trace v1.28.0 h1:GhQ9cUuQGmNDd5BTCP2dAvv75RdMxEfTmYejp+lkx9g= -go.opentelemetry.io/otel/trace v1.28.0/go.mod h1:jPyXzNPg6da9+38HEwElrQiHlVMTnVfM3/yv2OlIHaI= +go.opentelemetry.io/otel/trace v1.29.0 h1:J/8ZNK4XgR7a21DZUAsbF8pZ5Jcw1VhACmnYt39JTi4= +go.opentelemetry.io/otel/trace v1.29.0/go.mod h1:eHl3w0sp3paPkYstJOmAimxhiFXPg+MMTlEh3nsQgWQ= go.opentelemetry.io/proto/otlp v1.3.1 h1:TrMUixzpM0yuc/znrFTP9MMRh8trP93mkCiDVeXrui0= go.opentelemetry.io/proto/otlp v1.3.1/go.mod h1:0X1WI4de4ZsLrrJNLAQbFeLCm3T7yBkR0XqQ7niQU+8= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= @@ -302,10 +302,10 @@ golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 h1:H2TDz8ibqkAF6YGhCdN3j golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2/go.mod h1:K8+ghG5WaK9qNqU5K3HdILfMLy1f3aNYFI/wnl100a8= gonum.org/v1/gonum v0.12.0 h1:xKuo6hzt+gMav00meVPUlXwSdoEJP46BR+wdxQEFK2o= gonum.org/v1/gonum v0.12.0/go.mod h1:73TDxJfAAHeA8Mk9mf8NlIppyhQNo5GLTcYeqgo2lvY= -google.golang.org/genproto/googleapis/api v0.0.0-20240701130421-f6361c86f094 h1:0+ozOGcrp+Y8Aq8TLNN2Aliibms5LEzsq99ZZmAGYm0= -google.golang.org/genproto/googleapis/api v0.0.0-20240701130421-f6361c86f094/go.mod h1:fJ/e3If/Q67Mj99hin0hMhiNyCRmt6BQ2aWIJshUSJw= -google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 h1:BwIjyKYGsK9dMCBOorzRri8MQwmi7mT9rGHsCEinZkA= -google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094/go.mod h1:Ue6ibwXGpU+dqIcODieyLOcgj7z8+IcskoNIgZxtrFY= +google.golang.org/genproto/googleapis/api v0.0.0-20240822170219-fc7c04adadcd h1:BBOTEWLuuEGQy9n1y9MhVJ9Qt0BDu21X8qZs71/uPZo= +google.golang.org/genproto/googleapis/api v0.0.0-20240822170219-fc7c04adadcd/go.mod h1:fO8wJzT2zbQbAjbIoos1285VfEIYKDDY+Dt+WpTkh6g= +google.golang.org/genproto/googleapis/rpc v0.0.0-20240814211410-ddb44dafa142 h1:e7S5W7MGGLaSu8j3YjdezkZ+m1/Nm0uRVRMEMGk26Xs= +google.golang.org/genproto/googleapis/rpc v0.0.0-20240814211410-ddb44dafa142/go.mod h1:UqMtugtsSgubUsoxbuAoiCXvqvErP7Gf0so0mK9tHxU= google.golang.org/grpc v1.65.0 h1:bs/cUb4lp1G5iImFFd3u5ixQzweKizoZJAwBNLR42lc= google.golang.org/grpc v1.65.0/go.mod h1:WgYC2ypjlB0EiQi6wdKixMqukr6lBc0Vo+oOgjrM5ZQ= google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg=