[WIP] Basic tedious instrumentation

[kjvalencik]

Hi! We're currently working through New Relic instrumentation and MSSQL support is a must. This PR includes basic instrumentation.

However, it does not include any tests and may only work for specific versions of tedious. I mostly wanted to submit this to gauge interest and check to see if this might be something that is already on the road map.

Let me know if I'm the right track. Thanks!

[martinkuba]

@kjvalencik Thanks for the PR.  MS SQL is currently not on our near-term road map, but we are open to adding this in. Our main challenge would be to provide full test coverage with integration tests, as we are not currently setup to run tests with MS SQL.  In the meantime, we are happy to help you with this PR, but we would have to rely on you to fully test it.

[StyleT]

+1 for this PR

[kjvalencik]

Working through the unit tests right now. Is there a preferred method of skipping integration tests because dependencies (e.g., MSSQL instance) can't be met?

Maybe guard with an environment variable?

[martinkuba]

@kjvalencik We don't currently disable any tests based on environment (except versions of Node and modules). Tests take the DB config from this file
https://github.com/newrelic/node-newrelic/blob/master/test/lib/params.js

I would suggest that you add to this file for now, and we will probably want to update our test suite in general to skip tests that don't have the settings they need.

[hayes]

there is this: https://github.com/newrelic/node-newrelic/blob/master/test/integration/instrumentation/oracle.tap.js#L26-L39

[gfguthrie]

@kjvalencik I've been working on expanding the start that you've made (though so far have only made changes directly relevant to my project), and I'm wondering if you use tedious directly in your project or if you use the node-mssql library and thus also if it makes sense to hook into it instead to also support the other drivers that node-mssql supports.

[kjvalencik]

We mostly use tedious via Sequelize, but also use it with node-mssql. The best I can tell tedious is by far the most popular driver which, in my opinion, makes it a better target than node-mssql.

While we do use this fork in production, it only targets the features that Sequelize uses. I have been waiting for the new version of New Relic to land, which includes a plugins system so that this can be developed as a plugins instead of included in core before spending substantial time on it.

[martinkuba]

@gfguthrie @kjvalencik I would encourage you to start using the beta now. Seeing people use it and getting feedback will expedite making it a general release. Also note that the main difference between the 1.x line and the 2.x beta is the new API and our datastore instrumentations using it - the rest of the agent is pretty much the same.

https://docs.newrelic.com/docs/agents/nodejs-agent/nodejs-agent-v20-instrumentation-api-beta

[gfguthrie]

Just in case anyone comes looking, this is what I ended up with to instrument tedious.callProcedure

// New Relic custom MSSQL SP instrumentation
newrelic.instrumentDatastore('tedious', function (shim, tedious) {
	shim.setDatastore('MSSQL');
	shim.setParser(sql => ({operation: sql, collection: 'sp'}));
  
	shim.recordQuery(tedious.Connection.prototype, 'callProcedure', {
		query: (shim, func, name, args) => args[0].sqlTextOrProcedure,
		callback: function (shim, func, name, segment, args) {
			args[0].callback = shim.bindSegment(args[0].callback, segment);
		}
	});
});

[kjvalencik]

Thanks @gfguthrie. We will be open sourcing our tedious plugin soon. I'll be sure to include this! CC: @CKarper

[psvet]

@gfguthrie Thanks for the update! Due to the merge conflicts and how long it's been on the shelf, we're going to close this PR, but feel free to add some tests and submit an updated one in the future.

[CKarper]

https://github.com/ckarper/tedious-newrelic

Just require this lib, and you're gtg. Proper docs, etc forthcoming. I will integrate your source as well @gfguthrie . Thanks!