chore: Added otel compliant server.address, server.port, and `htt…

…p.request.method` to external http spans (#2169)

lib/instrumentation/core/http-outbound.js

@@ -17,6 +17,7 @@ const http = require('http')
 const synthetics = require('../../synthetics')
 
 const NAMES = require('../../metrics/names')
+const { DESTINATIONS } = require('../../config/attribute-filter')
 
 const DEFAULT_HOST = 'localhost'
 const DEFAULT_HTTP_PORT = 80
@@ -88,14 +89,15 @@ function parseOpts(opts) {
 module.exports = function instrumentOutbound(agent, opts, makeRequest) {
   opts = parseOpts(opts)
   const defaultPort = getDefaultPort(opts)
-  let hostname = getDefaultHostName(opts)
+  const host = getDefaultHostName(opts)
   const port = getPort(opts, defaultPort)
 
-  if (!hostname || port < 1) {
-    logger.warn('Invalid host name (%s) or port (%s) for outbound request.', hostname, port)
+  if (!host || port < 1) {
+    logger.warn('Invalid host name (%s) or port (%s) for outbound request.', host, port)
     return makeRequest(opts)
   }
 
+  let hostname = host
   if (port && port !== defaultPort) {
     hostname += ':' + port
   }
@@ -118,7 +120,7 @@ module.exports = function instrumentOutbound(agent, opts, makeRequest) {
     recordExternal(hostname, 'http'),
     parent,
     false,
-    instrumentRequest.bind(null, agent, opts, makeRequest, hostname)
+    instrumentRequest.bind(null, { agent, opts, makeRequest, host, port, hostname })
   )
 }
 
@@ -127,14 +129,17 @@ module.exports = function instrumentOutbound(agent, opts, makeRequest) {
  * Instruments the request.emit to properly handle the response of the
  * outbound http request
  *
- * @param {Agent} agent New Relic agent
- * @param {string|object} opts a url string or HTTP request options
- * @param {Function} makeRequest function to make request
- * @param {string} hostname host of outbound request
+ * @param {object} params to function
+ * @param {Agent} params.agent New Relic agent
+ * @param {string|object} params.opts a url string or HTTP request options
+ * @param {Function} params.makeRequest function to make request
+ * @param {string} params.hostname host + port of outbound request
+ * @param {string} params.host host of outbound request
+ * @param {string} params.port port of outbound request
  * @param {TraceSegment} segment outbound http segment
  * @returns {http.IncomingMessage} request actual http outbound request
  */
-function instrumentRequest(agent, opts, makeRequest, hostname, segment) {
+function instrumentRequest({ agent, opts, makeRequest, host, port, hostname }, segment) {
   const transaction = segment.transaction
   const outboundHeaders = Object.create(null)
 
@@ -144,7 +149,7 @@ function instrumentRequest(agent, opts, makeRequest, hostname, segment) {
   maybeAddDtCatHeaders(agent, transaction, outboundHeaders, opts?.headers)
   opts.headers = assignOutgoingHeaders(opts.headers, outboundHeaders)
 
-  const request = applySegment(opts, makeRequest, hostname, segment)
+  const request = applySegment({ opts, makeRequest, hostname, host, port, segment })
 
   instrumentRequestEmit(agent, hostname, segment, request)
 
@@ -205,13 +210,16 @@ function assignOutgoingHeaders(currentHeaders, outboundHeaders) {
 /**
  * Starts the http outbound segment and attaches relevant attributes to the segment/span.
  *
- * @param {string|object} opts a url string or HTTP request options
- * @param {Function} makeRequest function to make request
- * @param {string} hostname host of outbound request
- * @param {TraceSegment} segment outbound http segment
+ * @param {object} params to function
+ * @param {string|object} params.opts a url string or HTTP request options
+ * @param {Function} params.makeRequest function to make request
+ * @param {string} params.host host of outbound request
+ * @param {string} params.port port of outbound request
+ * @param {string} params.hostname host + port of outbound request
+ * @param {TraceSegment} params.segment outbound http segment
  * @returns {http.IncomingMessage} request actual http outbound request
  */
-function applySegment(opts, makeRequest, hostname, segment) {
+function applySegment({ opts, makeRequest, host, port, hostname, segment }) {
   segment.start()
   const request = makeRequest(opts)
   const parsed = urltils.scrubAndParseParameters(request.path)
@@ -227,6 +235,8 @@ function applySegment(opts, makeRequest, hostname, segment) {
       segment.addSpanAttribute(`request.parameters.${key}`, parsed.parameters[key])
     }
   }
+  segment.attributes.addAttribute(DESTINATIONS.SPAN_EVENT, 'host', host)
+  segment.attributes.addAttribute(DESTINATIONS.SPAN_EVENT, 'port', port)
   segment.addAttribute('url', `${proto}//${hostname}${parsed.path}`)
   segment.addAttribute('procedure', opts.method || 'GET')
   return request

lib/instrumentation/undici.js

@@ -13,6 +13,7 @@ const symbols = require('../symbols')
 const { executionAsyncResource } = require('async_hooks')
 const diagnosticsChannel = require('diagnostics_channel')
 const synthetics = require('../synthetics')
+const { DESTINATIONS } = require('../config/attribute-filter')
 
 const channels = [
   { channel: diagnosticsChannel.channel('undici:request:create'), hook: requestCreateHook },
@@ -128,6 +129,8 @@ function createExternalSegment({ shim, request, parentSegment }) {
     url.searchParams.forEach((value, key) => {
       segment.addSpanAttribute(`request.parameters.${key}`, value)
     })
+    segment.attributes.addAttribute(DESTINATIONS.SPAN_EVENT, 'host', url.hostname)
+    segment.attributes.addAttribute(DESTINATIONS.SPAN_EVENT, 'port', url.port)
     segment.addAttribute('procedure', request.method || 'GET')
     request[symbols.segment] = segment
   }

lib/spans/span-event.js

@@ -82,7 +82,7 @@ class SpanEvent {
    * category of the segment.
    *
    * @param {TraceSegment}  segment         - The segment to turn into a span event.
-   * @param {?string}       [parentId=null] - The ID of the segment's parent.
+   * @param {?string}       [parentId] - The ID of the segment's parent.
    * @param isRoot
    * @returns {SpanEvent} The constructed event.
    */
@@ -194,8 +194,19 @@ class HttpSpanEvent extends SpanEvent {
       attributes.url = null
     }
 
+    if (attributes.host) {
+      this.addAttribute('server.address', attributes.host)
+      attributes.host = null
+    }
+
+    if (attributes.port) {
+      this.addAttribute('server.port', attributes.port, true)
+      attributes.port = null
+    }
+
     if (attributes.procedure) {
       this.addAttribute('http.method', attributes.procedure)
+      this.addAttribute('http.request.method', attributes.procedure)
       attributes.procedure = null
     }
   }

lib/spans/streaming-span-event.js

@@ -64,7 +64,7 @@ class StreamingSpanEvent {
    *
    * @param {string} key Name of the attribute to be stored.
    * @param {string|boolean|number} value Value of the attribute to be stored.
-   * @param {boolean} [truncateExempt=false] Set to true if attribute should not be truncated.
+   * @param {boolean} [truncateExempt] Set to true if attribute should not be truncated.
    */
   addCustomAttribute(key, value, truncateExempt = false) {
     const shouldKeep = this._checkFilter(key)
@@ -79,7 +79,7 @@ class StreamingSpanEvent {
    *
    * @param {string} key Name of the attribute to be stored.
    * @param {string|boolean|number} value Value of the attribute to be stored.
-   * @param {boolean} [truncateExempt=false] Set to true if attribute should not be truncated.
+   * @param {boolean} [truncateExempt] Set to true if attribute should not be truncated.
    */
   addAgentAttribute(key, value, truncateExempt = false) {
     const shouldKeep = this._checkFilter(key)
@@ -197,8 +197,19 @@ class StreamingHttpSpanEvent extends StreamingSpanEvent {
       agentAttributes.url = null
     }
 
+    if (agentAttributes.host) {
+      this.addAgentAttribute('server.address', agentAttributes.host)
+      agentAttributes.host = null
+    }
+
+    if (agentAttributes.port) {
+      this.addAgentAttribute('server.port', agentAttributes.port, true)
+      agentAttributes.port = null
+    }
+
     if (agentAttributes.procedure) {
       this.addAgentAttribute('http.method', agentAttributes.procedure)
+      this.addAgentAttribute('http.request.method', agentAttributes.procedure)
       agentAttributes.procedure = null
     }
   }

test/integration/instrumentation/http-outbound.tap.js

@@ -182,8 +182,11 @@ tap.test('external requests', function (t) {
         res.resume()
         res.on('end', () => {
           const segment = tx.trace.root.children[0]
-          t.equal(segment.getAttributes().url, 'http://example.com/')
-          t.equal(segment.getAttributes().procedure, 'GET')
+          const attrs = segment.getAttributes()
+          t.same(attrs, {
+            url: 'http://example.com/',
+            procedure: 'GET'
+          })
           t.equal(reqSegment, segment, 'should expose external')
           t.end()
         })

test/unit/instrumentation/http/outbound.test.js

@@ -127,6 +127,8 @@ tap.test('instrumentOutbound', (t) => {
       t.same(
         transaction.trace.root.children[0].attributes.get(DESTINATIONS.SPAN_EVENT),
         {
+          'host': HOSTNAME,
+          'port': PORT,
           'url': `http://${HOSTNAME}:${PORT}/asdf`,
           'procedure': 'GET',
           'request.parameters.a': 'b',

test/unit/spans/span-event.test.js

@@ -164,10 +164,20 @@ tap.test('fromSegment()', (t) => {
 
           // Should have (most) http properties.
           t.equal(attributes['http.url'], 'https://example.com/')
+          t.equal(attributes['server.address'], 'example.com')
+          t.equal(attributes['server.port'], 443)
           t.ok(attributes['http.method'])
+          t.ok(attributes['http.request.method'])
           t.equal(attributes['http.statusCode'], 200)
           t.equal(attributes['http.statusText'], 'OK')
 
+          // should nullify mapped properties
+          t.notOk(attributes.library)
+          t.notOk(attributes.url)
+          t.notOk(attributes.host)
+          t.notOk(attributes.port)
+          t.notOk(attributes.procedure)
+
           // Should have no datastore properties.
           const hasOwnAttribute = Object.hasOwnProperty.bind(attributes)
           t.notOk(hasOwnAttribute('db.statement'))
@@ -251,7 +261,9 @@ tap.test('fromSegment()', (t) => {
         // Should have not http properties.
         const hasOwnAttribute = Object.hasOwnProperty.bind(attributes)
         t.notOk(hasOwnAttribute('http.url'))
+        t.notOk(hasOwnAttribute('server.address'))
         t.notOk(hasOwnAttribute('http.method'))
+        t.notOk(hasOwnAttribute('http.request.method'))
 
         // Should have (most) datastore properties.
         t.ok(attributes['db.instance'])

test/unit/spans/streaming-span-event.test.js

@@ -156,7 +156,10 @@ tap.test('fromSegment()', (t) => {
 
           // Should have (most) http properties.
           t.same(agentAttributes['http.url'], { [STRING_TYPE]: 'https://example.com/' })
+          t.same(agentAttributes['server.address'], { [STRING_TYPE]: 'example.com' })
+          t.same(agentAttributes['server.port'], { [INT_TYPE]: 443 })
           t.ok(agentAttributes['http.method'])
+          t.ok(agentAttributes['http.request.method'])
           t.same(agentAttributes['http.statusCode'], { [INT_TYPE]: 200 })
           t.same(agentAttributes['http.statusText'], { [STRING_TYPE]: 'OK' })
 
@@ -246,7 +249,9 @@ tap.test('fromSegment()', (t) => {
         // Should have not http properties.
         const hasOwnAttribute = Object.hasOwnProperty.bind(agentAttributes)
         t.notOk(hasOwnAttribute('http.url'))
+        t.notOk(hasOwnAttribute('server.address'))
         t.notOk(hasOwnAttribute('http.method'))
+        t.notOk(hasOwnAttribute('http.request.method'))
 
         // Should have (most) datastore properties.
         t.ok(agentAttributes['db.instance'])

test/versioned/undici/requests.tap.js

@@ -22,6 +22,7 @@ tap.test('Undici request tests', (t) => {
   let server
   let REQUEST_URL
   let HOST
+  let PORT
 
   function createServer() {
     server = http.createServer((req, res) => {
@@ -45,6 +46,7 @@ tap.test('Undici request tests', (t) => {
 
     server.listen(0)
     const { port } = server.address()
+    PORT = port
     HOST = `localhost:${port}`
     REQUEST_URL = `http://${HOST}`
     return server
@@ -146,7 +148,8 @@ tap.test('Undici request tests', (t) => {
       t.equal(spanAttrs['http.statusText'], 'OK')
       t.equal(spanAttrs['request.parameters.a'], 'b')
       t.equal(spanAttrs['request.parameters.c'], 'd')
-
+      t.equal(spanAttrs.host, 'localhost')
+      t.equal(spanAttrs.port, `${PORT}`)
       tx.end()
       t.end()
     })