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

Improve errors and warnings #1681

Open
na-- opened this issue Oct 22, 2020 · 5 comments
Open

Improve errors and warnings #1681

na-- opened this issue Oct 22, 2020 · 5 comments
Labels
enhancement evaluation needed proposal needs to be validated or tested before fully implementing it in k6 feature ux

Comments

@na--
Copy link
Member

na-- commented Oct 22, 2020

This started as a discussion with @sniku that a lot of k6 users probably don't realize that, even if 100% of their HTTP requests fail, k6 run their-script.js will exit with a 0 status (i.e. "all ok") if they have not defined some thresholds. So, he suggested that we can add a warning at the end of a k6 run, when we show the summary, if no thresholds are defined. He even proposed to use the summary data to suggest some appropriate http_req_duration threshold value for them, if it makes sense. Basically have a customized warning+hint combo if we have the data (e.g. more than 100 HTTP requests) and have a default generic "no thresholds" warning for every other scenario.

All of these things seem fine and an improvement in UX, but my biggest problem is that some percentage of users (more than 10%, I think) will purposefully not want to set any thresholds and this warning might get annoying. For example, someone doing simple performance measurements or exploratory testing, or people who are just writing their script and testing it with 1VU, not the SUT. Or, on the other end, people who just use k6 to generate load and are using some other tool to monitor their SUT. Or people using external outputs. So, I think we need some way for users to suppress the warning.

Also, there are other warnings/errors we log that users might want to suppress. The most obvious example is the HTTP request failure, which you can currently only turn off by enabling throw... And if we have the possibility to disable warnings, we probably would also want to use them for other things as well! Some ideas:

  • when we have shared memory (Shared state/memory between VUs #532) or streaming data (Proposal for a K6 stream API #592), we should probably start emitting warnings when users open() large files.
  • depends on when the new HTTP API lands, but a lot of people don't know about discardResponseBodies and responseType, so we might nudge them with a warning if they have large HTTP responses or something like that
  • suggest thresholds for other things, like failed or interrupted iterations
  • etc. other good practices that aren't followed

So, I think we should:

  • Give each error and warning a unique SEO-friendly ID, e.g. k6NoThresholdsDefined, k6OpenLargeFile, etc.
  • Allow users to explicitly disable specific warning/error messages with a new k6 option, e.g. options.ignoredWarnings = ['k6NoThresholdsDefined'] (any ideas for a better name?).
  • This option will probably also be re-used for cloud-only warnings/errors/smart-result hints, so we should simply build a set of ignored string IDs without validating it on the k6 side.
  • Have a page in the docs for each warning/error ID, with longer explanation and examples and information on how to fix the problem or disable the alarm. We already discussed having IDs and pages for fatal errors (cannot load script or open file, etc.), so this can be documented the same way.
  • Maybe even link to these pages from the actual error/warning messages and/or tell users how to disable them.

To get back to the original issue - having a warning for "no defined thresholds" will not solve all issues. It will just educate users that might not know that thresholds exist or their purpose. We'd still have many, many issues to fix in the thresholds themselves (#1443 (comment)) before they are useful enough for detecting all sorts of test issues, but the two problems are orthogonal and we should fix both.

@na-- na-- added enhancement ux feature evaluation needed proposal needs to be validated or tested before fully implementing it in k6 labels Oct 22, 2020
@kz
Copy link

kz commented Mar 1, 2021

I'm definitely keen to have this for my artifically low timeouts. I've set some requests with a timeout of 3s to mirror frontend behaviour. Under high load, this clutters my console with tens of thousands of error="...: context deadline exceeded"s. It would be useful to separate the noise out from useful warnings!

@na--
Copy link
Member Author

na-- commented Mar 1, 2021

@kz, here are some workaround suggestions until this issue is implemented:

  • enable the throw option, surround your HTTP requests in try {} catch block and silence all errors that have context deadline exceeded
  • use --logformat=json option and redirect stderr to a file. Then use something like jq to filter out the error messages you don't care about.
  • or, I guess just redirect stderr to a file and use grep 😅 or do it all inline, with something like [process substitution](https://tldp.org/LDP/abs/html/process-sub.html in bash), like this: k6 run short-timeouts.js 2> >(grep -v "context deadline exceeded")

@nicolevanderhoeven
Copy link

Just adding my +1 to this idea. I am in favour of more warnings, with a way to suppress them. This could also (later) feed into Performance Insights on k6 Cloud.

I also want to specifically mention @yorugac 's comment about the error Insufficient VUs, reached 500 active VUs and cannot initialize more, which we agreed needed to clearer.

@imiric
Copy link
Contributor

imiric commented Mar 28, 2023

As a way to reduce the log clutter, and instead of (or in addition to) having to configure k6 to ignore specific events, how about grouping repeated events in time buckets, with a counter of how many such events were logged within that period?

Let's say the default would be to group events in one minute buckets:

16:40 ERROR k6ScriptError (x42): TypeError: cannot read property '...' of undefined (./script.js:28:19)

New events would render at the bottom, and if they're repeated, only their counter would increase instead of writing an entire separate line. The time precision of individual events would be reduced, but in most cases exact timestamps aren't very important when the events are flooding the output.

This would require some event buffering for the configurable bucket period, and some tricky writing to stderr, but it shouldn't be too difficult. And, of course, it should be possible to disable this behavior.

@joanlopez
Copy link
Contributor

I think what Ivan suggested above around bucketing log events is probably a good idea, and in my opinion it deserves a time-boxed effort to build a small proof-of-concept to evaluate how feasible it would be, and what would be the impact. I'll try to do that during one of the upcoming cooldown periods.

Beyond that, and regarding what's suggested in the rest of the issue, I think (personal opinion) what we should do is to design some sort of Insights API (similar to what we have for Cloud now, either extend that or build a complementary one), where you can receive this kind of "warnings", telling you about potential ways of improving your tests.

I think that approach would be much better because, it would be decoupled from other kinds of error/warning messages, which won't just imply a better UX but would make it easier for instance to enable/disable, and also because I think most of the things mentioned above are kinda similar/related to some of the insights we already have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement evaluation needed proposal needs to be validated or tested before fully implementing it in k6 feature ux
Projects
None yet
Development

No branches or pull requests

5 participants