feat: Added timeslice metrics recorders for synthesized db segments (#2922)

bizob2828 · web-flow · commit 8606f789772b · 2025-02-06T09:48:12.000-05:00
diff --git a/lib/otel/segments/database.js b/lib/otel/segments/database.js
@@ -13,25 +13,70 @@ const {
   DbSystemValues
 } = require('@opentelemetry/semantic-conventions')
 const parseSql = require('../../db/query-parsers/sql')
+const recordQueryMetrics = require('../../metrics/recorders/database')
+const recordOperationMetrics = require('../../metrics/recorders/database-operation')
+const ParsedStatement = require('../../db/parsed-statement')
+const metrics = require('../../metrics/names')
 
 // TODO: This probably has some holes
 // I did analysis and tried to apply the best logic
 // to extract table/operation
 module.exports = function createDbSegment(agent, otelSpan) {
   const context = agent.tracer.getContext()
-  const name = setName(otelSpan)
+  const system = otelSpan.attributes[SEMATTRS_DB_SYSTEM]
+  const parsed = parseStatement(agent.config, otelSpan, system)
+  const { name, operation } = setName(parsed)
   const segment = agent.tracer.createSegment({
     name,
+    recorder: getRecorder({ operation, parsed, system }),
     parent: context.segment,
     transaction: context.transaction
   })
   return { segment, transaction: context.transaction }
 }
 
-function parseStatement(otelSpan, system) {
+/**
+ * Assigns the appropriate timeslice metrics recorder
+ * based on the otel span.
+ *
+ * @param {object} params to fn
+ * @param {ParsedStatement} params.parsed parsed statement of call
+ * @param {boolean} params.operation if span is an operation
+ * @param {string} params.system `db.system` value of otel span
+ * @returns {function} returns a timeslice metrics recorder function based on span
+ */
+function getRecorder({ parsed, operation, system }) {
+  if (operation) {
+    /**
+     * this metrics recorder expects to bound with
+     * a datastore-shim. But really the only thing it needs
+     * is `_metrics` with values for PREFIX and ALL
+     * This assigns what it needs to properly create
+     * the db operation time slice metrics
+     */
+    const scope = {}
+    scope._metrics = {
+      PREFIX: system,
+      ALL: metrics.DB.PREFIX + system + '/' + metrics.ALL
+    }
+    return recordOperationMetrics.bind(scope)
+  } else {
+    return recordQueryMetrics.bind(parsed)
+  }
+}
+
+/**
+ * Creates a parsed statement from various span attributes.
+ *
+ * @param {object} config agent config
+ * @param {object} otelSpan span getting parsed
+ * @param {string} system value of `db.system` on span
+ * @returns {ParsedStatement} instance of parsed statement
+ */
+function parseStatement(config, otelSpan, system) {
   let table = otelSpan.attributes[SEMATTRS_DB_SQL_TABLE]
   let operation = otelSpan.attributes[SEMATTRS_DB_OPERATION]
-  const statement = otelSpan.attributes[SEMATTRS_DB_STATEMENT]
+  let statement = otelSpan.attributes[SEMATTRS_DB_STATEMENT]
   if (statement && !(table || operation)) {
     const parsed = parseSql({ sql: statement })
     if (parsed.operation && !operation) {
@@ -41,6 +86,7 @@ function parseStatement(otelSpan, system) {
     if (parsed.collection && !table) {
       table = parsed.collection
     }
+    statement = parsed.query
   }
   if (system === DbSystemValues.MONGODB) {
     table = otelSpan.attributes[SEMATTRS_DB_MONGODB_COLLECTION]
@@ -52,17 +98,33 @@ function parseStatement(otelSpan, system) {
 
   table = table || 'Unknown'
   operation = operation || 'Unknown'
+  const queryRecorded =
+    config.transaction_tracer.record_sql === 'raw' ||
+    config.transaction_tracer.record_sql === 'obfuscated'
 
-  return { operation, table }
+  return new ParsedStatement(
+    system,
+    operation,
+    table,
+    queryRecorded ? statement : null
+  )
 }
 
-function setName(otelSpan) {
-  const system = otelSpan.attributes[SEMATTRS_DB_SYSTEM]
-  const { operation, table } = parseStatement(otelSpan, system)
-  let name = `Datastore/statement/${system}/${table}/${operation}`
+/**
+ * Creates name for db segment based on otel span
+ * If the system is redis or memcached the name is an operation name
+ *
+ * @param {ParsedStatement} parsed statement used for naming segment
+ * @returns {string} name of segment
+ */
+function setName(parsed) {
+  let operation = false
+  let name = `Datastore/statement/${parsed.type}/${parsed.collection}/${parsed.operation}`
   // All segment name shapes are same except redis/memcached
-  if (system === DbSystemValues.REDIS || system === DbSystemValues.MEMCACHED) {
-    name = `Datastore/operation/${system}/${operation}`
+  if (parsed.type === DbSystemValues.REDIS || parsed.type === DbSystemValues.MEMCACHED) {
+    name = `Datastore/operation/${parsed.type}/${parsed.operation}`
+    operation = true
   }
-  return name
+
+  return { name, operation }
 }
diff --git a/lib/otel/span-processor.js b/lib/otel/span-processor.js
@@ -7,6 +7,8 @@
 const SegmentSynthesizer = require('./segment-synthesis')
 const { otelSynthesis } = require('../symbols')
 const { hrTimeToMilliseconds } = require('@opentelemetry/core')
+const urltils = require('../util/urltils')
+const { SEMATTRS_NET_PEER_PORT, SEMATTRS_NET_PEER_NAME, SEMATTRS_DB_NAME, SEMATTRS_DB_SYSTEM, SEMATTRS_DB_STATEMENT } = require('@opentelemetry/semantic-conventions')
 
 module.exports = class NrSpanProcessor {
   constructor(agent) {
@@ -19,7 +21,7 @@ module.exports = class NrSpanProcessor {
    * Synthesize segment at start of span and assign to a symbol
    * that will be removed in `onEnd` once the correspondig
    * segment is read.
-   * @param span
+   * @param {object} span otel span getting tested
    */
   onStart(span) {
     span[otelSynthesis] = this.synthesizer.synthesize(span)
@@ -28,20 +30,51 @@ module.exports = class NrSpanProcessor {
   /**
    * Update the segment duration from span and reconcile
    * any attributes that were added after the start
-   * @param span
+   * @param {object} span otel span getting updated
    */
   onEnd(span) {
-    this.updateDuration(span)
-    // TODO: add attributes from span that did not exist at start
-  }
-
-  updateDuration(span) {
     if (span[otelSynthesis] && span[otelSynthesis].segment) {
       const { segment } = span[otelSynthesis]
-      segment.touch()
-      const duration = hrTimeToMilliseconds(span.duration)
-      segment.overwriteDurationInMillis(duration)
+      this.updateDuration(segment, span)
+      this.reconcileAttributes(segment, span)
       delete span[otelSynthesis]
     }
   }
+
+  updateDuration(segment, span) {
+    segment.touch()
+    const duration = hrTimeToMilliseconds(span.duration)
+    segment.overwriteDurationInMillis(duration)
+  }
+
+  // TODO: clean this up and break out by span.kind
+  reconcileAttributes(segment, span) {
+    for (const [prop, value] of Object.entries(span.attributes)) {
+      let key = prop
+      let sanitized = value
+      if (key === SEMATTRS_NET_PEER_PORT) {
+        key = 'port_path_or_id'
+      } else if (prop === SEMATTRS_NET_PEER_NAME) {
+        key = 'host'
+        if (urltils.isLocalhost(sanitized)) {
+          sanitized = this.agent.config.getHostnameSafe(sanitized)
+        }
+      } else if (prop === SEMATTRS_DB_NAME) {
+        key = 'database_name'
+      } else if (prop === SEMATTRS_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 === SEMATTRS_DB_STATEMENT) {
+        continue
+      }
+      segment.addAttribute(key, sanitized)
+    }
+  }
 }
diff --git a/test/versioned/otel-bridge/span.test.js b/test/versioned/otel-bridge/span.test.js
@@ -10,7 +10,7 @@ const helper = require('../../lib/agent_helper')
 const otel = require('@opentelemetry/api')
 const { hrTimeToMilliseconds } = require('@opentelemetry/core')
 const { otelSynthesis } = require('../../../lib/symbols')
-const { SEMATTRS_HTTP_HOST, SEMATTRS_HTTP_METHOD } = require('@opentelemetry/semantic-conventions')
+const { SEMATTRS_HTTP_HOST, SEMATTRS_HTTP_METHOD, SEMATTRS_DB_NAME, SEMATTRS_DB_STATEMENT, SEMATTRS_DB_SYSTEM, SEMATTRS_NET_PEER_PORT, SEMATTRS_NET_PEER_NAME, DbSystemValues } = require('@opentelemetry/semantic-conventions')
 
 test.beforeEach((ctx) => {
   const agent = helper.instrumentMockedAgent({
@@ -103,3 +103,89 @@ test('Otel http external span test', (t, end) => {
     })
   })
 })
+
+test('Otel db client span statement test', (t, end) => {
+  const { agent, tracer } = t.nr
+  const attributes = {
+    [SEMATTRS_DB_NAME]: 'test-db',
+    [SEMATTRS_DB_SYSTEM]: 'postgresql',
+    [SEMATTRS_DB_STATEMENT]: "select foo from test where foo = 'bar';",
+    [SEMATTRS_NET_PEER_PORT]: 5436,
+    [SEMATTRS_NET_PEER_NAME]: '127.0.0.1'
+  }
+  const expectedHost = agent.config.getHostnameSafe('127.0.0.1')
+  helper.runInTransaction(agent, (tx) => {
+    tx.name = 'db-test'
+    tracer.startActiveSpan('db-test', { kind: otel.SpanKind.CLIENT, attributes }, (span) => {
+      const segment = agent.tracer.getSegment()
+      assert.equal(segment.name, 'Datastore/statement/postgresql/test/select')
+      span.end()
+      const duration = hrTimeToMilliseconds(span.duration)
+      assert.equal(duration, segment.getDurationInMillis())
+      tx.end()
+      const attrs = segment.getAttributes()
+      assert.equal(attrs.host, expectedHost)
+      assert.equal(attrs.product, 'postgresql')
+      assert.equal(attrs.port_path_or_id, 5436)
+      assert.equal(attrs.database_name, 'test-db')
+      assert.equal(attrs.sql_obfuscated, 'select foo from test where foo = ?;')
+      const metrics = tx.metrics.scoped[tx.name]
+      assert.equal(metrics['Datastore/statement/postgresql/test/select'].callCount, 1)
+      const unscopedMetrics = tx.metrics.unscoped
+      ;[
+        'Datastore/all',
+        'Datastore/allWeb',
+        'Datastore/postgresql/all',
+        'Datastore/postgresql/allWeb',
+        'Datastore/operation/postgresql/select',
+        'Datastore/statement/postgresql/test/select',
+        `Datastore/instance/postgresql/${expectedHost}/5436`
+      ].forEach((expectedMetric) => {
+        assert.equal(unscopedMetrics[expectedMetric].callCount, 1)
+      })
+
+      end()
+    })
+  })
+})
+
+test('Otel db client span operation test', (t, end) => {
+  const { agent, tracer } = t.nr
+  const attributes = {
+    [SEMATTRS_DB_SYSTEM]: DbSystemValues.REDIS,
+    [SEMATTRS_DB_STATEMENT]: 'hset has random random',
+    [SEMATTRS_NET_PEER_PORT]: 5436,
+    [SEMATTRS_NET_PEER_NAME]: '127.0.0.1'
+  }
+  const expectedHost = agent.config.getHostnameSafe('127.0.0.1')
+  helper.runInTransaction(agent, (tx) => {
+    tx.name = 'db-test'
+    tracer.startActiveSpan('db-test', { kind: otel.SpanKind.CLIENT, attributes }, (span) => {
+      const segment = agent.tracer.getSegment()
+      assert.equal(segment.name, 'Datastore/operation/redis/hset')
+      span.end()
+      const duration = hrTimeToMilliseconds(span.duration)
+      assert.equal(duration, segment.getDurationInMillis())
+      tx.end()
+      const attrs = segment.getAttributes()
+      assert.equal(attrs.host, expectedHost)
+      assert.equal(attrs.product, 'redis')
+      assert.equal(attrs.port_path_or_id, 5436)
+      const metrics = tx.metrics.scoped[tx.name]
+      assert.equal(metrics['Datastore/operation/redis/hset'].callCount, 1)
+      const unscopedMetrics = tx.metrics.unscoped
+      ;[
+        'Datastore/all',
+        'Datastore/allWeb',
+        'Datastore/redis/all',
+        'Datastore/redis/allWeb',
+        'Datastore/operation/redis/hset',
+        `Datastore/instance/redis/${expectedHost}/5436`
+      ].forEach((expectedMetric) => {
+        assert.equal(unscopedMetrics[expectedMetric].callCount, 1)
+      })
+
+      end()
+    })
+  })
+})
