search

mikejihbe / metrics

A metrics library for Node.js
574 stars 58 forks source link

Corrects paste errors in graphite reporter for gauge #65

Closed ankitchiplunkar closed 5 years ago

ankitchiplunkar commented 5 years ago

Solves issue: https://github.com/mikejihbe/metrics/issues/64

The gauge was not properly configured in the graphite-reporter.js

sidshekhar commented 5 years ago

This is a key issue that affects a lot of users. We can't effectively track current websocket connections without using this patch

tolbertam commented 5 years ago

Good catch and thank you for the fix. I will be pushing a release for this shortly.

tolbertam commented 5 years ago

I was trying to figure out why our test/unit/graphite_reporter.js test did not catch this. It looks like the test was checking the number of metrics, and counters were being logged in place of gauge:

metrics host1.basicCount 5 1544152644
host1.myapp.Meter.count 10 1544152644
host1.myapp.Meter.mean_rate 2.4950099800399204 1544152644
...
host1.myapp.Histogram.p99 199.98 1544152644
host1.myapp.Histogram.p999 200 1544152644
host1.basicCount 5 1544152644

after the fix:

host1.basicCount 5 1544152809
host1.myapp.Meter.count 10 1544152809
host1.myapp.Meter.mean_rate 2.493765586034913 1544152809
...
host1.myapp.Histogram.p99 199.98 1544152809
host1.myapp.Histogram.p999 200 1544152809
host1.myapp.Gauge.value 0.8 1544152809

Perfect! The tests could use some refining, but for now manual verification works for me. I'll make a note to take care of that later.