diff --git a/pkg/querybuilder/where_clause_visitor.go b/pkg/querybuilder/where_clause_visitor.go index 3c89a8854e66..54fdd36403de 100644 --- a/pkg/querybuilder/where_clause_visitor.go +++ b/pkg/querybuilder/where_clause_visitor.go @@ -853,7 +853,7 @@ func (v *filterExpressionVisitor) VisitKey(ctx *grammar.KeyContext) any { } } - if len(fieldKeysForName) > 1 && !v.keysWithWarnings[keyName] { + if len(fieldKeysForName) > 1 { warnMsg := fmt.Sprintf( "Key `%s` is ambiguous, found %d different combinations of field context / data type: %v.", fieldKey.Name, @@ -865,6 +865,7 @@ func (v *filterExpressionVisitor) VisitKey(ctx *grammar.KeyContext) any { mixedFieldContext[item.FieldContext.StringValue()] = true } + // when there is both resource and attribute context, default to resource only if mixedFieldContext[telemetrytypes.FieldContextResource.StringValue()] && mixedFieldContext[telemetrytypes.FieldContextAttribute.StringValue()] { filteredKeys := []*telemetrytypes.TelemetryFieldKey{} @@ -878,9 +879,12 @@ func (v *filterExpressionVisitor) VisitKey(ctx *grammar.KeyContext) any { warnMsg += " " + "Using `resource` context by default. To query attributes explicitly, " + fmt.Sprintf("use the fully qualified name (e.g., 'attribute.%s')", fieldKey.Name) } - 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, warnMsg) + + if !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, warnMsg) + } v.keysWithWarnings[keyName] = true v.logger.Warn("ambiguous key", "field_key_name", fieldKey.Name) //nolint:sloglint } diff --git a/pkg/telemetrylogs/stmt_builder_test.go b/pkg/telemetrylogs/stmt_builder_test.go index b19a4d86f10c..cc2e64ae1bb4 100644 --- a/pkg/telemetrylogs/stmt_builder_test.go +++ b/pkg/telemetrylogs/stmt_builder_test.go @@ -488,3 +488,101 @@ func TestStatementBuilderTimeSeriesBodyGroupBy(t *testing.T) { }) } } + +func TestStatementBuilderListQueryServiceCollision(t *testing.T) { + cases := []struct { + name string + requestType qbtypes.RequestType + query qbtypes.QueryBuilderQuery[qbtypes.LogAggregation] + expected qbtypes.Statement + expectedErr error + expectWarn bool + }{ + { + name: "default list", + requestType: qbtypes.RequestTypeRaw, + query: qbtypes.QueryBuilderQuery[qbtypes.LogAggregation]{ + Signal: telemetrytypes.SignalLogs, + Filter: &qbtypes.Filter{ + Expression: "(service.name = 'cartservice' AND body CONTAINS 'error')", + }, + Limit: 10, + }, + 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 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 ((LOWER(body) LIKE LOWER(?))) AND timestamp >= ? AND ts_bucket_start >= ? AND timestamp < ? AND ts_bucket_start <= ? LIMIT ?", + Args: []any{"cartservice", "%service.name%", "%service.name\":\"cartservice%", uint64(1747945619), uint64(1747983448), "%error%", "1747947419000000000", uint64(1747945619), "1747983448000000000", uint64(1747983448), 10}, + }, + expectedErr: nil, + expectWarn: true, + }, + { + 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' AND body CONTAINS 'error'", + }, + 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 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 (LOWER(body) LIKE LOWER(?)) AND timestamp >= ? AND ts_bucket_start >= ? AND timestamp < ? 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), "%error%", "1747947419000000000", uint64(1747945619), "1747983448000000000", uint64(1747983448), 10}, + }, + expectedErr: nil, + expectWarn: true, + }, + } + + fm := NewFieldMapper() + cb := NewConditionBuilder(fm) + mockMetadataStore := telemetrytypestest.NewMockMetadataStore() + mockMetadataStore.KeysMap = buildCompleteFieldKeyMapCollision() + + aggExprRewriter := querybuilder.NewAggExprRewriter(instrumentationtest.New().ToProviderSettings(), 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) + if c.expectWarn { + require.True(t, len(q.Warnings) > 0) + } + } + }) + } +} diff --git a/pkg/telemetrylogs/test_data.go b/pkg/telemetrylogs/test_data.go index 3dd05933d9b2..da4045f299f2 100644 --- a/pkg/telemetrylogs/test_data.go +++ b/pkg/telemetrylogs/test_data.go @@ -899,3 +899,55 @@ func buildCompleteFieldKeyMap() map[string][]*telemetrytypes.TelemetryFieldKey { } return keysMap } + +func buildCompleteFieldKeyMapCollision() map[string][]*telemetrytypes.TelemetryFieldKey { + keysMap := map[string][]*telemetrytypes.TelemetryFieldKey{ + "service.name": { + { + Name: "service.name", + FieldContext: telemetrytypes.FieldContextResource, + FieldDataType: telemetrytypes.FieldDataTypeString, + }, + { + Name: "service.name", + FieldContext: telemetrytypes.FieldContextAttribute, + FieldDataType: telemetrytypes.FieldDataTypeString, + }, + }, + "body": { + { + Name: "body", + FieldContext: telemetrytypes.FieldContextLog, + FieldDataType: telemetrytypes.FieldDataTypeString, + }, + }, + "error.code": { + { + Name: "error.code", + FieldContext: telemetrytypes.FieldContextAttribute, + FieldDataType: telemetrytypes.FieldDataTypeInt64, + }, + }, + "environment": { + { + Name: "environment", + FieldContext: telemetrytypes.FieldContextResource, + FieldDataType: telemetrytypes.FieldDataTypeString, + }, + }, + "user.id": { + { + Name: "user.id", + FieldContext: telemetrytypes.FieldContextAttribute, + FieldDataType: telemetrytypes.FieldDataTypeString, + }, + }, + } + + for _, keys := range keysMap { + for _, key := range keys { + key.Signal = telemetrytypes.SignalLogs + } + } + return keysMap +}