Skip to content
This repository has been archived by the owner on Jan 5, 2019. It is now read-only.

Adding a api.all.<status>.count and measure #10

Merged
merged 2 commits into from
May 26, 2016
Merged

Adding a api.all.<status>.count and measure #10

merged 2 commits into from
May 26, 2016

Conversation

jonasfj
Copy link
Contributor

@jonasfj jonasfj commented May 19, 2016

I didn't run tests... Just quickly hacked it online, but I think this is what I meant...

@@ -38,6 +38,8 @@ export function expressMiddleware (monitor, name) {
monitor.measure(k, d[0] * 1000 + d[1] / 1000000);
monitor.count(k);
}
monitor.measure(['all', stat], d[0] * 1000 + d[1] / 1000000);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you want to either move this inside the loop and end up with
taskcluster-queue.api.all.client-error.5m.count and taskcluster-queue.api.all.all.5m.count or replace stat in these new lines with success and only end up with taskcluster-queue.api.all.client-error.5m.count.

I might be misunderstanding though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that might be right...
Actually, I'm little suspicious as to the utility of all these alls.. some of them sure...

But given that 99% is success, then we might not need both all and success. As the percentiles will be the same...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, now I see the bug, thanks...

@imbstack
Copy link
Contributor

r+

I'll merge this now.

@imbstack imbstack merged commit 9d358dc into master May 26, 2016
@imbstack imbstack deleted the api-all branch May 26, 2016 18:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants