Compare commits

...

4 Commits

Author SHA1 Message Date
Nikhil Soni
73d2c5d09c refactor(traces-qb): keep every metadata match for a resolved column
Resolving select through metadata before the probe worked, but it reordered
resolution for every context-carrying key to fix one thing: a key that resolves
to a column is not necessarily one field.

Say that where it belongs instead. logicalForResolvedColumn already asks
metadata; it just threw away all but a family and fell back to the key itself.
Returning every match it finds fixes the same ambiguity without moving
resolution around: ColumnExpressionFor keeps its probe-first shape and differs
from main by a single line.

The scope column resolves for both a declared path and a same-named scope
attribute, so `scope.name` with such an attribute present now yields both homes
rather than one. Output is identical to the reordered version on every case
that motivated it -- declared-only, attribute-shadowed, materialized attribute,
and an undeclared scope attribute.

Assisted-by: Claude Opus 5
2026-08-25 17:16:41 +05:30
Nikhil Soni
3425776ca6 test(traces-qb): cover semconv families for context-carrying select keys
TestColumnExpressionForFamilyGroupBy only exercised a bare key, which resolves
through the candidate path. A key carrying a context now resolves through
metadata instead, and nothing covered that the family is still reached there.

Run the existing assertion over all three key shapes. The expression is
identical for each: MatchingLogicalFields returns the grouped family directly,
which is what logicalForResolvedColumn was selecting from the same call.

Assisted-by: Claude Opus 5
2026-08-25 16:17:57 +05:30
Nikhil Soni
39b2fc5cef refactor(traces-qb): restore CandidateKeys metadata match to main's form
The context-filtered bare-name match and the appended `{context}.{name}`
spelling existed to let a scope key find its declared path once getColumn
declined scope `name`/`version`. Resolving select through metadata first moved
that job to MatchingLogicalFields, so the extra matching is dead weight: the
suite is green with main's two straight lookups restored.

Keep the scope synthesize case -- it is not redundant. A scope attribute
metadata does not know still reaches CandidateKeys from the filter path, which
passes nil keys for strict contexts, and without the case it resolves to
nothing and the filter fails with "key not found". No test covered that, so
add one.

Assisted-by: Claude Opus 5
2026-08-25 12:44:58 +05:30
Nikhil Soni
bae9a15f2a fix(traces-qb): ask metadata before the column probe when selecting
ColumnExpressionFor resolved a key by probing FieldFor first, while the filter
path asks metadata first. The probe only answers whether a key resolves to a
column, and was standing in for whether it names one field. For a JSON column
that resolves for either of two homes -- a declared scope path or a same-named
scope attribute -- it reported a single resolved field for an ambiguous name,
so getColumn had to decline scope `name` and `version` to force the key back
onto the candidate path. That made a scope attribute so named unselectable:
it failed with "field not found".

Ask metadata first for a key that carries a context, the way the filter path
does. A bare key is left exactly as it was -- it cannot resolve through the
probe at all, since getColumn needs a context, so it already reaches
CandidateKeys, which consults metadata itself.

The filter path is untouched: its rule that one home wins an ambiguous name is
a deliberate choice, and selecting still coalesces the homes instead.

Assisted-by: Claude Opus 5
2026-08-24 12:40:01 +05:30
4 changed files with 65 additions and 41 deletions

View File

