remove async storage from http2 server instrumentation (#5947)

* remove async storage from http2 instrumentation

Author: wconti27

packages/datadog-instrumentations/src/http2/server.js

@@ -5,14 +5,13 @@
 
 const {
   channel,
-  addHook,
-  AsyncResource
+  addHook
 } = require('../helpers/instrument')
 const shimmer = require('../../../datadog-shimmer')
 
 const startServerCh = channel('apm:http2:server:request:start')
 const errorServerCh = channel('apm:http2:server:request:error')
-const finishServerCh = channel('apm:http2:server:request:finish')
+const emitCh = channel('apm:http2:server:response:emit')
 
 const names = ['http2', 'node:http2']
 
@@ -30,18 +29,14 @@ function wrapCreateServer (createServer) {
   }
 }
 
-function wrapResponseEmit (emit) {
-  const asyncResource = new AsyncResource('bound-anonymous-fn')
+function wrapResponseEmit (emit, ctx) {
   return function (eventName, event) {
-    return asyncResource.runInAsyncScope(() => {
-      if (eventName === 'close' && finishServerCh.hasSubscribers) {
-        finishServerCh.publish({ req: this.req })
-      }
-
-      return emit.apply(this, arguments)
-    })
+    ctx.req = this.req
+    ctx.eventName = eventName
+    return emitCh.runStores(ctx, emit, this, ...arguments)
   }
 }
+
 function wrapEmit (emit) {
   return function (eventName, req, res) {
     if (!startServerCh.hasSubscribers) {
@@ -51,18 +46,17 @@ function wrapEmit (emit) {
     if (eventName === 'request') {
       res.req = req
 
-      const asyncResource = new AsyncResource('bound-anonymous-fn')
-      return asyncResource.runInAsyncScope(() => {
-        startServerCh.publish({ req, res })
-
-        shimmer.wrap(res, 'emit', wrapResponseEmit)
+      const ctx = { req, res }
+      return startServerCh.runStores(ctx, () => {
+        shimmer.wrap(res, 'emit', emit => wrapResponseEmit(emit, ctx))
 
         try {
           return emit.apply(this, arguments)
-        } catch (err) {
-          errorServerCh.publish(err)
+        } catch (error) {
+          ctx.error = error
+          errorServerCh.publish(ctx)
 
-          throw err
+          throw error
         }
       })
     }

packages/datadog-plugin-child_process/test/index.spec.js

@@ -52,6 +52,7 @@ describe('Child process plugin', () => {
         expect(tracerStub.startSpan).to.have.been.calledOnceWithExactly(
           'command_execution',
           {
+            startTime: undefined,
             childOf: undefined,
             tags: {
               component: 'subprocess',
@@ -75,6 +76,7 @@ describe('Child process plugin', () => {
         expect(tracerStub.startSpan).to.have.been.calledOnceWithExactly(
           'command_execution',
           {
+            startTime: undefined,
             childOf: undefined,
             tags: {
               component: 'subprocess',
@@ -100,6 +102,7 @@ describe('Child process plugin', () => {
         expect(tracerStub.startSpan).to.have.been.calledOnceWithExactly(
           'command_execution',
           {
+            startTime: undefined,
             childOf: undefined,
             tags: {
               component: 'subprocess',
@@ -126,6 +129,7 @@ describe('Child process plugin', () => {
         expect(tracerStub.startSpan).to.have.been.calledOnceWithExactly(
           'command_execution',
           {
+            startTime: undefined,
             childOf: undefined,
             tags: {
               component: 'subprocess',
@@ -153,6 +157,7 @@ describe('Child process plugin', () => {
         expect(tracerStub.startSpan).to.have.been.calledOnceWithExactly(
           'command_execution',
           {
+            startTime: undefined,
             childOf: undefined,
             tags: {
               component: 'subprocess',
@@ -180,6 +185,7 @@ describe('Child process plugin', () => {
         expect(tracerStub.startSpan).to.have.been.calledOnceWithExactly(
           'command_execution',
           {
+            startTime: undefined,
             childOf: undefined,
             tags: {
               component: 'subprocess',

packages/datadog-plugin-http/src/server.js

@@ -11,16 +11,16 @@ class HttpServerPlugin extends ServerPlugin {
     return 'http'
   }
 
+  static get prefix () {
+    return 'apm:http:server:request'
+  }
+
   constructor (...args) {
     super(...args)
     this._parentStore = undefined
     this.addTraceSub('exit', message => this.exit(message))
   }
 
-  addTraceSub (eventName, handler) {
-    this.addSub(`apm:${this.constructor.id}:server:${this.operation}:${eventName}`, handler)
-  }
-
   start ({ req, res, abortController }) {
     const store = storage('legacy').getStore()
     const span = web.startSpan(

packages/datadog-plugin-http2/src/server.js

@@ -3,21 +3,26 @@
 // Plugin temporarily disabled. See https://github.com/DataDog/dd-trace-js/issues/312
 
 const ServerPlugin = require('../../dd-trace/src/plugins/server')
-const { storage } = require('../../datadog-core')
 const web = require('../../dd-trace/src/plugins/util/web')
 const { COMPONENT } = require('../../dd-trace/src/constants')
 
 class Http2ServerPlugin extends ServerPlugin {
+  constructor (tracer, config) {
+    super(tracer, config)
+    this.addBind('apm:http2:server:response:emit', this.bindEmit)
+  }
+
   static get id () {
     return 'http2'
   }
 
-  addTraceSub (eventName, handler) {
-    this.addSub(`apm:${this.constructor.id}:server:${this.operation}:${eventName}`, handler)
+  static get prefix () {
+    return 'apm:http2:server:request'
   }
 
-  start ({ req, res }) {
-    const store = storage('legacy').getStore()
+  bindStart (ctx) {
+    const { req, res } = ctx
+
     const span = web.startSpan(
       this.tracer,
       {
@@ -26,28 +31,38 @@ class Http2ServerPlugin extends ServerPlugin {
       },
       req,
       res,
-      this.operationName()
+      this.operationName(),
+      ctx
     )
 
     span.setTag(COMPONENT, this.constructor.id)
     span._integrationName = this.constructor.id
 
-    this.enter(span, { ...store, req, res })
+    ctx.currentStore.req = req
+    ctx.currentStore.res = res
 
     const context = web.getContext(req)
 
     if (!context.instrumented) {
       context.res.writeHead = web.wrapWriteHead(context)
       context.instrumented = true
     }
+
+    return ctx.currentStore
   }
 
-  finish ({ req }) {
+  bindEmit (ctx) {
+    if (ctx.eventName !== 'close') return ctx.currentStore
+
+    const { req } = ctx
+
     const context = web.getContext(req)
 
     if (!context || !context.res) return // Not created by a http.Server instance.
 
     web.finishAll(context)
+
+    return ctx.currentStore
   }
 
   error (error) {

packages/dd-trace/src/plugins/plugin.js

@@ -60,7 +60,7 @@ module.exports = class Plugin {
   }
 
   get tracer () {
-    return this._tracer._tracer
+    return this._tracer?._tracer || this._tracer
   }
 
   enter (span, store) {

packages/dd-trace/src/plugins/tracing.js

@@ -103,28 +103,45 @@ class TracingPlugin extends Plugin {
   }
 
   startSpan (name, options = {}, enterOrCtx = true) {
-    let { childOf, integrationName, kind, meta, metrics, service, resource, type } = options
+    // TODO: modularize this code to a helper function
+    let {
+      component = this.component,
+      childOf,
+      integrationName,
+      kind,
+      meta,
+      metrics,
+      service,
+      startTime,
+      resource,
+      type
+    } = options
+
+    const tracer = options.tracer || this.tracer
+    const config = options.config || this.config
+
     const store = storage('legacy').getStore()
     if (store && childOf === undefined) {
       childOf = store.span
     }
 
-    const span = this.tracer.startSpan(name, {
+    const span = tracer.startSpan(name, {
+      startTime,
       childOf,
       tags: {
-        [COMPONENT]: this.component,
-        'service.name': service || this.tracer._service,
+        [COMPONENT]: component,
+        'service.name': service || meta?.service || tracer._service,
         'resource.name': resource,
         'span.kind': kind,
         'span.type': type,
         ...meta,
         ...metrics
       },
-      integrationName: integrationName || this.component,
+      integrationName: integrationName || component,
       links: childOf?._links
     })
 
-    analyticsSampler.sample(span, this.config.measured)
+    analyticsSampler.sample(span, config.measured)
 
     // TODO: Remove this after migration to TracingChannel is done.
     if (enterOrCtx === true) {

packages/dd-trace/src/plugins/util/inferred_proxy.js

@@ -22,7 +22,7 @@ const supportedProxies = {
   }
 }
 
-function createInferredProxySpan (headers, childOf, tracer, context) {
+function createInferredProxySpan (headers, childOf, tracer, reqCtx, traceCtx, config, startSpanHelper) {
   if (!headers) {
     return null
   }
@@ -41,26 +41,22 @@ function createInferredProxySpan (headers, childOf, tracer, context) {
 
   log.debug('Successfully extracted inferred span info %s for proxy:', proxyContext, proxyContext.proxySystemName)
 
-  const span = tracer.startSpan(
-    proxySpanInfo.spanName,
-    {
-      childOf,
-      type: 'web',
-      startTime: proxyContext.requestTime,
-      integrationName: proxySpanInfo.component,
-      tags: {
-        service: proxyContext.domainName || tracer._config.service,
-        component: proxySpanInfo.component,
-        [SPAN_TYPE]: 'web',
-        [HTTP_METHOD]: proxyContext.method,
-        [HTTP_URL]: proxyContext.domainName + proxyContext.path,
-        stage: proxyContext.stage
-      }
+  const span = startSpanHelper(tracer, proxySpanInfo.spanName, {
+    childOf,
+    type: 'web',
+    startTime: proxyContext.requestTime,
+    integrationName: proxySpanInfo.component,
+    meta: {
+      service: proxyContext.domainName || tracer._config.service,
+      component: proxySpanInfo.component,
+      [SPAN_TYPE]: 'web',
+      [HTTP_METHOD]: proxyContext.method,
+      [HTTP_URL]: proxyContext.domainName + proxyContext.path,
+      stage: proxyContext.stage
     }
-  )
+  }, traceCtx, config)
 
-  tracer.scope().activate(span)
-  context.inferredProxySpan = span
+  reqCtx.inferredProxySpan = span
   childOf = span
 
   log.debug('Successfully created inferred proxy span.')