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

Support for mysql connection pools. #119

Closed
atishay opened this issue Feb 19, 2014 · 8 comments
Closed

Support for mysql connection pools. #119

atishay opened this issue Feb 19, 2014 · 8 comments

Comments

@atishay
Copy link

atishay commented Feb 19, 2014

The newrelic module doesn't seems to support node-mysql 2.0.0's connection pool and pool clusters.

@alicegoldfuss
Copy link

Hi there,

Thanks for checking out our Node.js agent! We do support connection pooling and would love to get more info about what you are seeing. Would you mind submitting a ticket through http://support.newrelic.com with more details? We are especially interested in:

  • A permalink to the app you are having issues with (if it appears in the UI at all). To create a permalink to any page within the New Relic user interface, scroll to the bottom and click 'Permalink ∞' all the way on the right next to 'Kiosk Mode.' This will show us the exact page and time period that you are observing.
  • The file/script where you are integrating our agent. In other words, where you added require('newrelic');.
  • A description of your environment/stack.
  • Any error traces, if available. How often are these errors occurring? Are you only experiencing them when using our agent?

Thanks!
Alice

@atishay
Copy link
Author

atishay commented Feb 20, 2014

Hi, in instrumentation newrelic is only wrapping the createConnection method. We never use this. Instead we use PoolCluster.getConnection. No mysql data is seen if we use Pool or PoolCluster in node-mysql. Wrapping this is not trivial. Pool internally uses the constructor directly though a event is raised so the we can wrap the connection when listening to connection event. But PoolCluster doesn't even expose that.

@groundwater
Copy link
Contributor

Hi @atishay811, I had a look through node-mysql and you're absolutely right, using PoolCluster will bypass the current instrumentation.

We can definitely look at including this feature in an upcoming release. I will leave this issue open, but you may also want to submit a support ticket to help prioritize your feature request. If not, I'll also keep this issue updated to let you know when it appears on our roadmap.

@sidorares
Copy link

@groundwater I'd rather prefer all Pool* to call createConnection internally - please post your suggestions to node-mysql

I'm also interested to see what do I need to add to node-mysql2 for newrelic support

@atishay
Copy link
Author

atishay commented Feb 25, 2014

node-mysql also supports events. Unluckily, even that implementation is incomplete. Pool has an event for a new connection created but PoolCluster does not forward that event. Nor does it have an event for Pool created. The add calls to the cluster could be wrapped but that isn't a neat solution.

Meanwhile for others that come to this thread, I have a workaround. I do not use the raw createConnection call and have a PoolCluster present. At the startup before requiring newrelic, I create our poolcluster and modify the createConnection.

poolCluster = mysql.createPoolCluster();
pool.add("READ", {...});

var connection = null;
mysql.createConnection = function () {
  return connection;
};

poolCluster._nodes.READ.pool.on("connection", function (newConnection) {
  connection = newConnection;
  mysql.createConnection();
});

This approach is very fragile and will break with changes to newrelic or mysql, but is good enough to get working for now.

@etiennea
Copy link

plus one for added pool support to the agent!

@etiennea
Copy link

Still no solution, node-mysql has been updated the events are more consitent for pools this should be easy to fix. I have been waiting for months!

@wraithan
Copy link
Contributor

Easy to fix is relative. Instrumentation isn't the most simple problem due to how the call stack gets sliced up when you do something async. That said, +1 on these public issues don't translate very well to affecting our roadmap. Which is part of the reason we are in the process of phasing them out. Please contact support via https://support.newrelic.com/ so we can better serve you and track your request.

@wesrog wesrog closed this as completed Jun 10, 2019
cmcadams-newrelic pushed a commit to cmcadams-newrelic/node-newrelic that referenced this issue Jan 29, 2024
…/messaging-app/word-wrap-1.2.4

chore(deps-dev): bump word-wrap from 1.2.3 to 1.2.4 in /messaging-app
jsumners-nr pushed a commit to jsumners-nr/node-newrelic that referenced this issue Apr 11, 2024
jsumners-nr pushed a commit to jsumners-nr/node-newrelic that referenced this issue Apr 16, 2024
bizob2828 pushed a commit to bizob2828/node-newrelic that referenced this issue Apr 19, 2024
bizob2828 pushed a commit to bizob2828/node-newrelic that referenced this issue Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants