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

Include find in instrumented MongoDB operations #235

Closed
wants to merge 1 commit into from

Conversation

sgilroy
Copy link

@sgilroy sgilroy commented Dec 8, 2016

I noticed that find operations were not showing up in transaction traces and performance breakdowns. Looking at mongodb.js revealed that it was missing. Adding find here caused find to show up in the transactions in New Relic. Am I missing something? It seems odd that such a ubiquitous operation as "find" would have been omitted. Perhaps we are doing something else wrong/unusual in our app or how we are integrating with New Relic or MongoDB? Note that we are using Mongoose v4.3.7 at the moment.
image

@sgilroy
Copy link
Author

sgilroy commented Dec 11, 2016

Contrary to my previous theory, it looks like the absence of MongoDB find operations in New Relic is actually not a result of the omission of find from the list of operations to instrument in COLLECTION_OPS in newrelic/lib/instrumentation/mongodb.js. This list of operations only applies to instrumenting older versions of the mongodb driver. The mongodb 2.x driver used by mongoose 4.x is instrumented by newrelic via mongodb's new Application Performance Monitoring API (APM), and the find operation is intentionally excluded from being instrumented by New Relic because find itself has no callback (no asynchronous processing) and only creates a query. So for places in our app where we use find we should expect to see a corresponding toArray operation in New Relic, which is (confusingly) dissimilar to what we will see in New Relic for most other database operations.

Based on the above, I believe this PR is not actually appropriate.

@lykkin
Copy link
Contributor

lykkin commented Dec 12, 2016

@sgilroy thanks for the heads up! this wouldn't be the expected behavior, the find call should show up in metrics/traces. we'll take a look into this and keep you updated.

@michaelgoin
Copy link
Member

@sgilroy closing this PR given its age and looks like the initial intention was not met.

We appreciate you taking a stab at the problem. Capturing Mongo's chaining API, such as the synchronous find, is still an issue we have. We are not sure when it may be fixed, as it has not risen to the top of priorities.

Thanks again and don't hesitate to submit PR's again in the future.

bizob2828 added a commit to bizob2828/node-newrelic that referenced this pull request Apr 19, 2024
bizob2828 added a commit to bizob2828/node-newrelic that referenced this pull request Apr 23, 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

Successfully merging this pull request may close these issues.

None yet

3 participants