Skip to content

Commit 8c88d65

Browse files
Multimocletter7
andauthored
Mistyped macros are incorrectly reported as plugin error (#141)
* Mistyped macros are incorectly reported as plugin error * update method and deps * Update datasource_test.go Co-authored-by: Andriy <andriy.urbanas@grafana.com> --------- Co-authored-by: Andriy <andriy.urbanas@grafana.com>
1 parent 4b54dbb commit 8c88d65

File tree

7 files changed

+116
-24
lines changed

7 files changed

+116
-24
lines changed

dataframe_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ func TestNoRowsFrame(t *testing.T) {
158158
for _, tt := range tts {
159159
t.Run(tt.name, func(t *testing.T) {
160160
id := "empty-frames" + tt.name
161-
driver, _ := test.NewDriver(id, tt.data, nil, test.DriverOpts{})
161+
driver, _ := test.NewDriver(id, tt.data, nil, test.DriverOpts{}, nil)
162162
ds := sqlds.NewDatasource(driver)
163163

164164
settings := backend.DataSourceInstanceSettings{UID: id, JSONData: []byte("{}")}

datasource.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ func (ds *SQLDatasource) handleQuery(ctx context.Context, req backend.DataQuery,
153153
// Apply supported macros to the query
154154
q.RawSQL, err = Interpolate(ds.driver(), q)
155155
if err != nil {
156-
if errors.Is(err, sqlutil.ErrorBadArgumentCount) {
156+
if errors.Is(err, sqlutil.ErrorBadArgumentCount) || err.Error() == ErrorParsingMacroBrackets.Error() {
157157
err = backend.DownstreamError(err)
158158
}
159159
return sqlutil.ErrorFrameFromQuery(q), fmt.Errorf("%s: %w", "Could not apply macros", err)

datasource_test.go

Lines changed: 97 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ import (
44
"context"
55
"encoding/json"
66
"errors"
7+
"fmt"
78
"testing"
89

910
"github.com/grafana/grafana-plugin-sdk-go/backend"
11+
"github.com/grafana/grafana-plugin-sdk-go/data/sqlutil"
1012
"github.com/grafana/sqlds/v4"
1113
"github.com/grafana/sqlds/v4/test"
1214
"github.com/stretchr/testify/assert"
@@ -31,7 +33,7 @@ func Test_query_retries(t *testing.T) {
3133
QueryError: errors.New("foo"),
3234
}
3335

34-
req, handler, ds := queryRequest(t, "error", opts, cfg)
36+
req, handler, ds := queryRequest(t, "error", opts, cfg, nil)
3537

3638
data, err := ds.QueryData(context.Background(), req)
3739
assert.Nil(t, err)
@@ -56,7 +58,7 @@ func Test_query_apply_headers(t *testing.T) {
5658
}
5759
cfg := `{ "timeout": 0, "retries": 1, "retryOn": ["missing token"], "forwardHeaders": true }`
5860

59-
req, handler, ds := queryRequest(t, "headers", opts, cfg)
61+
req, handler, ds := queryRequest(t, "headers", opts, cfg, nil)
6062

6163
req.SetHTTPHeader("foo", "bar")
6264

@@ -99,8 +101,98 @@ func Test_no_errors(t *testing.T) {
99101
assert.Equal(t, expected, result.Message)
100102
}
101103

102-
func queryRequest(t *testing.T, name string, opts test.DriverOpts, cfg string) (*backend.QueryDataRequest, *test.SqlHandler, *sqlds.SQLDatasource) {
103-
driver, handler := test.NewDriver(name, test.Data{}, nil, opts)
104+
func Test_custom_marco_errors(t *testing.T) {
105+
cfg := `{ "timeout": 0, "retries": 0, "retryOn": ["foo"], query: "badArgumentCount" }`
106+
opts := test.DriverOpts{}
107+
108+
badArgumentCountFunc := func(query *sqlds.Query, args []string) (string, error) {
109+
return "", sqlutil.ErrorBadArgumentCount
110+
}
111+
macros := sqlds.Macros{
112+
"foo": badArgumentCountFunc,
113+
}
114+
115+
req, _, ds := queryRequest(t, "interpolate", opts, cfg, macros)
116+
117+
req.Queries[0].JSON = []byte(`{ "rawSql": "select $__foo from bar;" }`)
118+
119+
data, err := ds.QueryData(context.Background(), req)
120+
assert.Nil(t, err)
121+
122+
res := data.Responses["foo"]
123+
assert.NotNil(t, res.Error)
124+
assert.Equal(t, backend.ErrorSourceDownstream, res.ErrorSource)
125+
assert.Contains(t, res.Error.Error(), sqlutil.ErrorBadArgumentCount.Error())
126+
}
127+
128+
func Test_default_macro_errors(t *testing.T) {
129+
tests := []struct {
130+
name string
131+
rawSQL string
132+
wantError string
133+
}{
134+
{
135+
name: "missing parameters",
136+
rawSQL: "select * from bar where $__timeGroup(",
137+
wantError: "missing close bracket",
138+
},
139+
{
140+
name: "incorrect argument count 0 - timeGroup",
141+
rawSQL: "select * from bar where $__timeGroup()",
142+
wantError: sqlutil.ErrorBadArgumentCount.Error(),
143+
},
144+
{
145+
name: "incorrect argument count 3 - timeGroup",
146+
rawSQL: "select * from bar where $__timeGroup(1,2,3)",
147+
wantError: sqlutil.ErrorBadArgumentCount.Error(),
148+
},
149+
{
150+
name: "incorrect argument count 0 - timeFilter",
151+
rawSQL: "select * from bar where $__timeFilter",
152+
wantError: sqlutil.ErrorBadArgumentCount.Error(),
153+
},
154+
{
155+
name: "incorrect argument count 3 - timeFilter",
156+
rawSQL: "select * from bar where $__timeFilter(1,2,3)",
157+
wantError: sqlutil.ErrorBadArgumentCount.Error(),
158+
},
159+
{
160+
name: "incorrect argument count 0 - timeFrom",
161+
rawSQL: "select * from bar where $__timeFrom",
162+
wantError: sqlutil.ErrorBadArgumentCount.Error(),
163+
},
164+
{
165+
name: "incorrect argument count 3 - timeFrom",
166+
rawSQL: "select * from bar where $__timeFrom(1,2,3)",
167+
wantError: sqlutil.ErrorBadArgumentCount.Error(),
168+
},
169+
}
170+
171+
// Common test configuration
172+
cfg := `{ "timeout": 0, "retries": 0, "retryOn": ["foo"], query: "badArgumentCount" }`
173+
opts := test.DriverOpts{}
174+
175+
for _, tt := range tests {
176+
t.Run(tt.name, func(t *testing.T) {
177+
// Setup request
178+
req, _, ds := queryRequest(t, "interpolate", opts, cfg, nil)
179+
req.Queries[0].JSON = []byte(fmt.Sprintf(`{ "rawSql": "%s" }`, tt.rawSQL))
180+
181+
// Execute query
182+
data, err := ds.QueryData(context.Background(), req)
183+
assert.Nil(t, err)
184+
185+
// Verify response
186+
res := data.Responses["foo"]
187+
assert.NotNil(t, res.Error)
188+
assert.Equal(t, backend.ErrorSourceDownstream, res.ErrorSource)
189+
assert.Contains(t, res.Error.Error(), tt.wantError)
190+
})
191+
}
192+
}
193+
194+
func queryRequest(t *testing.T, name string, opts test.DriverOpts, cfg string, marcos sqlds.Macros) (*backend.QueryDataRequest, *test.SqlHandler, *sqlds.SQLDatasource) {
195+
driver, handler := test.NewDriver(name, test.Data{}, nil, opts, marcos)
104196
ds := sqlds.NewDatasource(driver)
105197

106198
req, settings := setupQueryRequest(name, cfg)
@@ -126,7 +218,7 @@ func setupQueryRequest(id string, cfg string) (*backend.QueryDataRequest, backen
126218
}
127219

128220
func healthRequest(t *testing.T, name string, opts test.DriverOpts, cfg string) (backend.CheckHealthRequest, *test.SqlHandler, *sqlds.SQLDatasource) {
129-
driver, handler := test.NewDriver(name, test.Data{}, nil, opts)
221+
driver, handler := test.NewDriver(name, test.Data{}, nil, opts, nil)
130222
ds := sqlds.NewDatasource(driver)
131223

132224
req, settings := setupHealthRequest(name, cfg)

go.mod

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ require (
88
github.com/go-sql-driver/mysql v1.8.1
99
github.com/google/go-cmp v0.6.0
1010
github.com/grafana/dataplane/sdata v0.0.9
11-
github.com/grafana/grafana-plugin-sdk-go v0.258.0
11+
github.com/grafana/grafana-plugin-sdk-go v0.259.0
1212
github.com/mithrandie/csvq-driver v1.7.0
1313
github.com/prometheus/client_golang v1.20.5
1414
github.com/stretchr/testify v1.9.0
@@ -76,7 +76,7 @@ require (
7676
github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus v1.0.1 // indirect
7777
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.1.0 // indirect
7878
github.com/hashicorp/go-hclog v1.6.3 // indirect
79-
github.com/hashicorp/go-plugin v1.6.1 // indirect
79+
github.com/hashicorp/go-plugin v1.6.2 // indirect
8080
github.com/hashicorp/yamux v0.1.1 // indirect
8181
github.com/json-iterator/go v1.1.12 // indirect
8282
github.com/jszwedko/go-datemath v0.1.1-0.20230526204004-640a500621d6 // indirect
@@ -86,7 +86,6 @@ require (
8686
github.com/mattn/go-isatty v0.0.19 // indirect
8787
github.com/mattn/go-runewidth v0.0.9 // indirect
8888
github.com/mitchellh/go-homedir v1.1.0 // indirect
89-
github.com/mitchellh/go-testing-interface v1.14.1 // indirect
9089
github.com/mithrandie/csvq v1.18.1 // indirect
9190
github.com/mithrandie/go-file/v2 v2.1.0 // indirect
9291
github.com/mithrandie/go-text v1.6.0 // indirect
@@ -99,7 +98,7 @@ require (
9998
github.com/pierrec/lz4/v4 v4.1.18 // indirect
10099
github.com/pmezard/go-difflib v1.0.0 // indirect
101100
github.com/prometheus/client_model v0.6.1 // indirect
102-
github.com/prometheus/common v0.60.0 // indirect
101+
github.com/prometheus/common v0.60.1 // indirect
103102
github.com/prometheus/procfs v0.15.1 // indirect
104103
golang.org/x/crypto v0.28.0 // indirect
105104
golang.org/x/exp v0.0.0-20231006140011-7918f672742d // indirect

go.sum

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY=
7171
github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ=
7272
github.com/grafana/dataplane/sdata v0.0.9 h1:AGL1LZnCUG4MnQtnWpBPbQ8ZpptaZs14w6kE/MWfg7s=
7373
github.com/grafana/dataplane/sdata v0.0.9/go.mod h1:Jvs5ddpGmn6vcxT7tCTWAZ1mgi4sbcdFt9utQx5uMAU=
74-
github.com/grafana/grafana-plugin-sdk-go v0.258.0 h1:rWsaD+5wuGUSNr9fFnSwS6t/jcRtAoEJ51pIR9bbPNs=
75-
github.com/grafana/grafana-plugin-sdk-go v0.258.0/go.mod h1:jN19FbzhAcPTLPIy31X5nvx38rR5eoD/1rASiip0GBY=
74+
github.com/grafana/grafana-plugin-sdk-go v0.259.0 h1:r/IbTzE89nqewLOpTVJUUnRaZUEu/3an+yIyBztUEd8=
75+
github.com/grafana/grafana-plugin-sdk-go v0.259.0/go.mod h1:W0oyXqr67j7VuFqGRz6bwYnIVO/O0pscPNOzpa/oeUA=
7676
github.com/grafana/otel-profiling-go v0.5.1 h1:stVPKAFZSa7eGiqbYuG25VcqYksR6iWvF3YH66t4qL8=
7777
github.com/grafana/otel-profiling-go v0.5.1/go.mod h1:ftN/t5A/4gQI19/8MoWurBEtC6gFw8Dns1sJZ9W4Tls=
7878
github.com/grafana/pyroscope-go/godeltaprof v0.1.8 h1:iwOtYXeeVSAeYefJNaxDytgjKtUuKQbJqgAIjlnicKg=
@@ -85,8 +85,8 @@ github.com/grpc-ecosystem/grpc-gateway/v2 v2.22.0 h1:asbCHRVmodnJTuQ3qamDwqVOIjw
8585
github.com/grpc-ecosystem/grpc-gateway/v2 v2.22.0/go.mod h1:ggCgvZ2r7uOoQjOyu2Y1NhHmEPPzzuhWgcza5M1Ji1I=
8686
github.com/hashicorp/go-hclog v1.6.3 h1:Qr2kF+eVWjTiYmU7Y31tYlP1h0q/X3Nl3tPGdaB11/k=
8787
github.com/hashicorp/go-hclog v1.6.3/go.mod h1:W4Qnvbt70Wk/zYJryRzDRU/4r0kIg0PVHBcfoyhpF5M=
88-
github.com/hashicorp/go-plugin v1.6.1 h1:P7MR2UP6gNKGPp+y7EZw2kOiq4IR9WiqLvp0XOsVdwI=
89-
github.com/hashicorp/go-plugin v1.6.1/go.mod h1:XPHFku2tFo3o3QKFgSYo+cghcUhw1NA1hZyMK0PWAw0=
88+
github.com/hashicorp/go-plugin v1.6.2 h1:zdGAEd0V1lCaU0u+MxWQhtSDQmahpkwOun8U8EiRVog=
89+
github.com/hashicorp/go-plugin v1.6.2/go.mod h1:CkgLQ5CZqNmdL9U9JzM532t8ZiYQ35+pj3b1FD37R0Q=
9090
github.com/hashicorp/yamux v0.1.1 h1:yrQxtgseBDrq9Y652vSRDvsKCJKOUD+GzTS4Y0Y8pvE=
9191
github.com/hashicorp/yamux v0.1.1/go.mod h1:CtWFDAQgb7dxtzFs4tWbplKIe2jSi3+5vKbgIO0SLnQ=
9292
github.com/invopop/yaml v0.2.0 h1:7zky/qH+O0DwAyoobXUqvVBwgBFRxKoQ/3FjcVpjTMY=
@@ -132,8 +132,6 @@ github.com/mattn/go-runewidth v0.0.9 h1:Lm995f3rfxdpd6TSmuVCHVb/QhupuXlYr8sCI/Qd
132132
github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI=
133133
github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=
134134
github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
135-
github.com/mitchellh/go-testing-interface v1.14.1 h1:jrgshOhYAUVNMAJiKbEu7EqAwgJJ2JqpQmpLJOu07cU=
136-
github.com/mitchellh/go-testing-interface v1.14.1/go.mod h1:gfgS7OtZj6MA4U1UrDRp04twqAjfvlZyCfX3sDjEym8=
137135
github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zxSIeXaQ=
138136
github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
139137
github.com/mithrandie/csvq v1.18.1 h1:f7NB2scbb7xx2ffPduJ2VtZ85RpWXfvanYskAkGlCBU=
@@ -169,8 +167,8 @@ github.com/prometheus/client_golang v1.20.5 h1:cxppBPuYhUnsO6yo/aoRol4L7q7UFfdm+
169167
github.com/prometheus/client_golang v1.20.5/go.mod h1:PIEt8X02hGcP8JWbeHyeZ53Y/jReSnHgO035n//V5WE=
170168
github.com/prometheus/client_model v0.6.1 h1:ZKSh/rekM+n3CeS952MLRAdFwIKqeY8b62p8ais2e9E=
171169
github.com/prometheus/client_model v0.6.1/go.mod h1:OrxVMOVHjw3lKMa8+x6HeMGkHMQyHDk9E3jmP2AmGiY=
172-
github.com/prometheus/common v0.60.0 h1:+V9PAREWNvJMAuJ1x1BaWl9dewMW4YrHZQbx0sJNllA=
173-
github.com/prometheus/common v0.60.0/go.mod h1:h0LYf1R1deLSKtD4Vdg8gy4RuOvENW2J/h19V5NADQw=
170+
github.com/prometheus/common v0.60.1 h1:FUas6GcOw66yB/73KC+BOZoFJmbo/1pojoILArPAaSc=
171+
github.com/prometheus/common v0.60.1/go.mod h1:h0LYf1R1deLSKtD4Vdg8gy4RuOvENW2J/h19V5NADQw=
174172
github.com/prometheus/procfs v0.15.1 h1:YagwOFzUgYfKKHX6Dr+sHT7km/hxC76UB0learggepc=
175173
github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoGhij/e3PBqk=
176174
github.com/rogpeppe/go-charset v0.0.0-20180617210344-2471d30d28b4/go.mod h1:qgYeAmZ5ZIpBWTGllZSQnw97Dj+woV0toclVaRGI8pc=

macros.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@ package sqlds
22

33
import (
44
"errors"
5+
56
"github.com/grafana/grafana-plugin-sdk-go/data/sqlutil"
67
)
78

89
var (
9-
// ErrorBadArgumentCount is returned from macros when the wrong number of arguments were provided
10-
ErrorBadArgumentCount = errors.New("unexpected number of arguments")
10+
ErrorParsingMacroBrackets = errors.New("failed to parse macro arguments (missing close bracket?)")
1111
)
1212

1313
// MacroFunc defines a signature for applying a query macro

test/driver.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import (
1919
var registered = map[string]*SqlHandler{}
2020

2121
// NewDriver creates and registers a new test datasource driver
22-
func NewDriver(name string, dbdata Data, converters []sqlutil.Converter, opts DriverOpts) (TestDS, *SqlHandler) {
22+
func NewDriver(name string, dbdata Data, converters []sqlutil.Converter, opts DriverOpts, macros sqlds.Macros) (TestDS, *SqlHandler) {
2323
if registered[name] == nil {
2424
handler := NewDriverHandler(dbdata, opts)
2525
registered[name] = &handler
@@ -34,14 +34,16 @@ func NewDriver(name string, dbdata Data, converters []sqlutil.Converter, opts Dr
3434
return sql.Open(name, "")
3535
},
3636
converters,
37+
macros,
3738
), registered[name]
3839
}
3940

4041
// NewTestDS creates a new test datasource driver
41-
func NewTestDS(openDBfn func(msg json.RawMessage) (*sql.DB, error), converters []sqlutil.Converter) TestDS {
42+
func NewTestDS(openDBfn func(msg json.RawMessage) (*sql.DB, error), converters []sqlutil.Converter, macros sqlds.Macros) TestDS {
4243
return TestDS{
4344
openDBfn: openDBfn,
4445
converters: converters,
46+
macros: macros,
4547
}
4648
}
4749

@@ -146,6 +148,7 @@ type Column struct {
146148
type TestDS struct {
147149
openDBfn func(msg json.RawMessage) (*sql.DB, error)
148150
converters []sqlutil.Converter
151+
macros sqlds.Macros
149152
sqlds.Driver
150153
}
151154

@@ -171,7 +174,7 @@ func (s TestDS) Settings(ctx context.Context, config backend.DataSourceInstanceS
171174

172175
// Macros - Macros for the test database
173176
func (s TestDS) Macros() sqlds.Macros {
174-
return sqlds.DefaultMacros
177+
return s.macros
175178
}
176179

177180
// Converters - Converters for the test database

0 commit comments

Comments
 (0)