mirror of
https://github.com/SigNoz/signoz.git
synced 2025-12-17 15:36:48 +00:00
fix: handle resource/attribute context collision with expression using paranthesis (#9240)
This commit is contained in:
parent
cbb24d9a34
commit
78e4f4f386
@ -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(
|
warnMsg := fmt.Sprintf(
|
||||||
"Key `%s` is ambiguous, found %d different combinations of field context / data type: %v.",
|
"Key `%s` is ambiguous, found %d different combinations of field context / data type: %v.",
|
||||||
fieldKey.Name,
|
fieldKey.Name,
|
||||||
@ -865,6 +865,7 @@ func (v *filterExpressionVisitor) VisitKey(ctx *grammar.KeyContext) any {
|
|||||||
mixedFieldContext[item.FieldContext.StringValue()] = true
|
mixedFieldContext[item.FieldContext.StringValue()] = true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// when there is both resource and attribute context, default to resource only
|
||||||
if mixedFieldContext[telemetrytypes.FieldContextResource.StringValue()] &&
|
if mixedFieldContext[telemetrytypes.FieldContextResource.StringValue()] &&
|
||||||
mixedFieldContext[telemetrytypes.FieldContextAttribute.StringValue()] {
|
mixedFieldContext[telemetrytypes.FieldContextAttribute.StringValue()] {
|
||||||
filteredKeys := []*telemetrytypes.TelemetryFieldKey{}
|
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, " +
|
warnMsg += " " + "Using `resource` context by default. To query attributes explicitly, " +
|
||||||
fmt.Sprintf("use the fully qualified name (e.g., 'attribute.%s')", fieldKey.Name)
|
fmt.Sprintf("use the fully qualified name (e.g., 'attribute.%s')", fieldKey.Name)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if !v.keysWithWarnings[keyName] {
|
||||||
v.mainWarnURL = "https://signoz.io/docs/userguide/field-context-data-types/"
|
v.mainWarnURL = "https://signoz.io/docs/userguide/field-context-data-types/"
|
||||||
// this is warning state, we must have a unambiguous key
|
// this is warning state, we must have a unambiguous key
|
||||||
v.warnings = append(v.warnings, warnMsg)
|
v.warnings = append(v.warnings, warnMsg)
|
||||||
|
}
|
||||||
v.keysWithWarnings[keyName] = true
|
v.keysWithWarnings[keyName] = true
|
||||||
v.logger.Warn("ambiguous key", "field_key_name", fieldKey.Name) //nolint:sloglint
|
v.logger.Warn("ambiguous key", "field_key_name", fieldKey.Name) //nolint:sloglint
|
||||||
}
|
}
|
||||||
|
|||||||
@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@ -899,3 +899,55 @@ func buildCompleteFieldKeyMap() map[string][]*telemetrytypes.TelemetryFieldKey {
|
|||||||
}
|
}
|
||||||
return keysMap
|
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
|
||||||
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user