9 Commits (ca19e8d7941bfac86355b2853ef2ea110f32075e)

Author	SHA1	Message	Date
Charles Korn	bde65667f7	Upgrade dskit to use packages migrated from weaveworks/common (#10164) ... **What this PR does / why we need it**: This PR upgrades dskit and replaces use of packages from weaveworks/common with their migrated equivalents in dskit. See https://github.com/grafana/dskit/pull/342 for more details. Note that Loki still uses some packages from weaveworks/common that I haven't migrated (`aws` and `test`) - I'll migrate these separately. If this PR needs to be rebuilt, I used `rewrite.sh` ([source](https://gist.github.com/charleskorn/48efe62a09d6d70f3de30327003df5c5#file-rewrite-sh)) to generate most of these changes. **Which issue(s) this PR fixes**: (none) **Special notes for your reviewer**: (none) **Checklist** - [x] Reviewed the [`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md) guide (**required**) - [n/a] Documentation added - [n/a] Tests updated - [n/a] `CHANGELOG.md` updated - [n/a] If the change is worth mentioning in the release notes, add `add-to-release-notes` label - [n/a] Changes that require user attention or interaction to upgrade are documented in `docs/sources/setup/upgrade/_index.md` - [n/a] For Helm chart changes bump the Helm chart version in `production/helm/loki/Chart.yaml` and update `production/helm/loki/CHANGELOG.md` and `production/helm/loki/README.md`. [Example PR](d10549e3ec) Co-authored-by: Kaviraj Kanagaraj <kavirajkanagaraj@gmail.com>	2 years ago
Sandeep Sukhani	3e1f2fc273	caching: do not try to fill the gap in log results cache when the new query interval does not overlap the cached query interval (#9757) ... **What this PR does / why we need it**: Currently, when we find a relevant cached negative response for a logs query, we do the following: * If the cached query completely covers the new query: * return back an empty response. * else: * fill the gaps on either/both sides of the cached query. The problem with filling the gaps is that when the cached query does not overlap at all with the new query, we have to extend the query beyond what the query requests for. However, with the logs query, we have a limit on the number of lines we can send back in the response. So, this could result in the query response having logs which were not requested by the query, which then get filtered out by the [response extractor](b78d3f0552/pkg/querier/queryrange/log_result_cache.go (L299)), unexpectedly resulting in an empty response. For example, if the query was cached for start=15, end=20 and we get a `backwards` query for start=5, end=10. To fill the gap, the query would be executed for start=5, end=15. Now, if we have logs more than the query `limit` in the range 10-15, we would filter out all the data in the response extractor and send back an empty response to the user. This PR fixes the issue by doing the following changes when handling cache hit: * If the cached query completely covers the new query: * return back an empty response[_existing_]. * else if the cached query does not overlap with the new query: * do the new query as requested. * If the new query results in an empty response and has a higher interval than the cached query: * update the cache * else: * query the data for missing intervals on both/either side[_existing_] * update the cache with extended intervals if the new queries resulted in an empty response[_existing_] **Special notes for your reviewer**: We could do further improvements in the handling of queries not overlapping with cached query by selectively extending the queries based on query direction and cached query lying before/after the new query. For example, if the new query is doing `backwards` query and the `cachedQuery.End` < `newQuery.Start`, it should be okay to extend the query and do `cachedQuery.End` to `newQuery.End` to fill the cache since query would first fill the most relevant data before hitting the limits. I did not want to complicate the fix so went without implementing this approach. We can revisit later if we feel we need to improve our caching. **Checklist** - [x] Tests updated - [x] `CHANGELOG.md` updated --------- Co-authored-by: Travis Patterson <travis.patterson@grafana.com>	3 years ago
Salva Corts	c6fbff26e1	Add config to avoid caching stats for recent data (#9537) ... **What this PR does / why we need it**: When we query the stats for recent data, we query both the ingesters and the index gateways for the stats. ebdb2b1800/pkg/storage/async_store.go (L112-L114) ebdb2b1800/pkg/storage/async_store.go (L126-L127) Then we merge all the responses, which means summing up all the stats ebdb2b1800/pkg/storage/async_store.go (L157-L158) ebdb2b1800/pkg/storage/stores/index/stats/stats.go (L23-L26) Because we have a replication factor of 3, this means that we will get the stats from the ingesters repeated up to 3 times, hence inflating the stats. In the stats cache, we store the stats for a given matcher set for the whole day, then we extract the stats from the cache by the factor of time from the request that is stored in the cache: 336283acad/pkg/querier/queryrange/index_stats_cache.go (L33) 336283acad/pkg/querier/queryrange/index_stats_cache.go (L40) Inflated stats for recent data will be cached, so subsequent stats extracted from the cache will be inflated regardless of the time. This PR adds a new per-tenant limit `max_stats_cache_freshness` to not cache requests with an end time that falls within Now minus this duration. Here's a scenario illustrating this. The graphs below show the bytes stats queried in the sharding middleware. We are running a log filter query that won't match any log, every 5 seconds with a length of 3h.  As can be seen, after enabling the stats cache and configuring`do_not_cache_request_within` to not cache stats for requests within 30m, the bytes stats used in the sharding middleware stopped increasing. In both cases the stats cache hit ration was 100%.  **Special notes for your reviewer**: - Blocked by https://github.com/grafana/loki/pull/9535 - Note that this PR doesn't fix the root issue of inflated stats form the ingesters, but rather buys us some time to work on that. **Checklist** - [x] Reviewed the [`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md) guide (**required**) - [x] Documentation added - [x] Tests updated - [ ] `CHANGELOG.md` updated - [ ] Changes that require user attention or interaction to upgrade are documented in `docs/sources/upgrading/_index.md` - [ ] For Helm chart changes bump the Helm chart version in `production/helm/loki/Chart.yaml` and update `production/helm/loki/CHANGELOG.md` and `production/helm/loki/README.md`. [Example PR](d10549e3ec)	3 years ago
Dylan Guedes	9159c1dac3	Loki: Improve spans usage (#8927) ... **What this PR does / why we need it**: - At different places, inherit the span/spanlogger from the given context instead of instantiating a new one from scratch, which fix spans being orphaned on a read/write operation. - At different places, turn spans into events. Events are lighter than spans and by having fewer spans in the trace, trace visualization will be cleaner without losing any details. - Adds new spans/events to places that might be a bottleneck for our writes/reads.	3 years ago
Callum Styan	9a2a038f43	Allow passing of context to query related limits functions (#8689) ... In this PR we're allowing for passing of a `context.Context` via the Limits interfaces (some of which are new, to clean up hardcoding/embedding of `validation.Overrides`) This is based on work/ideas by @jeschkies . Fixes #8694 --------- Signed-off-by: Callum Styan <callumstyan@gmail.com> Co-authored-by: Karsten Jeschkies <karsten.jeschkies@grafana.com>	3 years ago
Sandeep Sukhani	24deb6ed3b	fix bugs in logs results caching and its tests (#7925) ... **What this PR does / why we need it**: When a logs query results in an empty response, we cache it to avoid doing that query again and respond straight away with an empty response. However, we cache a single entry per time split interval with the query itself to keep things simple. For example, if the time split config for tenant `A` is `30m`, then queries for intervals `10m`-`20m` and `21m`-`25m` would have the same cache key. Here is roughly how cache hit is handled: * If the new query is within the cached query bounds, return empty results * If the start of new query is before the start time of the cached query, do a query from `newQuery.Start` to `cachedQuery.Start` * If the response of last query is also empty, set `cachedQuery.Start` = `newQuery.Start` * If the end of new query is after the end time of the cached query, do a query from `cachedQuery.End` to `newQuery.Start` * If the response of last query is also empty, set `cachedQuery.End` = `newQuery.End` * If we have changes in `cachedQuery.Start/End`, update it in the cache. The problem here is when we do queries to fill the gap, we sometimes do queries for the range outside of what the user requested and respond back without reducing the response to what the user requested. For example, if the cached query is from `21m`-`25m` and the user query is from `10m`-`15m`, we will query for the whole gap i.e `10m`-`21m`. If there are logs from `15m`-`21m` in the response, we will unexpectedly send it back to the user. This PR takes care of this issue by extracting the data and sending back only the user's requested logs. I have also found the tests for logs results cache were incorrect. They heavily use [mergeLokiResponse](e2842c69c5/pkg/querier/queryrange/codec.go (L1014)) for building the test data, but they were not setting the `limit` in test queries which was causing `mergeLokiResponse` to send back empty results. This means were were always comparing two empty results, which would always be the same. This PR also takes care of fixing it, and relevant changes to correct the test. **Checklist** - [x] Tests updated - [x] `CHANGELOG.md` updated	3 years ago
Sandeep Sukhani	020631ebac	add user-id transformer for logs results cache (#7581) ... **What this PR does / why we need it**: Adding transformer to results cache same as metrics cache done in PR #7542	3 years ago
Karsten Jeschkies	46956d0bf6	Use dskit/tenant intead of loki/pkg/tenant. (#5724)	4 years ago
Cyril Tovena	39101308f1	[Frontend] Log Result Cache (#5502) ... * Rough verion still require some fixes * with file saved * Finish off the code and logic Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com> * Refactors metrics and hook the log cache middleware Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com> * Add tests, fixes and lint Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com> * Hash the key Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com> * better comment Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com> * better comment Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com> * Update pkg/querier/queryrange/log_result_cache.go Co-authored-by: Christian Simon <simon@swine.de> * Update pkg/querier/queryrange/log_result_cache.go Co-authored-by: Christian Simon <simon@swine.de> * Fixes typo in variable namme Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com> * Use only one ok bool Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com> * Review feedbacks Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com> * check array boundaries Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com> Co-authored-by: Christian Simon <simon@swine.de>	4 years ago