diff --git a/CHANGELOG.md b/CHANGELOG.md index 0520c84e530..a2d95f7a7bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,5 @@ ## main / unreleased - +* [CHANGE] No longer return `status.code` from /api/search/tags unless it is an attribute present in the data [#2059](https://github.com/grafana/tempo/issues/2059) (@mdisibio) * [BUGFIX] Suppress logspam in single binary mode when metrics generator is disabled. [#2058](https://github.com/grafana/tempo/pull/2058) (@joe-elliott) * [BUGFIX] Error more gracefully while reading some blocks written by an interim commit between 1.5 and 2.0 [#2055](https://github.com/grafana/tempo/pull/2055) (@mdisibio) * [BUGFIX] Correctly coalesce trace level data when combining Parquet traces. [#2095](https://github.com/grafana/tempo/pull/2095) (@joe-elliott) diff --git a/Makefile b/Makefile index 79f54471fec..a48ae97efc5 100644 --- a/Makefile +++ b/Makefile @@ -35,7 +35,7 @@ ifeq ($(BUILD_DEBUG), 1) GO_OPT+= -gcflags="all=-N -l" endif -GOTEST_OPT?= -race -timeout 20m -count=1 +GOTEST_OPT?= -race -timeout 30m -count=1 GOTEST_OPT_WITH_COVERAGE = $(GOTEST_OPT) -cover GOTEST=go test LINT=golangci-lint diff --git a/integration/e2e/e2e_test.go b/integration/e2e/e2e_test.go index f893bed6f2a..127e5dde58c 100644 --- a/integration/e2e/e2e_test.go +++ b/integration/e2e/e2e_test.go @@ -119,7 +119,7 @@ func TestAllInOne(t *testing.T) { // test metrics require.NoError(t, tempo.WaitSumMetrics(e2e.Equals(1), "tempo_ingester_blocks_flushed_total")) require.NoError(t, tempo.WaitSumMetricsWithOptions(e2e.Equals(1), []string{"tempodb_blocklist_length"}, e2e.WaitMissingMetrics)) - require.NoError(t, tempo.WaitSumMetrics(e2e.Equals(5), "tempo_query_frontend_queries_total")) + require.NoError(t, tempo.WaitSumMetrics(e2e.Equals(4), "tempo_query_frontend_queries_total")) // query trace - should fetch from backend queryAndAssertTrace(t, apiClient, info) @@ -272,7 +272,7 @@ func TestMicroservicesWithKVStores(t *testing.T) { require.NoError(t, i.WaitSumMetrics(e2e.Equals(1), "tempo_ingester_blocks_flushed_total")) } require.NoError(t, tempoQuerier.WaitSumMetrics(e2e.Equals(3), "tempodb_blocklist_length")) - require.NoError(t, tempoQueryFrontend.WaitSumMetrics(e2e.Equals(4), "tempo_query_frontend_queries_total")) + require.NoError(t, tempoQueryFrontend.WaitSumMetrics(e2e.Equals(3), "tempo_query_frontend_queries_total")) // query trace - should fetch from backend queryAndAssertTrace(t, apiClient, info) diff --git a/integration/util.go b/integration/util.go index bc42bbe6bc0..1e3523fc866 100644 --- a/integration/util.go +++ b/integration/util.go @@ -223,10 +223,8 @@ func SearchAndAssertTrace(t *testing.T, client *tempoUtil.Client, info *tempoUti attr := tempoUtil.RandomAttrFromTrace(expected) - // verify attribute is present in tags - tagsResp, err := client.SearchTags() - require.NoError(t, err) - require.Contains(t, tagsResp.TagNames, attr.Key) + // NOTE: SearchTags doesn't include live traces anymore + // so don't check SearchTags // verify attribute value is present in tag values tagValuesResp, err := client.SearchTagValues(attr.Key) diff --git a/modules/ingester/instance_search.go b/modules/ingester/instance_search.go index 562df1bc171..5eb76b859df 100644 --- a/modules/ingester/instance_search.go +++ b/modules/ingester/instance_search.go @@ -336,19 +336,6 @@ func (i *instance) SearchTags(ctx context.Context) (*tempopb.SearchTagsResponse, limit := i.limiter.limits.MaxBytesPerTagValuesQuery(userID) distinctValues := util.NewDistinctStringCollector(limit) - // live traces - kv := &tempofb.KeyValues{} - err = i.visitSearchEntriesLiveTraces(ctx, func(entry *tempofb.SearchEntry) { - for i, ii := 0, entry.TagsLength(); i < ii; i++ { - entry.Tags(kv, i) - key := string(kv.Key()) - distinctValues.Collect(key) - } - }) - if err != nil { - return nil, err - } - // wal + search blocks if !distinctValues.Exceeded() { err = i.visitSearchableBlocks(ctx, func(block search.SearchableBlock) error { diff --git a/modules/querier/querier.go b/modules/querier/querier.go index e89a3fb5f15..292d3e7409c 100644 --- a/modules/querier/querier.go +++ b/modules/querier/querier.go @@ -353,11 +353,6 @@ func (q *Querier) SearchTags(ctx context.Context, req *tempopb.SearchTagsRequest limit := q.limits.MaxBytesPerTagValuesQuery(userID) distinctValues := util.NewDistinctStringCollector(limit) - // Virtual tags. Get these first - for _, k := range search.GetVirtualTags() { - distinctValues.Collect(k) - } - // Get results from all ingesters replicationSet, err := q.ring.GetReplicationSetForOperation(ring.Read) if err != nil { diff --git a/tempodb/encoding/vparquet/block_search_tags.go b/tempodb/encoding/vparquet/block_search_tags.go index 29316907d59..8c1d8de6c29 100644 --- a/tempodb/encoding/vparquet/block_search_tags.go +++ b/tempodb/encoding/vparquet/block_search_tags.go @@ -71,6 +71,11 @@ func searchTags(_ context.Context, cb common.TagCallback, pf *parquet.File) erro // find indexes of all special columns specialAttrIdxs := map[int]string{} for lbl, col := range labelMappings { + if lbl == LabelStatusCode { + // Don't include this in the list of tags (but it can still be used in SearchTagValues) + continue + } + idx, _ := pq.GetColumnIndexByPath(pf, col) if idx == -1 { continue diff --git a/tempodb/encoding/vparquet/block_search_tags_test.go b/tempodb/encoding/vparquet/block_search_tags_test.go index f53b178f4b5..643621a5955 100644 --- a/tempodb/encoding/vparquet/block_search_tags_test.go +++ b/tempodb/encoding/vparquet/block_search_tags_test.go @@ -31,6 +31,9 @@ func TestBackendBlockSearchTags(t *testing.T) { // test that all attrs are in found attrs for k := range attrs { + if k == LabelStatusCode { + continue + } _, ok := foundAttrs[k] require.True(t, ok) } diff --git a/tempodb/search/util.go b/tempodb/search/util.go index e3a4c6b81cf..e361c0fd6f7 100644 --- a/tempodb/search/util.go +++ b/tempodb/search/util.go @@ -8,10 +8,6 @@ import ( "github.com/grafana/tempo/pkg/util" ) -func GetVirtualTags() []string { - return []string{trace.ErrorTag} -} - func GetVirtualTagValues(tagName string) []string { switch tagName {