chore: Refactored otel attribute reconciling (#2964)

jsumners-nr · web-flow · commit 1b5ed2c2f7c7 · 2025-02-27T12:59:23.000-05:00
diff --git a/lib/otel/attr-reconciler.js b/lib/otel/attr-reconciler.js
@@ -0,0 +1,52 @@
+/*
+ * Copyright 2025 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+'use strict'
+
+const urltils = require('../util/urltils')
+const constants = require('./constants')
+
+const hostKeys = [
+  constants.ATTR_NET_HOST_NAME,
+  constants.ATTR_NET_PEER_NAME,
+  constants.ATTR_SERVER_ADDRESS
+]
+
+class AttributeReconciler {
+  #agent
+
+  constructor({ agent }) {
+    this.#agent = agent
+  }
+
+  #resolveHost(hostname) {
+    if (urltils.isLocalhost(hostname)) {
+      return this.#agent.config.getHostnameSafe(hostname)
+    }
+    return hostname
+  }
+
+  #isHostnameKey(key) {
+    return hostKeys.includes(key)
+  }
+
+  reconcile({ segment, otelSpan, mapper = {} }) {
+    for (const [key, srcValue] of Object.entries(otelSpan.attributes)) {
+      let value = srcValue
+
+      if (this.#isHostnameKey(key) === true) {
+        value = this.#resolveHost(srcValue)
+      }
+
+      if (Object.prototype.hasOwnProperty.call(mapper, key) === true) {
+        mapper[key](value)
+      } else {
+        segment.addAttribute(key, value)
+      }
+    }
+  }
+}
+
+module.exports = AttributeReconciler
diff --git a/lib/otel/span-processor.js b/lib/otel/span-processor.js
@@ -4,11 +4,14 @@
  */
 
 'use strict'
-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 AttributeReconciler = require('./attr-reconciler')
+const SegmentSynthesizer = require('./segment-synthesis')
+const { otelSynthesis } = require('../symbols')
+
 const {
   ATTR_DB_NAME,
   ATTR_DB_STATEMENT,
@@ -33,10 +36,14 @@ const {
 const { DESTINATIONS } = require('../config/attribute-filter')
 
 module.exports = class NrSpanProcessor {
+  #reconciler
+
   constructor(agent) {
     this.agent = agent
     this.synthesizer = new SegmentSynthesizer(agent)
     this.tracer = agent.tracer
+
+    this.#reconciler = new AttributeReconciler({ agent })
   }
 
   /**
@@ -93,49 +100,27 @@ module.exports = class NrSpanProcessor {
    * @param {Transaction} params.transaction The NR transaction to attach
    * the found attributes to.
    */
-  reconcileConsumerAttributes({ span, transaction }) { // eslint-disable-line sonarjs/cognitive-complexity
+  reconcileConsumerAttributes({ span, transaction }) {
     const baseSegment = transaction.baseSegment
     const trace = transaction.trace
     const isHighSecurity = this.agent.config.high_security ?? false
 
-    for (const [key, value] of Object.entries(span.attributes)) {
-      switch (key) {
-        case ATTR_SERVER_ADDRESS: {
-          if (value) {
-            let serverAddress = value
-            if (urltils.isLocalhost(value)) {
-              serverAddress = this.agent.config.getHostnameSafe(value)
-            }
-            baseSegment.addAttribute('host', serverAddress)
-          }
-          break
-        }
-
-        case ATTR_SERVER_PORT: {
-          baseSegment.addAttribute('port', value)
-          break
-        }
-
-        case ATTR_MESSAGING_RABBITMQ_DESTINATION_ROUTING_KEY: {
-          if (isHighSecurity === true || !value) break
-          trace.attributes.addAttribute(DESTINATIONS.TRANS_COMMON, 'message.routingKey', value)
-          baseSegment.addAttribute('message.routingKey', value)
-          break
-        }
-
-        case ATTR_MESSAGING_DESTINATION_NAME:
-        case ATTR_MESSAGING_DESTINATION: {
-          if (isHighSecurity === true || !value) break
-          trace.attributes.addAttribute(DESTINATIONS.TRANS_COMMON, 'message.queueName', value)
-          baseSegment.addAttribute('message.queueName', value)
-          break
-        }
-
-        default: {
-          baseSegment.addAttribute(key, value)
-        }
-      }
+    const queueNameMapper = (value) => {
+      if (isHighSecurity === true) return
+      trace.attributes.addAttribute(DESTINATIONS.TRANS_COMMON, 'message.queueName', value)
+      baseSegment.addAttribute('message.queueName', value)
+    }
+    const mapper = {
+      [ATTR_SERVER_ADDRESS]: (value) => baseSegment.addAttribute('host', value),
+      [ATTR_SERVER_PORT]: (value) => baseSegment.addAttribute('port', value),
+      [ATTR_MESSAGING_RABBITMQ_DESTINATION_ROUTING_KEY]: (value) => {
+        if (isHighSecurity === true) return
+        trace.attributes.addAttribute(DESTINATIONS.TRANS_COMMON, 'message.routingKey', value)
+      },
+      [ATTR_MESSAGING_DESTINATION_NAME]: queueNameMapper,
+      [ATTR_MESSAGING_DESTINATION]: queueNameMapper
     }
+    this.#reconciler.reconcile({ segment: baseSegment, otelSpan: span, mapper })
 
     transaction.end()
   }
@@ -157,98 +142,83 @@ module.exports = class NrSpanProcessor {
   }
 
   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 || key === ATTR_HTTP_RES_STATUS_CODE) {
-        key = 'http.statusCode'
-        transaction.statusCode = sanitized
-        transaction.trace.attributes.addAttribute(DESTINATIONS.TRANS_COMMON, key, sanitized)
-      // Not using const as it is not in semantic-conventions
-      } else if (key === ATTR_HTTP_STATUS_TEXT) {
-        key = 'http.statusText'
-        transaction.trace.attributes.addAttribute(DESTINATIONS.TRANS_COMMON, key, sanitized)
-      } 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)
+    const status = (value) => {
+      transaction.statusCode = value
+      transaction.trace.attributes.addAttribute(DESTINATIONS.TRANS_COMMON, 'http.statusCode', value)
+    }
+    const port = (value) => segment.addAttribute('port', value)
+    const host = (value) => segment.addAttribute('host', value)
+    const mapper = {
+      // TODO: if route params are available, assign them as well
+      [ATTR_HTTP_ROUTE]: (value) => {
+        transaction.nameState.appendPath(value)
+        segment.addAttribute('http.route', value)
+      },
+      [ATTR_HTTP_STATUS_CODE]: status,
+      [ATTR_HTTP_RES_STATUS_CODE]: status,
+      [ATTR_HTTP_STATUS_TEXT]: (value) => {
+        transaction.trace.attributes.addAttribute(DESTINATIONS.TRANS_COMMON, 'http.statusText', value)
+      },
+      [ATTR_SERVER_PORT]: port,
+      [ATTR_NET_HOST_PORT]: port,
+      [ATTR_SERVER_ADDRESS]: host,
+      [ATTR_NET_HOST_NAME]: host
     }
+    this.#reconciler.reconcile({ segment, otelSpan: span, mapper })
+
+    // 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
   }
 
   // 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) {
+    const mapper = {
+      [ATTR_GRPC_STATUS_CODE]: (value) => {
         transaction.trace.attributes.addAttribute(DESTINATIONS.TRANS_COMMON, 'response.status', value)
+        segment.addAttribute(ATTR_GRPC_STATUS_CODE, value)
       }
-      segment.addAttribute(prop, value)
     }
+    this.#reconciler.reconcile({ segment, otelSpan: span, mapper })
   }
 
   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) {
-        key = 'host'
-        if (urltils.isLocalhost(sanitized)) {
-          sanitized = this.agent.config.getHostnameSafe(sanitized)
-        }
-      } else if (prop === ATTR_DB_NAME) {
-        key = 'database_name'
-      } else if (prop === ATTR_DB_SYSTEM) {
-        key = 'product'
-      /**
-       * This attribute was collected in `onStart`
-       * and was passed to `ParsedStatement`. It adds
-       * this segment attribute as `sql` or `sql_obfuscated`
-       * and then when the span is built from segment
-       * re-assigns to `db.statement`. This needs
-       * to be skipped because it will be the raw value.
-       */
-      } else if (prop === ATTR_DB_STATEMENT) {
-        continue
-      }
-      segment.addAttribute(key, sanitized)
+    const mapper = {
+      [ATTR_NET_PEER_PORT]: (value) => {
+        segment.addAttribute('port_path_or_id', value)
+      },
+      [ATTR_NET_PEER_NAME]: (value) => {
+        segment.addAttribute('host', value)
+      },
+      [ATTR_DB_NAME]: (value) => {
+        segment.addAttribute('database_name', value)
+      },
+      [ATTR_DB_SYSTEM]: (value) => {
+        segment.addAttribute('product', value)
+        /*
+         * This attribute was collected in `onStart`
+         * and was passed to `ParsedStatement`. It adds
+         * this segment attribute as `sql` or `sql_obfuscated`
+         * and then when the span is built from segment
+         * re-assigns to `db.statement`. This needs
+         * to be skipped because it will be the raw value.
+         */
+      },
+      [ATTR_DB_STATEMENT]: () => {}
     }
+    this.#reconciler.reconcile({ segment, otelSpan: span, mapper })
   }
 
   reconcileProducerAttributes({ segment, span }) {
-    for (const [prop, value] of Object.entries(span.attributes)) {
-      let key = prop
-      let sanitized = value
-
-      if (prop === ATTR_SERVER_ADDRESS) {
-        key = 'host'
-        if (urltils.isLocalhost(sanitized)) {
-          sanitized = this.agent.config.getHostnameSafe(sanitized)
-        }
-      } else if (prop === ATTR_SERVER_PORT) {
-        key = 'port'
-      } else if (prop === ATTR_MESSAGING_MESSAGE_CONVERSATION_ID) {
-        key = 'correlation_id'
-      } else if (prop === ATTR_MESSAGING_RABBITMQ_DESTINATION_ROUTING_KEY) {
-        key = 'routing_key'
-      }
-
-      segment.addAttribute(key, sanitized)
+    const mapper = {
+      [ATTR_SERVER_ADDRESS]: (value) => segment.addAttribute('host', value),
+      [ATTR_SERVER_PORT]: (value) => segment.addAttribute('port', value),
+      [ATTR_MESSAGING_MESSAGE_CONVERSATION_ID]: (value) => segment.addAttribute('correlation_id', value),
+      [ATTR_MESSAGING_RABBITMQ_DESTINATION_ROUTING_KEY]: (value) => segment.addAttribute('routing_key', value)
     }
+    this.#reconciler.reconcile({ segment, otelSpan: span, mapper })
   }
 }
