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

Double-counting requests if using vhost functionality? #32

Closed
aseemk opened this issue May 20, 2013 · 9 comments
Closed

Double-counting requests if using vhost functionality? #32

aseemk opened this issue May 20, 2013 · 9 comments

Comments

@aseemk
Copy link

aseemk commented May 20, 2013

Hi guys,

We're running a Node.js app on Heroku and have node-newrelic capturing metrics. We've noticed that New Relic's count of requests per minute is consistently ~2x what we're seeing from Heroku's router logs (e.g. via log2viz, but also in Librato via l2met).

(I wish I could check Nodetime's count too. Unfortunately at the moment their site seems to be broken. I'll update this whenever it works again and I can check.)

It occurred to me that what could be happening is that requests might be getting double-counted, since we use Connect's/Express's vhost functionality, so all of our requests technically flow through two Express servers.

Simplified code: https://gist.github.com/aseemk/5613063

Is that a possibility? If so, it'd be great to fix.

We're still on Express 2 at the moment (and Node 0.6), but are actively looking to upgrade to Express 3 (and Node 0.10) very soon.

Thanks much!

@othiym23
Copy link
Contributor

You're right that it shouldn't be double-counting like that. We'll take a look and let you know what we find. Thanks for the report and the code sample!

@othiym23
Copy link
Contributor

Fixed via 209247e.

@aseemk
Copy link
Author

aseemk commented Oct 1, 2013

Deployed 0.11.2 to production and we're still seeing this FYI! Not urgent for us by any means, so don't feel the need to worry about this, but just wanted to let you know.

@othiym23
Copy link
Contributor

othiym23 commented Oct 2, 2013

Weird! The changes I made to the tracer should have put a stake through this. Can you put together a reduced test case for this so we can be sure we've killed the bug for good? It doesn't have to be pretty, it just has to reliably demonstrate the problem. Thanks, and sorry for closing this when it wasn't fixed. :/

@othiym23 othiym23 reopened this Oct 2, 2013
@othiym23
Copy link
Contributor

othiym23 commented Oct 2, 2013

Give 0.11.4 a shot and see if it makes a difference. It may not, but the change I made to the Connect instrumentation may affect this too.

@seth-admittedly
Copy link

I'm having a related problem with mounted apps. I've got both the relative and absolute routes ignored in my config file, but the requests are still showing in my reports and the logger is spitting out {"name":"newrelic","hostname":"local.admitted.ly","pid":20481,"component":"express","level":50,"msg":"No Express route to use for naming.","time":"2013-12-12T06:17:37.844Z","v":0} for each request.

@othiym23
Copy link
Contributor

👍 This is something I want us to get to soon, as part of a larger rewrite of router introspection to be more general. Thanks for moving this over here, @seth-admittedly!

@txase
Copy link

txase commented Jun 26, 2014

Hi @seth-admittedly and @aseemk,

This issue is a bit stale, and it's not clear if there is still an issue in the latest version of the agent. Please give it a try.

I am going to close this issue under the assumption that we have fixed the issue, but if not please feel free to reopen the issue. Thanks!

@txase txase closed this as completed Jun 26, 2014
@aseemk
Copy link
Author

aseemk commented Aug 8, 2014

Sorry for the delay here guys, and thanks for the work!

We are going to be upgrading our node-newrelic soon and will report back!

cmcadams-newrelic pushed a commit to cmcadams-newrelic/node-newrelic that referenced this issue Jan 29, 2024
…3b02682b68b783662c79c

[Snyk] Upgrade newrelic from 8.13.2 to 8.17.1
jsumners-nr pushed a commit to jsumners-nr/node-newrelic that referenced this issue Apr 16, 2024
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

No branches or pull requests

4 participants