From cd9768c73877c8cf84c3647d55bcc966bfe2e415 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Mon, 16 Jan 2023 14:57:04 +0530 Subject: [PATCH] feat: dashboard variable chaining (#2036) --- pkg/query-service/app/http_handler.go | 61 ++++++++++ pkg/query-service/app/http_handler_test.go | 132 +++++++++++++++++++++ pkg/query-service/app/logs/parser_test.go | 2 +- pkg/query-service/app/parser_test.go | 2 +- pkg/query-service/model/queryParams.go | 5 + 5 files changed, 200 insertions(+), 2 deletions(-) create mode 100644 pkg/query-service/app/http_handler_test.go diff --git a/pkg/query-service/app/http_handler.go b/pkg/query-service/app/http_handler.go index 6bb3a21999..58a060649a 100644 --- a/pkg/query-service/app/http_handler.go +++ b/pkg/query-service/app/http_handler.go @@ -346,6 +346,7 @@ func (aH *APIHandler) RegisterRoutes(router *mux.Router) { router.HandleFunc("/api/v1/dashboards/{uuid}", EditAccess(aH.updateDashboard)).Methods(http.MethodPut) router.HandleFunc("/api/v1/dashboards/{uuid}", EditAccess(aH.deleteDashboard)).Methods(http.MethodDelete) router.HandleFunc("/api/v1/variables/query", ViewAccess(aH.queryDashboardVars)).Methods(http.MethodGet) + router.HandleFunc("/api/v2/variables/query", ViewAccess(aH.queryDashboardVarsV2)).Methods(http.MethodPost) router.HandleFunc("/api/v1/feedback", OpenAccess(aH.submitFeedback)).Methods(http.MethodPost) // router.HandleFunc("/api/v1/get_percentiles", aH.getApplicationPercentiles).Methods(http.MethodGet) @@ -793,6 +794,66 @@ func (aH *APIHandler) queryDashboardVars(w http.ResponseWriter, r *http.Request) aH.Respond(w, dashboardVars) } +func prepareQuery(r *http.Request) (string, error) { + var postData *model.DashboardVars + + if err := json.NewDecoder(r.Body).Decode(&postData); err != nil { + return "", fmt.Errorf("failed to decode request body: %v", err) + } + + query := strings.TrimSpace(postData.Query) + + if query == "" { + return "", fmt.Errorf("query is required") + } + + notAllowedOps := []string{ + "alter table", + "drop table", + "truncate table", + "drop database", + "drop view", + "drop function", + } + + for _, op := range notAllowedOps { + if strings.Contains(strings.ToLower(query), op) { + return "", fmt.Errorf("Operation %s is not allowed", op) + } + } + + vars := make(map[string]string) + for k, v := range postData.Variables { + vars[k] = metrics.FormattedValue(v) + } + tmpl := template.New("dashboard-vars") + tmpl, tmplErr := tmpl.Parse(query) + if tmplErr != nil { + return "", tmplErr + } + var queryBuf bytes.Buffer + tmplErr = tmpl.Execute(&queryBuf, vars) + if tmplErr != nil { + return "", tmplErr + } + return queryBuf.String(), nil +} + +func (aH *APIHandler) queryDashboardVarsV2(w http.ResponseWriter, r *http.Request) { + query, err := prepareQuery(r) + if err != nil { + RespondError(w, &model.ApiError{Typ: model.ErrorBadData, Err: err}, nil) + return + } + + dashboardVars, err := aH.reader.QueryDashboardVars(r.Context(), query) + if err != nil { + RespondError(w, &model.ApiError{Typ: model.ErrorBadData, Err: err}, nil) + return + } + aH.Respond(w, dashboardVars) +} + func (aH *APIHandler) updateDashboard(w http.ResponseWriter, r *http.Request) { uuid := mux.Vars(r)["uuid"] diff --git a/pkg/query-service/app/http_handler_test.go b/pkg/query-service/app/http_handler_test.go new file mode 100644 index 0000000000..a84014eecd --- /dev/null +++ b/pkg/query-service/app/http_handler_test.go @@ -0,0 +1,132 @@ +package app + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "go.signoz.io/signoz/pkg/query-service/model" +) + +func TestPrepareQuery(t *testing.T) { + type testCase struct { + name string + postData *model.DashboardVars + query string + expectedErr bool + errMsg string + } + + testCases := []testCase{ + { + name: "test empty query", + postData: &model.DashboardVars{ + Query: "", + }, + expectedErr: true, + errMsg: "query is required", + }, + { + name: "test query with no variables", + postData: &model.DashboardVars{ + Query: "select * from table", + }, + expectedErr: false, + query: "select * from table", + }, + { + name: "test query with variables", + postData: &model.DashboardVars{ + Query: "select * from table where id = {{.id}}", + Variables: map[string]interface{}{ + "id": "1", + }, + }, + query: "select * from table where id = '1'", + expectedErr: false, + }, + // While this does seem like it should throw error, it shouldn't because empty value is a valid value + // for certain scenarios and should be treated as such. + { + name: "test query with variables and no value", + postData: &model.DashboardVars{ + Query: "select * from table where id = {{.id}}", + Variables: map[string]interface{}{ + "id": "", + }, + }, + query: "select * from table where id = ''", + expectedErr: false, + }, + { + name: "query contains alter table", + postData: &model.DashboardVars{ + Query: "ALTER TABLE signoz_table DELETE where true", + }, + expectedErr: true, + errMsg: "query shouldn't alter data", + }, + { + name: "query text produces template exec error", + postData: &model.DashboardVars{ + Query: "SELECT durationNano from signoz_traces.signoz_index_v2 WHERE id = {{if .X}}1{{else}}2{{else}}3{{end}}", + }, + expectedErr: true, + errMsg: "expected end; found {{else}}", + }, + { + name: "variables contain array", + postData: &model.DashboardVars{ + Query: "SELECT operation FROM table WHERE serviceName IN {{.serviceName}}", + Variables: map[string]interface{}{ + "serviceName": []string{"frontend", "route"}, + }, + }, + query: "SELECT operation FROM table WHERE serviceName IN ['frontend','route']", + }, + { + name: "query uses mixed types of variables", + postData: &model.DashboardVars{ + Query: "SELECT body FROM table Where id = {{.id}} AND elapsed_time >= {{.elapsed_time}} AND serviceName IN {{.serviceName}}", + Variables: map[string]interface{}{ + "serviceName": []string{"frontend", "route"}, + "id": "id", + "elapsed_time": 1.24, + }, + }, + query: "SELECT body FROM table Where id = 'id' AND elapsed_time >= 1.240000 AND serviceName IN ['frontend','route']", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var b bytes.Buffer + err := json.NewEncoder(&b).Encode(tc.postData) + if err != nil { + t.Fatal(err) + } + + req := httptest.NewRequest(http.MethodPost, "/api/v2/variables/query", &b) + query, err := prepareQuery(req) + + if tc.expectedErr { + if err == nil { + t.Errorf("expected error, but got nil") + } + if !strings.Contains(err.Error(), tc.errMsg) { + t.Errorf("expected error message contain: %s, but got: %s", tc.errMsg, err.Error()) + } + } else { + if err != nil { + t.Errorf("expected no error, but got: %s", err.Error()) + } + if query != tc.query { + t.Errorf("expected query: %s, but got: %s", tc.query, query) + } + } + }) + } +} diff --git a/pkg/query-service/app/logs/parser_test.go b/pkg/query-service/app/logs/parser_test.go index d51f5b554f..b139038b8f 100644 --- a/pkg/query-service/app/logs/parser_test.go +++ b/pkg/query-service/app/logs/parser_test.go @@ -372,7 +372,7 @@ var generateSQLQueryTestCases = []struct { func TestGenerateSQLQuery(t *testing.T) { for _, test := range generateSQLQueryTestCases { Convey("testGenerateSQL", t, func() { - res, _ := GenerateSQLWhere(&generateSQLQueryFields, &test.Filter) + res, _, _ := GenerateSQLWhere(&generateSQLQueryFields, &test.Filter) So(res, ShouldEqual, test.SqlFilter) }) } diff --git a/pkg/query-service/app/parser_test.go b/pkg/query-service/app/parser_test.go index 1ce5ec488b..53e08125e6 100644 --- a/pkg/query-service/app/parser_test.go +++ b/pkg/query-service/app/parser_test.go @@ -23,7 +23,7 @@ func TestParseFilterSingleFilter(t *testing.T) { req, _ := http.NewRequest("POST", "", bytes.NewReader(postBody)) res, _ := parseFilterSet(req) query, _ := metrics.BuildMetricsTimeSeriesFilterQuery(res, []string{}, "table", model.NOOP) - So(query, ShouldContainSubstring, "signoz_metrics.time_series_v2 WHERE metric_name = 'table' AND JSONExtractString(labels, 'namespace') = 'a'") + So(query, ShouldContainSubstring, "signoz_metrics.distributed_time_series_v2 WHERE metric_name = 'table' AND JSONExtractString(labels, 'namespace') = 'a'") }) } diff --git a/pkg/query-service/model/queryParams.go b/pkg/query-service/model/queryParams.go index da0d274406..7262b238b7 100644 --- a/pkg/query-service/model/queryParams.go +++ b/pkg/query-service/model/queryParams.go @@ -131,6 +131,11 @@ type QueryRangeParamsV2 struct { Variables map[string]interface{} `json:"variables,omitempty"` } +type DashboardVars struct { + Query string `json:"query"` + Variables map[string]interface{} `json:"variables,omitempty"` +} + // Metric auto complete types type metricTags map[string]string