refactor

Author: amychisholm03

lib/agent.js

@@ -288,7 +288,7 @@ function Agent(config) {
     this.harvester
   )
   this.transactionSampler = determineSampler({ agent: this, config, sampler: 'root' })
-  // TODO: Does 3 AdaptiveSamplers triple the data being sent up?
+  // TODO: This could create duplicate sampler instances, fix will be addressed in https://github.com/newrelic/node-newrelic/issues/3519.
   this.remoteParentSampledSampler = determineSampler({ agent: this, config, sampler: 'remote_parent_sampled' })
   this.remoteParentNotSampledSampler = determineSampler({ agent: this, config, sampler: 'remote_parent_not_sampled' })
 

lib/samplers/adaptive-sampler.js

@@ -5,7 +5,9 @@
 
 'use strict'
 
-class AdaptiveSampler {
+const Sampler = require('./sampler')
+
+class AdaptiveSampler extends Sampler {
   /**
    *
    * @param {object} opts Sampler options.
@@ -15,6 +17,7 @@ class AdaptiveSampler {
    * @param {boolean} opts.serverless - Indicates if the environment is serverless.
    */
   constructor(opts) {
+    super() // no-op
     this._serverless = opts.serverless
     this._seen = 0
     this._sampled = 0
@@ -103,6 +106,17 @@ class AdaptiveSampler {
     return false
   }
 
+  applySamplingDecision(transaction) {
+    if (!transaction) return
+    // eslint-disable-next-line sonarjs/pseudo-random
+    const initPriority = Math.random()
+    transaction.sampled = this.shouldSample(initPriority)
+    transaction.priority = transaction.sampled ? initPriority + 1 : initPriority
+    // Truncate the priority after potentially modifying it to avoid floating
+    // point errors.
+    transaction.priority = ((transaction.priority * 1e6) | 0) / 1e6
+  }
+
   /**
    * Starts a new sample period after adjusting the sampling statistics.
    */

lib/samplers/always-off-sampler.js

@@ -0,0 +1,16 @@
+/*
+ * Copyright 2025 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+const Sampler = require('./sampler')
+
+class AlwaysOffSampler extends Sampler {
+  applySamplingDecision(transaction) {
+    if (!transaction) return
+    transaction.priority = 0
+    transaction.sampled = false
+  }
+}
+
+module.exports = AlwaysOffSampler

lib/samplers/always-on-sampler.js

@@ -0,0 +1,16 @@
+/*
+ * Copyright 2025 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+const Sampler = require('./sampler')
+
+class AlwaysOnSampler extends Sampler {
+  applySamplingDecision(transaction) {
+    if (!transaction) return
+    transaction.priority = 2.0
+    transaction.sampled = true
+  }
+}
+
+module.exports = AlwaysOnSampler

lib/samplers/determine-sampler.js

@@ -4,7 +4,10 @@
  */
 
 const AdaptiveSampler = require('./adaptive-sampler')
+const AlwaysOffSampler = require('./always-off-sampler')
+const AlwaysOnSampler = require('./always-on-sampler')
 const TraceIdRatioBasedSampler = require('./ratio-based-sampler')
+const Sampler = require('./sampler')
 
 /**
  * A helper function for the Agent to determine what sampler
@@ -13,13 +16,23 @@ const TraceIdRatioBasedSampler = require('./ratio-based-sampler')
  * @param {Agent} params.agent The New Relic agent instance.
  * @param {object} params.config The entire agent config.
  * @param {string} params.sampler The sampler type to use: 'root', 'remote_parent_sampled', or 'remote_parent_not_sampled'.
- * @returns {object} An AdaptiveSampler or a TraceIdRatioBasedSampler, depending on given configuration.
+ * @returns {Sampler} A Sampler e.g. AdaptiveSampler or TraceIdRatioBasedSampler
  */
 function determineSampler({ agent, config, sampler = 'root' }) {
-  const ratioBasedSampler = config?.distributed_tracing?.sampler?.[sampler]?.trace_id_ratio_based
+  const samplerDefinition = config?.distributed_tracing?.sampler?.[sampler]
 
-  // This slightly complicated boolean check is for the use case
-  // where the customer sets `ratio` to 0, which is a falsey value.
+  // Always on?
+  if (samplerDefinition === 'always_on') {
+    return new AlwaysOnSampler()
+  }
+
+  // Always off?
+  if (samplerDefinition === 'always_off') {
+    return new AlwaysOffSampler()
+  }
+
+  // Is it TraceIdRatioBased?
+  const ratioBasedSampler = samplerDefinition?.trace_id_ratio_based
   const validRatio = (typeof ratioBasedSampler?.ratio === 'number' && ratioBasedSampler.ratio >= 0 && ratioBasedSampler.ratio <= 1)
   if (validRatio) {
     return new TraceIdRatioBasedSampler({
@@ -28,11 +41,13 @@ function determineSampler({ agent, config, sampler = 'root' }) {
     })
   }
 
+  // Our default ^.^
+  // We'll ignore https://github.com/newrelic/node-newrelic/issues/3519 for now 0.o
   return new AdaptiveSampler({
     agent,
     serverless: config.serverless_mode.enabled,
     period: config.sampling_target_period_in_seconds * 1000,
-    target: config.sampling_target // alias for dt.sampler.adaptive_sampling_target, see config/index.js
+    target: config.sampling_target // getter/setter for distributed_tracing.sampler.adaptive_sampling_target
   })
 }
 

lib/samplers/ratio-based-sampler.js

@@ -4,30 +4,42 @@
  */
 
 'use strict'
+const Sampler = require('./sampler')
 
 /**
  * Trace ID ratio-based sampler adapted for New Relic agent use,
  * based on OpenTelemetry's sampler of the same name.
  */
-class TraceIdRatioBasedSampler {
+class TraceIdRatioBasedSampler extends Sampler {
   /**
    * @param {object} opts Sampler options.
    * @param {Agent} opts.agent - The New Relic agent instance.
    * @param {number} opts.ratio - The sampling ratio (0 to 1).
    */
   constructor(opts) {
+    super() // no-op
     if (!opts.agent) {
       throw new Error('must have an agent instance')
     }
-    this._logger = opts.agent.logger
     this._ratio = this._normalize(opts.ratio)
     this._upperBound = Math.floor(this._ratio * 0xffffffff)
   }
 
+  applySamplingDecision(transaction) {
+    if (!transaction) return
+    // eslint-disable-next-line sonarjs/pseudo-random
+    const initPriority = Math.random()
+    transaction.sampled = this.shouldSample(transaction.traceId)
+    transaction.priority = transaction.sampled ? initPriority + 1 : initPriority
+    // Truncate the priority after potentially modifying it to avoid floating
+    // point errors.
+    transaction.priority = ((transaction.priority * 1e6) | 0) / 1e6
+  }
+
   /**
    *
-   * @param {string} traceId trace id
-   * @returns {boolean}
+   * @param {string} traceId transaction trace id
+   * @returns {boolean} whether to sample a transaction based on given trace id
    */
   shouldSample(traceId) {
     const accumulated = this._accumulate(traceId)
@@ -45,7 +57,6 @@ class TraceIdRatioBasedSampler {
    */
   _normalize(ratio) {
     if (typeof ratio !== 'number' || isNaN(ratio)) {
-      this._logger.warn('TraceIdRatioBasedSampler: invalid `ratio` provided, defaulting to 0')
       return 0
     }
     if (ratio >= 1) return 1

lib/samplers/sampler.js

@@ -0,0 +1,21 @@
+/*
+ * Copyright 2025 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/**
+ * @private
+ * @interface
+ */
+class Sampler {
+  /**
+   * Sets `priority` and `sampled` on the transaction
+   * in respect to this sampler's decision.
+   * @param {Transaction} transaction the transaction to update
+   */
+  applySamplingDecision(transaction) {
+    throw new Error('must implement applySamplingDecision for %s', transaction)
+  }
+}
+
+module.exports = Sampler

lib/transaction/index.js

@@ -1028,32 +1028,11 @@ function acceptTraceContextPayload(traceparentHeader, tracestateHeader, transpor
     }
   }
 
-  decideSamplingFromW3cData({ transaction: this, traceparent, tracestate })
-}
-
-/**
- * Updates the transaction with sampling configuration as determined by the
- * agent's distributed tracing sampler configuration. In short, updates the
- * transaction to always sample, never sample, or let the standard sampling
- * algorithm make the decision.
- *
- * @param {object} params Input parameters.
- * @param {Transaction} params.transaction The transaction to update.
- * @param {Traceparent} params.traceparent The object representation of a
- * traceparent header.
- * @param {Tracestate} params.tracestate The object representation of a
- * tracestate header.
- */
-function decideSamplingFromW3cData({ transaction, traceparent, tracestate }) {
-  const agent = transaction.agent
-  const {
-    remote_parent_sampled: parentSampled,
-    remote_parent_not_sampled: parentNotSampled
-  } = agent.config.distributed_tracing.sampler
+  // Decide sampling from w3c data
   if (traceparent.isSampled === true) {
-    applySamplingStrategy({ transaction, tracestate, sampler: agent.remoteParentSampledSampler, samplerType: parentSampled })
+    applySamplingStrategy({ transaction: this, tracestate, sampler: this.agent.remoteParentSampledSampler })
   } else if (traceparent.isSampled === false) {
-    applySamplingStrategy({ transaction, tracestate, sampler: agent.remoteParentNotSampledSampler, samplerType: parentNotSampled })
+    applySamplingStrategy({ transaction: this, tracestate, sampler: this.agent.remoteParentNotSampledSampler })
   }
 }
 
@@ -1063,51 +1042,22 @@ function decideSamplingFromW3cData({ transaction, traceparent, tracestate }) {
  *
  * @param {*} params Function parameters.
  * @param {Transaction} params.transaction The transaction to update.
- * @param {Tracestate} params.tracestate A tracestate object with embedded New Relic intrinsics.
- * @param {object} params.sampler The sampler to use, AdaptiveSampler or TraceIdRatioBasedSampler.
- * @param {string|object} params.samplerType A string describing the sampler being used
- * ('default', 'always_on', 'always_off'), or an object if 'trace_id_ratio_based'.
+ * @param {Tracestate} [params.tracestate] A tracestate object with embedded New Relic intrinsics.
+ * @param {object} params.sampler The sampler to use, e.g. AdaptiveSampler or TraceIdRatioBasedSampler.
  */
-function applySamplingStrategy({ transaction, tracestate, sampler, samplerType }) {
-  if (sampler?.toString() === 'TraceIdRatioBasedSampler') {
-    transaction.sampled = sampler.shouldSample(transaction.traceId)
-    // Force calc priority because transaction.sampled likekly has changed
-    // eslint-disable-next-line sonarjs/pseudo-random
-    transaction.priority = Math.random()
-    if (transaction.sampled) {
-      transaction.priority += 1
-    }
-
-    // Truncate the priority after potentially modifying it to avoid floating
-    // point errors.
-    transaction.priority = ((transaction.priority * 1e6) | 0) / 1e6
-  }
-
-  // TODO: Make these their own samplers?
-  switch (samplerType) {
-    case 'default': {
-      // This case is if we're coming from the deprecated newrelic header logic
-      // where we don't want to override the sampled and priority values
-      if (!tracestate) {
-        return
-      }
-
-      transaction.sampled = tracestate?.intrinsics ? tracestate.isSampled : null
-      transaction.priority = tracestate?.intrinsics ? tracestate.priority : null
-      break
-    }
-
-    case 'always_on': {
-      transaction.sampled = true
-      transaction.priority = 2.0
-      break
+function applySamplingStrategy({ transaction, tracestate = null, sampler }) {
+  if (sampler?.toString() !== 'AdaptiveSampler') {
+    sampler.applySamplingDecision(transaction)
+  } else {
+    // This case is if we're coming from the deprecated newrelic header logic
+    // and we don't want to override the sampled and priority values
+    if (!tracestate) {
+      return
     }
 
-    case 'always_off': {
-      transaction.sampled = false
-      transaction.priority = 0
-      break
-    }
+    // Explicitly set sampled and priority from tracestate intrinsics if available
+    transaction.sampled = tracestate?.intrinsics ? tracestate.isSampled : null
+    transaction.priority = tracestate?.intrinsics ? tracestate.priority : null
   }
 }
 
@@ -1209,19 +1159,15 @@ const _dtDefineAttrsFromTraceData = function _dtDefineAttrsFromTraceData(data, t
   this.traceId = data.tr
 
   if (data.pr) {
-    // TODO: We need to consider the sampler before blindly accepting this value.
     this.priority = data.pr
     this.sampled = data.sa != null ? data.sa : this.sampled
-    // we don't have traceparent or tracestate, but this will update
-    // sampling decision based on our samplers
-    const {
-      remote_parent_sampled: parentSampled,
-      remote_parent_not_sampled: parentNotSampled
-    } = this.agent.config.distributed_tracing.sampler
+    // Even though New Relic headers are deprecated,
+    // we still have to apply our sampling decision on top
+    // of the priority and sampled values we receive.
     if (data?.sa) {
-      applySamplingStrategy({ transaction: this, sampler: this.agent.remoteParentSampledSampler, samplerType: parentSampled })
+      applySamplingStrategy({ transaction: this, sampler: this.agent.remoteParentSampledSampler })
     } else {
-      applySamplingStrategy({ transaction: this, sampler: this.agent.remoteParentNotSampledSampler, samplerType: parentNotSampled })
+      applySamplingStrategy({ transaction: this, sampler: this.agent.remoteParentNotSampledSampler })
     }
   }
 
@@ -1430,48 +1376,15 @@ Transaction.prototype.isSampled = function isSampled() {
 }
 
 /**
- * Generates a priority for the transaction if it does not have one already.
- * @param {object} sampler The sampler to calculate priority with. Defaults to `this.agent.transactionSampler`.
+ * If `priority` does not exist on this transaction already,
+ * this.agent.transactionSampler will apply a sampling decision
+ * to the transaction, which assigns `priority` and updates `sampled`
+ * in respect to this sampling decision.
  */
-Transaction.prototype._calculatePriority = function _calculatePriority(sampler) {
-  if (!sampler) {
-    sampler = this.agent.transactionSampler
-  }
+Transaction.prototype._calculatePriority = function _calculatePriority() {
   if (this.priority === null) {
-    // eslint-disable-next-line sonarjs/pseudo-random
-    this.priority = Math.random()
-
-    // Determine sampling parameter based on type of sampler being used
-    const shouldSampleParam = sampler.toString() === 'TraceIdRatioBasedSampler'
-      ? this.traceId
-      // We want to separate the priority roll from the decision roll to
-      // avoid biasing the priority range
-      // eslint-disable-next-line sonarjs/pseudo-random
-      : Math.random()
-    this.sampled = sampler.shouldSample(shouldSampleParam)
-    if (this.sampled) {
-      this.priority += 1
-    }
-
-    // Truncate the priority after potentially modifying it to avoid floating
-    // point errors.
-    this.priority = ((this.priority * 1e6) | 0) / 1e6
-
-    // IGNORE THIS
-    if (sampler === this.agent.transactionSampler) {
-      const samplerType = this.agent.config.distributed_tracing.sampler.root
-
-      // TODO: have to account if there's no headers
-      if (samplerType === 'always_on') {
-        this.sampled = true
-        this.priority = 2.0
-      }
-
-      if (samplerType === 'always_off') {
-        this.sampled = false
-        this.priority = 0
-      }
-    }
+    const sampler = this.agent.transactionSampler
+    sampler.applySamplingDecision(this)
   }
 }