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

Add the ability to filter by request type in page.on('metric') #1487

Open
5 tasks done
ankur22 opened this issue Oct 16, 2024 · 2 comments
Open
5 tasks done

Add the ability to filter by request type in page.on('metric') #1487

ankur22 opened this issue Oct 16, 2024 · 2 comments
Assignees
Labels
feature A new feature next Might be eligible for the next planning (not guaranteed!)

Comments

@ankur22
Copy link
Collaborator

ankur22 commented Oct 16, 2024

Feature Description

We have page.on('metric') which allows us to group metrics when a URL match is found. The issue arises when the website under test makes different types of requests using the same URL, for example a GET to api/cart, and a POST to api/cart. In this case it's not going to be useful to group the same request together as the behaviour of them will be different.

What would be useful is to be able to filter/group also by the request type.

Suggested Solution (optional)

A possible solution is to extend the existing API so that it can also take a type field. If the type field isn't present, it will assume that all requests are to be grouped regardless of the type. If a type field is present it will take that into account:

  page.on('metric', (metric) => {
    metric.tag({
      urls: [
        {url: /^https:\/\/test\.k6\.io\/\?q=[0-9a-z]+$/, type:'GET', name:'test'},
      ]
    });
  });

Already existing or connected issues / PRs (optional)

#371
#1434

@ankur22 ankur22 added feature A new feature next Might be eligible for the next planning (not guaranteed!) labels Oct 16, 2024
@inancgumus
Copy link
Member

inancgumus commented Oct 16, 2024

The suggested (and the current) solution is mixing the tag name and URL properties.

It was fine when there was only url and name. I believe the API would be less confusing if it'd group urls by a name instead of having name as another property. Maybe something like:

page.on('metric', metric => {
  metric.tag({
    name: 'tests',
    urls: [
      {url: /^https:\/\/test\.k6\.io\/\?q=[0-9a-z]+$/, type:'GET'},
    ]
  }, {
    name: 'other',
    urls: [
      {url: /^https:\/\/test\.k6\.io\/\?q=[0-9a-z]+$/, type:'POST'},
      {url: /^https:\/\/foo\.com/, type:'GET'},
    ]
  });
});

@ankur22
Copy link
Collaborator Author

ankur22 commented Oct 17, 2024

@inancgumus I can see where you are coming from, I like this approach 👍

WDYT of calling it method instead of type?

page.on('metric', metric => {
  metric.tag({
    name: 'tests',
    urls: [
      {url: /^https:\/\/test\.k6\.io\/\?q=[0-9a-z]+$/, method:'GET'},
    ]
  }, {
    name: 'other',
    urls: [
      {url: /^https:\/\/test\.k6\.io\/\?q=[0-9a-z]+$/, method:'POST'},
      {url: /^https:\/\/foo\.com/, method:'GET'},
    ]
  });
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature next Might be eligible for the next planning (not guaranteed!)
Projects
None yet
Development

No branches or pull requests

2 participants