Threshold sub-metric selectors containing reserved symbols would fail

[efdknittlfrank]

Brief summary

Kudos on the 0.38.0 release! 🎉

k6 0.38.0 now applies stricter validation to thresholds, both for metric names as well as threshold expressions. The error message however is not really helpful in telling users which threshold definition is actually wrong:

With scripts which worked fine in the past, the execution is aborted with an error:

ERRO[0012] unable to validate threshold expressions: parsing metric name failed

I understand that something is wrong with my script and it should be fixed, but it would be really helpful if the error message contained (at least one of) the following:

• A line number
• The name of metric that failed
• The threshold expression that's invalid

Adding -v doesn't produce more (useful) information in this case:

DEBU[0012] Parsing thresholds and validating config...
ERRO[0012] unable to validate threshold expressions: parsing metric name failed

I will now go strip out thresholds until I find the culprit 😄 (and there are lots of them 😆)

k6 version

0.38.0

OS

Windows, Linux, Docker

Docker version and image (if applicable)

grafana/k6:0.38.0

Steps to reproduce the problem

Have a script with invalid thresholds

Expected behaviour

The error message points the user to the invalid threshold

Actual behaviour

The error message is too generic and simply says "invalid threshold", but not which threshold or metric is invalid.

[na--]

Ah, sorry, we'll definitely have to fix this... 😅 Thanks for reporting it!

[oleiade]

Hi @efdknittlfrank,

thanks for the feedback 🙇🏻 We've tried to make sure error messages would be as helpful as possible, but it seems you've found an execution path where it is not good enough indeed. Could you provide an example script to reproduce the error you're encountering?

Errors were supposed to provide the metric, and expression that failed. Line number would be trickier, and I'm not certain that would be possible at the moment. Will look into it 👍🏻

[efdknittlfrank]

@na-- @oleiade Thanks for the replies. I was too quick to submit the initial bug report. Of course, I will provide the faulty threshold configuration here for further analysis.

I'm pretty sure that some of them are actual errors, other should maybe be discussed.

One thing I already noticed: I have several "default" thresholds defined in my scripts (via a library). Not all scripts that use this library create all metrics that are listed in the threshold definition. With 0.38.0 there's no other way then to always define those metrics (and never submit a value)? Would be cool if a threshold/metric could be marked as "optional" for the "iDoNotExist" case. But this already sounds more like a feature request to bring back functionality that has worked before :)

I will follow up, give me a few hours.

[efdknittlfrank]

Alright, I think I've hit a real bug here. The repro is trivial:

import http from 'k6/http';

export const options = {
	thresholds: { 'http_req_duration{name:http://${}.com}': [ 'max < 1000' ] },
};

export default function() {
	const host = 'example';
	http.get(http.url`http://${host}.com`);
}

This has worked in versions up to and including 0.37.0 and is explained in the docs under URL Grouping

And no, "parsing metric name failed" does not mean "parsing metric 'name' failed" – it's the same message regardless of the actual tag name :)

Do those tag values now need special handling such as escaping or quoting? I tried http_req_duration{name:"http://${}.com"} to no avail.

For the other breaking change (thresholds on "optional metrics" no longer supported), I can implement an easy workaround on my part, if it is decided that all metrics used in thresholds must exist.

[oleiade]

@efdknittlfrank thanks for providing us this example. I can reproduce the issue indeed 🙇🏻

I have some intuition as to what's happening here, and I can confirm this is very likely a limitation introduced by the new thresholds' interpreter. It could be that in the example you've provided, the threshold tag parser is probably confused by all the non-alphanumeric symbols it did not expect.

I was not aware of this specific use case for thresholds, so thanks for bringing it to my attention. Sorry for the inconvenience... I'll work on a fix and will let you know how we intend to proceed when it's ready and how we intend to proceed further 👍🏻

[oleiade]

@efdknittlfrank we have opened a pull request to tentatively resolve the issue you've encountered. If you have the opportunity, would you mind giving the fix a try and let us know if that addresses your issue indeed? Cheers, 🙇🏻

If things go as expected, we would publish a dot release: 0.38.1 by tomorrow.

[oleiade]

@efdknittlfrank we have now released v0.38.1 which contains the hotfix for this issue 🙇🏻

[efdknittlfrank]

@oleiade wonderful! I can confirm that 0.31.1 accepts grouped URLs in threshold definitions again 👍

With new error message I have found 2 metrics already which were used in thresholds, but didn't exist 😄

I will implement a workaround for my "optional metrics", which should be simple enough to do on my part.

I have noticed a few minor glitches, but need to investigate further if that deviates from the old behavior and if it's an actual bug/issue. Could be that my script was defining metric expressions which were never valid in the first place.

Thank you very much, uncertainly the best bug response time!