CodeFoodPixels / node-promise-mysql

A wrapper for mysqljs/mysql that wraps function calls with Bluebird promises.
MIT License
338 stars 64 forks source link

add support for `AWS X-Ray` wrapped `mysql` module #46

Closed Kosta-Github closed 5 years ago

Kosta-Github commented 7 years ago

To inject an augmented mysql module (e.g., wrapped for tracing), an additional mysql property can be passed in as part of the config object. E.g., for using an AWS X-Ray wrapped mysql implementation use something like this:

var AWSXRay   = require('aws-xray-sdk');
var mysqlXRay = AWSXRay.captureMySQL(require('mysql'));
var mysql     = require('promise-mysql');
var connection;

mysql.createConnection({
    mysql: mysqlXRay,
    host: 'localhost',
    user: 'sauron',
    password: 'theonetruering',
    database: 'mordor'
}).then(function(conn){
    connection = conn;
});
CodeFoodPixels commented 7 years ago

Thanks for this, but I'm unsure. Passing a MySQL module in introduces potential breakages if the user uses a more up to date or older version of the node MySQL module. I like the idea of being able to use a wrapped module, but perhaps a different approach is needed.

marc-at commented 7 years ago

Is it possible to pass the wrapper function in the config?

CodeFoodPixels commented 7 years ago

Yeah, I've had an idea based around that. I'll have a shot at it tomorrow.

CodeFoodPixels commented 7 years ago

Just to update you, I started putting the ability to pass a wrapper in and sort of started doing a bit of a rewrite too. You can have a look in the mysql-wrapper branch

Kosta-Github commented 7 years ago

any news/progress on this?

CodeFoodPixels commented 7 years ago

Sorry, I've been preoccupied with other things. I'll be getting back to it this week.

On Mon, 10 Jul 2017 at 14:59 Kosta notifications@github.com wrote:

any news/progress on this?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/lukeb-uk/node-promise-mysql/pull/46#issuecomment-314113858, or mute the thread https://github.com/notifications/unsubscribe-auth/AEjy1NO1WD9DIilsX5tHvHhYhKNiRdSQks5sMi4lgaJpZM4ND9rk .

Kosta-Github commented 7 years ago

I didn't want to rush you; just checking in again into this topic since I am again trying to use your lib together with the AWS x-ray SDK...

CodeFoodPixels commented 6 years ago

Just to let you know, I've been working on the tests this week for the mysql-wrapper branch. Once the tests are done I'll be pushing it up.

Sorry it's taken a while.

robertbullen commented 5 years ago

I'd really like to be able to instrument an AWS Lambda function that uses promise-mysql with X-Ray as well. But this PR looks like it has been abandoned. Is there an alternative way of accomplishing this besides what has been proposed by this PR?

CodeFoodPixels commented 5 years ago

@Kosta-Github @robertbullen If you're still looking to wrap mysql with x-ray, please give the beta of v4 a go: https://www.npmjs.com/package/promise-mysql/v/4.0.0-beta.0

If you have any feedback, please post them in #111

I'm going to close this.