feat: Propagate agent root context when opentelemetry ROOT_CONTEXT is passed in to trace propagator. Added logic to handle properly naming and ending transactions for server spans. (#2940)

Author: bizob2828

lib/otel/constants.js

@@ -68,6 +68,11 @@ module.exports = {
    */
   ATTR_FULL_URL: 'url.full',
 
+  /**
+   * The [numeric status code](https://github.com/grpc/grpc/blob/v1.33.2/doc/statuscodes.md) of the gRPC request.
+   */
+  ATTR_GRPC_STATUS_CODE: 'rpc.grpc.status_code',
+
   /**
    * Value of the HTTP `host` header.
    *
@@ -101,6 +106,20 @@ module.exports = {
    */
   ATTR_HTTP_URL: 'http.url',
 
+  /**
+   * The http response status code
+   *
+   * @example 200
+   */
+  ATTR_HTTP_STATUS_CODE: 'http.response.status_code',
+
+  /**
+   * The http response status text
+   *
+   * @example OK
+   */
+  ATTR_HTTP_STATUS_TEXT: 'http.status_text',
+
   /**
    * The message destination name.
    *
@@ -174,6 +193,15 @@ module.exports = {
    * @example /tmp/my.sock
    */
   ATTR_SERVER_ADDRESS: 'server.address',
+  ATTR_NET_HOST_NAME: 'net.host.name',
+
+  /**
+   * Poort of the local HTTP server that received the request.
+   *
+   * @example 80
+   */
+  ATTR_SERVER_PORT: 'server.port',
+  ATTR_NET_HOST_PORT: 'net.host.port',
 
   /**
    * Logical name of the local service being instrumented.

lib/otel/segments/server.js

@@ -9,12 +9,12 @@ const Transaction = require('../../transaction')
 const httpRecorder = require('../../metrics/recorders/http')
 const urltils = require('../../util/urltils')
 const url = require('node:url')
+const { NODEJS, ACTION_DELIMITER } = require('../../metrics/names')
 
 const DESTINATION = Transaction.DESTINATIONS.TRANS_COMMON
 const {
   ATTR_HTTP_METHOD,
   ATTR_HTTP_REQUEST_METHOD,
-  ATTR_HTTP_ROUTE,
   ATTR_HTTP_URL,
   ATTR_RPC_METHOD,
   ATTR_RPC_SERVICE,
@@ -24,15 +24,15 @@ const {
 module.exports = function createServerSegment(agent, otelSpan) {
   const transaction = new Transaction(agent)
   transaction.type = 'web'
+  transaction.nameState.setPrefix(NODEJS.PREFIX)
+  transaction.nameState.setPrefix(ACTION_DELIMITER)
   const rpcSystem = otelSpan.attributes[ATTR_RPC_SYSTEM]
   const httpMethod = otelSpan.attributes[ATTR_HTTP_METHOD] ?? otelSpan.attributes[ATTR_HTTP_REQUEST_METHOD]
   let segment
   if (rpcSystem) {
     segment = rpcSegment({ agent, otelSpan, transaction, rpcSystem })
-  } else if (httpMethod) {
-    segment = httpSegment({ agent, otelSpan, transaction, httpMethod })
   } else {
-    segment = genericHttpSegment({ agent, transaction })
+    segment = httpSegment({ agent, otelSpan, transaction, httpMethod })
   }
   transaction.baseSegment = segment
   return { segment, transaction }
@@ -41,11 +41,12 @@ module.exports = function createServerSegment(agent, otelSpan) {
 function rpcSegment({ agent, otelSpan, transaction, rpcSystem }) {
   const rpcService = otelSpan.attributes[ATTR_RPC_SERVICE] || 'Unknown'
   const rpcMethod = otelSpan.attributes[ATTR_RPC_METHOD] || 'Unknown'
-  const name = `WebTransaction/WebFrameworkUri/${rpcSystem}/${rpcService}.${rpcMethod}`
-  transaction.name = name
-  transaction.trace.attributes.addAttribute(DESTINATION, 'request.method', rpcMethod)
-  transaction.trace.attributes.addAttribute(DESTINATION, 'request.uri', name)
+  const name = `${rpcService}/${rpcMethod}`
   transaction.url = name
+  transaction.trace.attributes.addAttribute(DESTINATION, 'request.method', rpcMethod)
+  transaction.trace.attributes.addAttribute(DESTINATION, 'request.uri', transaction.url)
+  transaction.nameState.setPrefix(rpcSystem)
+  transaction.nameState.appendPath(transaction.url)
   const segment = agent.tracer.createSegment({
     name,
     recorder: httpRecorder,
@@ -56,34 +57,22 @@ function rpcSegment({ agent, otelSpan, transaction, rpcSystem }) {
   return segment
 }
 
-// most instrumentation will hit this case
-// I find that if the request is in a web framework, the web framework instrumentation
-// sets `http.route` and when the span closes it pulls that attribute in
-// we'll most likely need to wire up some naming reconciliation
-// to handle this use case.
 function httpSegment({ agent, otelSpan, transaction, httpMethod }) {
-  const httpRoute = otelSpan.attributes[ATTR_HTTP_ROUTE] || 'Unknown'
   const httpUrl = otelSpan.attributes[ATTR_HTTP_URL] || '/Unknown'
+  transaction.nameState.setVerb(httpMethod)
   const requestUrl = url.parse(httpUrl, true)
-  const name = `WebTransaction/Nodejs/${httpMethod}/${httpRoute}`
-  transaction.name = name
+  transaction.parsedUrl = requestUrl
   transaction.url = urltils.obfuscatePath(agent.config, requestUrl.pathname)
   transaction.trace.attributes.addAttribute(DESTINATION, 'request.uri', transaction.url)
-  transaction.trace.attributes.addAttribute(DESTINATION, 'request.method', httpMethod)
-  return agent.tracer.createSegment({
-    name,
-    recorder: httpRecorder,
-    parent: transaction.trace.root,
-    transaction
-  })
-}
-
-function genericHttpSegment({ agent, transaction }) {
-  const name = 'WebTransaction/NormalizedUri/*'
-  transaction.name = name
+  if (httpMethod) {
+    transaction.trace.attributes.addAttribute(DESTINATION, 'request.method', httpMethod)
+  }
+  transaction.applyUserNamingRules(requestUrl.pathname)
+  // accept dt headers?
+  // synthetics.assignHeadersToTransaction(agent.config, transaction, )
   return agent.tracer.createSegment({
-    name,
     recorder: httpRecorder,
+    name: requestUrl.pathname,
     parent: transaction.trace.root,
     transaction
   })

lib/otel/setup.js

@@ -10,6 +10,7 @@ const NrSpanProcessor = require('./span-processor')
 const ContextManager = require('./context-manager')
 const defaultLogger = require('../logger').child({ component: 'opentelemetry-bridge' })
 const createOtelLogger = require('./logger')
+const TracePropagator = require('./trace-propagator')
 
 const { ATTR_SERVICE_NAME } = require('./constants')
 
@@ -34,8 +35,8 @@ module.exports = function setupOtel(agent, logger = defaultLogger) {
 
   })
   provider.register({
-    contextManager: new ContextManager(agent)
-    // propagator: // todo: https://github.com/newrelic/node-newrelic/issues/2662
+    contextManager: new ContextManager(agent),
+    propagator: new TracePropagator(agent)
   })
 
   agent.metrics

lib/otel/span-processor.js

@@ -7,16 +7,25 @@
 const SegmentSynthesizer = require('./segment-synthesis')
 const { otelSynthesis } = require('../symbols')
 const { hrTimeToMilliseconds } = require('@opentelemetry/core')
+const { SpanKind } = require('@opentelemetry/api')
 const urltils = require('../util/urltils')
 const {
   ATTR_DB_NAME,
   ATTR_DB_STATEMENT,
   ATTR_DB_SYSTEM,
-  ATTR_HTTP_HOST,
+  ATTR_GRPC_STATUS_CODE,
+  ATTR_HTTP_ROUTE,
+  ATTR_HTTP_STATUS_CODE,
+  ATTR_HTTP_STATUS_TEXT,
   ATTR_NET_PEER_NAME,
   ATTR_NET_PEER_PORT,
-  ATTR_SERVER_ADDRESS,
+  ATTR_NET_HOST_NAME,
+  ATTR_NET_HOST_PORT,
+  ATTR_RPC_SYSTEM,
+  ATTR_SERVER_PORT,
+  ATTR_SERVER_ADDRESS
 } = require('./constants')
+const { DESTINATIONS } = require('../config/attribute-filter')
 
 module.exports = class NrSpanProcessor {
   constructor(agent) {
@@ -42,9 +51,9 @@ module.exports = class NrSpanProcessor {
    */
   onEnd(span) {
     if (span[otelSynthesis] && span[otelSynthesis].segment) {
-      const { segment } = span[otelSynthesis]
+      const { segment, transaction } = span[otelSynthesis]
       this.updateDuration(segment, span)
-      this.reconcileAttributes(segment, span)
+      this.reconcileAttributes({ segment, span, transaction })
       delete span[otelSynthesis]
     }
   }
@@ -55,14 +64,79 @@ module.exports = class NrSpanProcessor {
     segment.overwriteDurationInMillis(duration)
   }
 
-  // TODO: clean this up and break out by span.kind
-  reconcileAttributes(segment, span) {
+  reconcileAttributes({ segment, span, transaction }) {
+    if (span.kind === SpanKind.SERVER) {
+      this.reconcileServerAttributes({ segment, span, transaction })
+    } else if (span.kind === SpanKind.CLIENT && span.attributes[ATTR_DB_SYSTEM]) {
+      this.reconcileDbAttributes({ segment, span })
+    }
+    // TODO: add http external checks
+  }
+
+  reconcileServerAttributes({ segment, span, transaction }) {
+    if (span.attributes[ATTR_RPC_SYSTEM]) {
+      this.reconcileRpcAttributes({ segment, span, transaction })
+    } else {
+      this.reconcileHttpAttributes({ segment, span, transaction })
+    }
+
+    // End the corresponding transaction for the entry point server span.
+    // We do then when the span ends to ensure all data has been processed
+    // for the correspondig server span.
+    transaction.end()
+  }
+
+  reconcileHttpAttributes({ segment, span, transaction }) {
+    for (const [prop, value] of Object.entries(span.attributes)) {
+      let key = prop
+      let sanitized = value
+      if (key === ATTR_HTTP_ROUTE) {
+        // TODO: can we get the route params?
+        transaction.nameState.appendPath(sanitized)
+      } else if (key === ATTR_HTTP_STATUS_CODE) {
+        transaction.finalizeNameFromUri(transaction.parsedUrl, sanitized)
+        transaction.trace.attributes.addAttribute(DESTINATIONS.TRANS_COMMON, 'http.statusCode', sanitized)
+        key = 'http.statusCode'
+      // Not using const as it is not in semantic-conventions
+      } else if (key === ATTR_HTTP_STATUS_TEXT) {
+        transaction.trace.attributes.addAttribute(DESTINATIONS.TRANS_COMMON, 'http.statusText', sanitized)
+        key = 'http.statusText'
+      } else if (key === ATTR_SERVER_PORT || key === ATTR_NET_HOST_PORT) {
+        key = 'port'
+      } else if (key === ATTR_SERVER_ADDRESS || key === ATTR_NET_HOST_NAME) {
+        key = 'host'
+        if (urltils.isLocalhost(sanitized)) {
+          sanitized = this.agent.config.getHostnameSafe(sanitized)
+        }
+      }
+
+      // TODO: otel instrumentation does not collect headers
+      // a customer can specify which ones, we also specify this
+      // so i think we'd have to cross reference our list
+      // it also looks like we add all headers to the trace
+      // this isn't doing that
+      segment.addAttribute(key, sanitized)
+    }
+  }
+
+  // TODO: our grpc instrumentation handles errors when the status code is not 0
+  // we should prob do this here too
+  reconcileRpcAttributes({ segment, span, transaction }) {
+    for (const [prop, value] of Object.entries(span.attributes)) {
+      if (prop === ATTR_GRPC_STATUS_CODE) {
+        transaction.trace.attributes.addAttribute(DESTINATIONS.TRANS_COMMON, 'response.status', value)
+      }
+      segment.addAttribute(prop, value)
+    }
+  }
+
+  reconcileDbAttributes({ segment, span }) {
     for (const [prop, value] of Object.entries(span.attributes)) {
       let key = prop
       let sanitized = value
       if (key === ATTR_NET_PEER_PORT) {
         key = 'port_path_or_id'
-      } else if (prop === ATTR_NET_PEER_NAME || prop === ATTR_SERVER_ADDRESS || prop === ATTR_HTTP_HOST) {
+      } else if (prop === ATTR_NET_PEER_NAME) {
         key = 'host'
         if (urltils.isLocalhost(sanitized)) {
           sanitized = this.agent.config.getHostnameSafe(sanitized)