diff --git a/CHANGELOG.md b/CHANGELOG.md index 188784bab44..8bc2a932a95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * [ENHANCEMENT] Add string interning to TraceQL queries [#3411](https://github.com/grafana/tempo/pull/3411) (@mapno) * [ENHANCEMENT] Add new (unsafe) query hints for metrics queries [#3396](https://github.com/grafana/tempo/pull/3396) (@mdisibio) * [ENHANCEMENT] Add nestedSetLeft/Right/Parent instrinsics to TraceQL. [#3497](https://github.com/grafana/tempo/pull/3497) (@joe-elliott) +* [ENHANCEMENT] Add tenant to frontend job cache key. [#3527](https://github.com/grafana/tempo/pull/3527) (@joe-elliott) * [BUGFIX] Fix metrics query results when filtering and rating on the same attribute [#3428](https://github.com/grafana/tempo/issues/3428) (@mdisibio) * [BUGFIX] Fix metrics query results when series contain empty strings or nil values [#3429](https://github.com/grafana/tempo/issues/3429) (@mdisibio) * [BUGFIX] Fix metrics query duration check, add per-tenant override for max metrics query duration [#3479](https://github.com/grafana/tempo/issues/3479) (@mdisibio) diff --git a/modules/frontend/cache_keys.go b/modules/frontend/cache_keys.go index b5665650d97..e0fd5dd4bc1 100644 --- a/modules/frontend/cache_keys.go +++ b/modules/frontend/cache_keys.go @@ -13,13 +13,13 @@ const ( cacheKeyPrefixSearchTagValues = "stv:" ) -func searchJobCacheKey(queryHash uint64, start int64, end int64, meta *backend.BlockMeta, startPage, pagesToSearch int) string { - return cacheKey(cacheKeyPrefixSearchJob, queryHash, start, end, meta, startPage, pagesToSearch) +func searchJobCacheKey(tenant string, queryHash uint64, start int64, end int64, meta *backend.BlockMeta, startPage, pagesToSearch int) string { + return cacheKey(cacheKeyPrefixSearchJob, tenant, queryHash, start, end, meta, startPage, pagesToSearch) } // cacheKey returns a string that can be used as a cache key for a backend search job. if a valid key cannot be calculated // it returns an empty string. -func cacheKey(prefix string, queryHash uint64, start int64, end int64, meta *backend.BlockMeta, startPage, pagesToSearch int) string { +func cacheKey(prefix string, tenant string, queryHash uint64, start int64, end int64, meta *backend.BlockMeta, startPage, pagesToSearch int) string { // if the query hash is 0 we can't cache. this may occur if the user is using the old search api if queryHash == 0 { return "" @@ -33,8 +33,19 @@ func cacheKey(prefix string, queryHash uint64, start int64, end int64, meta *bac } sb := strings.Builder{} - sb.Grow(3 + 20 + 1 + 36 + 1 + 3 + 1 + 2) // 3 for prefix, 20 for query hash, 1 for :, 36 for block id, 1 for :, 3 for start page, 1 for :, 2 for pages to search - sb.WriteString(prefix) // sj for search job. prefix prevents unexpected collisions and an easy way to version for future iterations + sb.Grow(len(prefix) + + len(tenant) + + 1 + // : + 20 + // query hash + 1 + // : + 36 + // block id + 1 + // : + 3 + // start page + 1 + // : + 2) // 2 for pages to search + sb.WriteString(prefix) + sb.WriteString(tenant) + sb.WriteString(":") sb.WriteString(strconv.FormatUint(queryHash, 10)) sb.WriteString(":") sb.WriteString(meta.BlockID.String()) diff --git a/modules/frontend/cache_keys_test.go b/modules/frontend/cache_keys_test.go index ef822ba803d..44845898fcc 100644 --- a/modules/frontend/cache_keys_test.go +++ b/modules/frontend/cache_keys_test.go @@ -13,6 +13,7 @@ import ( func TestCacheKeyForJob(t *testing.T) { tcs := []struct { name string + tenant string queryHash uint64 req *tempopb.SearchRequest meta *backend.BlockMeta @@ -23,6 +24,7 @@ func TestCacheKeyForJob(t *testing.T) { }{ { name: "valid!", + tenant: "foo", queryHash: 42, req: &tempopb.SearchRequest{ Start: 10, @@ -35,7 +37,7 @@ func TestCacheKeyForJob(t *testing.T) { }, searchPage: 1, pagesToSearch: 2, - expected: "sj:42:00000000-0000-0000-0000-000000000123:1:2", + expected: "sj:foo:42:00000000-0000-0000-0000-000000000123:1:2", }, { name: "no query hash means no query cache", @@ -137,7 +139,7 @@ func TestCacheKeyForJob(t *testing.T) { for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - actual := searchJobCacheKey(tc.queryHash, int64(tc.req.Start), int64(tc.req.End), tc.meta, tc.searchPage, tc.pagesToSearch) + actual := searchJobCacheKey(tc.tenant, tc.queryHash, int64(tc.req.Start), int64(tc.req.End), tc.meta, tc.searchPage, tc.pagesToSearch) require.Equal(t, tc.expected, actual) }) } @@ -155,7 +157,7 @@ func BenchmarkCacheKeyForJob(b *testing.B) { } for i := 0; i < b.N; i++ { - s := searchJobCacheKey(10, int64(req.Start), int64(req.End), meta, 1, 2) + s := searchJobCacheKey("foo", 10, int64(req.Start), int64(req.End), meta, 1, 2) if len(s) == 0 { b.Fatalf("expected non-empty string") } diff --git a/modules/frontend/search_handlers_test.go b/modules/frontend/search_handlers_test.go index 34b5c8fd5c1..6e13f8f80f1 100644 --- a/modules/frontend/search_handlers_test.go +++ b/modules/frontend/search_handlers_test.go @@ -526,6 +526,7 @@ func TestSearchFailurePropagatesFromQueriers(t *testing.T) { } func TestSearchAccessesCache(t *testing.T) { + tenant := "foo" meta := &backend.BlockMeta{ StartTime: time.Unix(15, 0), EndTime: time.Unix(16, 0), @@ -550,7 +551,7 @@ func TestSearchAccessesCache(t *testing.T) { hash := hashForTraceQLQuery(query) start := uint32(10) end := uint32(20) - cacheKey := searchJobCacheKey(hash, int64(start), int64(end), meta, 0, 1) + cacheKey := searchJobCacheKey(tenant, hash, int64(start), int64(end), meta, 0, 1) // confirm cache key coesn't exist _, bufs, _ := c.Fetch(context.Background(), []string{cacheKey}) @@ -560,7 +561,7 @@ func TestSearchAccessesCache(t *testing.T) { path := fmt.Sprintf("/?start=%d&end=%d&q=%s", start, end, query) // encapsulates block above req := httptest.NewRequest("GET", path, nil) ctx := req.Context() - ctx = user.InjectOrgID(ctx, "blerg") + ctx = user.InjectOrgID(ctx, tenant) req = req.WithContext(ctx) respWriter := httptest.NewRecorder() diff --git a/modules/frontend/search_sharder.go b/modules/frontend/search_sharder.go index 3ee9d97feb7..f025250e10b 100644 --- a/modules/frontend/search_sharder.go +++ b/modules/frontend/search_sharder.go @@ -306,7 +306,7 @@ func buildBackendRequests(ctx context.Context, tenantID string, parent *http.Req } subR.RequestURI = buildUpstreamRequestURI(parent.URL.Path, subR.URL.Query()) - key := searchJobCacheKey(queryHash, int64(searchReq.Start), int64(searchReq.End), m, startPage, pages) + key := searchJobCacheKey(tenantID, queryHash, int64(searchReq.Start), int64(searchReq.End), m, startPage, pages) if len(key) > 0 { subR = pipeline.AddCacheKey(key, subR) } diff --git a/modules/frontend/tag_handlers_test.go b/modules/frontend/tag_handlers_test.go index 200aef3e15a..9b7e219f326 100644 --- a/modules/frontend/tag_handlers_test.go +++ b/modules/frontend/tag_handlers_test.go @@ -428,11 +428,12 @@ func TestSearchTagsV2AccessesCache(t *testing.T) { }, rdr, nil, p) // setup query + tenant := "foo" scope := "resource" hash := fnv1a.HashString64(scope) start := uint32(10) end := uint32(20) - cacheKey := cacheKey(cacheKeyPrefixSearchTag, hash, int64(start), int64(end), meta, 0, 1) + cacheKey := cacheKey(cacheKeyPrefixSearchTag, tenant, hash, int64(start), int64(end), meta, 0, 1) // confirm cache key coesn't exist _, bufs, _ := c.Fetch(context.Background(), []string{cacheKey}) @@ -442,7 +443,7 @@ func TestSearchTagsV2AccessesCache(t *testing.T) { path := fmt.Sprintf("/?start=%d&end=%d&scope=%s", start, end, scope) // encapsulates block above req := httptest.NewRequest("GET", path, nil) ctx := req.Context() - ctx = user.InjectOrgID(ctx, "blerg") + ctx = user.InjectOrgID(ctx, tenant) req = req.WithContext(ctx) respWriter := httptest.NewRecorder() diff --git a/modules/frontend/tag_sharder.go b/modules/frontend/tag_sharder.go index 7733c8fdeb4..f4438c834fd 100644 --- a/modules/frontend/tag_sharder.go +++ b/modules/frontend/tag_sharder.go @@ -301,7 +301,7 @@ func (s searchTagSharder) buildBackendRequests(ctx context.Context, tenantID str } subR.RequestURI = buildUpstreamRequestURI(parent.URL.Path, subR.URL.Query()) - key := cacheKey(keyPrefix, hash, int64(searchReq.start()), int64(searchReq.end()), m, startPage, pages) + key := cacheKey(keyPrefix, tenantID, hash, int64(searchReq.start()), int64(searchReq.end()), m, startPage, pages) if len(key) > 0 { subR = pipeline.AddCacheKey(key, subR) }