Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion lib/samplers/determine-sampler.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
const AdaptiveSampler = require('./adaptive-sampler')
const AlwaysOffSampler = require('./always-off-sampler')
const AlwaysOnSampler = require('./always-on-sampler')
const TraceIdRatioBasedSampler = require('./ratio-based-sampler')

/**
* A helper function for the Agent to determine what sampler
Expand All @@ -14,7 +15,7 @@ const AlwaysOnSampler = require('./always-on-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 {Sampler} A Sampler e.g. AdaptiveSampler
* @returns {Sampler} A Sampler e.g. AdaptiveSampler or TraceIdRatioBasedSampler
*/
function determineSampler({ agent, config, sampler = 'root' }) {
const samplerDefinition = config?.distributed_tracing?.sampler?.[sampler]
Expand All @@ -29,6 +30,16 @@ function determineSampler({ agent, config, sampler = 'root' }) {
return new AlwaysOffSampler()
}

// Is it TraceIdRatioBased?
const ratioBasedSampler = samplerDefinition?.trace_id_ratio_based
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this logic should live in the config. we shouldn't have to check if it's a valid ratio here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we shouldn't but it's there as kind of a fail safe

const validRatio = (typeof ratioBasedSampler?.ratio === 'number' && ratioBasedSampler.ratio >= 0 && ratioBasedSampler.ratio <= 1)
if (validRatio) {
return new TraceIdRatioBasedSampler({
agent,
ratio: ratioBasedSampler.ratio
})
}

// Our default ^.^
// We'll ignore https://github.com/newrelic/node-newrelic/issues/3519 for now 0.o
return new AdaptiveSampler({
Expand Down
83 changes: 83 additions & 0 deletions lib/samplers/ratio-based-sampler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* Copyright 2025 New Relic Corporation. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

'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 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._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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the sample logic as the adaptive, it's just that it calls its shouldSample method. Feels like this should be in the default Sampler class

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, that was my first thought too. However the parameters passed into shouldSample differ, so there's no real repeated logic except for the priority calc and truncation.

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 transaction trace id
* @returns {boolean} whether to sample a transaction based on given trace id
*/
shouldSample(traceId) {
const accumulated = this._accumulate(traceId)
return accumulated <= this._upperBound
}

toString() {
return 'TraceIdRatioBasedSampler'
}

/**
* Normalizes the sampling ratio.
* @param {number} ratio The ratio provided by the config.
* @returns {number} The normalized ratio [0, 1].
*/
_normalize(ratio) {
if (typeof ratio !== 'number' || isNaN(ratio)) {
return 0
}
if (ratio >= 1) return 1

return ratio <= 0 ? 0 : ratio
}

/**
* Accumulates a value from the trace ID.
* @param {string} traceId The trace ID to accumulate.
* @returns {number} The accumulated value.
*/
_accumulate(traceId) {
let accumulation = 0
for (let i = 0; i < traceId.length / 8; i++) {
const pos = i * 8
const part = parseInt(traceId.slice(pos, pos + 8), 16)
accumulation = (accumulation ^ part) >>> 0
}
return accumulation
}
}

module.exports = TraceIdRatioBasedSampler
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const recordSupportability = require('../../../lib/agent').prototype.recordSuppo

const getDescendantValue = function (object, descendants) {
const arrayDescendants = descendants.split('.')
const noop = () => {}
const noop = () => { }
while (arrayDescendants.length && (object = object[arrayDescendants.shift()])) {
noop()
}
Expand Down Expand Up @@ -268,7 +268,6 @@ async function runTestCase(testCase, parentTest) {
testExpectedFixtureKeys(testCase, [
'account_id',
'expected_metrics',
'force_sampled_true',
'inbound_headers',
'intrinsics',
'outbound_payloads',
Expand All @@ -279,7 +278,14 @@ async function runTestCase(testCase, parentTest) {
'trusted_account_key',
'web_transaction',
'comment',
'transaction_events_enabled'
'transaction_events_enabled',
// Sampling
'force_adaptive_sampled',
'remote_parent_sampled',
'remote_parent_not_sampled',
'root',
'ratio',
'expected_priority_between'
])

if (testCase.outbound_payloads) {
Expand Down Expand Up @@ -325,14 +331,53 @@ async function runTestCase(testCase, parentTest) {
})

await parentTest.test('trace context: ' + testCase.test_name, (t, end) => {
const agent = helper.instrumentMockedAgent({})
// Sampling config has to be given on agent initialization
const initConfig = {
distributed_tracing: {
enabled: true,
sampler: {
root: {},
remote_parent_sampled: {},
remote_parent_not_sampled: {}
}
}
}
initConfig.distributed_tracing.sampler.remote_parent_sampled = testCase.remote_parent_sampled ?? 'default'
initConfig.distributed_tracing.sampler.remote_parent_not_sampled = testCase.remote_parent_not_sampled ?? 'default'
initConfig.distributed_tracing.sampler.root = testCase.root ?? 'default'
if (testCase.ratio) {
// The ratio to use for all of the trace ID ratio samplers defined in the test.
// For testing purposes we are not defining different ratios for each trace ID ratio sampler instance.
if (initConfig.distributed_tracing.sampler.root === 'trace_id_ratio_based') {
initConfig.distributed_tracing.sampler.root = {
trace_id_ratio_based: {
ratio: testCase.ratio
}
}
}
if (initConfig.distributed_tracing.sampler.remote_parent_sampled === 'trace_id_ratio_based') {
initConfig.distributed_tracing.sampler.remote_parent_sampled = {
trace_id_ratio_based: {
ratio: testCase.ratio
}
}
}
if (initConfig.distributed_tracing.sampler.remote_parent_not_sampled === 'trace_id_ratio_based') {
initConfig.distributed_tracing.sampler.remote_parent_not_sampled = {
trace_id_ratio_based: {
ratio: testCase.ratio
}
}
}
}
const agent = helper.instrumentMockedAgent(initConfig)
agent.recordSupportability = recordSupportability
agent.config.trusted_account_key = testCase.trusted_account_key
agent.config.account_id = testCase.account_id
agent.config.primary_application_id = 4657
agent.config.span_events.enabled = testCase.span_events_enabled
agent.config.transaction_events.enabled = testCase.transaction_events_enabled
agent.config.distributed_tracing.enabled = true

t.after(() => helper.unloadAgent(agent))

const agentApi = new API(agent)
Expand All @@ -354,13 +399,7 @@ async function runTestCase(testCase, parentTest) {
agentApi.noticeError(new Error('should error'))
}

// monkey patch this transaction object
// to force sampled to be true.
if (testCase.force_sampled_true) {
transaction.agent.sampler.root.shouldSample = function stubShouldSample() {
return true
}
}
forceAdaptiveSamplers(agent, testCase.force_adaptive_sampled)

for (const inboundHeader of testCase.inbound_headers.values()) {
transaction.acceptDistributedTraceHeaders(testCase.transport_type, inboundHeader)
Expand Down Expand Up @@ -406,6 +445,16 @@ async function runTestCase(testCase, parentTest) {
transaction.trace.root.touch()
transaction.end()

// check `expected_priority_between`
if (testCase.expected_priority_between) {
const [minPriority, maxPriority] = testCase.expected_priority_between
const actualPriority = transaction.priority
assert.ok(
actualPriority >= minPriority && actualPriority <= maxPriority,
`Expected transaction.priority (${actualPriority}) to be between ${minPriority} and ${maxPriority}`
)
}

// These tests assume setting a transport type even when there are not valid
// trace headers. This is slightly inconsistent with the spec. Given DT
// (NR format) does not include transport when there is no trace AND the
Expand Down Expand Up @@ -446,3 +495,21 @@ async function runTestCase(testCase, parentTest) {
end()
})
}

function forceAdaptiveSamplers(agent, forceAdaptiveSampled) {
if (forceAdaptiveSampled === undefined || forceAdaptiveSampled === null) return
// "The sampling decision to force on a transaction whenever the adaptive sampler is used"
// implies this affects all samplers that are adaptive samplers
const samplers = [
agent.sampler.root,
agent.sampler.remoteParentSampled,
agent.sampler.remoteParentNotSampled
]
for (const sampler of samplers) {
if (sampler?.toString() === 'AdaptiveSampler') {
sampler.shouldSample = function stubShouldSample() {
return forceAdaptiveSampled
}
}
}
}
15 changes: 14 additions & 1 deletion test/lib/cross_agent_tests/distributed_tracing/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ the agent under test. Here's what the various fields in each test case mean:
| `account_id` | The account id the agent would receive on connect. |
| `web_transaction` | Whether the transaction that's tested is a web transaction or not. |
| `raises_exception` | Whether to simulate an exception happening within the transaction or not, resulting in a transaction error event. |
| `force_sampled_true` | Whether to force a transaction to be sampled or not. |
| `root` | The sampler to use for transactions at the root of a trace. |
| `remote_parent_sampled` | The sampler to use for transactions with a remote parent. |
| `remote_parent_not_sampled` | The sampler to use for transactions with a remote parent that is not sampled. |
| `expected_priority_between` | The inclusive range of the expected priority value on the generated transaction event. |
| `force_adaptive_sampled` | The sampling decision to force on a transaction whenever the adaptive sampler is used. |
| `ratio` | The ratio to use for all of the trace ID ratio samplers defined in the test. For testing purposes we are not defining different ratios for each trace ID ratio sampler instance. If that is necessary, we will need a different way to configure the ratios. |
| `transport_type` | The transport type for the inbound request. |
| `inbound_headers` | The headers you should mock coming into the agent. |
| `outbound_payloads` | The exact/expected/unexpected values for outbound `w3c` headers. |
Expand All @@ -23,6 +28,14 @@ the agent under test. Here's what the various fields in each test case mean:
| `span_events_enabled` | Whether span events are enabled in the agent or not. |
| `transaction_events_enabled` | Whether transaction events are enabled in the agent or not. |

The samplers that can referenced in the `root`, `remote_parent_sampled`, and `remote_parent_not_sampled` fields are:

- `default`: Use the adaptive sampler.
- `adaptive`: Use the adaptive sampler.
- `trace_id_ratio_based`: Use the trace ID ratio sampler.
- `always_on`: Use the always on sampler.
- `always_off`: Use the always off sampler.

The `outbound_payloads` and `intrinsics` field can have nested values, for example:
```javascript
...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -796,78 +796,6 @@
["Supportability/DistributedTrace/CreatePayload/Success", 2]
]
},
{
"test_name": "payload_from_trusted_partnership_account",
"trusted_account_key": "44",
"account_id": "11",
"web_transaction": true,
"raises_exception": false,
"force_sampled_true": false,
"span_events_enabled": true,
"major_version": 0,
"minor_version": 1,
"transport_type": "HTTP",
"inbound_payloads": [
{
"v": [0, 1],
"d": {
"ac": "33",
"ap": "2827902",
"tx": "e8b91a159289ff74",
"pr": 0.123456,
"sa": false,
"ti": 1518469636035,
"tr": "d6b4ba0c3a712ca",
"tk": "44",
"ty": "App"
}
}
],
"outbound_payloads": [
{
"exact": {
"v": [0, 1],
"d.ac": "11",
"d.pr": 0.123456,
"d.sa": false,
"d.tr": "d6b4ba0c3a712ca",
"d.tk": "44",
"d.ty": "App"
},
"expected": ["d.ap", "d.tx", "d.ti"],
"unexpected": ["d.id"]
}
],
"intrinsics": {
"target_events": ["Transaction"],
"common":{
"exact": {
"parent.type": "App",
"parent.app": "2827902",
"parent.account": "33",
"parent.transportType": "HTTP",
"traceId": "d6b4ba0c3a712ca",
"priority": 0.123456,
"sampled": false
},
"expected": ["parent.transportDuration", "guid"],
"unexpected": ["grandparentId", "cross_process_id", "nr.tripId", "nr.pathHash", "nr.referringPathHash", "nr.guid", "nr.referringTransactionGuid", "nr.alternatePathHashes"]
},
"Transaction": {
"exact": {
"parentId": "e8b91a159289ff74"
}
}
},
"expected_metrics": [
["DurationByCaller/App/33/2827902/HTTP/all", 1],
["DurationByCaller/App/33/2827902/HTTP/allWeb", 1],
["TransportDuration/App/33/2827902/HTTP/all", 1],
["TransportDuration/App/33/2827902/HTTP/allWeb", 1],
["Supportability/DistributedTrace/AcceptPayload/Success", 1],
["Supportability/DistributedTrace/CreatePayload/Success", 1]
]
},
{
"test_name": "payload_has_larger_minor_version",
"trusted_account_key": "33",
Expand Down
Loading
Loading