mirror of
https://github.com/SigNoz/signoz.git
synced 2025-12-17 15:36:48 +00:00
feat: warn when LIKE/ILIKE is used without any %/_ (#9098)
* feat: warn when LIKE/ILIKE is used without any %/_ Signed-off-by: “niladrix719” <niladrix719@gmail.com>
This commit is contained in:
parent
2acdd101d8
commit
c5051128fa
@ -18,6 +18,8 @@ import (
|
|||||||
|
|
||||||
var searchTroubleshootingGuideURL = "https://signoz.io/docs/userguide/search-troubleshooting/"
|
var searchTroubleshootingGuideURL = "https://signoz.io/docs/userguide/search-troubleshooting/"
|
||||||
|
|
||||||
|
const stringMatchingOperatorDocURL = "https://signoz.io/docs/userguide/operators-reference/#string-matching-operators"
|
||||||
|
|
||||||
// filterExpressionVisitor implements the FilterQueryVisitor interface
|
// filterExpressionVisitor implements the FilterQueryVisitor interface
|
||||||
// to convert the parsed filter expressions into ClickHouse WHERE clause
|
// to convert the parsed filter expressions into ClickHouse WHERE clause
|
||||||
type filterExpressionVisitor struct {
|
type filterExpressionVisitor struct {
|
||||||
@ -533,11 +535,13 @@ func (v *filterExpressionVisitor) VisitComparison(ctx *grammar.ComparisonContext
|
|||||||
if ctx.NOT() != nil {
|
if ctx.NOT() != nil {
|
||||||
op = qbtypes.FilterOperatorNotLike
|
op = qbtypes.FilterOperatorNotLike
|
||||||
}
|
}
|
||||||
|
v.warnIfLikeWithoutWildcards("LIKE", value)
|
||||||
} else if ctx.ILIKE() != nil {
|
} else if ctx.ILIKE() != nil {
|
||||||
op = qbtypes.FilterOperatorILike
|
op = qbtypes.FilterOperatorILike
|
||||||
if ctx.NOT() != nil {
|
if ctx.NOT() != nil {
|
||||||
op = qbtypes.FilterOperatorNotILike
|
op = qbtypes.FilterOperatorNotILike
|
||||||
}
|
}
|
||||||
|
v.warnIfLikeWithoutWildcards("ILIKE", value)
|
||||||
} else if ctx.REGEXP() != nil {
|
} else if ctx.REGEXP() != nil {
|
||||||
op = qbtypes.FilterOperatorRegexp
|
op = qbtypes.FilterOperatorRegexp
|
||||||
if ctx.NOT() != nil {
|
if ctx.NOT() != nil {
|
||||||
@ -571,6 +575,19 @@ func (v *filterExpressionVisitor) VisitComparison(ctx *grammar.ComparisonContext
|
|||||||
return "" // Should not happen with valid input
|
return "" // Should not happen with valid input
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// warnIfLikeWithoutWildcards adds a guidance warning when LIKE/ILIKE is used without wildcards
|
||||||
|
func (v *filterExpressionVisitor) warnIfLikeWithoutWildcards(op string, value any) {
|
||||||
|
if hasLikeWildcards(value) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
msg := op + " operator used without wildcards (% or _). Consider using = operator for exact matches or add wildcards for pattern matching."
|
||||||
|
v.warnings = append(v.warnings, msg)
|
||||||
|
if v.mainWarnURL == "" {
|
||||||
|
v.mainWarnURL = stringMatchingOperatorDocURL
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// VisitInClause handles IN expressions
|
// VisitInClause handles IN expressions
|
||||||
func (v *filterExpressionVisitor) VisitInClause(ctx *grammar.InClauseContext) any {
|
func (v *filterExpressionVisitor) VisitInClause(ctx *grammar.InClauseContext) any {
|
||||||
if ctx.ValueList() != nil {
|
if ctx.ValueList() != nil {
|
||||||
@ -871,6 +888,15 @@ func (v *filterExpressionVisitor) VisitKey(ctx *grammar.KeyContext) any {
|
|||||||
return fieldKeysForName
|
return fieldKeysForName
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// hasLikeWildcards checks if a value contains LIKE wildcards (% or _)
|
||||||
|
func hasLikeWildcards(value any) bool {
|
||||||
|
str, ok := value.(string)
|
||||||
|
if !ok {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
return strings.Contains(str, "%") || strings.Contains(str, "_")
|
||||||
|
}
|
||||||
|
|
||||||
func trimQuotes(txt string) string {
|
func trimQuotes(txt string) string {
|
||||||
if len(txt) >= 2 {
|
if len(txt) >= 2 {
|
||||||
if (txt[0] == '"' && txt[len(txt)-1] == '"') ||
|
if (txt[0] == '"' && txt[len(txt)-1] == '"') ||
|
||||||
|
|||||||
82
pkg/telemetrylogs/filter_expr_like_warning_test.go
Normal file
82
pkg/telemetrylogs/filter_expr_like_warning_test.go
Normal file
@ -0,0 +1,82 @@
|
|||||||
|
package telemetrylogs
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/SigNoz/signoz/pkg/instrumentation/instrumentationtest"
|
||||||
|
"github.com/SigNoz/signoz/pkg/querybuilder"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestLikeAndILikeWithoutWildcards_Warns Tests that LIKE/ILIKE without wildcards add warnings and include docs URL
|
||||||
|
func TestLikeAndILikeWithoutWildcards_Warns(t *testing.T) {
|
||||||
|
fm := NewFieldMapper()
|
||||||
|
cb := NewConditionBuilder(fm)
|
||||||
|
|
||||||
|
keys := buildCompleteFieldKeyMap()
|
||||||
|
|
||||||
|
opts := querybuilder.FilterExprVisitorOpts{
|
||||||
|
Logger: instrumentationtest.New().Logger(),
|
||||||
|
FieldMapper: fm,
|
||||||
|
ConditionBuilder: cb,
|
||||||
|
FieldKeys: keys,
|
||||||
|
FullTextColumn: DefaultFullTextColumn,
|
||||||
|
JsonBodyPrefix: BodyJSONStringSearchPrefix,
|
||||||
|
JsonKeyToKey: GetBodyJSONKey,
|
||||||
|
}
|
||||||
|
|
||||||
|
tests := []string{
|
||||||
|
"service.name LIKE 'demo-backend'",
|
||||||
|
"service.name ILIKE 'demo-backend'",
|
||||||
|
"service.name NOT LIKE 'demo-backend'",
|
||||||
|
"service.name NOT ILIKE 'demo-backend'",
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, expr := range tests {
|
||||||
|
t.Run(expr, func(t *testing.T) {
|
||||||
|
clause, err := querybuilder.PrepareWhereClause(expr, opts)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.NotNil(t, clause)
|
||||||
|
|
||||||
|
require.NotEmpty(t, clause.Warnings, "expected warning for: %s", expr)
|
||||||
|
require.Contains(t, clause.Warnings[0], "operator used without wildcards")
|
||||||
|
require.Contains(t, clause.WarningsDocURL, "operators-reference/#string-matching-operators")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestLikeAndILikeWithWildcards_NoWarn Tests that LIKE/ILIKE with wildcards do not add warnings
|
||||||
|
func TestLikeAndILikeWithWildcards_NoWarn(t *testing.T) {
|
||||||
|
fm := NewFieldMapper()
|
||||||
|
cb := NewConditionBuilder(fm)
|
||||||
|
|
||||||
|
keys := buildCompleteFieldKeyMap()
|
||||||
|
|
||||||
|
opts := querybuilder.FilterExprVisitorOpts{
|
||||||
|
Logger: instrumentationtest.New().Logger(),
|
||||||
|
FieldMapper: fm,
|
||||||
|
ConditionBuilder: cb,
|
||||||
|
FieldKeys: keys,
|
||||||
|
FullTextColumn: DefaultFullTextColumn,
|
||||||
|
JsonBodyPrefix: BodyJSONStringSearchPrefix,
|
||||||
|
JsonKeyToKey: GetBodyJSONKey,
|
||||||
|
}
|
||||||
|
|
||||||
|
tests := []string{
|
||||||
|
"service.name LIKE 'demo-%'",
|
||||||
|
"service.name LIKE '%demo'",
|
||||||
|
"service.name ILIKE '_demo'",
|
||||||
|
"service.name ILIKE '%demo%'",
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, expr := range tests {
|
||||||
|
t.Run(expr, func(t *testing.T) {
|
||||||
|
clause, err := querybuilder.PrepareWhereClause(expr, opts)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.NotNil(t, clause)
|
||||||
|
|
||||||
|
require.Empty(t, clause.Warnings, "did not expect warnings for: %s", expr)
|
||||||
|
require.Empty(t, clause.WarningsDocURL)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
x
Reference in New Issue
Block a user