From 161a69fbe9c3ae4a0750a24247c0468544ce37bb Mon Sep 17 00:00:00 2001 From: Raj Kamal Singh <1133322+raj-k-singh@users.noreply.github.com> Date: Tue, 2 Jul 2024 17:14:08 +0530 Subject: [PATCH] chore: remove workaround for supporting pipeline filters using attribs with . replaced with _ (#5405) --- .../pipelineBuilder_test.go | 73 ------------------- .../queryBuilderToExpr/queryBuilderToExpr.go | 54 +++++--------- 2 files changed, 20 insertions(+), 107 deletions(-) diff --git a/pkg/query-service/app/logparsingpipeline/pipelineBuilder_test.go b/pkg/query-service/app/logparsingpipeline/pipelineBuilder_test.go index 19ffd4780f..82bb2ab2bf 100644 --- a/pkg/query-service/app/logparsingpipeline/pipelineBuilder_test.go +++ b/pkg/query-service/app/logparsingpipeline/pipelineBuilder_test.go @@ -803,76 +803,3 @@ func TestContainsFilterIsCaseInsensitive(t *testing.T) { _, test2Exists := result[0].Attributes_string["test2"] require.False(test2Exists) } - -func TestTemporaryWorkaroundForSupportingAttribsContainingDots(t *testing.T) { - // TODO(Raj): Remove this after dots are supported - - require := require.New(t) - - testPipeline := Pipeline{ - OrderId: 1, - Name: "pipeline1", - Alias: "pipeline1", - Enabled: true, - Filter: &v3.FilterSet{ - Operator: "AND", - Items: []v3.FilterItem{ - { - Key: v3.AttributeKey{ - Key: "k8s_deployment_name", - DataType: v3.AttributeKeyDataTypeString, - Type: v3.AttributeKeyTypeResource, - }, - Operator: "=", - Value: "ingress", - }, - }, - }, - Config: []PipelineOperator{ - { - ID: "add", - Type: "add", - Enabled: true, - Name: "add", - Field: "attributes.test", - Value: "test-value", - }, - }, - } - - testLogs := []model.SignozLog{{ - Timestamp: uint64(time.Now().UnixNano()), - Body: "test log", - Attributes_string: map[string]string{}, - Resources_string: map[string]string{ - "k8s_deployment_name": "ingress", - }, - SeverityText: entry.Info.String(), - SeverityNumber: uint8(entry.Info), - SpanID: "", - TraceID: "", - }, { - Timestamp: uint64(time.Now().UnixNano()), - Body: "test log", - Attributes_string: map[string]string{}, - Resources_string: map[string]string{ - "k8s.deployment.name": "ingress", - }, - SeverityText: entry.Info.String(), - SeverityNumber: uint8(entry.Info), - SpanID: "", - TraceID: "", - }} - - result, collectorWarnAndErrorLogs, err := SimulatePipelinesProcessing( - context.Background(), - []Pipeline{testPipeline}, - testLogs, - ) - require.Nil(err) - require.Equal(0, len(collectorWarnAndErrorLogs), strings.Join(collectorWarnAndErrorLogs, "\n")) - require.Equal(2, len(result)) - for _, processedLog := range result { - require.Equal(processedLog.Attributes_string["test"], "test-value") - } -} diff --git a/pkg/query-service/queryBuilderToExpr/queryBuilderToExpr.go b/pkg/query-service/queryBuilderToExpr/queryBuilderToExpr.go index e853a37685..bda8c446b1 100644 --- a/pkg/query-service/queryBuilderToExpr/queryBuilderToExpr.go +++ b/pkg/query-service/queryBuilderToExpr/queryBuilderToExpr.go @@ -53,49 +53,35 @@ func Parse(filters *v3.FilterSet) (string, error) { return "", fmt.Errorf("operator not supported") } - // TODO(Raj): Remove the use of dot replaced alternative when key - // contains underscore after dots are supported in keys - names := []string{getName(v.Key)} - if strings.Contains(v.Key.Key, "_") { - dotKey := v.Key - dotKey.Key = strings.Replace(v.Key.Key, "_", ".", -1) - names = append(names, getName(dotKey)) - } + name := getName(v.Key) - filterParts := []string{} - for _, name := range names { - var filter string + var filter string - switch v.Operator { - // uncomment following lines when new version of expr is used - // case v3.FilterOperatorIn, v3.FilterOperatorNotIn: - // filter = fmt.Sprintf("%s %s list%s", name, logOperatorsToExpr[v.Operator], exprFormattedValue(v.Value)) + switch v.Operator { + // uncomment following lines when new version of expr is used + // case v3.FilterOperatorIn, v3.FilterOperatorNotIn: + // filter = fmt.Sprintf("%s %s list%s", name, logOperatorsToExpr[v.Operator], exprFormattedValue(v.Value)) - case v3.FilterOperatorExists, v3.FilterOperatorNotExists: - filter = fmt.Sprintf("%s %s %s", exprFormattedValue(v.Key.Key), logOperatorsToExpr[v.Operator], getTypeName(v.Key.Type)) + case v3.FilterOperatorExists, v3.FilterOperatorNotExists: + filter = fmt.Sprintf("%s %s %s", exprFormattedValue(v.Key.Key), logOperatorsToExpr[v.Operator], getTypeName(v.Key.Type)) - default: - filter = fmt.Sprintf("%s %s %s", name, logOperatorsToExpr[v.Operator], exprFormattedValue(v.Value)) + default: + filter = fmt.Sprintf("%s %s %s", name, logOperatorsToExpr[v.Operator], exprFormattedValue(v.Value)) - if v.Operator == v3.FilterOperatorContains || v.Operator == v3.FilterOperatorNotContains { - // `contains` and `ncontains` should be case insensitive to match how they work when querying logs. - filter = fmt.Sprintf( - "lower(%s) %s lower(%s)", - name, logOperatorsToExpr[v.Operator], exprFormattedValue(v.Value), - ) - } - - // Avoid running operators on nil values - if v.Operator != v3.FilterOperatorEqual && v.Operator != v3.FilterOperatorNotEqual { - filter = fmt.Sprintf("%s != nil && %s", name, filter) - } + if v.Operator == v3.FilterOperatorContains || v.Operator == v3.FilterOperatorNotContains { + // `contains` and `ncontains` should be case insensitive to match how they work when querying logs. + filter = fmt.Sprintf( + "lower(%s) %s lower(%s)", + name, logOperatorsToExpr[v.Operator], exprFormattedValue(v.Value), + ) } - filterParts = append(filterParts, filter) + // Avoid running operators on nil values + if v.Operator != v3.FilterOperatorEqual && v.Operator != v3.FilterOperatorNotEqual { + filter = fmt.Sprintf("%s != nil && %s", name, filter) + } } - filter := strings.Join(filterParts, " || ") - // check if the filter is a correct expression language _, err := expr.Compile(filter) if err != nil {