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

Update Custom Metrics / Performance Measure with Trend param isTime == true #1436

Merged
merged 7 commits into from
Dec 8, 2023

Conversation

dirtydiesel46
Copy link
Contributor

@dirtydiesel46 dirtydiesel46 commented Dec 2, 2023

Add isTime == true param to Trends
Fixed value being added to the myTrend trend too

What?

  1. The documentation isn't declaring the trend metric as a time based trend metric

Documentation shows

const myTrend = new Trend('total_action_time');

and not

const myTrend = new Trend('total_action_time',true);

  1. Passed correct value to the add method of Trend

Documentation shows

myTrend.add(total_action_time);

should be

myTrend.add(totalActionTime);

  1. The results that has a missing s has been added, it's supposed to be a time based metric

Documentation shows

  iteration_duration..........: avg=1.06s    min=1.06s    med=1.06s    max=1.06s    p(90)=1.06s    p(95)=1.06s
  iterations..................: 1      0.70866/s
  total_action_time.............: avg=295.3    min=295.3    med=295.3    max=295.3    p(90)=295.3    p(95)=295.3

and not

  iteration_duration..........: avg=1.06s    min=1.06s    med=1.06s    max=1.06s    p(90)=1.06s    p(95)=1.06s
  iterations..................: 1      0.70866/s
  total_action_time.............: avg=295.3s    min=295.3s    med=295.3s    max=295.3s    p(90)=295.3s    p(95)=295.3s

Checklist

Please fill in this template:

  • I have used a meaningful title for the PR.
  • I have described the changes I've made in the "What?" section above.
  • I have performed a self-review of my changes.
  • I have run the make docs command locally and verified that the changes look good.

Select one of these and delete the others:

If updating the documentation for the most recent release of k6:

  • I have made my changes in the docs/sources/v{most_recent_release} folder of the documentation.

If updating the documentation for the next release of k6:

  • I have made my changes in the docs/sources/next folder of the documentation.

Related PR(s)/Issue(s)

Add isTime == true param to Trends
@CLAassistant
Copy link

CLAassistant commented Dec 2, 2023

CLA assistant check
All committers have signed the CLA.

@dirtydiesel46 dirtydiesel46 changed the title Update metrics.md Update Custom Metrics / Performance Measure with Trend param isTime == true Dec 2, 2023
@dirtydiesel46 dirtydiesel46 marked this pull request as draft December 2, 2023 10:47
@dirtydiesel46 dirtydiesel46 marked this pull request as ready for review December 2, 2023 10:56
Copy link
Contributor

@mdcruz mdcruz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, Daniel! Just one minor comment :)

Copy link
Contributor

@mdcruz mdcruz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just ran the test locally, and I've updated the time from s to ms to reflect it accurately.

docs/sources/next/using-k6-browser/metrics.md Outdated Show resolved Hide resolved
docs/sources/v0.47.x/using-k6-browser/metrics.md Outdated Show resolved Hide resolved
dirtydiesel46 and others added 3 commits December 4, 2023 22:20
Co-authored-by: Marie Cruz <mdcruz@users.noreply.github.com>
Co-authored-by: Marie Cruz <mdcruz@users.noreply.github.com>
@mdcruz mdcruz merged commit d6d264c into grafana:main Dec 8, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants