yaorg / node-measured

A Node metrics library for measuring and reporting application-level metrics, inspired by Coda Hale, Yammer Inc's Dropwizard Metrics Libraries
https://yaorg.github.io/node-measured/
MIT License
517 stars 52 forks source link

Report statusCode and uri accurately for POSTs #57 #58

Closed avolfson closed 5 years ago

avolfson commented 5 years ago

With a listener on the req.on('end') event, req.route and res.statusCode are undefined. Switching to res.on('finish') makes these values available.

https://github.com/yaorg/node-measured/issues/57

fieldju commented 5 years ago

image

The build appears to be failing with this PR.

~Which is weird because res.on was already there.~

Are you sure this isn't supposed to be req.on('finish')

avolfson commented 5 years ago

Added a reproducing integration test and updated the unit test, so now the fix's PR is in good shape.

avolfson commented 5 years ago

Is there anyway to update the example at https://yaorg.github.io/node-measured/packages/measured-signalfx-reporter/tutorial-SignalFx%20Express%20Full%20End%20to%20End%20Example.html to mention that this middleware must precede bodyParser middleware? This was another bug that was hard to discover for me :laughing:

fieldju commented 5 years ago

Yeah it's in the tutorial folder at the root of the project.

On Sat, Feb 2, 2019 at 8:40 AM avolfson notifications@github.com wrote:

Is there anyway to update the example at https://yaorg.github.io/node-measured/packages/measured-signalfx-reporter/tutorial-SignalFx%20Express%20Full%20End%20to%20End%20Example.html to mention that this middleware must precede bodyParser middleware? This was another bug that was hard to discover for me 😆

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/yaorg/node-measured/pull/58#issuecomment-459979281, or mute the thread https://github.com/notifications/unsubscribe-auth/AArcLkmX4-qgV7S27zi3cCM74IcWxmTlks5vJb-QgaJpZM4aZZls .

-- Justin Field fieldju@gmail.com LinkedIn https://www.linkedin.com/in/fieldju | @fieldju

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 353


Totals Coverage Status
Change from base Build 347: 0.0%
Covered Lines: 749
Relevant Lines: 827

💛 - Coveralls