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

Metrics refactor #69

Merged
merged 5 commits into from
Feb 14, 2016
Merged

Metrics refactor #69

merged 5 commits into from
Feb 14, 2016

Conversation

banks
Copy link
Member

@banks banks commented Feb 11, 2016

See #68

Not sure about name counters but it's as good as any I came up with.

I tested this ad-hoc by opening three terminals and these commands, on in each:

$ ./centrifugo --insecure_api
$ watch -n 1 -d 'curl -s -X POST -H "Content-Type: application/json" -d "[{\"method\":\"counters\",\"params\":{}}]" localhost:8000/api/ | jq "."'
$ watch -n 1 -d 'curl -s -X POST -H "Content-Type: application/json" -d "[{\"method\":\"stats\",\"params\":{}}]" localhost:8000/api/ | jq "."'

See
image

You can see the api requests counter has changed in the 1 second since last update in the middle (counters command) and not changed on the right (stats command). Counters command also shows cumulative total, while stats only last minute (which is ~120 since there are two watchers running about every 1 second).

@banks
Copy link
Member Author

banks commented Feb 12, 2016

@FZambia now it's node :)

Also note: I removed go-metrics from Godep file by hand because I'm actually running go1.6rc and didn't want to save that in Godep file.

I noticed though that there are a couple of other packages in Godep file that godep diff seems to think are no longer used in the code - in a separate PR/branch would be good to clean those up at some point.

@FZambia
Copy link
Member

FZambia commented Feb 14, 2016

@banks wonderful! Thanks a lot!

FZambia added a commit that referenced this pull request Feb 14, 2016
@FZambia FZambia merged commit f3a6f7d into master Feb 14, 2016
@banks banks deleted the metrics_refactor branch February 25, 2016 17:05
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.

2 participants