Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
watson committed Oct 10, 2024
1 parent 29c4fae commit ec2975d
Show file tree
Hide file tree
Showing 10 changed files with 44 additions and 53 deletions.
1 change: 1 addition & 0 deletions CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
/packages/dd-trace/test/appsec/ @DataDog/asm-js

/integration-tests/debugger/ @DataDog/dd-trace-js @DataDog/debugger
/packages/datadog-plugin-code-origin/ @DataDog/dd-trace-js @DataDog/debugger
/packages/dd-trace/src/debugger/ @DataDog/dd-trace-js @DataDog/debugger
/packages/dd-trace/test/debugger/ @DataDog/dd-trace-js @DataDog/debugger

Expand Down
13 changes: 3 additions & 10 deletions packages/datadog-instrumentations/src/fastify.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const { addHook, channel, AsyncResource } = require('./helpers/instrument')

const errorChannel = channel('apm:fastify:middleware:error')
const handleChannel = channel('apm:fastify:request:handle')
const codeOriginForSpansChannel = channel('datadog:code-origin-for-spans')
const routeAddedChannel = channel('apm:fastify:route:added')

const parsingResources = new WeakMap()

Expand All @@ -17,6 +17,7 @@ function wrapFastify (fastify, hasParsingEvents) {

if (!app || typeof app.addHook !== 'function') return app

app.addHook('onRoute', onRoute)
app.addHook('onRequest', onRequest)
app.addHook('preHandler', preHandler)

Expand All @@ -28,11 +29,6 @@ function wrapFastify (fastify, hasParsingEvents) {
app.addHook('preHandler', preValidation)
}

// No need to add the onRoute hook unless Code Origin for Spans is enabled
if (codeOriginForSpansChannel.hasSubscribers) {
app.addHook('onRoute', onRoute)
}

app.addHook = wrapAddHook(app.addHook)

return app
Expand Down Expand Up @@ -162,10 +158,7 @@ function publishError (error, req) {
}

function onRoute (routeOptions) {
codeOriginForSpansChannel.publish({
routeOptions,
topOfStackFunc: onRoute
})
routeAddedChannel.publish({ routeOptions, onRoute })
}

addHook({ name: 'fastify', versions: ['>=3'] }, fastify => {
Expand Down
20 changes: 20 additions & 0 deletions packages/datadog-plugin-code-origin/src/fastify.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
'use strict'

const { entryTag } = require('./tags')
const web = require('../../dd-trace/src/plugins/util/web')

const kCodeOriginForSpansTagsSym = Symbol('datadog.codeOriginForSpansTags')

module.exports = function (plugin) {
plugin.addSub('apm:fastify:request:handle', ({ req, routeConfig }) => {
const tags = routeConfig?.[kCodeOriginForSpansTagsSym]
if (!tags) return
const context = web.getContext(req)
context.span?.addTags(tags)
})

plugin.addSub('apm:fastify:route:added', ({ routeOptions, onRoute }) => {
if (!routeOptions.config) routeOptions.config = {}
routeOptions.config[kCodeOriginForSpansTagsSym] = entryTag(onRoute)
})
}
17 changes: 17 additions & 0 deletions packages/datadog-plugin-code-origin/src/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
'use strict'

const fastify = require('./fastify')
const RouterPlugin = require('../../datadog-plugin-router/src')

class CodeOriginForSpansPlugin extends RouterPlugin {
static get id () {
return 'code-origin-for-spans'
}

constructor (...args) {
super(...args)
fastify(this)
}
}

module.exports = CodeOriginForSpansPlugin
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict'

const { getUserLandFrames } = require('./plugins/util/stacktrace')
const { getUserLandFrames } = require('../../dd-trace/src/plugins/util/stacktrace')

const limit = Number(process.env._DD_CODE_ORIGIN_MAX_USER_FRAMES) || 8

Expand Down
14 changes: 1 addition & 13 deletions packages/datadog-plugin-fastify/src/index.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
'use strict'

const { entryTag } = require('../../dd-trace/src/code_origin')
const RouterPlugin = require('../../datadog-plugin-router/src')

const kCodeOriginForSpansTagsSym = Symbol('datadog.codeOriginForSpansTags')

class FastifyPlugin extends RouterPlugin {
static get id () {
return 'fastify'
Expand All @@ -13,18 +10,9 @@ class FastifyPlugin extends RouterPlugin {
constructor (...args) {
super(...args)

this.addSub('apm:fastify:request:handle', ({ req, routeConfig }) => {
this.addSub('apm:fastify:request:handle', ({ req }) => {
this.setFramework(req, 'fastify', this.config)
const tags = routeConfig?.[kCodeOriginForSpansTagsSym]
if (tags) this.setSpanTags(req, tags)
})

if (this._tracerConfig.codeOriginForSpansEnabled) {
this.addSub('datadog:code-origin-for-spans', ({ routeOptions, topOfStackFunc }) => {
if (!routeOptions.config) routeOptions.config = {}
routeOptions.config[kCodeOriginForSpansTagsSym] = entryTag(topOfStackFunc)
})
}
}
}

Expand Down
4 changes: 0 additions & 4 deletions packages/datadog-plugin-web/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ class WebPlugin extends Plugin {
setFramework (req, name, config) {
web.setFramework(req, name, config)
}

setSpanTags (req, tags) {
web.setSpanTags(req, tags)
}
}

module.exports = WebPlugin
1 change: 1 addition & 0 deletions packages/dd-trace/src/plugins/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ module.exports = {
get bunyan () { return require('../../../datadog-plugin-bunyan/src') },
get 'cassandra-driver' () { return require('../../../datadog-plugin-cassandra-driver/src') },
get child_process () { return require('../../../datadog-plugin-child_process/src') },
get 'code-origin' () { return require('../../../datadog-plugin-code-origin/src') },
get connect () { return require('../../../datadog-plugin-connect/src') },
get couchbase () { return require('../../../datadog-plugin-couchbase/src') },
get cypress () { return require('../../../datadog-plugin-cypress/src') },
Expand Down
9 changes: 0 additions & 9 deletions packages/dd-trace/src/plugins/util/web.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,6 @@ const web = {
web.setConfig(req, config)
},

setSpanTags (req, tags) {
const context = this.patch(req)
const span = context.span

if (!span) return

span.addTags(tags)
},

setConfig (req, config) {
const context = contexts.get(req)
const span = context.span
Expand Down
16 changes: 0 additions & 16 deletions packages/dd-trace/test/plugins/util/web.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -612,22 +612,6 @@ describe('plugins/util/web', () => {
})
})

describe('setSpanTags', () => {
it('should add expected tags', () => {
web.instrument(tracer, config, req, res, 'test.request', span => {
web.setSpanTags(req, { foo: 'bar' })

const tags = span.context()._tags

res.end()

expect(tags).to.include({
foo: 'bar'
})
})
})
})

describe('enterRoute', () => {
beforeEach(() => {
config = web.normalizeConfig(config)
Expand Down

0 comments on commit ec2975d

Please sign in to comment.