Fix thresholds evaluated incorrectly #4787
Conversation
Signed-off-by: Fernando-hub527 <fernandocoelhosaraivanando@gmail.com>
Signed-off-by: Fernando-hub527 <fernandocoelhosaraivanando@gmail.com>
|
As far as I could determine, the issue was happening because, to avoid a division by zero, the rate metric wasn’t populated when the total number of checks was zero (see code here). That means if an error occurred before any checks were executed, the threshold failure was never triggered. Here’s an example of the threshold evaluation after the fix: code: result: |
ankur22
left a comment
There was a problem hiding this comment.
Thanks for the PR @Fernando-hub527!
I took a look at the change and used the following script to test before and after the fix:
import { check } from "k6";
import http from "k6/http";
export const options = {
stages: [
{ duration: "2s", target: 1 },
],
thresholds: {
"checks": [{threshold: "rate>0.9", abortOnFail: true}],
},
};
export default function () {
const res = http.get("https://test-api.k6.io/public/crocodiles");
throw new Error('This is an error');
check(res, {
"status was 200": (res) => res.status === 200,
});
}Before the fix:
█ THRESHOLDS
checks
✓ 'rate>0.9' rate=0.00%
█ TOTAL RESULTS
checks_total.......................: 0 0/s
checks_succeeded...................: 0.00% 0 out of 0
checks_failed......................: NaN% 0 out of 0
After the fix:
█ THRESHOLDS
checks
✗ 'rate>0.9' rate=0.00%
█ TOTAL RESULTS
checks_total.......................: 0 0/s
checks_succeeded...................: 0.00% 0 out of 0
checks_failed......................: 0.00% 0 out of 0
NOTE: The results are focusing on the areas that differ between the two test runs. Other metrics are presented such as HTTP, but they look correct in both test runs.
I think with the fix is a better outcome than before, so i'm approving it.
|
Hi @Fernando-hub527, We're discussing internally if really want to merge this new behavior for thresholds as-is, or perhaps offer another workaround to sort it out. So, could you please split it into two different PRs, one for changing the threshold evaluation behavior and the other one to fix the This way we can merge this part of your contribution and close #4842. Thanks! 🙇🏻 🙌🏻 |
|
Hi @joanlopez, sorry for the delay. |
|
Hey @Fernando-hub527, I'm going to close this pull request because after some internal discussions, I even stronger believe we won't want to change this behavior on thresholds, as it would introduce a breaking change, and it's unlikely that's always going to be the desired behavior by users, while it won't be configurable. So, we'll likely agree on another workaround, like "exposing" the "total" value for Rate metrics, letting users to define another threshold that ensures "total > 0"; or adding a new option, like Anyway, thanks for the other half of the PR that you split and that has already been merged, as that was really a small bug to be fixed. Really appreciated! 🙌🏻 |
What?
This PR ensures that once thresholds are defined, they are still evaluated correctly even if check(...) is never called.
Why?
Guarantee that if an error occurs before any checks are executed, the threshold evaluation will correctly register a failure.
Checklist
make check) and all pass.Checklist: Documentation (only for k6 maintainers and if relevant)
Please do not merge this PR until the following items are filled out.
Related PR(s)/Issue(s)
Closes #4283