Skip to content
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

fix: handle fractional "check result" logs #1074

Merged
merged 1 commit into from
Feb 26, 2025
Merged

Conversation

nadiamoe
Copy link
Member

What this PR does / why we need it:

When grafana/xk6-sm#34 is merged, it will be possible for check result logs to have fractional values, if the same check name is used twice. For example, the following snippet:

  check(res, {
      'is 200': () => true,
    }
  );
  check(res, {
      'is 200': () => false,
    }
  );

Currently yields:

time="2025-01-14T17:32:54+01:00" level=info msg="check result" check="is 200" metric=checks_total scenario=default source=sm value=1
time="2025-01-14T17:32:55+01:00" level=info msg="check result" check="is 200" metric=checks_total scenario=default source=sm value=0

After grafana/xk6-sm#34 is merged, it will yield

time="2025-01-14T17:32:54+01:00" level=info msg="check result" check="is 200" metric=checks_total scenario=default source=sm value=0.5

In terms of reporting, and given this will only happen if two checks with the same name are defined, I think it is okay to simplify and assume that if not 100% of the checks with the same name are okay, the check has failed. So this PR changes the query to look for != 1 instead of = 0. Other queries already look for = 1, so those shouldn't need changing.

@nadiamoe nadiamoe requested a review from a team as a code owner February 21, 2025 18:00
@nadiamoe nadiamoe requested a review from VikaCep February 21, 2025 18:00
@VikaCep
Copy link
Contributor

VikaCep commented Feb 25, 2025

In the example you provided, then with these changes both checks would fail, is that right?

@nadiamoe
Copy link
Member Author

Yes, I think this would be a good approximation to handle this corner case: If at least one instance of check of a given name fails, then the check should be reported as failed.

Copy link
Contributor

@VikaCep VikaCep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@nadiamoe nadiamoe merged commit d50448f into main Feb 26, 2025
7 checks passed
@nadiamoe nadiamoe deleted the fractional-check-logs branch February 26, 2025 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants