From f350b0e2f0a9a34216e5d709c8805076835de759 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Mon, 4 Aug 2025 21:02:54 +0530 Subject: [PATCH 1/4] chore: add endpoint to replace varibales (#8689) --- ee/anomaly/seasonal.go | 20 +- ee/query-service/app/api/api.go | 2 + ee/query-service/app/api/queryrange.go | 6 +- ee/query-service/rules/anomaly.go | 3 +- pkg/querier/api.go | 55 ++ pkg/querier/builder_query.go | 10 +- pkg/querier/consume.go | 2 +- pkg/querier/querier.go | 47 +- pkg/query-service/app/http_handler.go | 1 + pkg/query-service/model/v3/v3.go | 2 +- pkg/query-service/rules/threshold_rule.go | 14 +- pkg/querybuilder/agg_rewrite.go | 6 +- .../resourcefilter/statement_builder.go | 4 +- pkg/querybuilder/where_clause_visitor.go | 84 ++- .../filter_expr_logs_body_json_test.go | 4 +- pkg/telemetrylogs/filter_expr_logs_test.go | 4 +- pkg/telemetrylogs/statement_builder.go | 67 ++- pkg/telemetrylogs/stmt_builder_test.go | 109 ++++ pkg/telemetrymetadata/metadata.go | 4 +- pkg/telemetrymetrics/statement_builder.go | 8 +- pkg/telemetrytraces/span_scope_simple_test.go | 6 +- pkg/telemetrytraces/statement_builder.go | 81 ++- pkg/telemetrytraces/stmt_builder_test.go | 31 +- .../querybuildertypesv5/builder_elements.go | 3 +- .../querybuildertypesv5/builder_query.go | 2 +- .../querybuildertypesv5/qb.go | 7 +- .../querybuildertypesv5/query.go | 9 +- .../querybuildertypesv5/resp.go | 58 +- pkg/types/ruletypes/alerting.go | 2 +- pkg/variables/variable_replace_visitor.go | 528 ++++++++++++++++++ .../variable_replace_visitor_test.go | 463 +++++++++++++++ 31 files changed, 1484 insertions(+), 158 deletions(-) create mode 100644 pkg/variables/variable_replace_visitor.go create mode 100644 pkg/variables/variable_replace_visitor_test.go diff --git a/ee/anomaly/seasonal.go b/ee/anomaly/seasonal.go index abd455e0e005..6636188b6d04 100644 --- a/ee/anomaly/seasonal.go +++ b/ee/anomaly/seasonal.go @@ -50,19 +50,14 @@ func (p *BaseSeasonalProvider) getQueryParams(req *AnomaliesRequest) *anomalyQue func (p *BaseSeasonalProvider) toTSResults(ctx context.Context, resp *qbtypes.QueryRangeResponse) []*qbtypes.TimeSeriesData { - if resp == nil || resp.Data == nil { + tsData := []*qbtypes.TimeSeriesData{} + + if resp == nil { p.logger.InfoContext(ctx, "nil response from query range") + return tsData } - data, ok := resp.Data.(struct { - Results []any `json:"results"` - Warnings []string `json:"warnings"` - }) - if !ok { - return nil - } - tsData := []*qbtypes.TimeSeriesData{} - for _, item := range data.Results { + for _, item := range resp.Data.Results { if resultData, ok := item.(*qbtypes.TimeSeriesData); ok { tsData = append(tsData, resultData) } @@ -395,6 +390,11 @@ func (p *BaseSeasonalProvider) getAnomalies(ctx context.Context, orgID valuer.UU continue } + // no data; + if len(result.Aggregations) == 0 { + continue + } + aggOfInterest := result.Aggregations[0] for _, series := range aggOfInterest.Series { diff --git a/ee/query-service/app/api/api.go b/ee/query-service/app/api/api.go index 26f69087b164..c5fcbf64f473 100644 --- a/ee/query-service/app/api/api.go +++ b/ee/query-service/app/api/api.go @@ -113,6 +113,8 @@ func (ah *APIHandler) RegisterRoutes(router *mux.Router, am *middleware.AuthZ) { // v5 router.HandleFunc("/api/v5/query_range", am.ViewAccess(ah.queryRangeV5)).Methods(http.MethodPost) + router.HandleFunc("/api/v5/substitute_vars", am.ViewAccess(ah.QuerierAPI.ReplaceVariables)).Methods(http.MethodPost) + // Gateway router.PathPrefix(gateway.RoutePrefix).HandlerFunc(am.EditAccess(ah.ServeGatewayHTTP)) diff --git a/ee/query-service/app/api/queryrange.go b/ee/query-service/app/api/queryrange.go index 85d60d4b6b43..698ac2d91c72 100644 --- a/ee/query-service/app/api/queryrange.go +++ b/ee/query-service/app/api/queryrange.go @@ -260,11 +260,9 @@ func (aH *APIHandler) queryRangeV5(rw http.ResponseWriter, req *http.Request) { finalResp := &qbtypes.QueryRangeResponse{ Type: queryRangeRequest.RequestType, Data: struct { - Results []any `json:"results"` - Warnings []string `json:"warnings"` + Results []any `json:"results"` }{ - Results: results, - Warnings: make([]string, 0), // TODO(srikanthccv): will there be any warnings here? + Results: results, }, Meta: struct { RowsScanned uint64 `json:"rowsScanned"` diff --git a/ee/query-service/rules/anomaly.go b/ee/query-service/rules/anomaly.go index 52226bd08b7f..3fbf1e32b1f2 100644 --- a/ee/query-service/rules/anomaly.go +++ b/ee/query-service/rules/anomaly.go @@ -211,7 +211,8 @@ func (r *AnomalyRule) prepareQueryRangeV5(ctx context.Context, ts time.Time) (*q }, NoCache: true, } - copy(r.Condition().CompositeQuery.Queries, req.CompositeQuery.Queries) + req.CompositeQuery.Queries = make([]qbtypes.QueryEnvelope, len(r.Condition().CompositeQuery.Queries)) + copy(req.CompositeQuery.Queries, r.Condition().CompositeQuery.Queries) return req, nil } diff --git a/pkg/querier/api.go b/pkg/querier/api.go index 38e1b567c92e..ba49b7dbe392 100644 --- a/pkg/querier/api.go +++ b/pkg/querier/api.go @@ -14,6 +14,7 @@ import ( "github.com/SigNoz/signoz/pkg/types/authtypes" qbtypes "github.com/SigNoz/signoz/pkg/types/querybuildertypes/querybuildertypesv5" "github.com/SigNoz/signoz/pkg/valuer" + "github.com/SigNoz/signoz/pkg/variables" ) type API struct { @@ -85,6 +86,60 @@ func (a *API) QueryRange(rw http.ResponseWriter, req *http.Request) { render.Success(rw, http.StatusOK, queryRangeResponse) } +// TODO(srikanthccv): everything done here can be done on frontend as well +// For the time being I am adding a helper function +func (a *API) ReplaceVariables(rw http.ResponseWriter, req *http.Request) { + + var queryRangeRequest qbtypes.QueryRangeRequest + if err := json.NewDecoder(req.Body).Decode(&queryRangeRequest); err != nil { + render.Error(rw, err) + return + } + + errs := []error{} + + for idx, item := range queryRangeRequest.CompositeQuery.Queries { + if item.Type == qbtypes.QueryTypeBuilder { + switch spec := item.Spec.(type) { + case qbtypes.QueryBuilderQuery[qbtypes.LogAggregation]: + if spec.Filter != nil && spec.Filter.Expression != "" { + replaced, err := variables.ReplaceVariablesInExpression(spec.Filter.Expression, queryRangeRequest.Variables) + if err != nil { + errs = append(errs, err) + } + spec.Filter.Expression = replaced + } + queryRangeRequest.CompositeQuery.Queries[idx].Spec = spec + case qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]: + if spec.Filter != nil && spec.Filter.Expression != "" { + replaced, err := variables.ReplaceVariablesInExpression(spec.Filter.Expression, queryRangeRequest.Variables) + if err != nil { + errs = append(errs, err) + } + spec.Filter.Expression = replaced + } + queryRangeRequest.CompositeQuery.Queries[idx].Spec = spec + case qbtypes.QueryBuilderQuery[qbtypes.MetricAggregation]: + if spec.Filter != nil && spec.Filter.Expression != "" { + replaced, err := variables.ReplaceVariablesInExpression(spec.Filter.Expression, queryRangeRequest.Variables) + if err != nil { + errs = append(errs, err) + } + spec.Filter.Expression = replaced + } + queryRangeRequest.CompositeQuery.Queries[idx].Spec = spec + } + } + } + + if len(errs) != 0 { + render.Error(rw, errors.NewInvalidInputf(errors.CodeInvalidInput, errors.Join(errs...).Error())) + return + } + + render.Success(rw, http.StatusOK, queryRangeRequest) +} + func (a *API) logEvent(ctx context.Context, referrer string, event *qbtypes.QBEvent) { claims, err := authtypes.ClaimsFromContext(ctx) if err != nil { diff --git a/pkg/querier/builder_query.go b/pkg/querier/builder_query.go index 756dbd318b7b..3b1e4166daf3 100644 --- a/pkg/querier/builder_query.go +++ b/pkg/querier/builder_query.go @@ -194,7 +194,9 @@ func (q *builderQuery[T]) Execute(ctx context.Context) (*qbtypes.Result, error) if err != nil { return nil, err } + result.Warnings = stmt.Warnings + result.WarningsDocURL = stmt.WarningsDocURL return result, nil } @@ -297,6 +299,9 @@ func (q *builderQuery[T]) executeWindowList(ctx context.Context) (*qbtypes.Resul } } + var warnings []string + var warningsDocURL string + for _, r := range buckets { q.spec.Offset = 0 q.spec.Limit = need @@ -305,7 +310,8 @@ func (q *builderQuery[T]) executeWindowList(ctx context.Context) (*qbtypes.Resul if err != nil { return nil, err } - + warnings = stmt.Warnings + warningsDocURL = stmt.WarningsDocURL // Execute with proper context for partial value detection res, err := q.executeWithContext(ctx, stmt.Query, stmt.Args) if err != nil { @@ -345,6 +351,8 @@ func (q *builderQuery[T]) executeWindowList(ctx context.Context) (*qbtypes.Resul Rows: rows, NextCursor: nextCursor, }, + Warnings: warnings, + WarningsDocURL: warningsDocURL, Stats: qbtypes.ExecStats{ RowsScanned: totalRows, BytesScanned: totalBytes, diff --git a/pkg/querier/consume.go b/pkg/querier/consume.go index b43f4461ef65..127530a597aa 100644 --- a/pkg/querier/consume.go +++ b/pkg/querier/consume.go @@ -332,7 +332,7 @@ func readAsScalar(rows driver.Rows, queryName string) (*qbtypes.ScalarData, erro }, nil } -func derefValue(v interface{}) interface{} { +func derefValue(v any) any { if v == nil { return nil } diff --git a/pkg/querier/querier.go b/pkg/querier/querier.go index db8820f089eb..148e5b872156 100644 --- a/pkg/querier/querier.go +++ b/pkg/querier/querier.go @@ -290,6 +290,7 @@ func (q *querier) run( ) (*qbtypes.QueryRangeResponse, error) { results := make(map[string]any) warnings := make([]string, 0) + warningsDocURL := "" stats := qbtypes.ExecStats{} hasData := func(result *qbtypes.Result) bool { @@ -338,6 +339,7 @@ func (q *querier) run( } results[name] = result.Value warnings = append(warnings, result.Warnings...) + warningsDocURL = result.WarningsDocURL stats.RowsScanned += result.Stats.RowsScanned stats.BytesScanned += result.Stats.BytesScanned stats.DurationMS += result.Stats.DurationMS @@ -349,6 +351,7 @@ func (q *querier) run( } results[name] = result.Value warnings = append(warnings, result.Warnings...) + warningsDocURL = result.WarningsDocURL stats.RowsScanned += result.Stats.RowsScanned stats.BytesScanned += result.Stats.BytesScanned stats.DurationMS += result.Stats.DurationMS @@ -360,14 +363,10 @@ func (q *querier) run( return nil, err } - return &qbtypes.QueryRangeResponse{ + resp := &qbtypes.QueryRangeResponse{ Type: req.RequestType, - Data: struct { - Results []any `json:"results"` - Warnings []string `json:"warnings"` - }{ - Results: maps.Values(processedResults), - Warnings: warnings, + Data: qbtypes.QueryData{ + Results: maps.Values(processedResults), }, Meta: struct { RowsScanned uint64 `json:"rowsScanned"` @@ -378,7 +377,23 @@ func (q *querier) run( BytesScanned: stats.BytesScanned, DurationMS: stats.DurationMS, }, - }, nil + } + + if len(warnings) != 0 { + warns := make([]qbtypes.QueryWarnDataAdditional, len(warnings)) + for i, warning := range warnings { + warns[i] = qbtypes.QueryWarnDataAdditional{ + Message: warning, + } + } + + resp.Warning = qbtypes.QueryWarnData{ + Message: "Encountered warnings", + Url: warningsDocURL, + Warnings: warns, + } + } + return resp, nil } // executeWithCache executes a query using the bucket cache @@ -520,9 +535,10 @@ func (q *querier) mergeResults(cached *qbtypes.Result, fresh []*qbtypes.Result) // If cached is nil but we have multiple fresh results, we need to merge them // We need to merge all fresh results properly to avoid duplicates merged := &qbtypes.Result{ - Type: fresh[0].Type, - Stats: fresh[0].Stats, - Warnings: fresh[0].Warnings, + Type: fresh[0].Type, + Stats: fresh[0].Stats, + Warnings: fresh[0].Warnings, + WarningsDocURL: fresh[0].WarningsDocURL, } // Merge all fresh results including the first one @@ -537,10 +553,11 @@ func (q *querier) mergeResults(cached *qbtypes.Result, fresh []*qbtypes.Result) // Start with cached result merged := &qbtypes.Result{ - Type: cached.Type, - Value: cached.Value, - Stats: cached.Stats, - Warnings: cached.Warnings, + Type: cached.Type, + Value: cached.Value, + Stats: cached.Stats, + Warnings: cached.Warnings, + WarningsDocURL: cached.WarningsDocURL, } // If no fresh results, return cached diff --git a/pkg/query-service/app/http_handler.go b/pkg/query-service/app/http_handler.go index 69be0b7f18fb..1b580a0f5141 100644 --- a/pkg/query-service/app/http_handler.go +++ b/pkg/query-service/app/http_handler.go @@ -470,6 +470,7 @@ func (aH *APIHandler) RegisterQueryRangeV4Routes(router *mux.Router, am *middlew func (aH *APIHandler) RegisterQueryRangeV5Routes(router *mux.Router, am *middleware.AuthZ) { subRouter := router.PathPrefix("/api/v5").Subrouter() subRouter.HandleFunc("/query_range", am.ViewAccess(aH.QuerierAPI.QueryRange)).Methods(http.MethodPost) + subRouter.HandleFunc("/substitute_vars", am.ViewAccess(aH.QuerierAPI.ReplaceVariables)).Methods(http.MethodPost) } // todo(remove): Implemented at render package (github.com/SigNoz/signoz/pkg/http/render) with the new error structure diff --git a/pkg/query-service/model/v3/v3.go b/pkg/query-service/model/v3/v3.go index 67bbf65af586..c989cb9218a3 100644 --- a/pkg/query-service/model/v3/v3.go +++ b/pkg/query-service/model/v3/v3.go @@ -603,7 +603,7 @@ func (c *CompositeQuery) Validate() error { return fmt.Errorf("composite query is required") } - if c.BuilderQueries == nil && c.ClickHouseQueries == nil && c.PromQueries == nil { + if c.BuilderQueries == nil && c.ClickHouseQueries == nil && c.PromQueries == nil && len(c.Queries) == 0 { return fmt.Errorf("composite query must contain at least one query type") } diff --git a/pkg/query-service/rules/threshold_rule.go b/pkg/query-service/rules/threshold_rule.go index 9ca3850defc4..a2dc1db1e176 100644 --- a/pkg/query-service/rules/threshold_rule.go +++ b/pkg/query-service/rules/threshold_rule.go @@ -291,7 +291,8 @@ func (r *ThresholdRule) prepareQueryRangeV5(ctx context.Context, ts time.Time) ( }, NoCache: true, } - copy(r.Condition().CompositeQuery.Queries, req.CompositeQuery.Queries) + req.CompositeQuery.Queries = make([]qbtypes.QueryEnvelope, len(r.Condition().CompositeQuery.Queries)) + copy(req.CompositeQuery.Queries, r.Condition().CompositeQuery.Queries) return req, nil } @@ -503,16 +504,7 @@ func (r *ThresholdRule) buildAndRunQueryV5(ctx context.Context, orgID valuer.UUI return nil, fmt.Errorf("internal error while querying") } - data, ok := v5Result.Data.(struct { - Results []any `json:"results"` - Warnings []string `json:"warnings"` - }) - - if !ok { - return nil, fmt.Errorf("unexpected result from v5 querier") - } - - for _, item := range data.Results { + for _, item := range v5Result.Data.Results { if tsData, ok := item.(*qbtypes.TimeSeriesData); ok { results = append(results, transition.ConvertV5TimeSeriesDataToV4Result(tsData)) } else { diff --git a/pkg/querybuilder/agg_rewrite.go b/pkg/querybuilder/agg_rewrite.go index 1b661a388220..7f49aafc2295 100644 --- a/pkg/querybuilder/agg_rewrite.go +++ b/pkg/querybuilder/agg_rewrite.go @@ -152,7 +152,7 @@ func (v *exprVisitor) VisitFunctionExpr(fn *chparser.FunctionExpr) error { aggFunc, ok := AggreFuncMap[valuer.NewString(name)] if !ok { - return nil + return errors.NewInvalidInputf(errors.CodeInvalidInput, "unrecognized function: %s", name) } var args []chparser.Expr @@ -180,7 +180,7 @@ func (v *exprVisitor) VisitFunctionExpr(fn *chparser.FunctionExpr) error { if aggFunc.FuncCombinator { // Map the predicate (last argument) origPred := args[len(args)-1].String() - whereClause, _, err := PrepareWhereClause( + whereClause, err := PrepareWhereClause( origPred, FilterExprVisitorOpts{ FieldKeys: v.fieldKeys, @@ -195,7 +195,7 @@ func (v *exprVisitor) VisitFunctionExpr(fn *chparser.FunctionExpr) error { return err } - newPred, chArgs := whereClause.BuildWithFlavor(sqlbuilder.ClickHouse) + newPred, chArgs := whereClause.WhereClause.BuildWithFlavor(sqlbuilder.ClickHouse) newPred = strings.TrimPrefix(newPred, "WHERE") parsedPred, err := parseFragment(newPred) if err != nil { diff --git a/pkg/querybuilder/resourcefilter/statement_builder.go b/pkg/querybuilder/resourcefilter/statement_builder.go index 3a3cb7fe283d..e9a61a8864f9 100644 --- a/pkg/querybuilder/resourcefilter/statement_builder.go +++ b/pkg/querybuilder/resourcefilter/statement_builder.go @@ -146,7 +146,7 @@ func (b *resourceFilterStatementBuilder[T]) addConditions( if query.Filter != nil && query.Filter.Expression != "" { // warnings would be encountered as part of the main condition already - filterWhereClause, _, err := querybuilder.PrepareWhereClause(query.Filter.Expression, querybuilder.FilterExprVisitorOpts{ + filterWhereClause, err := querybuilder.PrepareWhereClause(query.Filter.Expression, querybuilder.FilterExprVisitorOpts{ FieldMapper: b.fieldMapper, ConditionBuilder: b.conditionBuilder, FieldKeys: keys, @@ -164,7 +164,7 @@ func (b *resourceFilterStatementBuilder[T]) addConditions( return err } if filterWhereClause != nil { - sb.AddWhereClause(filterWhereClause) + sb.AddWhereClause(filterWhereClause.WhereClause) } } diff --git a/pkg/querybuilder/where_clause_visitor.go b/pkg/querybuilder/where_clause_visitor.go index 936803cd6276..f262573f6b88 100644 --- a/pkg/querybuilder/where_clause_visitor.go +++ b/pkg/querybuilder/where_clause_visitor.go @@ -15,14 +15,18 @@ import ( sqlbuilder "github.com/huandu/go-sqlbuilder" ) +var searchTroubleshootingGuideURL = "https://signoz.io/docs/userguide/search-troubleshooting/" + // filterExpressionVisitor implements the FilterQueryVisitor interface // to convert the parsed filter expressions into ClickHouse WHERE clause type filterExpressionVisitor struct { fieldMapper qbtypes.FieldMapper conditionBuilder qbtypes.ConditionBuilder warnings []string + mainWarnURL string fieldKeys map[string][]*telemetrytypes.TelemetryFieldKey errors []string + mainErrorURL string builder *sqlbuilder.SelectBuilder fullTextColumn *telemetrytypes.TelemetryFieldKey jsonBodyPrefix string @@ -70,8 +74,14 @@ func newFilterExpressionVisitor(opts FilterExprVisitorOpts) *filterExpressionVis } } +type PreparedWhereClause struct { + WhereClause *sqlbuilder.WhereClause + Warnings []string + WarningsDocURL string +} + // PrepareWhereClause generates a ClickHouse compatible WHERE clause from the filter query -func PrepareWhereClause(query string, opts FilterExprVisitorOpts) (*sqlbuilder.WhereClause, []string, error) { +func PrepareWhereClause(query string, opts FilterExprVisitorOpts) (*PreparedWhereClause, error) { // Setup the ANTLR parsing pipeline input := antlr.NewInputStream(query) lexer := grammar.NewFilterQueryLexer(input) @@ -102,14 +112,17 @@ func PrepareWhereClause(query string, opts FilterExprVisitorOpts) (*sqlbuilder.W combinedErrors := errors.Newf( errors.TypeInvalidInput, errors.CodeInvalidInput, - "found %d syntax errors while parsing the filter expression", + "Found %d syntax errors while parsing the search expression.", len(parserErrorListener.SyntaxErrors), ) additionals := make([]string, len(parserErrorListener.SyntaxErrors)) for _, err := range parserErrorListener.SyntaxErrors { - additionals = append(additionals, err.Error()) + if err.Error() != "" { + additionals = append(additionals, err.Error()) + } } - return nil, nil, combinedErrors.WithAdditional(additionals...) + + return nil, combinedErrors.WithAdditional(additionals...).WithUrl(searchTroubleshootingGuideURL) } // Visit the parse tree with our ClickHouse visitor @@ -120,15 +133,23 @@ func PrepareWhereClause(query string, opts FilterExprVisitorOpts) (*sqlbuilder.W combinedErrors := errors.Newf( errors.TypeInvalidInput, errors.CodeInvalidInput, - "found %d errors while parsing the search expression", + "Found %d errors while parsing the search expression.", len(visitor.errors), ) - return nil, nil, combinedErrors.WithAdditional(visitor.errors...) + url := visitor.mainErrorURL + if url == "" { + url = searchTroubleshootingGuideURL + } + return nil, combinedErrors.WithAdditional(visitor.errors...).WithUrl(url) + } + + if cond == "" { + cond = "true" } whereClause := sqlbuilder.NewWhereClause().AddWhereExpr(visitor.builder.Args, cond) - return whereClause, visitor.warnings, nil + return &PreparedWhereClause{whereClause, visitor.warnings, visitor.mainWarnURL}, nil } // Visit dispatches to the specific visit method based on node type @@ -194,7 +215,13 @@ func (v *filterExpressionVisitor) VisitOrExpression(ctx *grammar.OrExpressionCon andExpressionConditions := make([]string, len(andExpressions)) for i, expr := range andExpressions { - andExpressionConditions[i] = v.Visit(expr).(string) + if condExpr, ok := v.Visit(expr).(string); ok && condExpr != "" { + andExpressionConditions[i] = condExpr + } + } + + if len(andExpressionConditions) == 0 { + return "" } if len(andExpressionConditions) == 1 { @@ -210,7 +237,13 @@ func (v *filterExpressionVisitor) VisitAndExpression(ctx *grammar.AndExpressionC unaryExpressionConditions := make([]string, len(unaryExpressions)) for i, expr := range unaryExpressions { - unaryExpressionConditions[i] = v.Visit(expr).(string) + if condExpr, ok := v.Visit(expr).(string); ok && condExpr != "" { + unaryExpressionConditions[i] = condExpr + } + } + + if len(unaryExpressionConditions) == 0 { + return "" } if len(unaryExpressionConditions) == 1 { @@ -236,7 +269,10 @@ func (v *filterExpressionVisitor) VisitUnaryExpression(ctx *grammar.UnaryExpress func (v *filterExpressionVisitor) VisitPrimary(ctx *grammar.PrimaryContext) any { if ctx.OrExpression() != nil { // This is a parenthesized expression - return fmt.Sprintf("(%s)", v.Visit(ctx.OrExpression()).(string)) + if condExpr, ok := v.Visit(ctx.OrExpression()).(string); ok && condExpr != "" { + return fmt.Sprintf("(%s)", v.Visit(ctx.OrExpression()).(string)) + } + return "" } else if ctx.Comparison() != nil { return v.Visit(ctx.Comparison()) } else if ctx.FunctionCall() != nil { @@ -248,7 +284,7 @@ func (v *filterExpressionVisitor) VisitPrimary(ctx *grammar.PrimaryContext) any // Handle standalone key/value as a full text search term if ctx.GetChildCount() == 1 { if v.skipFullTextFilter { - return "true" + return "" } if v.fullTextColumn == nil { @@ -297,11 +333,7 @@ func (v *filterExpressionVisitor) VisitComparison(ctx *grammar.ComparisonContext // if key is missing and can be ignored, the condition is ignored if len(keys) == 0 && v.ignoreNotFoundKeys { - // Why do we return "true"? to prevent from create a empty tuple - // example, if the condition is (x AND (y OR z)) - // if we find ourselves ignoring all, then it creates and invalid - // condition (()) which throws invalid tuples error - return "true" + return "" } // this is used to skip the resource filtering on main table if @@ -315,11 +347,7 @@ func (v *filterExpressionVisitor) VisitComparison(ctx *grammar.ComparisonContext } keys = filteredKeys if len(keys) == 0 { - // Why do we return "true"? to prevent from create a empty tuple - // example, if the condition is (resource.service.name='api' AND (env='prod' OR env='production')) - // if we find ourselves skipping all, then it creates and invalid - // condition (()) which throws invalid tuples error - return "true" + return "" } } @@ -368,7 +396,7 @@ func (v *filterExpressionVisitor) VisitComparison(ctx *grammar.ComparisonContext var varItem qbtypes.VariableItem varItem, ok = v.variables[var_] // if not present, try without `$` prefix - if !ok { + if !ok && len(var_) > 0 { varItem, ok = v.variables[var_[1:]] } @@ -547,7 +575,7 @@ func (v *filterExpressionVisitor) VisitValueList(ctx *grammar.ValueListContext) func (v *filterExpressionVisitor) VisitFullText(ctx *grammar.FullTextContext) any { if v.skipFullTextFilter { - return "true" + return "" } var text string @@ -573,7 +601,7 @@ func (v *filterExpressionVisitor) VisitFullText(ctx *grammar.FullTextContext) an // VisitFunctionCall handles function calls like has(), hasAny(), etc. func (v *filterExpressionVisitor) VisitFunctionCall(ctx *grammar.FunctionCallContext) any { if v.skipFunctionCalls { - return "true" + return "" } // Get function name based on which token is present @@ -609,6 +637,10 @@ func (v *filterExpressionVisitor) VisitFunctionCall(ctx *grammar.FunctionCallCon if strings.HasPrefix(key.Name, v.jsonBodyPrefix) { fieldName, _ = v.jsonKeyToKey(context.Background(), key, qbtypes.FilterOperatorUnknown, value) } else { + // TODO(add docs for json body search) + if v.mainErrorURL == "" { + v.mainErrorURL = "https://signoz.io/docs/userguide/search-troubleshooting/#function-supports-only-body-json-search" + } v.errors = append(v.errors, fmt.Sprintf("function `%s` supports only body JSON search", functionName)) return "" } @@ -736,13 +768,15 @@ func (v *filterExpressionVisitor) VisitKey(ctx *grammar.KeyContext) any { // TODO(srikanthccv): do we want to return an error here? // should we infer the type and auto-magically build a key for expression? v.errors = append(v.errors, fmt.Sprintf("key `%s` not found", fieldKey.Name)) + v.mainErrorURL = "https://signoz.io/docs/userguide/search-troubleshooting/#key-fieldname-not-found" } } if len(fieldKeysForName) > 1 && !v.keysWithWarnings[keyName] { + v.mainWarnURL = "https://signoz.io/docs/userguide/field-context-data-types/" // this is warning state, we must have a unambiguous key v.warnings = append(v.warnings, fmt.Sprintf( - "key `%s` is ambiguous, found %d different combinations of field context and data type: %v", + "key `%s` is ambiguous, found %d different combinations of field context / data type: %v", fieldKey.Name, len(fieldKeysForName), fieldKeysForName, diff --git a/pkg/telemetrylogs/filter_expr_logs_body_json_test.go b/pkg/telemetrylogs/filter_expr_logs_body_json_test.go index a57371cb5558..3d3140768dc7 100644 --- a/pkg/telemetrylogs/filter_expr_logs_body_json_test.go +++ b/pkg/telemetrylogs/filter_expr_logs_body_json_test.go @@ -161,7 +161,7 @@ func TestFilterExprLogsBodyJSON(t *testing.T) { for _, tc := range testCases { t.Run(fmt.Sprintf("%s: %s", tc.category, limitString(tc.query, 50)), func(t *testing.T) { - clause, _, err := querybuilder.PrepareWhereClause(tc.query, opts) + clause, err := querybuilder.PrepareWhereClause(tc.query, opts) if tc.shouldPass { if err != nil { @@ -175,7 +175,7 @@ func TestFilterExprLogsBodyJSON(t *testing.T) { } // Build the SQL and print it for debugging - sql, args := clause.BuildWithFlavor(sqlbuilder.ClickHouse) + sql, args := clause.WhereClause.BuildWithFlavor(sqlbuilder.ClickHouse) require.Equal(t, tc.expectedQuery, sql) require.Equal(t, tc.expectedArgs, args) diff --git a/pkg/telemetrylogs/filter_expr_logs_test.go b/pkg/telemetrylogs/filter_expr_logs_test.go index 6b9d5c64539d..7096ee875c90 100644 --- a/pkg/telemetrylogs/filter_expr_logs_test.go +++ b/pkg/telemetrylogs/filter_expr_logs_test.go @@ -2297,7 +2297,7 @@ func TestFilterExprLogs(t *testing.T) { for _, tc := range testCases { t.Run(fmt.Sprintf("%s: %s", tc.category, limitString(tc.query, 50)), func(t *testing.T) { - clause, _, err := querybuilder.PrepareWhereClause(tc.query, opts) + clause, err := querybuilder.PrepareWhereClause(tc.query, opts) if tc.shouldPass { if err != nil { @@ -2311,7 +2311,7 @@ func TestFilterExprLogs(t *testing.T) { } // Build the SQL and print it for debugging - sql, args := clause.BuildWithFlavor(sqlbuilder.ClickHouse) + sql, args := clause.WhereClause.BuildWithFlavor(sqlbuilder.ClickHouse) require.Equal(t, tc.expectedQuery, sql) require.Equal(t, tc.expectedArgs, args) diff --git a/pkg/telemetrylogs/statement_builder.go b/pkg/telemetrylogs/statement_builder.go index 8c393a1f08ca..8cf87ce4e58d 100644 --- a/pkg/telemetrylogs/statement_builder.go +++ b/pkg/telemetrylogs/statement_builder.go @@ -228,7 +228,8 @@ func (b *logQueryStatementBuilder) buildListQuery( sb.From(fmt.Sprintf("%s.%s", DBName, LogsV2TableName)) // Add filter conditions - warnings, err := b.addFilterCondition(ctx, sb, start, end, query, keys, variables) + preparedWhereClause, err := b.addFilterCondition(ctx, sb, start, end, query, keys, variables) + if err != nil { return nil, err } @@ -258,11 +259,16 @@ func (b *logQueryStatementBuilder) buildListQuery( finalSQL := querybuilder.CombineCTEs(cteFragments) + mainSQL finalArgs := querybuilder.PrependArgs(cteArgs, mainArgs) - return &qbtypes.Statement{ - Query: finalSQL, - Args: finalArgs, - Warnings: warnings, - }, nil + stmt := &qbtypes.Statement{ + Query: finalSQL, + Args: finalArgs, + } + if preparedWhereClause != nil { + stmt.Warnings = preparedWhereClause.Warnings + stmt.WarningsDocURL = preparedWhereClause.WarningsDocURL + } + + return stmt, nil } func (b *logQueryStatementBuilder) buildTimeSeriesQuery( @@ -322,7 +328,8 @@ func (b *logQueryStatementBuilder) buildTimeSeriesQuery( } sb.From(fmt.Sprintf("%s.%s", DBName, LogsV2TableName)) - warnings, err := b.addFilterCondition(ctx, sb, start, end, query, keys, variables) + preparedWhereClause, err := b.addFilterCondition(ctx, sb, start, end, query, keys, variables) + if err != nil { return nil, err } @@ -401,11 +408,16 @@ func (b *logQueryStatementBuilder) buildTimeSeriesQuery( finalArgs = querybuilder.PrependArgs(cteArgs, mainArgs) } - return &qbtypes.Statement{ - Query: finalSQL, - Args: finalArgs, - Warnings: warnings, - }, nil + stmt := &qbtypes.Statement{ + Query: finalSQL, + Args: finalArgs, + } + if preparedWhereClause != nil { + stmt.Warnings = preparedWhereClause.Warnings + stmt.WarningsDocURL = preparedWhereClause.WarningsDocURL + } + + return stmt, nil } // buildScalarQuery builds a query for scalar panel type @@ -469,7 +481,8 @@ func (b *logQueryStatementBuilder) buildScalarQuery( sb.From(fmt.Sprintf("%s.%s", DBName, LogsV2TableName)) // Add filter conditions - warnings, err := b.addFilterCondition(ctx, sb, start, end, query, keys, variables) + preparedWhereClause, err := b.addFilterCondition(ctx, sb, start, end, query, keys, variables) + if err != nil { return nil, err } @@ -511,11 +524,16 @@ func (b *logQueryStatementBuilder) buildScalarQuery( finalSQL := querybuilder.CombineCTEs(cteFragments) + mainSQL finalArgs := querybuilder.PrependArgs(cteArgs, mainArgs) - return &qbtypes.Statement{ - Query: finalSQL, - Args: finalArgs, - Warnings: warnings, - }, nil + stmt := &qbtypes.Statement{ + Query: finalSQL, + Args: finalArgs, + } + if preparedWhereClause != nil { + stmt.Warnings = preparedWhereClause.Warnings + stmt.WarningsDocURL = preparedWhereClause.WarningsDocURL + } + + return stmt, nil } // buildFilterCondition builds SQL condition from filter expression @@ -526,15 +544,14 @@ func (b *logQueryStatementBuilder) addFilterCondition( query qbtypes.QueryBuilderQuery[qbtypes.LogAggregation], keys map[string][]*telemetrytypes.TelemetryFieldKey, variables map[string]qbtypes.VariableItem, -) ([]string, error) { +) (*querybuilder.PreparedWhereClause, error) { - var filterWhereClause *sqlbuilder.WhereClause - var warnings []string + var preparedWhereClause *querybuilder.PreparedWhereClause var err error if query.Filter != nil && query.Filter.Expression != "" { // add filter expression - filterWhereClause, warnings, err = querybuilder.PrepareWhereClause(query.Filter.Expression, querybuilder.FilterExprVisitorOpts{ + preparedWhereClause, err = querybuilder.PrepareWhereClause(query.Filter.Expression, querybuilder.FilterExprVisitorOpts{ FieldMapper: b.fm, ConditionBuilder: b.cb, FieldKeys: keys, @@ -550,8 +567,8 @@ func (b *logQueryStatementBuilder) addFilterCondition( } } - if filterWhereClause != nil { - sb.AddWhereClause(filterWhereClause) + if preparedWhereClause != nil { + sb.AddWhereClause(preparedWhereClause.WhereClause) } // add time filter @@ -560,7 +577,7 @@ func (b *logQueryStatementBuilder) addFilterCondition( sb.Where(sb.GE("timestamp", fmt.Sprintf("%d", start)), sb.L("timestamp", fmt.Sprintf("%d", end)), sb.GE("ts_bucket_start", startBucket), sb.LE("ts_bucket_start", endBucket)) - return warnings, nil + return preparedWhereClause, nil } func aggOrderBy(k qbtypes.OrderBy, q qbtypes.QueryBuilderQuery[qbtypes.LogAggregation]) (int, bool) { diff --git a/pkg/telemetrylogs/stmt_builder_test.go b/pkg/telemetrylogs/stmt_builder_test.go index 64d483858991..9993367edd0a 100644 --- a/pkg/telemetrylogs/stmt_builder_test.go +++ b/pkg/telemetrylogs/stmt_builder_test.go @@ -245,3 +245,112 @@ func TestStatementBuilderListQuery(t *testing.T) { }) } } + +func TestStatementBuilderListQueryResourceTests(t *testing.T) { + cases := []struct { + name string + requestType qbtypes.RequestType + query qbtypes.QueryBuilderQuery[qbtypes.LogAggregation] + expected qbtypes.Statement + expectedErr error + }{ + { + name: "List with full text search", + requestType: qbtypes.RequestTypeRaw, + query: qbtypes.QueryBuilderQuery[qbtypes.LogAggregation]{ + Signal: telemetrytypes.SignalLogs, + Filter: &qbtypes.Filter{ + Expression: "hello", + }, + Limit: 10, + }, + expected: qbtypes.Statement{ + Query: "WITH __resource_filter AS (SELECT fingerprint FROM signoz_logs.distributed_logs_v2_resource WHERE true AND seen_at_ts_bucket_start >= ? AND seen_at_ts_bucket_start <= ?) SELECT timestamp, id, trace_id, span_id, trace_flags, severity_text, severity_number, scope_name, scope_version, body, attributes_string, attributes_number, attributes_bool, resources_string, scope_string FROM signoz_logs.distributed_logs_v2 WHERE resource_fingerprint GLOBAL IN (SELECT fingerprint FROM __resource_filter) AND match(body, ?) AND timestamp >= ? AND timestamp < ? AND ts_bucket_start >= ? AND ts_bucket_start <= ? LIMIT ?", + Args: []any{uint64(1747945619), uint64(1747983448), "hello", "1747947419000000000", "1747983448000000000", uint64(1747945619), uint64(1747983448), 10}, + }, + expectedErr: nil, + }, + { + name: "list query with mat col order by", + requestType: qbtypes.RequestTypeRaw, + query: qbtypes.QueryBuilderQuery[qbtypes.LogAggregation]{ + Signal: telemetrytypes.SignalLogs, + Filter: &qbtypes.Filter{ + Expression: "service.name = 'cartservice' hello", + }, + Limit: 10, + Order: []qbtypes.OrderBy{ + { + Key: qbtypes.OrderByKey{ + TelemetryFieldKey: telemetrytypes.TelemetryFieldKey{ + Name: "materialized.key.name", + FieldContext: telemetrytypes.FieldContextAttribute, + FieldDataType: telemetrytypes.FieldDataTypeString, + }, + }, + Direction: qbtypes.OrderDirectionDesc, + }, + }, + }, + expected: qbtypes.Statement{ + Query: "WITH __resource_filter AS (SELECT fingerprint FROM signoz_logs.distributed_logs_v2_resource WHERE ((simpleJSONExtractString(labels, 'service.name') = ? AND labels LIKE ? AND labels LIKE ?)) AND seen_at_ts_bucket_start >= ? AND seen_at_ts_bucket_start <= ?) SELECT timestamp, id, trace_id, span_id, trace_flags, severity_text, severity_number, scope_name, scope_version, body, attributes_string, attributes_number, attributes_bool, resources_string, scope_string FROM signoz_logs.distributed_logs_v2 WHERE resource_fingerprint GLOBAL IN (SELECT fingerprint FROM __resource_filter) AND (match(body, ?)) AND timestamp >= ? AND timestamp < ? AND ts_bucket_start >= ? AND ts_bucket_start <= ? ORDER BY `attribute_string_materialized$$key$$name` AS `materialized.key.name` desc LIMIT ?", + Args: []any{"cartservice", "%service.name%", "%service.name\":\"cartservice%", uint64(1747945619), uint64(1747983448), "hello", "1747947419000000000", "1747983448000000000", uint64(1747945619), uint64(1747983448), 10}, + }, + expectedErr: nil, + }, + { + name: "List with json search", + requestType: qbtypes.RequestTypeRaw, + query: qbtypes.QueryBuilderQuery[qbtypes.LogAggregation]{ + Signal: telemetrytypes.SignalLogs, + Filter: &qbtypes.Filter{ + Expression: "body.status = 'success'", + }, + Limit: 10, + }, + expected: qbtypes.Statement{ + Query: "WITH __resource_filter AS (SELECT fingerprint FROM signoz_logs.distributed_logs_v2_resource WHERE true AND seen_at_ts_bucket_start >= ? AND seen_at_ts_bucket_start <= ?) SELECT timestamp, id, trace_id, span_id, trace_flags, severity_text, severity_number, scope_name, scope_version, body, attributes_string, attributes_number, attributes_bool, resources_string, scope_string FROM signoz_logs.distributed_logs_v2 WHERE resource_fingerprint GLOBAL IN (SELECT fingerprint FROM __resource_filter) AND (JSON_VALUE(body, '$.\"status\"') = ? AND JSON_EXISTS(body, '$.\"status\"')) AND timestamp >= ? AND timestamp < ? AND ts_bucket_start >= ? AND ts_bucket_start <= ? LIMIT ?", + Args: []any{uint64(1747945619), uint64(1747983448), "success", "1747947419000000000", "1747983448000000000", uint64(1747945619), uint64(1747983448), 10}, + }, + expectedErr: nil, + }, + } + + fm := NewFieldMapper() + cb := NewConditionBuilder(fm) + mockMetadataStore := telemetrytypestest.NewMockMetadataStore() + mockMetadataStore.KeysMap = buildCompleteFieldKeyMap() + + aggExprRewriter := querybuilder.NewAggExprRewriter(nil, fm, cb, "", nil) + + resourceFilterStmtBuilder := resourceFilterStmtBuilder() + + statementBuilder := NewLogQueryStatementBuilder( + instrumentationtest.New().ToProviderSettings(), + mockMetadataStore, + fm, + cb, + resourceFilterStmtBuilder, + aggExprRewriter, + DefaultFullTextColumn, + BodyJSONStringSearchPrefix, + GetBodyJSONKey, + ) + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + + q, err := statementBuilder.Build(context.Background(), 1747947419000, 1747983448000, c.requestType, c.query, nil) + + if c.expectedErr != nil { + require.Error(t, err) + require.Contains(t, err.Error(), c.expectedErr.Error()) + } else { + require.NoError(t, err) + require.Equal(t, c.expected.Query, q.Query) + require.Equal(t, c.expected.Args, q.Args) + require.Equal(t, c.expected.Warnings, q.Warnings) + } + }) + } +} diff --git a/pkg/telemetrymetadata/metadata.go b/pkg/telemetrymetadata/metadata.go index 4fd86e29f146..80c8d0d0594f 100644 --- a/pkg/telemetrymetadata/metadata.go +++ b/pkg/telemetrymetadata/metadata.go @@ -701,13 +701,13 @@ func (t *telemetryMetaStore) getRelatedValues(ctx context.Context, fieldValueSel return nil, err } - whereClause, _, err := querybuilder.PrepareWhereClause(fieldValueSelector.ExistingQuery, querybuilder.FilterExprVisitorOpts{ + whereClause, err := querybuilder.PrepareWhereClause(fieldValueSelector.ExistingQuery, querybuilder.FilterExprVisitorOpts{ FieldMapper: t.fm, ConditionBuilder: t.conditionBuilder, FieldKeys: keys, }) if err == nil { - sb.AddWhereClause(whereClause) + sb.AddWhereClause(whereClause.WhereClause) } else { t.logger.WarnContext(ctx, "error parsing existing query for related values", "error", err) } diff --git a/pkg/telemetrymetrics/statement_builder.go b/pkg/telemetrymetrics/statement_builder.go index e8ffeabb3a5c..3b0a39ed30ad 100644 --- a/pkg/telemetrymetrics/statement_builder.go +++ b/pkg/telemetrymetrics/statement_builder.go @@ -289,11 +289,11 @@ func (b *metricQueryStatementBuilder) buildTimeSeriesCTE( ) (string, []any, error) { sb := sqlbuilder.NewSelectBuilder() - var filterWhere *sqlbuilder.WhereClause + var preparedWhereClause *querybuilder.PreparedWhereClause var err error if query.Filter != nil && query.Filter.Expression != "" { - filterWhere, _, err = querybuilder.PrepareWhereClause(query.Filter.Expression, querybuilder.FilterExprVisitorOpts{ + preparedWhereClause, err = querybuilder.PrepareWhereClause(query.Filter.Expression, querybuilder.FilterExprVisitorOpts{ FieldMapper: b.fm, ConditionBuilder: b.cb, FieldKeys: keys, @@ -332,8 +332,8 @@ func (b *metricQueryStatementBuilder) buildTimeSeriesCTE( sb.EQ("__normalized", false), ) - if filterWhere != nil { - sb.AddWhereClause(filterWhere) + if preparedWhereClause != nil { + sb.AddWhereClause(preparedWhereClause.WhereClause) } sb.GroupBy("fingerprint") diff --git a/pkg/telemetrytraces/span_scope_simple_test.go b/pkg/telemetrytraces/span_scope_simple_test.go index d9558d52d63c..3fd43b21069c 100644 --- a/pkg/telemetrytraces/span_scope_simple_test.go +++ b/pkg/telemetrytraces/span_scope_simple_test.go @@ -63,7 +63,7 @@ func TestSpanScopeFilterExpression(t *testing.T) { FieldContext: telemetrytypes.FieldContextSpan, }} - whereClause, _, err := querybuilder.PrepareWhereClause(tt.expression, querybuilder.FilterExprVisitorOpts{ + whereClause, err := querybuilder.PrepareWhereClause(tt.expression, querybuilder.FilterExprVisitorOpts{ FieldMapper: fm, ConditionBuilder: cb, FieldKeys: fieldKeys, @@ -77,7 +77,7 @@ func TestSpanScopeFilterExpression(t *testing.T) { require.NotNil(t, whereClause) // Apply the where clause to the builder and get the SQL - sb.AddWhereClause(whereClause) + sb.AddWhereClause(whereClause.WhereClause) whereSQL, _ := sb.BuildWithFlavor(sqlbuilder.ClickHouse) t.Logf("Generated SQL: %s", whereSQL) assert.Contains(t, whereSQL, tt.expectedCondition) @@ -129,7 +129,7 @@ func TestSpanScopeWithResourceFilter(t *testing.T) { FieldContext: telemetrytypes.FieldContextResource, }} - _, _, err := querybuilder.PrepareWhereClause(tt.expression, querybuilder.FilterExprVisitorOpts{ + _, err := querybuilder.PrepareWhereClause(tt.expression, querybuilder.FilterExprVisitorOpts{ FieldMapper: fm, ConditionBuilder: cb, FieldKeys: fieldKeys, diff --git a/pkg/telemetrytraces/statement_builder.go b/pkg/telemetrytraces/statement_builder.go index a614563f01e7..fd2296fca72f 100644 --- a/pkg/telemetrytraces/statement_builder.go +++ b/pkg/telemetrytraces/statement_builder.go @@ -303,7 +303,7 @@ func (b *traceQueryStatementBuilder) buildListQuery( sb.From(fmt.Sprintf("%s.%s", DBName, SpanIndexV3TableName)) // Add filter conditions - warnings, err := b.addFilterCondition(ctx, sb, start, end, query, keys, variables) + preparedWhereClause, err := b.addFilterCondition(ctx, sb, start, end, query, keys, variables) if err != nil { return nil, err } @@ -333,11 +333,16 @@ func (b *traceQueryStatementBuilder) buildListQuery( finalSQL := querybuilder.CombineCTEs(cteFragments) + mainSQL finalArgs := querybuilder.PrependArgs(cteArgs, mainArgs) - return &qbtypes.Statement{ - Query: finalSQL, - Args: finalArgs, - Warnings: warnings, - }, nil + stmt := &qbtypes.Statement{ + Query: finalSQL, + Args: finalArgs, + } + if preparedWhereClause != nil { + stmt.Warnings = preparedWhereClause.Warnings + stmt.WarningsDocURL = preparedWhereClause.WarningsDocURL + } + + return stmt, nil } func (b *traceQueryStatementBuilder) buildTraceQuery( @@ -369,7 +374,7 @@ func (b *traceQueryStatementBuilder) buildTraceQuery( } // Add filter conditions - warnings, err := b.addFilterCondition(ctx, distSB, start, end, query, keys, variables) + preparedWhereClause, err := b.addFilterCondition(ctx, distSB, start, end, query, keys, variables) if err != nil { return nil, err } @@ -441,11 +446,16 @@ func (b *traceQueryStatementBuilder) buildTraceQuery( finalSQL := querybuilder.CombineCTEs(cteFragments) + mainSQL + " SETTINGS distributed_product_mode='allow', max_memory_usage=10000000000" finalArgs := querybuilder.PrependArgs(cteArgs, mainArgs) - return &qbtypes.Statement{ - Query: finalSQL, - Args: finalArgs, - Warnings: warnings, - }, nil + stmt := &qbtypes.Statement{ + Query: finalSQL, + Args: finalArgs, + } + if preparedWhereClause != nil { + stmt.Warnings = preparedWhereClause.Warnings + stmt.WarningsDocURL = preparedWhereClause.WarningsDocURL + } + + return stmt, nil } func (b *traceQueryStatementBuilder) buildTimeSeriesQuery( @@ -505,7 +515,7 @@ func (b *traceQueryStatementBuilder) buildTimeSeriesQuery( } sb.From(fmt.Sprintf("%s.%s", DBName, SpanIndexV3TableName)) - warnings, err := b.addFilterCondition(ctx, sb, start, end, query, keys, variables) + preparedWhereClause, err := b.addFilterCondition(ctx, sb, start, end, query, keys, variables) if err != nil { return nil, err } @@ -581,11 +591,16 @@ func (b *traceQueryStatementBuilder) buildTimeSeriesQuery( finalArgs = querybuilder.PrependArgs(cteArgs, mainArgs) } - return &qbtypes.Statement{ - Query: finalSQL, - Args: finalArgs, - Warnings: warnings, - }, nil + stmt := &qbtypes.Statement{ + Query: finalSQL, + Args: finalArgs, + } + if preparedWhereClause != nil { + stmt.Warnings = preparedWhereClause.Warnings + stmt.WarningsDocURL = preparedWhereClause.WarningsDocURL + } + + return stmt, nil } // buildScalarQuery builds a query for scalar panel type @@ -649,7 +664,7 @@ func (b *traceQueryStatementBuilder) buildScalarQuery( sb.From(fmt.Sprintf("%s.%s", DBName, SpanIndexV3TableName)) // Add filter conditions - warnings, err := b.addFilterCondition(ctx, sb, start, end, query, keys, variables) + preparedWhereClause, err := b.addFilterCondition(ctx, sb, start, end, query, keys, variables) if err != nil { return nil, err } @@ -691,11 +706,16 @@ func (b *traceQueryStatementBuilder) buildScalarQuery( finalSQL := querybuilder.CombineCTEs(cteFragments) + mainSQL finalArgs := querybuilder.PrependArgs(cteArgs, mainArgs) - return &qbtypes.Statement{ - Query: finalSQL, - Args: finalArgs, - Warnings: warnings, - }, nil + stmt := &qbtypes.Statement{ + Query: finalSQL, + Args: finalArgs, + } + if preparedWhereClause != nil { + stmt.Warnings = preparedWhereClause.Warnings + stmt.WarningsDocURL = preparedWhereClause.WarningsDocURL + } + + return stmt, nil } // buildFilterCondition builds SQL condition from filter expression @@ -706,15 +726,14 @@ func (b *traceQueryStatementBuilder) addFilterCondition( query qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation], keys map[string][]*telemetrytypes.TelemetryFieldKey, variables map[string]qbtypes.VariableItem, -) ([]string, error) { +) (*querybuilder.PreparedWhereClause, error) { - var filterWhereClause *sqlbuilder.WhereClause - var warnings []string + var preparedWhereClause *querybuilder.PreparedWhereClause var err error if query.Filter != nil && query.Filter.Expression != "" { // add filter expression - filterWhereClause, warnings, err = querybuilder.PrepareWhereClause(query.Filter.Expression, querybuilder.FilterExprVisitorOpts{ + preparedWhereClause, err = querybuilder.PrepareWhereClause(query.Filter.Expression, querybuilder.FilterExprVisitorOpts{ FieldMapper: b.fm, ConditionBuilder: b.cb, FieldKeys: keys, @@ -727,8 +746,8 @@ func (b *traceQueryStatementBuilder) addFilterCondition( } } - if filterWhereClause != nil { - sb.AddWhereClause(filterWhereClause) + if preparedWhereClause != nil { + sb.AddWhereClause(preparedWhereClause.WhereClause) } // add time filter @@ -737,7 +756,7 @@ func (b *traceQueryStatementBuilder) addFilterCondition( sb.Where(sb.GE("timestamp", fmt.Sprintf("%d", start)), sb.L("timestamp", fmt.Sprintf("%d", end)), sb.GE("ts_bucket_start", startBucket), sb.LE("ts_bucket_start", endBucket)) - return warnings, nil + return preparedWhereClause, nil } func aggOrderBy(k qbtypes.OrderBy, q qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]) (int, bool) { diff --git a/pkg/telemetrytraces/stmt_builder_test.go b/pkg/telemetrytraces/stmt_builder_test.go index 95e53b0d6b1b..e94916018e4b 100644 --- a/pkg/telemetrytraces/stmt_builder_test.go +++ b/pkg/telemetrytraces/stmt_builder_test.go @@ -64,6 +64,35 @@ func TestStatementBuilder(t *testing.T) { }, expectedErr: nil, }, + { + name: "OR b/w resource attr and attribute", + requestType: qbtypes.RequestTypeTimeSeries, + query: qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]{ + Signal: telemetrytypes.SignalTraces, + StepInterval: qbtypes.Step{Duration: 30 * time.Second}, + Aggregations: []qbtypes.TraceAggregation{ + { + Expression: "count()", + }, + }, + Filter: &qbtypes.Filter{ + Expression: "service.name = 'redis-manual' OR http.request.method = 'GET'", + }, + Limit: 10, + GroupBy: []qbtypes.GroupByKey{ + { + TelemetryFieldKey: telemetrytypes.TelemetryFieldKey{ + Name: "service.name", + }, + }, + }, + }, + expected: qbtypes.Statement{ + Query: "WITH __resource_filter AS (SELECT fingerprint FROM signoz_traces.distributed_traces_v3_resource WHERE ((simpleJSONExtractString(labels, 'service.name') = ? AND labels LIKE ? AND labels LIKE ?) OR true) AND seen_at_ts_bucket_start >= ? AND seen_at_ts_bucket_start <= ?), __limit_cte AS (SELECT toString(multiIf(mapContains(resources_string, 'service.name') = ?, resources_string['service.name'], NULL)) AS `service.name`, count() AS __result_0 FROM signoz_traces.distributed_signoz_index_v3 WHERE resource_fingerprint GLOBAL IN (SELECT fingerprint FROM __resource_filter) AND ((attributes_string['http.request.method'] = ? AND mapContains(attributes_string, 'http.request.method') = ?)) AND timestamp >= ? AND timestamp < ? AND ts_bucket_start >= ? AND ts_bucket_start <= ? GROUP BY `service.name` ORDER BY __result_0 DESC LIMIT ?) SELECT toStartOfInterval(timestamp, INTERVAL 30 SECOND) AS ts, toString(multiIf(mapContains(resources_string, 'service.name') = ?, resources_string['service.name'], NULL)) AS `service.name`, count() AS __result_0 FROM signoz_traces.distributed_signoz_index_v3 WHERE resource_fingerprint GLOBAL IN (SELECT fingerprint FROM __resource_filter) AND ((attributes_string['http.request.method'] = ? AND mapContains(attributes_string, 'http.request.method') = ?)) AND timestamp >= ? AND timestamp < ? AND ts_bucket_start >= ? AND ts_bucket_start <= ? AND (`service.name`) GLOBAL IN (SELECT `service.name` FROM __limit_cte) GROUP BY ts, `service.name`", + Args: []any{"redis-manual", "%service.name%", "%service.name\":\"redis-manual%", uint64(1747945619), uint64(1747983448), true, "GET", true, "1747947419000000000", "1747983448000000000", uint64(1747945619), uint64(1747983448), 10, true, "GET", true, "1747947419000000000", "1747983448000000000", uint64(1747945619), uint64(1747983448)}, + }, + expectedErr: nil, + }, { name: "legacy httpRoute in group by", requestType: qbtypes.RequestTypeTimeSeries, @@ -128,7 +157,7 @@ func TestStatementBuilder(t *testing.T) { }, }, expected: qbtypes.Statement{ - Query: "WITH __resource_filter AS (SELECT fingerprint FROM signoz_traces.distributed_traces_v3_resource WHERE (true AND true AND true) AND seen_at_ts_bucket_start >= ? AND seen_at_ts_bucket_start <= ?), __limit_cte AS (SELECT toString(multiIf(attribute_string_http$$route <> ?, attribute_string_http$$route, NULL)) AS `httpRoute`, toString(multiIf(http_method <> ?, http_method, NULL)) AS `httpMethod`, count() AS __result_0 FROM signoz_traces.distributed_signoz_index_v3 WHERE resource_fingerprint GLOBAL IN (SELECT fingerprint FROM __resource_filter) AND ((resource_string_service$$name = ? AND resource_string_service$$name <> ?) AND http_method <> ? AND kind_string = ?) AND timestamp >= ? AND timestamp < ? AND ts_bucket_start >= ? AND ts_bucket_start <= ? GROUP BY `httpRoute`, `httpMethod` ORDER BY __result_0 DESC LIMIT ?) SELECT toStartOfInterval(timestamp, INTERVAL 30 SECOND) AS ts, toString(multiIf(attribute_string_http$$route <> ?, attribute_string_http$$route, NULL)) AS `httpRoute`, toString(multiIf(http_method <> ?, http_method, NULL)) AS `httpMethod`, count() AS __result_0 FROM signoz_traces.distributed_signoz_index_v3 WHERE resource_fingerprint GLOBAL IN (SELECT fingerprint FROM __resource_filter) AND ((resource_string_service$$name = ? AND resource_string_service$$name <> ?) AND http_method <> ? AND kind_string = ?) AND timestamp >= ? AND timestamp < ? AND ts_bucket_start >= ? AND ts_bucket_start <= ? AND (`httpRoute`, `httpMethod`) GLOBAL IN (SELECT `httpRoute`, `httpMethod` FROM __limit_cte) GROUP BY ts, `httpRoute`, `httpMethod`", + Query: "WITH __resource_filter AS (SELECT fingerprint FROM signoz_traces.distributed_traces_v3_resource WHERE true AND seen_at_ts_bucket_start >= ? AND seen_at_ts_bucket_start <= ?), __limit_cte AS (SELECT toString(multiIf(attribute_string_http$$route <> ?, attribute_string_http$$route, NULL)) AS `httpRoute`, toString(multiIf(http_method <> ?, http_method, NULL)) AS `httpMethod`, count() AS __result_0 FROM signoz_traces.distributed_signoz_index_v3 WHERE resource_fingerprint GLOBAL IN (SELECT fingerprint FROM __resource_filter) AND ((resource_string_service$$name = ? AND resource_string_service$$name <> ?) AND http_method <> ? AND kind_string = ?) AND timestamp >= ? AND timestamp < ? AND ts_bucket_start >= ? AND ts_bucket_start <= ? GROUP BY `httpRoute`, `httpMethod` ORDER BY __result_0 DESC LIMIT ?) SELECT toStartOfInterval(timestamp, INTERVAL 30 SECOND) AS ts, toString(multiIf(attribute_string_http$$route <> ?, attribute_string_http$$route, NULL)) AS `httpRoute`, toString(multiIf(http_method <> ?, http_method, NULL)) AS `httpMethod`, count() AS __result_0 FROM signoz_traces.distributed_signoz_index_v3 WHERE resource_fingerprint GLOBAL IN (SELECT fingerprint FROM __resource_filter) AND ((resource_string_service$$name = ? AND resource_string_service$$name <> ?) AND http_method <> ? AND kind_string = ?) AND timestamp >= ? AND timestamp < ? AND ts_bucket_start >= ? AND ts_bucket_start <= ? AND (`httpRoute`, `httpMethod`) GLOBAL IN (SELECT `httpRoute`, `httpMethod` FROM __limit_cte) GROUP BY ts, `httpRoute`, `httpMethod`", Args: []any{uint64(1747945619), uint64(1747983448), "", "", "redis-manual", "", "", "Server", "1747947419000000000", "1747983448000000000", uint64(1747945619), uint64(1747983448), 10, "", "", "redis-manual", "", "", "Server", "1747947419000000000", "1747983448000000000", uint64(1747945619), uint64(1747983448)}, }, expectedErr: nil, diff --git a/pkg/types/querybuildertypes/querybuildertypesv5/builder_elements.go b/pkg/types/querybuildertypes/querybuildertypesv5/builder_elements.go index c303c203647e..2738b9e81082 100644 --- a/pkg/types/querybuildertypes/querybuildertypesv5/builder_elements.go +++ b/pkg/types/querybuildertypes/querybuildertypesv5/builder_elements.go @@ -47,8 +47,7 @@ func (s *Step) UnmarshalJSON(b []byte) error { } func (s Step) MarshalJSON() ([]byte, error) { - // Emit human‑friendly string → "30s" - return json.Marshal(s.Duration.String()) + return json.Marshal(s.Duration.Seconds()) } // FilterOperator is the operator for the filter. diff --git a/pkg/types/querybuildertypes/querybuildertypesv5/builder_query.go b/pkg/types/querybuildertypes/querybuildertypesv5/builder_query.go index c81433d1ed02..5f0b3db11d08 100644 --- a/pkg/types/querybuildertypes/querybuildertypesv5/builder_query.go +++ b/pkg/types/querybuildertypes/querybuildertypesv5/builder_query.go @@ -37,7 +37,7 @@ type QueryBuilderQuery[T any] struct { Limit int `json:"limit,omitempty"` // limitBy fields to limit by - LimitBy LimitBy `json:"limitBy,omitempty"` + LimitBy *LimitBy `json:"limitBy,omitempty"` // offset the number of rows to skip // TODO: remove this once we have cursor-based pagination everywhere? diff --git a/pkg/types/querybuildertypes/querybuildertypesv5/qb.go b/pkg/types/querybuildertypes/querybuildertypesv5/qb.go index c65671659cc8..de709787e59a 100644 --- a/pkg/types/querybuildertypes/querybuildertypesv5/qb.go +++ b/pkg/types/querybuildertypes/querybuildertypesv5/qb.go @@ -41,9 +41,10 @@ type AggExprRewriter interface { } type Statement struct { - Query string - Args []any - Warnings []string + Query string + Args []any + Warnings []string + WarningsDocURL string } // StatementBuilder builds the query. diff --git a/pkg/types/querybuildertypes/querybuildertypesv5/query.go b/pkg/types/querybuildertypes/querybuildertypesv5/query.go index d56e7b0fa646..0fe2f1514352 100644 --- a/pkg/types/querybuildertypes/querybuildertypesv5/query.go +++ b/pkg/types/querybuildertypes/querybuildertypesv5/query.go @@ -15,10 +15,11 @@ type Query interface { } type Result struct { - Type RequestType - Value any // concrete Go value (to be type asserted based on the RequestType) - Stats ExecStats - Warnings []string + Type RequestType + Value any // concrete Go value (to be type asserted based on the RequestType) + Stats ExecStats + Warnings []string + WarningsDocURL string } type ExecStats struct { diff --git a/pkg/types/querybuildertypes/querybuildertypesv5/resp.go b/pkg/types/querybuildertypes/querybuildertypesv5/resp.go index 1ed3b6c3a9b5..dc8c21bd592a 100644 --- a/pkg/types/querybuildertypes/querybuildertypesv5/resp.go +++ b/pkg/types/querybuildertypes/querybuildertypesv5/resp.go @@ -6,6 +6,7 @@ import ( "math" "reflect" "slices" + "strconv" "strings" "time" @@ -26,11 +27,27 @@ type QBEvent struct { HasData bool `json:"-"` } +type QueryWarnData struct { + Message string `json:"message"` + Url string `json:"url,omitempty"` + Warnings []QueryWarnDataAdditional `json:"warnings,omitempty"` +} + +type QueryWarnDataAdditional struct { + Message string `json:"message"` +} + +type QueryData struct { + Results []any `json:"results"` +} + type QueryRangeResponse struct { Type RequestType `json:"type"` - Data any `json:"data"` + Data QueryData `json:"data"` Meta ExecStats `json:"meta"` + Warning QueryWarnData `json:"warning,omitempty"` + QBEvent *QBEvent `json:"-"` } @@ -168,6 +185,41 @@ type RawRow struct { Data map[string]any `json:"data"` } +func roundToNonZeroDecimals(val float64, n int) float64 { + if val == 0 || math.IsNaN(val) || math.IsInf(val, 0) { + return val + } + + absVal := math.Abs(val) + + // For numbers >= 1, we want to round to n decimal places total + if absVal >= 1 { + // Round to n decimal places + multiplier := math.Pow(10, float64(n)) + rounded := math.Round(val*multiplier) / multiplier + + // If the result is a whole number, return it as such + if rounded == math.Trunc(rounded) { + return rounded + } + + // Remove trailing zeros by converting to string and back + str := strconv.FormatFloat(rounded, 'f', -1, 64) + result, _ := strconv.ParseFloat(str, 64) + return result + } + + // For numbers < 1, count n significant figures after first non-zero digit + order := math.Floor(math.Log10(absVal)) + scale := math.Pow(10, -order+float64(n)-1) + rounded := math.Round(val*scale) / scale + + // Clean up floating point precision + str := strconv.FormatFloat(rounded, 'f', -1, 64) + result, _ := strconv.ParseFloat(str, 64) + return result +} + func sanitizeValue(v any) any { if v == nil { return nil @@ -181,7 +233,7 @@ func sanitizeValue(v any) any { } else if math.IsInf(f, -1) { return "-Inf" } - return f + return roundToNonZeroDecimals(f, 3) } if f, ok := v.(float32); ok { @@ -193,7 +245,7 @@ func sanitizeValue(v any) any { } else if math.IsInf(f64, -1) { return "-Inf" } - return f + return float32(roundToNonZeroDecimals(f64, 3)) // ADD ROUNDING HERE } rv := reflect.ValueOf(v) diff --git a/pkg/types/ruletypes/alerting.go b/pkg/types/ruletypes/alerting.go index c358c1cf9b2b..f76d5a3ec79d 100644 --- a/pkg/types/ruletypes/alerting.go +++ b/pkg/types/ruletypes/alerting.go @@ -175,7 +175,7 @@ func (rc *RuleCondition) IsValid() bool { } if rc.QueryType() == v3.QueryTypePromQL { - if len(rc.CompositeQuery.PromQueries) == 0 { + if len(rc.CompositeQuery.PromQueries) == 0 && len(rc.CompositeQuery.Queries) == 0 { return false } } diff --git a/pkg/variables/variable_replace_visitor.go b/pkg/variables/variable_replace_visitor.go new file mode 100644 index 000000000000..146b4ddcbf4a --- /dev/null +++ b/pkg/variables/variable_replace_visitor.go @@ -0,0 +1,528 @@ +package variables + +import ( + "fmt" + "strconv" + "strings" + + grammar "github.com/SigNoz/signoz/pkg/parser/grammar" + qbtypes "github.com/SigNoz/signoz/pkg/types/querybuildertypes/querybuildertypesv5" + "github.com/antlr4-go/antlr/v4" +) + +// ErrorListener collects syntax errors during parsing +type ErrorListener struct { + *antlr.DefaultErrorListener + SyntaxErrors []error +} + +// NewErrorListener creates a new error listener +func NewErrorListener() *ErrorListener { + return &ErrorListener{ + DefaultErrorListener: antlr.NewDefaultErrorListener(), + SyntaxErrors: []error{}, + } +} + +// SyntaxError is called when a syntax error is encountered +func (e *ErrorListener) SyntaxError(recognizer antlr.Recognizer, offendingSymbol any, line, column int, msg string, ex antlr.RecognitionException) { + e.SyntaxErrors = append(e.SyntaxErrors, fmt.Errorf("line %d:%d %s", line, column, msg)) +} + +// variableReplacementVisitor implements the visitor interface +// to replace variables in filter expressions with their actual values +type variableReplacementVisitor struct { + variables map[string]qbtypes.VariableItem + errors []string +} + +// specialSkipMarker is used to indicate that a condition should be removed +const specialSkipMarker = "__SKIP_CONDITION__" + +// ReplaceVariablesInExpression takes a filter expression and returns it with variables replaced +func ReplaceVariablesInExpression(expression string, variables map[string]qbtypes.VariableItem) (string, error) { + input := antlr.NewInputStream(expression) + lexer := grammar.NewFilterQueryLexer(input) + + visitor := &variableReplacementVisitor{ + variables: variables, + errors: []string{}, + } + + lexerErrorListener := NewErrorListener() + lexer.RemoveErrorListeners() + lexer.AddErrorListener(lexerErrorListener) + + tokens := antlr.NewCommonTokenStream(lexer, 0) + parserErrorListener := NewErrorListener() + parser := grammar.NewFilterQueryParser(tokens) + parser.RemoveErrorListeners() + parser.AddErrorListener(parserErrorListener) + + tree := parser.Query() + + if len(parserErrorListener.SyntaxErrors) > 0 { + return "", fmt.Errorf("syntax errors in expression: %v", parserErrorListener.SyntaxErrors) + } + + result := visitor.Visit(tree).(string) + + if len(visitor.errors) > 0 { + return "", fmt.Errorf("errors processing expression: %v", visitor.errors) + } + + // If the entire expression should be skipped, return empty string + if result == specialSkipMarker { + return "", nil + } + + return result, nil +} + +// Visit dispatches to the specific visit method based on node type +func (v *variableReplacementVisitor) Visit(tree antlr.ParseTree) any { + if tree == nil { + return "" + } + + switch t := tree.(type) { + case *grammar.QueryContext: + return v.VisitQuery(t) + case *grammar.ExpressionContext: + return v.VisitExpression(t) + case *grammar.OrExpressionContext: + return v.VisitOrExpression(t) + case *grammar.AndExpressionContext: + return v.VisitAndExpression(t) + case *grammar.UnaryExpressionContext: + return v.VisitUnaryExpression(t) + case *grammar.PrimaryContext: + return v.VisitPrimary(t) + case *grammar.ComparisonContext: + return v.VisitComparison(t) + case *grammar.InClauseContext: + return v.VisitInClause(t) + case *grammar.NotInClauseContext: + return v.VisitNotInClause(t) + case *grammar.ValueListContext: + return v.VisitValueList(t) + case *grammar.FullTextContext: + return v.VisitFullText(t) + case *grammar.FunctionCallContext: + return v.VisitFunctionCall(t) + case *grammar.FunctionParamListContext: + return v.VisitFunctionParamList(t) + case *grammar.FunctionParamContext: + return v.VisitFunctionParam(t) + case *grammar.ArrayContext: + return v.VisitArray(t) + case *grammar.ValueContext: + return v.VisitValue(t) + case *grammar.KeyContext: + return v.VisitKey(t) + default: + // For unknown types, return the original text + return tree.GetText() + } +} + +func (v *variableReplacementVisitor) VisitQuery(ctx *grammar.QueryContext) any { + return v.Visit(ctx.Expression()) +} + +func (v *variableReplacementVisitor) VisitExpression(ctx *grammar.ExpressionContext) any { + return v.Visit(ctx.OrExpression()) +} + +func (v *variableReplacementVisitor) VisitOrExpression(ctx *grammar.OrExpressionContext) any { + andExpressions := ctx.AllAndExpression() + + parts := make([]string, 0, len(andExpressions)) + for _, expr := range andExpressions { + part := v.Visit(expr).(string) + // Skip conditions that should be removed + if part != specialSkipMarker && part != "" { + parts = append(parts, part) + } + } + + if len(parts) == 0 { + return specialSkipMarker + } + + if len(parts) == 1 { + return parts[0] + } + + return strings.Join(parts, " OR ") +} + +func (v *variableReplacementVisitor) VisitAndExpression(ctx *grammar.AndExpressionContext) any { + unaryExpressions := ctx.AllUnaryExpression() + + parts := make([]string, 0, len(unaryExpressions)) + for _, expr := range unaryExpressions { + part := v.Visit(expr).(string) + // Skip conditions that should be removed + if part != specialSkipMarker && part != "" { + parts = append(parts, part) + } + } + + if len(parts) == 0 { + return specialSkipMarker + } + + if len(parts) == 1 { + return parts[0] + } + + return strings.Join(parts, " AND ") +} + +func (v *variableReplacementVisitor) VisitUnaryExpression(ctx *grammar.UnaryExpressionContext) any { + result := v.Visit(ctx.Primary()).(string) + + // If the condition should be skipped, propagate it up + if result == specialSkipMarker { + return specialSkipMarker + } + + if ctx.NOT() != nil { + return "NOT " + result + } + + return result +} + +func (v *variableReplacementVisitor) VisitPrimary(ctx *grammar.PrimaryContext) any { + if ctx.OrExpression() != nil { + return "(" + v.Visit(ctx.OrExpression()).(string) + ")" + } else if ctx.Comparison() != nil { + return v.Visit(ctx.Comparison()) + } else if ctx.FunctionCall() != nil { + return v.Visit(ctx.FunctionCall()) + } else if ctx.FullText() != nil { + return v.Visit(ctx.FullText()) + } + + // Handle standalone key/value + if ctx.GetChildCount() == 1 { + child := ctx.GetChild(0) + if parseTree, ok := child.(antlr.ParseTree); ok { + return v.Visit(parseTree).(string) + } + // Fallback to getting text from the context + return ctx.GetText() + } + + return ctx.GetText() +} + +func (v *variableReplacementVisitor) VisitComparison(ctx *grammar.ComparisonContext) any { + // First check if any value contains __all__ variable + values := ctx.AllValue() + for _, val := range values { + valueResult := v.Visit(val).(string) + if valueResult == specialSkipMarker { + return specialSkipMarker + } + } + + // Also check in IN/NOT IN clauses + if ctx.InClause() != nil { + inResult := v.Visit(ctx.InClause()).(string) + if inResult == specialSkipMarker { + return specialSkipMarker + } + } + + if ctx.NotInClause() != nil { + notInResult := v.Visit(ctx.NotInClause()).(string) + if notInResult == specialSkipMarker { + return specialSkipMarker + } + } + + var parts []string + + // Add key + parts = append(parts, v.Visit(ctx.Key()).(string)) + + // Handle EXISTS + if ctx.EXISTS() != nil { + if ctx.NOT() != nil { + parts = append(parts, " NOT") + } + parts = append(parts, " EXISTS") + return strings.Join(parts, "") + } + + // Handle IN/NOT IN + if ctx.InClause() != nil { + parts = append(parts, " IN ") + parts = append(parts, v.Visit(ctx.InClause()).(string)) + return strings.Join(parts, "") + } + + if ctx.NotInClause() != nil { + parts = append(parts, " NOT IN ") + parts = append(parts, v.Visit(ctx.NotInClause()).(string)) + return strings.Join(parts, "") + } + + // Handle BETWEEN + if ctx.BETWEEN() != nil { + if ctx.NOT() != nil { + parts = append(parts, " NOT") + } + parts = append(parts, " BETWEEN ") + values := ctx.AllValue() + parts = append(parts, v.Visit(values[0]).(string)) + parts = append(parts, " AND ") + parts = append(parts, v.Visit(values[1]).(string)) + return strings.Join(parts, "") + } + + // Handle other operators + if ctx.EQUALS() != nil { + parts = append(parts, " = ") + } else if ctx.NOT_EQUALS() != nil { + parts = append(parts, " != ") + } else if ctx.NEQ() != nil { + parts = append(parts, " <> ") + } else if ctx.LT() != nil { + parts = append(parts, " < ") + } else if ctx.LE() != nil { + parts = append(parts, " <= ") + } else if ctx.GT() != nil { + parts = append(parts, " > ") + } else if ctx.GE() != nil { + parts = append(parts, " >= ") + } else if ctx.LIKE() != nil { + parts = append(parts, " LIKE ") + } else if ctx.ILIKE() != nil { + parts = append(parts, " ILIKE ") + } else if ctx.NOT_LIKE() != nil { + parts = append(parts, " NOT LIKE ") + } else if ctx.NOT_ILIKE() != nil { + parts = append(parts, " NOT ILIKE ") + } else if ctx.REGEXP() != nil { + if ctx.NOT() != nil { + parts = append(parts, " NOT") + } + parts = append(parts, " REGEXP ") + } else if ctx.CONTAINS() != nil { + if ctx.NOT() != nil { + parts = append(parts, " NOT") + } + parts = append(parts, " CONTAINS ") + } + + // Add value + if len(values) > 0 { + parts = append(parts, v.Visit(values[0]).(string)) + } + + return strings.Join(parts, "") +} + +func (v *variableReplacementVisitor) VisitInClause(ctx *grammar.InClauseContext) any { + if ctx.ValueList() != nil { + result := v.Visit(ctx.ValueList()).(string) + if result == specialSkipMarker { + return specialSkipMarker + } + return result + } + result := v.Visit(ctx.Value()).(string) + if result == specialSkipMarker { + return specialSkipMarker + } + return result +} + +func (v *variableReplacementVisitor) VisitNotInClause(ctx *grammar.NotInClauseContext) any { + if ctx.ValueList() != nil { + result := v.Visit(ctx.ValueList()).(string) + if result == specialSkipMarker { + return specialSkipMarker + } + return result + } + result := v.Visit(ctx.Value()).(string) + if result == specialSkipMarker { + return specialSkipMarker + } + return result +} + +func (v *variableReplacementVisitor) VisitValueList(ctx *grammar.ValueListContext) any { + values := ctx.AllValue() + + // Check if any value is __all__ + for _, val := range values { + result := v.Visit(val).(string) + if result == specialSkipMarker { + return specialSkipMarker + } + } + + parts := make([]string, 0, len(values)) + for i, val := range values { + if i > 0 { + parts = append(parts, ", ") + } + parts = append(parts, v.Visit(val).(string)) + } + + return "(" + strings.Join(parts, "") + ")" +} + +func (v *variableReplacementVisitor) VisitFullText(ctx *grammar.FullTextContext) any { + if ctx.QUOTED_TEXT() != nil { + return ctx.QUOTED_TEXT().GetText() + } else if ctx.FREETEXT() != nil { + return ctx.FREETEXT().GetText() + } + return "" +} + +func (v *variableReplacementVisitor) VisitFunctionCall(ctx *grammar.FunctionCallContext) any { + var functionName string + if ctx.HAS() != nil { + functionName = "has" + } else if ctx.HASANY() != nil { + functionName = "hasAny" + } else if ctx.HASALL() != nil { + functionName = "hasAll" + } + + params := v.Visit(ctx.FunctionParamList()).(string) + return functionName + "(" + params + ")" +} + +func (v *variableReplacementVisitor) VisitFunctionParamList(ctx *grammar.FunctionParamListContext) any { + params := ctx.AllFunctionParam() + parts := make([]string, 0, len(params)) + + for i, param := range params { + if i > 0 { + parts = append(parts, ", ") + } + parts = append(parts, v.Visit(param).(string)) + } + + return strings.Join(parts, "") +} + +func (v *variableReplacementVisitor) VisitFunctionParam(ctx *grammar.FunctionParamContext) any { + if ctx.Key() != nil { + return v.Visit(ctx.Key()) + } else if ctx.Value() != nil { + return v.Visit(ctx.Value()) + } else if ctx.Array() != nil { + return v.Visit(ctx.Array()) + } + return "" +} + +func (v *variableReplacementVisitor) VisitArray(ctx *grammar.ArrayContext) any { + valueList := v.Visit(ctx.ValueList()).(string) + // Don't wrap in brackets if it's already wrapped in parentheses + if strings.HasPrefix(valueList, "(") { + return valueList + } + return "[" + valueList + "]" +} + +func (v *variableReplacementVisitor) VisitValue(ctx *grammar.ValueContext) any { + // First get the original value + var originalValue string + if ctx.QUOTED_TEXT() != nil { + originalValue = ctx.QUOTED_TEXT().GetText() + } else if ctx.NUMBER() != nil { + originalValue = ctx.NUMBER().GetText() + } else if ctx.KEY() != nil { + originalValue = ctx.KEY().GetText() + } + + // Check if this is a variable (starts with $) + if strings.HasPrefix(originalValue, "$") { + varName := originalValue + + // Try with $ prefix first + varItem, ok := v.variables[varName] + if !ok && len(varName) > 1 { + // Try without $ prefix + varItem, ok = v.variables[varName[1:]] + } + + if ok { + // Handle dynamic variable with __all__ value + if varItem.Type == qbtypes.DynamicVariableType { + if allVal, ok := varItem.Value.(string); ok && allVal == "__all__" { + // Return special marker to indicate this condition should be removed + return specialSkipMarker + } + } + + // Replace variable with its value + return v.formatVariableValue(varItem.Value) + } + } + + // Return original value if not a variable or variable not found + return originalValue +} + +func (v *variableReplacementVisitor) VisitKey(ctx *grammar.KeyContext) any { + keyText := ctx.GetText() + + // Check if this key is actually a variable + if strings.HasPrefix(keyText, "$") { + varName := keyText + + // Try with $ prefix first + varItem, ok := v.variables[varName] + if !ok && len(varName) > 1 { + // Try without $ prefix + varItem, ok = v.variables[varName[1:]] + } + + if ok { + // Handle dynamic variable with __all__ value + if varItem.Type == qbtypes.DynamicVariableType { + if allVal, ok := varItem.Value.(string); ok && allVal == "__all__" { + return specialSkipMarker + } + } + // Replace variable with its value + return v.formatVariableValue(varItem.Value) + } + } + + return keyText +} + +// formatVariableValue formats a variable value for inclusion in the expression +func (v *variableReplacementVisitor) formatVariableValue(value any) string { + switch val := value.(type) { + case string: + // Quote string values + return fmt.Sprintf("'%s'", strings.ReplaceAll(val, "'", "\\'")) + case []any: + // Format array values + parts := make([]string, len(val)) + for i, item := range val { + parts[i] = v.formatVariableValue(item) + } + return "(" + strings.Join(parts, ", ") + ")" + case int, int32, int64, float32, float64: + return fmt.Sprintf("%v", val) + case bool: + return strconv.FormatBool(val) + default: + return fmt.Sprintf("%v", val) + } +} diff --git a/pkg/variables/variable_replace_visitor_test.go b/pkg/variables/variable_replace_visitor_test.go new file mode 100644 index 000000000000..e8b9ae848549 --- /dev/null +++ b/pkg/variables/variable_replace_visitor_test.go @@ -0,0 +1,463 @@ +package variables + +import ( + "testing" + + qbtypes "github.com/SigNoz/signoz/pkg/types/querybuildertypes/querybuildertypesv5" + "github.com/stretchr/testify/assert" +) + +func TestReplaceVariablesInExpression(t *testing.T) { + tests := []struct { + name string + expression string + variables map[string]qbtypes.VariableItem + expected string + wantErr bool + }{ + { + name: "simple string variable replacement", + expression: "service.name = $service", + variables: map[string]qbtypes.VariableItem{ + "service": { + Type: qbtypes.DynamicVariableType, + Value: "auth-service", + }, + }, + expected: "service.name = 'auth-service'", + }, + { + name: "simple string variable replacement", + expression: "service.name = $service", + variables: map[string]qbtypes.VariableItem{ + "service": { + Type: qbtypes.QueryVariableType, + Value: "auth-service", + }, + }, + expected: "service.name = 'auth-service'", + }, + { + name: "simple string variable replacement", + expression: "service.name = $service", + variables: map[string]qbtypes.VariableItem{ + "service": { + Type: qbtypes.CustomVariableType, + Value: "auth-service", + }, + }, + expected: "service.name = 'auth-service'", + }, + { + name: "simple string variable replacement", + expression: "service.name = $service", + variables: map[string]qbtypes.VariableItem{ + "service": { + Type: qbtypes.TextBoxVariableType, + Value: "auth-service", + }, + }, + expected: "service.name = 'auth-service'", + }, + { + name: "variable with dollar sign prefix in map", + expression: "service.name = $service", + variables: map[string]qbtypes.VariableItem{ + "$service": { + Type: qbtypes.DynamicVariableType, + Value: "auth-service", + }, + }, + expected: "service.name = 'auth-service'", + }, + { + name: "numeric variable replacement", + expression: "http.status_code > $threshold", + variables: map[string]qbtypes.VariableItem{ + "threshold": { + Type: qbtypes.DynamicVariableType, + Value: 400, + }, + }, + expected: "http.status_code > 400", + }, + { + name: "boolean variable replacement", + expression: "is_error = $error_flag", + variables: map[string]qbtypes.VariableItem{ + "error_flag": { + Type: qbtypes.DynamicVariableType, + Value: true, + }, + }, + expected: "is_error = true", + }, + { + name: "array variable in IN clause", + expression: "service.name IN $services", + variables: map[string]qbtypes.VariableItem{ + "services": { + Type: qbtypes.DynamicVariableType, + Value: []any{"auth", "api", "web"}, + }, + }, + expected: "service.name IN ('auth', 'api', 'web')", + }, + { + name: "array variable with mixed types", + expression: "id IN $ids", + variables: map[string]qbtypes.VariableItem{ + "ids": { + Type: qbtypes.DynamicVariableType, + Value: []any{1, 2, "three", 4.5}, + }, + }, + expected: "id IN (1, 2, 'three', 4.5)", + }, + { + name: "multiple variables in expression", + expression: "service.name = $service AND env = $environment AND status_code >= $min_code", + variables: map[string]qbtypes.VariableItem{ + "service": { + Type: qbtypes.DynamicVariableType, + Value: "auth-service", + }, + "environment": { + Type: qbtypes.DynamicVariableType, + Value: "production", + }, + "min_code": { + Type: qbtypes.DynamicVariableType, + Value: 400, + }, + }, + expected: "service.name = 'auth-service' AND env = 'production' AND status_code >= 400", + }, + { + name: "variable in complex expression with parentheses", + expression: "(service.name = $service OR service.name = 'default') AND status_code > $threshold", + variables: map[string]qbtypes.VariableItem{ + "service": { + Type: qbtypes.DynamicVariableType, + Value: "auth", + }, + "threshold": { + Type: qbtypes.DynamicVariableType, + Value: 200, + }, + }, + expected: "(service.name = 'auth' OR service.name = 'default') AND status_code > 200", + }, + { + name: "variable not found - preserved as is", + expression: "service.name = $unknown_service", + variables: map[string]qbtypes.VariableItem{}, + expected: "service.name = $unknown_service", + }, + { + name: "string with quotes needs escaping", + expression: "message = $msg", + variables: map[string]qbtypes.VariableItem{ + "msg": { + Type: qbtypes.DynamicVariableType, + Value: "user's request", + }, + }, + expected: "message = 'user\\'s request'", + }, + { + name: "dynamic variable with __all__ value", + expression: "service.name = $all_services", + variables: map[string]qbtypes.VariableItem{ + "all_services": { + Type: qbtypes.DynamicVariableType, + Value: "__all__", + }, + }, + expected: "", // Special value preserved + }, + { + name: "variable in NOT IN clause", + expression: "service.name NOT IN $excluded", + variables: map[string]qbtypes.VariableItem{ + "excluded": { + Type: qbtypes.DynamicVariableType, + Value: []any{"test", "debug"}, + }, + }, + expected: "service.name NOT IN ('test', 'debug')", + }, + { + name: "variable in BETWEEN clause", + expression: "latency BETWEEN $min AND $max", + variables: map[string]qbtypes.VariableItem{ + "min": { + Type: qbtypes.DynamicVariableType, + Value: 100, + }, + "max": { + Type: qbtypes.DynamicVariableType, + Value: 500, + }, + }, + expected: "latency BETWEEN 100 AND 500", + }, + { + name: "variable in LIKE expression", + expression: "service.name LIKE $pattern", + variables: map[string]qbtypes.VariableItem{ + "pattern": { + Type: qbtypes.DynamicVariableType, + Value: "%auth%", + }, + }, + expected: "service.name LIKE '%auth%'", + }, + { + name: "variable in function call", + expression: "has(tags, $tag)", + variables: map[string]qbtypes.VariableItem{ + "tag": { + Type: qbtypes.DynamicVariableType, + Value: "error", + }, + }, + expected: "has(tags, 'error')", + }, + { + name: "variable in hasAny function", + expression: "hasAny(tags, $tags)", + variables: map[string]qbtypes.VariableItem{ + "tags": { + Type: qbtypes.DynamicVariableType, + Value: []any{"error", "warning", "info"}, + }, + }, + expected: "hasAny(tags, ('error', 'warning', 'info'))", + }, + { + name: "empty array variable", + expression: "service.name IN $services", + variables: map[string]qbtypes.VariableItem{ + "services": { + Type: qbtypes.DynamicVariableType, + Value: []any{}, + }, + }, + expected: "service.name IN ()", + }, + { + name: "expression with OR and variables", + expression: "env = $env1 OR env = $env2", + variables: map[string]qbtypes.VariableItem{ + "env1": { + Type: qbtypes.DynamicVariableType, + Value: "staging", + }, + "env2": { + Type: qbtypes.DynamicVariableType, + Value: "production", + }, + }, + expected: "env = 'staging' OR env = 'production'", + }, + { + name: "NOT expression with variable", + expression: "NOT service.name = $service", + variables: map[string]qbtypes.VariableItem{ + "service": { + Type: qbtypes.DynamicVariableType, + Value: "test-service", + }, + }, + expected: "NOT service.name = 'test-service'", + }, + { + name: "variable in EXISTS clause", + expression: "tags EXISTS", + variables: map[string]qbtypes.VariableItem{}, + expected: "tags EXISTS", + }, + { + name: "complex nested expression", + expression: "(service.name IN $services AND env = $env) OR (status_code >= $error_code)", + variables: map[string]qbtypes.VariableItem{ + "services": { + Type: qbtypes.DynamicVariableType, + Value: []any{"auth", "api"}, + }, + "env": { + Type: qbtypes.DynamicVariableType, + Value: "prod", + }, + "error_code": { + Type: qbtypes.DynamicVariableType, + Value: 500, + }, + }, + expected: "(service.name IN ('auth', 'api') AND env = 'prod') OR (status_code >= 500)", + }, + { + name: "float variable", + expression: "cpu_usage > $threshold", + variables: map[string]qbtypes.VariableItem{ + "threshold": { + Type: qbtypes.DynamicVariableType, + Value: 85.5, + }, + }, + expected: "cpu_usage > 85.5", + }, + { + name: "variable in REGEXP expression", + expression: "message REGEXP $pattern", + variables: map[string]qbtypes.VariableItem{ + "pattern": { + Type: qbtypes.DynamicVariableType, + Value: "^ERROR.*", + }, + }, + expected: "message REGEXP '^ERROR.*'", + }, + { + name: "variable in NOT REGEXP expression", + expression: "message NOT REGEXP $pattern", + variables: map[string]qbtypes.VariableItem{ + "pattern": { + Type: qbtypes.DynamicVariableType, + Value: "^DEBUG.*", + }, + }, + expected: "message NOT REGEXP '^DEBUG.*'", + }, + { + name: "invalid syntax", + expression: "service.name = = $service", + variables: map[string]qbtypes.VariableItem{}, + wantErr: true, + }, + { + name: "full text search not affected by variables", + expression: "'error message'", + variables: map[string]qbtypes.VariableItem{}, + expected: "'error message'", + }, + { + name: "comparison operators", + expression: "a < $v1 AND b <= $v2 AND c > $v3 AND d >= $v4 AND e != $v5 AND f <> $v6", + variables: map[string]qbtypes.VariableItem{ + "v1": {Type: qbtypes.DynamicVariableType, Value: 10}, + "v2": {Type: qbtypes.DynamicVariableType, Value: 20}, + "v3": {Type: qbtypes.DynamicVariableType, Value: 30}, + "v4": {Type: qbtypes.DynamicVariableType, Value: 40}, + "v5": {Type: qbtypes.DynamicVariableType, Value: "test"}, + "v6": {Type: qbtypes.DynamicVariableType, Value: "other"}, + }, + expected: "a < 10 AND b <= 20 AND c > 30 AND d >= 40 AND e != 'test' AND f <> 'other'", + }, + { + name: "CONTAINS operator with variable", + expression: "message CONTAINS $text", + variables: map[string]qbtypes.VariableItem{ + "text": { + Type: qbtypes.DynamicVariableType, + Value: "error", + }, + }, + expected: "message CONTAINS 'error'", + }, + { + name: "NOT CONTAINS operator with variable", + expression: "message NOT CONTAINS $text", + variables: map[string]qbtypes.VariableItem{ + "text": { + Type: qbtypes.DynamicVariableType, + Value: "debug", + }, + }, + expected: "message NOT CONTAINS 'debug'", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := ReplaceVariablesInExpression(tt.expression, tt.variables) + + if tt.wantErr { + assert.Error(t, err) + return + } + + assert.NoError(t, err) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestFormatVariableValue(t *testing.T) { + visitor := &variableReplacementVisitor{} + + tests := []struct { + name string + value any + expected string + }{ + { + name: "string value", + value: "hello", + expected: "'hello'", + }, + { + name: "string with single quote", + value: "user's data", + expected: "'user\\'s data'", + }, + { + name: "integer value", + value: 42, + expected: "42", + }, + { + name: "float value", + value: 3.14, + expected: "3.14", + }, + { + name: "boolean true", + value: true, + expected: "true", + }, + { + name: "boolean false", + value: false, + expected: "false", + }, + { + name: "array of strings", + value: []any{"a", "b", "c"}, + expected: "('a', 'b', 'c')", + }, + { + name: "array of mixed types", + value: []any{"string", 123, true, 45.6}, + expected: "('string', 123, true, 45.6)", + }, + { + name: "empty array", + value: []any{}, + expected: "()", + }, + { + name: "nil value", + value: nil, + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := visitor.formatVariableValue(tt.value) + assert.Equal(t, tt.expected, result) + }) + } +} From 301d9ca4dd4493bc7329e658319a5c90dc2efca2 Mon Sep 17 00:00:00 2001 From: Amlan Kumar Nandy <45410599+amlannandy@users.noreply.github.com> Date: Tue, 5 Aug 2025 09:40:19 +0700 Subject: [PATCH 2/4] fix: disabling alert from overview page doesn't work (#8640) --- .../AlertDetails/AlertHeader/ActionButtons/ActionButtons.tsx | 2 +- frontend/src/pages/AlertDetails/hooks.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/pages/AlertDetails/AlertHeader/ActionButtons/ActionButtons.tsx b/frontend/src/pages/AlertDetails/AlertHeader/ActionButtons/ActionButtons.tsx index c6b65a28d406..04bd456db7d9 100644 --- a/frontend/src/pages/AlertDetails/AlertHeader/ActionButtons/ActionButtons.tsx +++ b/frontend/src/pages/AlertDetails/AlertHeader/ActionButtons/ActionButtons.tsx @@ -111,7 +111,7 @@ function AlertActionButtons({ return ( <>
- + {isAlertRuleDisabled !== undefined && ( { setAlertRuleState(data?.payload?.state); - + queryClient.refetchQueries([REACT_QUERY_KEY.ALERT_RULE_DETAILS, ruleId]); notifications.success({ message: `Alert has been ${ data?.payload?.state === 'disabled' ? 'disabled' : 'enabled' From 8e5b1be10621c0fe3344da4dc82a2ee305b170f0 Mon Sep 17 00:00:00 2001 From: Shaheer Kochai Date: Tue, 5 Aug 2025 09:29:52 +0430 Subject: [PATCH 3/4] fix: improve the UX of trying to create a funnel with existing name (#8693) --- .../components/CreateFunnel/CreateFunnel.tsx | 38 +++++++++++++++---- .../RenameFunnel/RenameFunnel.styles.scss | 20 ++++++++++ 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/frontend/src/pages/TracesFunnels/components/CreateFunnel/CreateFunnel.tsx b/frontend/src/pages/TracesFunnels/components/CreateFunnel/CreateFunnel.tsx index b98053ae90d5..29fb1fbfddbd 100644 --- a/frontend/src/pages/TracesFunnels/components/CreateFunnel/CreateFunnel.tsx +++ b/frontend/src/pages/TracesFunnels/components/CreateFunnel/CreateFunnel.tsx @@ -2,7 +2,7 @@ import '../RenameFunnel/RenameFunnel.styles.scss'; import { Input } from 'antd'; import logEvent from 'api/common/logEvent'; -import { AxiosError } from 'axios'; +import axios from 'axios'; import SignozModal from 'components/SignozModal/SignozModal'; import { REACT_QUERY_KEY } from 'constants/reactQueryKeys'; import ROUTES from 'constants/routes'; @@ -26,6 +26,7 @@ function CreateFunnel({ redirectToDetails, }: CreateFunnelProps): JSX.Element { const [funnelName, setFunnelName] = useState(''); + const [inputError, setInputError] = useState(''); const createFunnelMutation = useCreateFunnel(); const { notifications } = useNotifications(); const queryClient = useQueryClient(); @@ -51,6 +52,7 @@ function CreateFunnel({ logEvent(eventMessage, {}); setFunnelName(''); + setInputError(''); queryClient.invalidateQueries([REACT_QUERY_KEY.GET_FUNNELS_LIST]); const funnelId = data?.payload?.funnel_id; @@ -65,11 +67,17 @@ function CreateFunnel({ } }, onError: (error) => { - notifications.error({ - message: - ((error as AxiosError)?.response?.data as string) || - 'Failed to create funnel', - }); + if (axios.isAxiosError(error) && error.response?.status === 400) { + const errorMessage = + error.response?.data?.error?.message || 'Invalid funnel name'; + setInputError(errorMessage); + } else { + notifications.error({ + message: axios.isAxiosError(error) + ? error.response?.data?.error?.message + : 'Failed to create funnel', + }); + } }, }, ); @@ -77,9 +85,17 @@ function CreateFunnel({ const handleCancel = (): void => { setFunnelName(''); + setInputError(''); onClose(); }; + const handleInputChange = (e: React.ChangeEvent): void => { + setFunnelName(e.target.value); + if (inputError) { + setInputError(''); + } + }; + return ( Enter funnel name setFunnelName(e.target.value)} + onChange={handleInputChange} placeholder="Eg. checkout dropoff funnel" autoFocus + status={inputError && 'error'} /> + {inputError && ( + {inputError} + )}
); diff --git a/frontend/src/pages/TracesFunnels/components/RenameFunnel/RenameFunnel.styles.scss b/frontend/src/pages/TracesFunnels/components/RenameFunnel/RenameFunnel.styles.scss index 7cc6be8473cb..0a020e7bec56 100644 --- a/frontend/src/pages/TracesFunnels/components/RenameFunnel/RenameFunnel.styles.scss +++ b/frontend/src/pages/TracesFunnels/components/RenameFunnel/RenameFunnel.styles.scss @@ -63,6 +63,18 @@ font-weight: 400; line-height: 18px; letter-spacing: -0.07px; + + &--error { + border-color: var(--bg-cherry-500); + } + } + + &__error { + color: var(--bg-cherry-500); + font-size: 12px; + font-weight: 400; + line-height: 16px; + margin-top: 4px; } } @@ -82,6 +94,14 @@ &:focus { border-color: var(--bg-robin-500); } + + &--error { + border-color: var(--bg-cherry-500); + } + } + + &__error { + color: var(--bg-cherry-500); } } .funnel-modal__cancel-btn { From 372372694ee726f64983cd0380845be1dce7ea37 Mon Sep 17 00:00:00 2001 From: Amlan Kumar Nandy <45410599+amlannandy@users.noreply.github.com> Date: Tue, 5 Aug 2025 12:39:31 +0700 Subject: [PATCH 4/4] chore: update home page task tracker for metrics explorer (#8631) --- frontend/src/constants/orgPreferences.ts | 4 +- frontend/src/container/Home/Home.tsx | 60 +++++----- .../container/Home/SavedViews/SavedViews.tsx | 112 ++++++++++++++---- frontend/src/container/Home/constants.ts | 16 ++- 4 files changed, 124 insertions(+), 68 deletions(-) diff --git a/frontend/src/constants/orgPreferences.ts b/frontend/src/constants/orgPreferences.ts index 83a4a1e3f341..f96aa4a014ed 100644 --- a/frontend/src/constants/orgPreferences.ts +++ b/frontend/src/constants/orgPreferences.ts @@ -7,8 +7,8 @@ export const ORG_PREFERENCES = { 'welcome_checklist_setup_alerts_skipped', WELCOME_CHECKLIST_SETUP_SAVED_VIEW_SKIPPED: 'welcome_checklist_setup_saved_view_skipped', - WELCOME_CHECKLIST_SEND_INFRA_METRICS_SKIPPED: - 'welcome_checklist_send_infra_metrics_skipped', + WELCOME_CHECKLIST_SEND_METRICS_SKIPPED: + 'welcome_checklist_send_metrics_skipped', WELCOME_CHECKLIST_SETUP_DASHBOARDS_SKIPPED: 'welcome_checklist_setup_dashboards_skipped', WELCOME_CHECKLIST_SETUP_WORKSPACE_SKIPPED: diff --git a/frontend/src/container/Home/Home.tsx b/frontend/src/container/Home/Home.tsx index 33e96374224d..d9133095b95d 100644 --- a/frontend/src/container/Home/Home.tsx +++ b/frontend/src/container/Home/Home.tsx @@ -4,21 +4,17 @@ import './Home.styles.scss'; import { Color } from '@signozhq/design-tokens'; import { Button, Popover } from 'antd'; import logEvent from 'api/common/logEvent'; -import { HostListPayload } from 'api/infraMonitoring/getHostLists'; -import { K8sPodsListPayload } from 'api/infraMonitoring/getK8sPodsList'; import listUserPreferences from 'api/v1/user/preferences/list'; import updateUserPreferenceAPI from 'api/v1/user/preferences/name/update'; import Header from 'components/Header/Header'; import { ENTITY_VERSION_V5 } from 'constants/app'; -import { FeatureKeys } from 'constants/features'; import { LOCALSTORAGE } from 'constants/localStorage'; import { ORG_PREFERENCES } from 'constants/orgPreferences'; import { initialQueriesMap, PANEL_TYPES } from 'constants/queryBuilder'; import { REACT_QUERY_KEY } from 'constants/reactQueryKeys'; import ROUTES from 'constants/routes'; -import { getHostListsQuery } from 'container/InfraMonitoringHosts/utils'; -import { useGetHostList } from 'hooks/infraMonitoring/useGetHostList'; -import { useGetK8sPodsList } from 'hooks/infraMonitoring/useGetK8sPodsList'; +import { getMetricsListQuery } from 'container/MetricsExplorer/Summary/utils'; +import { useGetMetricsList } from 'hooks/metricsExplorer/useGetMetricsList'; import { useGetQueryRange } from 'hooks/queryBuilder/useGetQueryRange'; import { useGetTenantLicense } from 'hooks/useGetTenantLicense'; import history from 'lib/history'; @@ -132,9 +128,9 @@ export default function Home(): JSX.Element { }, ); - // Detect Infra Metrics - Hosts + // Detect Metrics const query = useMemo(() => { - const baseQuery = getHostListsQuery(); + const baseQuery = getMetricsListQuery(); let queryStartTime = startTime; let queryEndTime = endTime; @@ -161,26 +157,11 @@ export default function Home(): JSX.Element { }; }, [startTime, endTime]); - const { data: hostData } = useGetHostList(query as HostListPayload, { - queryKey: ['hostList', query], + const { data: metricsData } = useGetMetricsList(query, { enabled: !!query, + queryKey: ['metricsList', query], }); - const { featureFlags } = useAppContext(); - const dotMetricsEnabled = - featureFlags?.find((flag) => flag.name === FeatureKeys.DOT_METRICS_ENABLED) - ?.active || false; - - const { data: k8sPodsData } = useGetK8sPodsList( - query as K8sPodsListPayload, - { - queryKey: ['K8sPodsList', query], - enabled: !!query, - }, - undefined, - dotMetricsEnabled, - ); - const [isLogsIngestionActive, setIsLogsIngestionActive] = useState(false); const [isTracesIngestionActive, setIsTracesIngestionActive] = useState(false); const [isMetricsIngestionActive, setIsMetricsIngestionActive] = useState( @@ -305,15 +286,14 @@ export default function Home(): JSX.Element { }, [tracesData, handleUpdateChecklistDoneItem]); useEffect(() => { - const hostDataTotal = hostData?.payload?.data?.total ?? 0; - const k8sPodsDataTotal = k8sPodsData?.payload?.data?.total ?? 0; + const metricsDataTotal = metricsData?.payload?.data?.total ?? 0; - if (hostDataTotal > 0 || k8sPodsDataTotal > 0) { + if (metricsDataTotal > 0) { setIsMetricsIngestionActive(true); handleUpdateChecklistDoneItem('ADD_DATA_SOURCE'); - handleUpdateChecklistDoneItem('SEND_INFRA_METRICS'); + handleUpdateChecklistDoneItem('SEND_METRICS'); } - }, [hostData, k8sPodsData, handleUpdateChecklistDoneItem]); + }, [metricsData, handleUpdateChecklistDoneItem]); useEffect(() => { logEvent('Homepage: Visited', {}); @@ -520,19 +500,19 @@ export default function Home(): JSX.Element { logEvent('Homepage: Ingestion Active Explore clicked', { source: 'Metrics', }); - history.push(ROUTES.INFRASTRUCTURE_MONITORING_HOSTS); + history.push(ROUTES.METRICS_EXPLORER); }} onKeyDown={(e): void => { if (e.key === 'Enter') { logEvent('Homepage: Ingestion Active Explore clicked', { source: 'Metrics', }); - history.push(ROUTES.INFRASTRUCTURE_MONITORING_HOSTS); + history.push(ROUTES.METRICS_EXPLORER); } }} > - Explore Infra Metrics + Explore Metrics @@ -593,6 +573,20 @@ export default function Home(): JSX.Element { > Open Traces Explorer + + diff --git a/frontend/src/container/Home/SavedViews/SavedViews.tsx b/frontend/src/container/Home/SavedViews/SavedViews.tsx index a89f898de55c..0e3272fc36e4 100644 --- a/frontend/src/container/Home/SavedViews/SavedViews.tsx +++ b/frontend/src/container/Home/SavedViews/SavedViews.tsx @@ -7,6 +7,7 @@ import { useHandleExplorerTabChange } from 'hooks/useHandleExplorerTabChange'; import { ArrowRight, ArrowUpRight, + BarChart, CompassIcon, DraftingCompass, } from 'lucide-react'; @@ -42,6 +43,12 @@ export default function SavedViews({ isError: tracesViewsError, } = useGetAllViews(DataSource.TRACES); + const { + data: metricsViewsData, + isLoading: metricsViewsLoading, + isError: metricsViewsError, + } = useGetAllViews(DataSource.METRICS); + const logsViews = useMemo(() => [...(logsViewsData?.data.data || [])], [ logsViewsData, ]); @@ -50,14 +57,25 @@ export default function SavedViews({ tracesViewsData, ]); + const metricsViews = useMemo(() => [...(metricsViewsData?.data.data || [])], [ + metricsViewsData, + ]); + useEffect(() => { - setSelectedEntityViews(selectedEntity === 'logs' ? logsViews : tracesViews); - }, [selectedEntity, logsViews, tracesViews]); + if (selectedEntity === 'logs') { + setSelectedEntityViews(logsViews); + } else if (selectedEntity === 'traces') { + setSelectedEntityViews(tracesViews); + } else if (selectedEntity === 'metrics') { + setSelectedEntityViews(metricsViews); + } + }, [selectedEntity, logsViews, tracesViews, metricsViews]); const hasTracesViews = tracesViews.length > 0; const hasLogsViews = logsViews.length > 0; + const hasMetricsViews = metricsViews.length > 0; - const hasSavedViews = hasTracesViews || hasLogsViews; + const hasSavedViews = hasTracesViews || hasLogsViews || hasMetricsViews; const { handleExplorerTabChange } = useHandleExplorerTabChange(); @@ -68,10 +86,16 @@ export default function SavedViews({ entity: selectedEntity, }); - const currentViewDetails = getViewDetailsUsingViewKey( - view.id, - selectedEntity === 'logs' ? logsViews : tracesViews, - ); + let currentViews: ViewProps[] = []; + if (selectedEntity === 'logs') { + currentViews = logsViews; + } else if (selectedEntity === 'traces') { + currentViews = tracesViews; + } else if (selectedEntity === 'metrics') { + currentViews = metricsViews; + } + + const currentViewDetails = getViewDetailsUsingViewKey(view.id, currentViews); if (!currentViewDetails) return; const { query, name, id, panelType: currentPanelType } = currentViewDetails; @@ -94,6 +118,32 @@ export default function SavedViews({ } }, [hasSavedViews, onUpdateChecklistDoneItem, loadingUserPreferences]); + const footerLink = useMemo(() => { + if (selectedEntity === 'logs') { + return ROUTES.LOGS_SAVE_VIEWS; + } + if (selectedEntity === 'traces') { + return ROUTES.TRACES_SAVE_VIEWS; + } + if (selectedEntity === 'metrics') { + return ROUTES.METRICS_EXPLORER_VIEWS; + } + return ''; + }, [selectedEntity]); + + const getStartedLink = useMemo(() => { + if (selectedEntity === 'logs') { + return ROUTES.LOGS_EXPLORER; + } + if (selectedEntity === 'traces') { + return ROUTES.TRACES_EXPLORER; + } + if (selectedEntity === 'metrics') { + return ROUTES.METRICS_EXPLORER_EXPLORER; + } + return ''; + }, [selectedEntity]); + const emptyStateCard = (): JSX.Element => (
@@ -115,13 +165,7 @@ export default function SavedViews({ {user?.role !== USER_ROLES.VIEWER && (
- +
)} + + {selectedEntity === 'metrics' && metricsViewsError && ( +
+
+ Oops, something went wrong while loading your saved views. +
+
+ )}
); @@ -246,11 +298,19 @@ export default function SavedViews({ logEvent('Homepage: Saved views switched', { tab, }); - setSelectedEntityViews(tab === 'logs' ? logsViews : tracesViews); + let currentViews: ViewProps[] = []; + if (tab === 'logs') { + currentViews = logsViews; + } else if (tab === 'traces') { + currentViews = tracesViews; + } else if (tab === 'metrics') { + currentViews = metricsViews; + } + setSelectedEntityViews(currentViews); setSelectedEntity(tab); }; - if (logsViewsLoading || tracesViewsLoading) { + if (logsViewsLoading || tracesViewsLoading || metricsViewsLoading) { return ( @@ -260,7 +320,7 @@ export default function SavedViews({ ); } - if (logsViewsError || tracesViewsError) { + if (logsViewsError || tracesViewsError || metricsViewsError) { return ( @@ -299,6 +359,16 @@ export default function SavedViews({ > Traces + @@ -312,13 +382,7 @@ export default function SavedViews({ {selectedEntityViews.length > 0 && (
- +