querymiddleware: cache instant query errors

[narqo]

What this PR does

The PR adds errors caching middleware for instant query. This is basically the amendment to the original #9028 (I haven't tested it yet, but I think it should just work).

Update I ran some manual testing in a dev environment, and it works as I'd expected:

Details

Non-cachable errors aren't cached:

Cachable errors are stored in the cache on first occurrence:

...and are retrived from the cache on subsequent requests

Which issue(s) this PR fixes or relates to

Fixes #11029

Checklist

• Tests updated.
• Documentation added.
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
• about-versioning.md updated with experimental features.

[56quarters]

Changes LGTM assuming you've tested locally. Do you mind expanding the unit tests for this middleware to make sure instant queries behave how we expect?

[narqo]

@56quarters I've updated the tests and also ran the tests (see the updated description of the PR). PHAL

[56quarters]

Thanks for adding tests but I don't think this will work as written. The way the cache key for errors is built includes req.GetStart() and req.GetEnd() in the key. For an instant query this is the timestamp the query is executed at which will be different for every instant query.

Additionally, the screenshots of testing you did indicate another bug. A context deadline exceeded error is a timeout and shouldn't be cached.

[56quarters]

Thanks for adding tests but I don't think this will work as written. The way the cache key for errors is built includes req.GetStart() and req.GetEnd() in the key. For an instant query this is the timestamp the query is executed at which will be different for every instant query.

Additionally, the screenshots of testing you did indicate another bug. A context deadline exceeded error is a timeout and shouldn't be cached.

The first issue can be fix by changes to DefaultCacheKeyGenerator which can detect instant queries by comparing GetStart() and GetEnd() and build the key without start/end/step parameters.

[56quarters]

Additionally, the screenshots of testing you did indicate another bug. A context deadline exceeded error is a timeout and shouldn't be cached.

Opened #11198 to fix this

[narqo]

For an instant query this is the timestamp the query is executed at which will be different for every instant query.

That's a great point. I've updated the default cache key generator to handle that.

[56quarters]

LGTM with a few comments. Thanks!