Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .chloggen/tagvalues-empty-join-panic.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Changelog entry. Generate a copy with `make chlog-new`, then fill in the fields.

# One of: breaking | change | feature | enhancement | bug_fix | security
change_type: bug_fix

# Component or area of the change. Must be one of the components in .chloggen/config.yaml,
# e.g. distributor, querier, query-frontend, storage, operations.
component: storage

# A brief description of the change. Surround with quotes ("") if it must start with a backtick (`).
note: Fix querier panic on tag value autocomplete when every condition is a metadata-only intrinsic, e.g. `span:id` filtered by `{trace:id="..."}`

# (Optional) PR number(s), e.g. [7339]. Leave blank to auto-fill at release. See
# .chloggen/README.md.
issues: []

# (Optional) Additional lines rendered under the note. Use a pipe (|) for multiline text.
subtext: |
Metadata-only intrinsics such as `trace:id`, `trace:start`, `span:id` and `span:startTime` are
deliberately not fetched from columns. When a tag value autocomplete request consisted solely of
those, the trace-level join was built with zero sub-iterators and panicked on the first `Next()`,
restarting the querier. Affects vparquet3, vparquet4 and vparquet5.

# The GitHub handle (without the leading @) of the change's author. Rendered as "(@handle)".
user: zalegrala
14 changes: 14 additions & 0 deletions tempodb/encoding/vparquet3/block_autocomplete.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ func (b *backendBlock) FetchTagNames(ctx context.Context, req traceql.FetchTagsR
if err != nil {
return fmt.Errorf("creating fetch iter: %w", err)
}
if iter == nil {
continue // nothing to read for this condition group
}

done, iterErr := func() (bool, error) {
defer iter.Close()
Expand Down Expand Up @@ -232,6 +235,9 @@ func (b *backendBlock) FetchTagValues(ctx context.Context, req traceql.FetchTagV
if err != nil {
return fmt.Errorf("creating fetch iter: %w", err)
}
if iter == nil {
continue // nothing to read for this condition group
}

done, iterErr := func() (bool, error) {
defer iter.Close()
Expand Down Expand Up @@ -869,6 +875,14 @@ func createDistinctTraceIterator(
traceIters = append(traceIters, resourceIter)
}

// Every condition may have been metadata-only (trace:id, trace:start) and the
// lower scopes may have collapsed to nothing, leaving no iterators at all. A
// join over zero iterators has nothing to read, so report that rather than
// building a degenerate one.
if len(traceIters) == 0 {
return nil, nil
}

// Final trace iterator
// Join iterator means it requires matching resources to have been found
// TraceCollor adds trace-level data to the spansets
Expand Down
53 changes: 53 additions & 0 deletions tempodb/encoding/vparquet3/block_autocomplete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,21 @@ func TestFetchTagValues(t *testing.T) {
tag, query string
expectedValues []tempopb.TagValue
}{
{
// Both span:id and trace:id are metadata-only intrinsics: neither
// contributes a column iterator, so the trace-level join ends up with
// zero sub-iterators. It must return no values instead of panicking.
name: "metadata-only intrinsic tag with metadata-only intrinsic condition",
tag: "span:id",
query: `{trace:id="000000000000000000000000000000ff"}`,
expectedValues: []tempopb.TagValue{},
},
{
name: "metadata-only intrinsic tag with span-scoped metadata-only condition",
tag: "span:id",
query: `{span:id="0000000000000001"}`,
expectedValues: []tempopb.TagValue{},
},
{
name: "intrinsic with no query - match",
tag: "name",
Expand Down Expand Up @@ -815,6 +830,44 @@ func TestFetchTagNamesWithOrConditions(t *testing.T) {
}
}

// A WAL block runs the same autocomplete iterator as a backend block, so it hits
// the same empty-join and nil-iterator cases when every condition is a
// metadata-only intrinsic.
func TestWalBlockFetchTagValuesMetadataOnlyIntrinsics(t *testing.T) {
queries := []string{
`{trace:id="000000000000000000000000000000ff"}`,
`{span:id="0000000000000001"}`,
}

testWalBlock(t, func(w *walBlock, _ []common.ID, _ []*tempopb.Trace) {
for _, query := range queries {
t.Run(query, func(t *testing.T) {
req, err := traceql.ExtractFetchSpansRequest(query)
require.NoError(t, err)

tag, err := traceql.ParseIdentifier("span:id")
require.NoError(t, err)

var (
mc = collector.NewMetricsCollector()
distinctValues = collector.NewDistinctValue(1_000_000, 0, 0, func(v tempopb.TagValue) int { return len(v.Type) + len(v.Value) })
autocompletedReq = traceql.FetchTagValuesRequest{
TagName: tag,
ConditionGroups: [][]traceql.Condition{append(
req.Conditions,
traceql.Condition{Attribute: tag, Op: traceql.OpNone},
)},
}
)

err = w.FetchTagValues(t.Context(), autocompletedReq, traceql.MakeCollectTagValueFunc(distinctValues.Collect), mc.Add, common.DefaultSearchOptions())
require.NoError(t, err)
require.Empty(t, distinctValues.Values())
})
}
})
}

func TestFetchTagValuesWithOrConditions(t *testing.T) {
testCases := []struct {
name string
Expand Down
6 changes: 6 additions & 0 deletions tempodb/encoding/vparquet3/wal_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,9 @@ func (b *walBlock) FetchTagValues(ctx context.Context, req traceql.FetchTagValue
if err != nil {
return false, fmt.Errorf("creating fetch iter: %w", err)
}
if iter == nil {
continue // nothing to read for this condition group
}

for {
// Exhaust the iterator
Expand Down Expand Up @@ -848,6 +851,9 @@ func (b *walBlock) FetchTagNames(ctx context.Context, req traceql.FetchTagsReque
if err != nil {
return false, fmt.Errorf("creating fetch iter: %w", err)
}
if iter == nil {
continue // nothing to read for this condition group
}

for {
// Exhaust the iterator
Expand Down
14 changes: 14 additions & 0 deletions tempodb/encoding/vparquet4/block_autocomplete.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ func (b *backendBlock) FetchTagNames(ctx context.Context, req traceql.FetchTagsR
if err != nil {
return fmt.Errorf("creating fetch iter: %w", err)
}
if iter == nil {
continue // nothing to read for this condition group
}

done, iterErr := func() (bool, error) {
defer iter.Close()
Expand Down Expand Up @@ -262,6 +265,9 @@ func (b *backendBlock) FetchTagValues(ctx context.Context, req traceql.FetchTagV
if err != nil {
return fmt.Errorf("creating fetch iter: %w", err)
}
if iter == nil {
continue // nothing to read for this condition group
}

done, iterErr := func() (bool, error) {
defer iter.Close()
Expand Down Expand Up @@ -1125,6 +1131,14 @@ func createDistinctTraceIterator(
traceIters = append(traceIters, resourceIter)
}

// Every condition may have been metadata-only (trace:id, trace:start) and the
// lower scopes may have collapsed to nothing, leaving no iterators at all. A
// join over zero iterators has nothing to read, so report that rather than
// building a degenerate one.
if len(traceIters) == 0 {
return nil, nil
}

// Final trace iterator
// Join iterator means it requires matching resources to have been found
// TraceCollor adds trace-level data to the spansets
Expand Down
53 changes: 53 additions & 0 deletions tempodb/encoding/vparquet4/block_autocomplete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,21 @@ func TestFetchTagValues(t *testing.T) {
tag, query string
expectedValues []tempopb.TagValue
}{
{
// Both span:id and trace:id are metadata-only intrinsics: neither
// contributes a column iterator, so the trace-level join ends up with
// zero sub-iterators. It must return no values instead of panicking.
name: "metadata-only intrinsic tag with metadata-only intrinsic condition",
tag: "span:id",
query: `{trace:id="000000000000000000000000000000ff"}`,
expectedValues: []tempopb.TagValue{},
},
{
name: "metadata-only intrinsic tag with span-scoped metadata-only condition",
tag: "span:id",
query: `{span:id="0000000000000001"}`,
expectedValues: []tempopb.TagValue{},
},
{
name: "intrinsic with no query - match",
tag: "name",
Expand Down Expand Up @@ -1074,6 +1089,44 @@ func TestFetchTagNamesWithOrConditions(t *testing.T) {
}
}

// A WAL block runs the same autocomplete iterator as a backend block, so it hits
// the same empty-join and nil-iterator cases when every condition is a
// metadata-only intrinsic.
func TestWalBlockFetchTagValuesMetadataOnlyIntrinsics(t *testing.T) {
queries := []string{
`{trace:id="000000000000000000000000000000ff"}`,
`{span:id="0000000000000001"}`,
}

testWalBlock(t, func(w *walBlock, _ []common.ID, _ []*tempopb.Trace) {
for _, query := range queries {
t.Run(query, func(t *testing.T) {
req, err := traceql.ExtractFetchSpansRequest(query)
require.NoError(t, err)

tag, err := traceql.ParseIdentifier("span:id")
require.NoError(t, err)

var (
mc = collector.NewMetricsCollector()
distinctValues = collector.NewDistinctValue(1_000_000, 0, 0, func(v tempopb.TagValue) int { return len(v.Type) + len(v.Value) })
autocompletedReq = traceql.FetchTagValuesRequest{
TagName: tag,
ConditionGroups: [][]traceql.Condition{append(
req.Conditions,
traceql.Condition{Attribute: tag, Op: traceql.OpNone},
)},
}
)

err = w.FetchTagValues(t.Context(), autocompletedReq, traceql.MakeCollectTagValueFunc(distinctValues.Collect), mc.Add, common.DefaultSearchOptions())
require.NoError(t, err)
require.Empty(t, distinctValues.Values())
})
}
})
}

func TestFetchTagValuesWithOrConditions(t *testing.T) {
testCases := []struct {
name string
Expand Down
6 changes: 6 additions & 0 deletions tempodb/encoding/vparquet4/wal_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,9 @@ func (b *walBlock) FetchTagValues(ctx context.Context, req traceql.FetchTagValue
if err != nil {
return false, fmt.Errorf("creating fetch iter: %w", err)
}
if iter == nil {
continue // nothing to read for this condition group
}

for {
// Exhaust the iterator
Expand Down Expand Up @@ -912,6 +915,9 @@ func (b *walBlock) FetchTagNames(ctx context.Context, req traceql.FetchTagsReque
if err != nil {
return false, fmt.Errorf("creating fetch iter: %w", err)
}
if iter == nil {
continue // nothing to read for this condition group
}

for {
// Exhaust the iterator
Expand Down
14 changes: 14 additions & 0 deletions tempodb/encoding/vparquet5/block_autocomplete.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ func (b *backendBlock) FetchTagNames(ctx context.Context, req traceql.FetchTagsR
if err != nil {
return fmt.Errorf("creating fetch iter: %w", err)
}
if iter == nil {
continue // nothing to read for this condition group
}

done, iterErr := func() (bool, error) {
defer iter.Close()
Expand Down Expand Up @@ -274,6 +277,9 @@ func (b *backendBlock) FetchTagValues(ctx context.Context, req traceql.FetchTagV
if err != nil {
return fmt.Errorf("creating fetch iter: %w", err)
}
if iter == nil {
continue // nothing to read for this condition group
}

done, iterErr := func() (bool, error) {
defer iter.Close()
Expand Down Expand Up @@ -1170,6 +1176,14 @@ func createDistinctTraceIterator(
traceIters = append(traceIters, resourceIter)
}

// Every condition may have been metadata-only (trace:id, trace:start) and the
// lower scopes may have collapsed to nothing, leaving no iterators at all. A
// join over zero iterators has nothing to read, so report that rather than
// building a degenerate one.
if len(traceIters) == 0 {
return nil, nil
}

// Final trace iterator
// Join iterator means it requires matching resources to have been found
// TraceCollor adds trace-level data to the spansets
Expand Down
53 changes: 53 additions & 0 deletions tempodb/encoding/vparquet5/block_autocomplete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,21 @@ func TestFetchTagValues(t *testing.T) {
tr *Trace
dc backend.DedicatedColumns
}{
{
// Both span:id and trace:id are metadata-only intrinsics: neither
// contributes a column iterator, so the trace-level join ends up with
// zero sub-iterators. It must return no values instead of panicking.
name: "metadata-only intrinsic tag with metadata-only intrinsic condition",
tag: "span:id",
query: `{trace:id="000000000000000000000000000000ff"}`,
expectedValues: []tempopb.TagValue{},
},
{
name: "metadata-only intrinsic tag with span-scoped metadata-only condition",
tag: "span:id",
query: `{span:id="0000000000000001"}`,
expectedValues: []tempopb.TagValue{},
},
{
name: "intrinsic with no query - match",
tag: "name",
Expand Down Expand Up @@ -1079,6 +1094,44 @@ func TestFetchTagNamesWithOrConditions(t *testing.T) {
}
}

// A WAL block runs the same autocomplete iterator as a backend block, so it hits
// the same empty-join and nil-iterator cases when every condition is a
// metadata-only intrinsic.
func TestWalBlockFetchTagValuesMetadataOnlyIntrinsics(t *testing.T) {
queries := []string{
`{trace:id="000000000000000000000000000000ff"}`,
`{span:id="0000000000000001"}`,
}

testWalBlock(t, func(w *walBlock, _ []common.ID, _ []*tempopb.Trace) {
for _, query := range queries {
t.Run(query, func(t *testing.T) {
req, err := traceql.ExtractFetchSpansRequest(query)
require.NoError(t, err)

tag, err := traceql.ParseIdentifier("span:id")
require.NoError(t, err)

var (
mc = collector.NewMetricsCollector()
distinctValues = collector.NewDistinctValue(1_000_000, 0, 0, func(v tempopb.TagValue) int { return len(v.Type) + len(v.Value) })
autocompletedReq = traceql.FetchTagValuesRequest{
TagName: tag,
ConditionGroups: [][]traceql.Condition{append(
req.Conditions,
traceql.Condition{Attribute: tag, Op: traceql.OpNone},
)},
}
)

err = w.FetchTagValues(t.Context(), autocompletedReq, traceql.MakeCollectTagValueFunc(distinctValues.Collect), mc.Add, common.DefaultSearchOptions())
require.NoError(t, err)
require.Empty(t, distinctValues.Values())
})
}
})
}

func TestFetchTagValuesWithOrConditions(t *testing.T) {
testCases := []struct {
name string
Expand Down
6 changes: 6 additions & 0 deletions tempodb/encoding/vparquet5/wal_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,9 @@ func (b *walBlock) FetchTagValues(ctx context.Context, req traceql.FetchTagValue
if err != nil {
return false, fmt.Errorf("creating fetch iter: %w", err)
}
if iter == nil {
continue // nothing to read for this condition group
}

for {
// Exhaust the iterator
Expand Down Expand Up @@ -968,6 +971,9 @@ func (b *walBlock) FetchTagNames(ctx context.Context, req traceql.FetchTagsReque
if err != nil {
return false, fmt.Errorf("creating fetch iter: %w", err)
}
if iter == nil {
continue // nothing to read for this condition group
}

for {
// Exhaust the iterator
Expand Down