@@ -504,6 +504,17 @@ func TestConditionForSynthesizedKeys(t *testing.T) {
assert.Contains(t, args, "timeout")
})
t.Run("scope context with no metadata -> scope attribute", func(t *testing.T) {
sb := sqlbuilder.NewSelectBuilder()
key := telemetrytypes.TelemetryFieldKey{Name: "custom.attr", FieldContext: telemetrytypes.FieldContextScope}
conds, warnings, err := cb.ConditionFor(ctx, valuer.UUID{}, 0, 0, &key, noMatches, qbtypes.ConditionBuilderOptions{}, qbtypes.FilterOperatorEqual, "v", sb)
assert.NoError(t, err, "an undeclared scope attribute must still be filterable")
assert.NotEmpty(t, warnings)
sb.Where(conds...)
sql, _ := sb.BuildWithFlavor(sqlbuilder.ClickHouse)
assert.Contains(t, sql, "scope.attributes.`custom.attr`")
})
t.Run("bare key with number operand -> attribute number", func(t *testing.T) {
sb := sqlbuilder.NewSelectBuilder()
key := telemetrytypes.TelemetryFieldKey{Name: "http.status"}

View File

@@ -236,8 +236,30 @@ func TestColumnExpressionForFamilyGroupBy(t *testing.T) {
}
fm := NewFieldMapper(familyFlagOn(t))
expr, err := fm.ColumnExpressionFor(context.Background(), valuer.UUID{}, startNs, endNs,
&telemetrytypes.TelemetryFieldKey{Name: "deployment.environment.name"}, telemetrytypes.FieldDataTypeString, fieldKeys)
require.NoError(t, err)
require.Equal(t, "multiIf((multiIf(resource.`deployment.environment.name` IS NOT NULL, resource.`deployment.environment.name`::String, mapContains(resources_string, 'deployment.environment.name'), resources_string['deployment.environment.name'], NULL) IS NOT NULL OR multiIf(resource.`deployment.environment` IS NOT NULL, resource.`deployment.environment`::String, mapContains(resources_string, 'deployment.environment'), resources_string['deployment.environment'], NULL) IS NOT NULL), COALESCE(NULLIF(multiIf(resource.`deployment.environment.name` IS NOT NULL, resource.`deployment.environment.name`::String, mapContains(resources_string, 'deployment.environment.name'), resources_string['deployment.environment.name'], NULL), ''), NULLIF(multiIf(resource.`deployment.environment` IS NOT NULL, resource.`deployment.environment`::String, mapContains(resources_string, 'deployment.environment'), resources_string['deployment.environment'], NULL), ''), ''), NULL)", expr)
// The family must be reached whether or not the key carries a context: a bare key
// resolves through the candidate path, a context-carrying one through metadata.
keys := []struct {
name string
key telemetrytypes.TelemetryFieldKey
}{
{"bare", telemetrytypes.TelemetryFieldKey{Name: "deployment.environment.name"}},
{"with context", telemetrytypes.TelemetryFieldKey{
Name: "deployment.environment.name",
FieldContext: telemetrytypes.FieldContextResource,
}},
{"with context and data type", telemetrytypes.TelemetryFieldKey{
Name: "deployment.environment.name",
FieldContext: telemetrytypes.FieldContextResource,
FieldDataType: telemetrytypes.FieldDataTypeString,
}},
}
for _, tc := range keys {
t.Run(tc.name, func(t *testing.T) {
key := tc.key
expr, err := fm.ColumnExpressionFor(context.Background(), valuer.UUID{}, startNs, endNs,
&key, telemetrytypes.FieldDataTypeString, fieldKeys)
require.NoError(t, err)
require.Equal(t, "multiIf((multiIf(resource.`deployment.environment.name` IS NOT NULL, resource.`deployment.environment.name`::String, mapContains(resources_string, 'deployment.environment.name'), resources_string['deployment.environment.name'], NULL) IS NOT NULL OR multiIf(resource.`deployment.environment` IS NOT NULL, resource.`deployment.environment`::String, mapContains(resources_string, 'deployment.environment'), resources_string['deployment.environment'], NULL) IS NOT NULL), COALESCE(NULLIF(multiIf(resource.`deployment.environment.name` IS NOT NULL, resource.`deployment.environment.name`::String, mapContains(resources_string, 'deployment.environment.name'), resources_string['deployment.environment.name'], NULL), ''), NULLIF(multiIf(resource.`deployment.environment` IS NOT NULL, resource.`deployment.environment`::String, mapContains(resources_string, 'deployment.environment'), resources_string['deployment.environment'], NULL), ''), ''), NULL)", expr)
})
}
}

View File

@@ -182,10 +182,6 @@ func (m *fieldMapper) getColumn(
case telemetrytypes.FieldContextResource:
return []*schema.Column{indexV3Columns["resource"], indexV3Columns["resources_string"]}, nil
case telemetrytypes.FieldContextScope:
// scope attributes with same name as declared paths create ambiguity and can't be queried directly.
if key.Name == "name" || key.Name == "version" {
return nil, qbtypes.ErrColumnNotFound
}
return []*schema.Column{indexV3Columns["scope"]}, nil
case telemetrytypes.FieldContextAttribute:
switch key.FieldDataType {
@@ -357,18 +353,17 @@ func (m *fieldMapper) resolveColumnExprs(
return exprs, existExprs, columns, nil
}
// logicalForResolvedColumn returns the logical field for a directly-resolvable key: its
// semantic-convention family when the metadata map proves membership, otherwise the
// single-member field for the key as given.
func (m *fieldMapper) logicalForResolvedColumn(ctx context.Context, orgID valuer.UUID, field *telemetrytypes.TelemetryFieldKey, keys map[string][]*telemetrytypes.TelemetryFieldKey) *telemetrytypes.LogicalField {
for _, logical := range querybuilder.MatchingLogicalFields(ctx, orgID, m.fl, field, keys) {
if logical.IsFamily() &&
logical.FieldContext == field.FieldContext &&
(field.FieldDataType == telemetrytypes.FieldDataTypeUnspecified || logical.FieldDataType == field.FieldDataType) {
return logical
}
// logicalForResolvedColumn returns the logical field(s) a directly-resolvable key names.
// Resolving to a column answers only that the key is addressable, not that it names one
// field: the scope JSON column resolves for both a declared path and a same-named scope
// attribute. Metadata is what tells those homes apart, so every match it reports is kept,
// the way the filter path keeps them. Only a name metadata does not know falls back to
// the key as given.
func (m *fieldMapper) logicalForResolvedColumn(ctx context.Context, orgID valuer.UUID, field *telemetrytypes.TelemetryFieldKey, keys map[string][]*telemetrytypes.TelemetryFieldKey) []*telemetrytypes.LogicalField {
if matches := querybuilder.MatchingLogicalFields(ctx, orgID, m.fl, field, keys); len(matches) > 0 {
return matches
}
return telemetrytypes.SingleLogicalField(field.Name, field)
return []*telemetrytypes.LogicalField{telemetrytypes.SingleLogicalField(field.Name, field)}
}
// upgradeToFamilies swaps single-member candidates for their family when the
@@ -435,7 +430,7 @@ func (m *fieldMapper) ColumnExpressionFor(
var candidates []*telemetrytypes.LogicalField
switch _, err := m.FieldFor(ctx, orgID, startNs, endNs, field); {
case err == nil:
candidates = []*telemetrytypes.LogicalField{m.logicalForResolvedColumn(ctx, orgID, field, keys)}
candidates = m.logicalForResolvedColumn(ctx, orgID, field, keys)
case errors.Is(err, qbtypes.ErrColumnNotFound):
raw := m.CandidateKeys(ctx, orgID, field, nil, keys)
if len(raw) == 0 {
@@ -590,28 +585,10 @@ func (m *fieldMapper) CandidateKeys(ctx context.Context, _ valuer.UUID, field *t
// Metadata match by name, then the literal `{context}.{name}` spelling (a context can be
// a legitimate prefix in user data, e.g. `metric.max_count`). For a forgiving context
// this is the correction step (span.http.method -> attribute http.method).
matches := keys[field.Name]
if field.FieldContext != telemetrytypes.FieldContextUnspecified {
// A bare-name match must agree on context; a same-named key under a different
// context is a different field. The literal `{context}.{name}` spelling is a real
// key in whatever context it was stored (e.g. an attribute named `span.test`), so
// it is taken regardless of context.
validMatches := make([]*telemetrytypes.TelemetryFieldKey, 0, len(matches))
for _, match := range matches {
if match.FieldContext == field.FieldContext {
validMatches = append(validMatches, match)
}
}
compoundName := fmt.Sprintf("%s.%s", field.FieldContext.StringValue(), field.Name)
compoundMatches := keys[compoundName]
validMatches = append(validMatches, compoundMatches...)
matches = append(matches, compoundMatches...)
if len(validMatches) > 0 {
return validMatches
}
if matches := keys[field.Name]; len(matches) > 0 {
return matches
}
if len(matches) > 0 {
if matches := keys[fmt.Sprintf("%s.%s", field.FieldContext.StringValue(), field.Name)]; len(matches) > 0 {
return matches
}

View File

@@ -405,6 +405,20 @@ func TestColumnExpressionForScopeDeclaredPath(t *testing.T) {
keys: withAttr,
expectedResult: "multiIf(scope.attributes.`name` IS NOT NULL, scope.attributes.`name`::String, NULL)",
},
{
// metadata knows both homes under this name, so the short spelling coalesces
// them instead of being rejected as ambiguous
name: "short name coalesces a known scope attribute with the declared path",
key: telemetrytypes.TelemetryFieldKey{Name: "name", FieldContext: telemetrytypes.FieldContextScope},
keys: withAttr,
expectedResult: "multiIf(scope.attributes.`name` IS NOT NULL, toString(scope.attributes.`name`::String), scope.name::String <> '', toString(scope.name::String), NULL)",
},
{
name: "short version coalesces a known scope attribute with the declared path",
key: telemetrytypes.TelemetryFieldKey{Name: "version", FieldContext: telemetrytypes.FieldContextScope},
keys: withAttr,
expectedResult: "multiIf(scope.attributes.`version` IS NOT NULL, toString(scope.attributes.`version`::String), scope.version::String <> '', toString(scope.version::String), NULL)",
},
}
for _, tc := range testCases {