-
Notifications
You must be signed in to change notification settings - Fork 569
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Chore: add instant query to query-frontend limitsMiddleware tests #8410
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also add
"instant query": &PrometheusInstantQueryRequest{
queryExpr: parseQuery(t, testData.query),
time: endMs,
},
to TestLimitsMiddleware_MaxQueryExpressionSizeBytes
expectedSkipped bool | ||
expectedTime time.Time | ||
}{ | ||
"should not manipulate request time if maxQueryLookback and blocksRetentionPeriod are both disabled": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NitX3: If I understand correctly, start==end for instant query so either we allow it or not, never manipulate it so this is kind of weird name for the testcase.
"should not manipulate request time if maxQueryLookback and blocksRetentionPeriod are both disabled": { | |
"should allow executing a request if maxQueryLookback and blocksRetentionPeriod are both disabled": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, absolutely. Going to fix the descriptions in this test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 7fac981
Signed-off-by: Marco Pracucci <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved with nit
Signed-off-by: Marco Pracucci <[email protected]>
96c150c
to
7fac981
Compare
What this PR does
While working on #8374 we noticed that we don't test
limitsMiddleware
against instant queries. In this PR I'm doing it. Specifically:TestLimitsMiddleware_MaxQueryLookback
: create a newTestLimitsMiddleware_MaxQueryLookback_InstantQuery
because the assertions for instant queries are different than range queries and remote readsTestLimitsMiddleware_MaxQueryExpressionSizeBytes
: added instant queriesTestLimitsMiddleware_MaxQueryLength
: doesn't apply to instant queries because we run the check against the input query time range (and instant queries don't have a time range)Which issue(s) this PR fixes or relates to
N/A
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]
.about-versioning.md
updated with experimental features.