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

Comma character in the url causes parsing issue, so it's not possible to use url containing comma for threshold expression #3173

Open
ArturSuchowierski opened this issue Jul 6, 2023 · 1 comment
Labels

Comments

@ArturSuchowierski
Copy link

ArturSuchowierski commented Jul 6, 2023

Brief summary

Following example of threshold for URL will run into error: ERRO [0000] unable to validate threshold expressions; reason: parsing metric name failed, metric "cde" tag expression is malformed

export const options = { thresholds: { ‘webvital_largest_content_paint{url:www.example-url.com/somepage?abc,cde,def}’ : [‘p(90) < 2500’], }, },

Maybe it's worth checking even for other characters, that might be allowed in the URL's

k6 version

v0.45.0

OS

macOS Ventura 13.4.1

Docker version and image (if applicable)

n/a

Steps to reproduce the problem

Run any test for following threshold:

export const options = {
  thresholds: {
    ‘webvital_largest_content_paint{url:www.example-url.com/somepage?abc,cde,def}’ : [‘p(90) < 2500’],
  },
 },

Expected behaviour

Url including commas should be properly parsed for the threshold

Actual behaviour

When running test there's error returned: ERRO [0000] unable to validate threshold expressions; reason: parsing metric name failed, metric "cde" tag expression is malformed

@olegbespalov
Copy link
Contributor

Hi @ArturSuchowierski

Sorry for the late response 😢

Yes, unfortunately, I can only confirm the fact that the comma there could cause the issue for the threshold parser since it tries to tread cde as the metric. As @mstoykov mentioned to you in another issue, it's due to the fact that the current threshold syntax, format, and functionality is kind of basic.

It seems like the proper solution for the issue is to keep such a case in mind during the implementation of a bigger thing like #3198.

@codebien codebien added feature and removed bug labels Oct 13, 2023
@olegbespalov olegbespalov removed their assignment Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants