Skip to content

Commit

Permalink
Change to ProcID, add unwrap, remove unnecessary binding
Browse files Browse the repository at this point in the history
Also add execBatchSql tests
  • Loading branch information
rishabh committed Jul 3, 2019
1 parent 9c0ed43 commit c4f46fe
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 17 deletions.
22 changes: 12 additions & 10 deletions packages/datadog-plugin-tedious/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,7 @@ const Kinds = require('../../../ext/kinds')
const analyticsSampler = require('../../dd-trace/src/analytics_sampler')
const tx = require('../../dd-trace/src/plugins/util/tx')

const sqlMapping = {
'sp_execute': 'execute',
'sp_prepare': 'prepare',
'sp_unprepare': 'unprepare'
}
const SQL_BATCH = 1

function createWrapRequestClass (tracer) {
return function wrapRequestClass (Request) {
Expand Down Expand Up @@ -46,7 +42,7 @@ function createWrapMakeRequest (tracer, config) {
const query = getQuery(request)

if (!query) {
return scope.activate(childOf, () => makeRequest.apply(this, arguments))
return makeRequest.apply(this, arguments)
}

const span = tracer.startSpan(`tedious.request`, {
Expand All @@ -63,7 +59,10 @@ function createWrapMakeRequest (tracer, config) {

addConnectionTags(span, connectionConfig)
addDatabaseTags(span, connectionConfig)
addQueryTags(span, request)

if (packetType !== SQL_BATCH) {
addProcIdTags(span, request)
}

analyticsSampler.sample(span, config.analytics)
request.callback = tx.wrap(span, request.callback)
Expand Down Expand Up @@ -106,10 +105,9 @@ function addDatabaseTags (span, connectionConfig) {
span.setTag('db.instance', connectionConfig.options.instanceName)
}

function addQueryTags (span, request) {
function addProcIdTags (span, request) {
const transformedPacketType = request.sqlTextOrProcedure
const resourceType = sqlMapping[transformedPacketType] || 'query'
span.setTag('resource.type', resourceType)
span.setTag('request.procid', transformedPacketType)
}

module.exports = [
Expand All @@ -128,6 +126,10 @@ module.exports = [
},
unpatch (tedious) {
this.unwrap(tedious.Connection.prototype, 'makeRequest')

if (tedious.BulkLoad && tedious.BulkLoad.prototype.getRowStream) {
this.unwrap(tedious.BulkLoad.prototype, 'getRowStream')
}
}
}
]
34 changes: 27 additions & 7 deletions packages/datadog-plugin-tedious/test/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ describe('Plugin', () => {
expect(traces[0][0]).to.have.property('service', 'test-mssql')
expect(traces[0][0]).to.have.property('resource', query)
expect(traces[0][0]).to.have.property('type', 'sql')
expect(traces[0][0].meta).to.have.property('resource.type', 'query')
expect(traces[0][0].meta).to.have.property('request.procid', 'sp_executesql')
expect(traces[0][0].meta).to.have.property('component', 'tedious')
expect(traces[0][0].meta).to.have.property('db.name', 'master')
expect(traces[0][0].meta).to.have.property('db.user', 'sa')
Expand All @@ -133,7 +133,7 @@ describe('Plugin', () => {
expect(traces[0][0]).to.have.property('name', 'tedious.request')
expect(traces[0][0]).to.have.property('service', 'test-mssql')
expect(traces[0][0]).to.have.property('resource', query)
expect(traces[0][0].meta).to.have.property('resource.type', 'query')
expect(traces[0][0].meta).to.have.property('request.procid', 'sp_executesql')
})
.then(done)
.catch(done)
Expand All @@ -145,6 +145,26 @@ describe('Plugin', () => {
connection.execSql(request)
})

it('should handle batch queries', done => {
const query = 'SELECT 1 + 1 AS solution1;\n' +
'SELECT 1 + 2 AS solution2'

agent
.use(traces => {
expect(traces[0][0]).to.have.property('name', 'tedious.request')
expect(traces[0][0]).to.have.property('service', 'test-mssql')
expect(traces[0][0]).to.have.property('resource', query)
expect(traces[0][0].meta).to.not.have.property('request.procid')
})
.then(done)
.catch(done)

const request = new tds.Request(query, (err) => {
if (err) done(err)
})
connection.execSqlBatch(request)
})

it('should handle prepare requests', done => {
const query = 'SELECT 1 + @num AS solution'

Expand All @@ -153,7 +173,7 @@ describe('Plugin', () => {
expect(traces[0][0]).to.have.property('name', 'tedious.request')
expect(traces[0][0]).to.have.property('service', 'test-mssql')
expect(traces[0][0]).to.have.property('resource', query)
expect(traces[0][0].meta).to.have.property('resource.type', 'prepare')
expect(traces[0][0].meta).to.have.property('request.procid', 'sp_prepare')
})
.then(done)
.catch(done)
Expand All @@ -171,7 +191,7 @@ describe('Plugin', () => {
expect(traces[0][0]).to.have.property('name', 'tedious.request')
expect(traces[0][0]).to.have.property('service', 'test-mssql')
expect(traces[0][0]).to.have.property('resource', 'SELECT 1 + @num AS solution')
expect(traces[0][0].meta).to.have.property('resource.type', 'execute')
expect(traces[0][0].meta).to.have.property('request.procid', 'sp_execute')
})
.then(done)
.catch(done)
Expand All @@ -194,7 +214,7 @@ describe('Plugin', () => {
expect(traces[0][0]).to.have.property('name', 'tedious.request')
expect(traces[0][0]).to.have.property('service', 'test-mssql')
expect(traces[0][0]).to.have.property('resource', query)
expect(traces[0][0].meta).to.have.property('resource.type', 'unprepare')
expect(traces[0][0].meta).to.have.property('request.procid', 'sp_unprepare')
})
.then(done)
.catch(done)
Expand Down Expand Up @@ -287,7 +307,7 @@ describe('Plugin', () => {
.use(traces => {
expect(traces[0][0]).to.have.property('name', 'tedious.request')
expect(traces[0][0]).to.have.property('resource', bulkLoad.getBulkInsertSql())
expect(traces[0][0].meta).to.have.property('resource.type', 'query')
expect(traces[0][0].meta).to.not.have.property('request.procid')
})
.then(done)
.catch(done)
Expand All @@ -304,7 +324,7 @@ describe('Plugin', () => {
.use(traces => {
expect(traces[0][0]).to.have.property('name', 'tedious.request')
expect(traces[0][0]).to.have.property('resource', bulkLoad.getBulkInsertSql())
expect(traces[0][0].meta).to.have.property('resource.type', 'query')
expect(traces[0][0].meta).to.not.have.property('request.procid')
})
.then(done)
.catch(done)
Expand Down

0 comments on commit c4f46fe

Please sign in to comment.