-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: allow name tag, and overwrite url tag with name tag if set #60
feat: allow name tag, and overwrite url tag with name tag if set #60
Conversation
…th url and name tags.
bump @oleiade @olegbespalov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay in reviewing; the changes look good to me 🙌 Thank you for the contribution!
No problem, thanks for the review. What is the merge process? Do we need another review, apparently I can't merge. |
Yes, we do allow the merge only from maintainers. Once the @oleiade (as the remaining reviewer) reviews and approves the changes, it could be possible. Later, we will issue the tag (or bump an updated version of the module to the k6 core. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
The k6's PR for the reference grafana/k6#3606 |
What?
This PR allows users to manually set name tag. Also, when name tag is present, it will overwrite the url tag.
Why?
Query parameters in the url, cause too many trends to appear in metrics. This is explained in the issue as well.
Checklist
make lint
) and all checks pass.make test
) and all tests pass.Related PR(s)/Issue(s)
Spiritual equivelant in k6 repo: grafana/k6#2703
Closes #33