Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Update plugin to account for removing segment from transaction and child segments #337

Merged
merged 5 commits into from
Jan 14, 2025
Merged
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
12 changes: 6 additions & 6 deletions THIRD_PARTY_NOTICES.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ code, the source code can be found at [https://github.com/newrelic/newrelic-node

### @apollo/server

This product includes source derived from [@apollo/server](https://github.com/apollographql/apollo-server) ([v4.10.3](https://github.com/apollographql/apollo-server/tree/v4.10.3)), distributed under the [MIT License](https://github.com/apollographql/apollo-server/blob/v4.10.3/README.md):
This product includes source derived from [@apollo/server](https://github.com/apollographql/apollo-server) ([v4.11.2](https://github.com/apollographql/apollo-server/tree/v4.11.2)), distributed under the [MIT License](https://github.com/apollographql/apollo-server/blob/v4.11.2/README.md):

```
MIT License
Expand Down Expand Up @@ -509,7 +509,7 @@ This product includes source derived from [@newrelic/newrelic-oss-cli](https://g

### @newrelic/test-utilities

This product includes source derived from [@newrelic/test-utilities](https://github.com/newrelic/node-test-utilities) ([v8.5.0](https://github.com/newrelic/node-test-utilities/tree/v8.5.0)), distributed under the [Apache-2.0 License](https://github.com/newrelic/node-test-utilities/blob/v8.5.0/LICENSE):
This product includes source derived from [@newrelic/test-utilities](https://github.com/newrelic/node-test-utilities) ([v8.7.0](https://github.com/newrelic/node-test-utilities/tree/v8.7.0)), distributed under the [Apache-2.0 License](https://github.com/newrelic/node-test-utilities/blob/v8.7.0/LICENSE):

```
Apache License
Expand Down Expand Up @@ -905,7 +905,7 @@ OTHER DEALINGS IN THE SOFTWARE.

### eslint

This product includes source derived from [eslint](https://github.com/eslint/eslint) ([v8.57.0](https://github.com/eslint/eslint/tree/v8.57.0)), distributed under the [MIT License](https://github.com/eslint/eslint/blob/v8.57.0/LICENSE):
This product includes source derived from [eslint](https://github.com/eslint/eslint) ([v8.57.1](https://github.com/eslint/eslint/tree/v8.57.1)), distributed under the [MIT License](https://github.com/eslint/eslint/blob/v8.57.1/LICENSE):

```
Copyright OpenJS Foundation and other contributors, <www.openjsf.org>
Expand All @@ -932,7 +932,7 @@ THE SOFTWARE.

### graphql

This product includes source derived from [graphql](https://github.com/graphql/graphql-js) ([v16.8.1](https://github.com/graphql/graphql-js/tree/v16.8.1)), distributed under the [MIT License](https://github.com/graphql/graphql-js/blob/v16.8.1/LICENSE):
This product includes source derived from [graphql](https://github.com/graphql/graphql-js) ([v16.9.0](https://github.com/graphql/graphql-js/tree/v16.9.0)), distributed under the [MIT License](https://github.com/graphql/graphql-js/blob/v16.9.0/LICENSE):

```
MIT License
Expand Down Expand Up @@ -1019,7 +1019,7 @@ SOFTWARE.

### lockfile-lint

This product includes source derived from [lockfile-lint](https://github.com/lirantal/lockfile-lint) ([v4.13.2](https://github.com/lirantal/lockfile-lint/tree/v4.13.2)), distributed under the [Apache-2.0 License](https://github.com/lirantal/lockfile-lint/blob/v4.13.2/LICENSE):
This product includes source derived from [lockfile-lint](https://github.com/lirantal/lockfile-lint) ([v4.14.0](https://github.com/lirantal/lockfile-lint/tree/v4.14.0)), distributed under the [Apache-2.0 License](https://github.com/lirantal/lockfile-lint/blob/v4.14.0/LICENSE):

```

Expand Down Expand Up @@ -1217,7 +1217,7 @@ This product includes source derived from [lockfile-lint](https://github.com/lir

### newrelic

This product includes source derived from [newrelic](https://github.com/newrelic/node-newrelic) ([v11.15.0](https://github.com/newrelic/node-newrelic/tree/v11.15.0)), distributed under the [Apache-2.0 License](https://github.com/newrelic/node-newrelic/blob/v11.15.0/LICENSE):
This product includes source derived from [newrelic](https://github.com/newrelic/node-newrelic) ([v11.23.2](https://github.com/newrelic/node-newrelic/tree/v11.23.2)), distributed under the [Apache-2.0 License](https://github.com/newrelic/node-newrelic/blob/v11.23.2/LICENSE):

```
Apache License
Expand Down
19 changes: 8 additions & 11 deletions lib/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,12 @@ function createResolverSegment({ shim, parent, resolver, formattedPath }) {
* @param {Object} params.config plugin config
* @param {Object} segment relevant resolver segment
* @param {string} scope name of transaction
* @param {Transaction} transaction active transaction
*/
function recordResolveSegment(segment, scope) {
function recordResolveSegment(segment, scope, transaction) {
const duration = segment.getDurationInMillis()
const exclusive = segment.getExclusiveDurationInMillis()

const transaction = segment.transaction

const attributes = segment.getAttributes()
const fieldName = attributes[FIELD_NAME_ATTR]
const fieldType = attributes[PARENT_TYPE_ATTR]
Expand All @@ -102,12 +101,10 @@ function recordResolveSegment(segment, scope) {
}
}

function recordOperationSegment(segment, scope) {
function recordOperationSegment(segment, scope, transaction) {
const duration = segment.getDurationInMillis()
const exclusive = segment.getExclusiveDurationInMillis()

const transaction = segment.transaction

createMetricPairs(transaction, segment.name, scope, duration, exclusive)
}

Expand All @@ -132,9 +129,10 @@ function createModuleUsageMetric(agent) {
*
* @param {Object} context apollo request context
* @param {Segment} operationSegment default segment created in request start
* @param {Transaction} transaction active transaction
* @return {Boolean} true if document could be parsed from context
*/
function updateOperationSegmentName(context, operationSegment) {
function updateOperationSegmentName(context, operationSegment, transaction) {
const operationDetails = getOperationDetails(context)
if (operationDetails) {
const { operationName, operationType, deepestUniquePath, cleanedQuery } = operationDetails
Expand All @@ -157,7 +155,7 @@ function updateOperationSegmentName(context, operationSegment) {

const segmentName = formattedOperation
const transactionName = formattedOperation
setTransactionName(operationSegment.transaction, transactionName)
setTransactionName(transaction, transactionName)
operationSegment.name = `${OPERATION_PREFIX}/${segmentName}`
return true
}
Expand All @@ -173,13 +171,12 @@ function updateOperationSegmentName(context, operationSegment) {
* @param {Object} params
* @param {Object} params.config plugin config
* @param {Object} params.info info key from resolver context
* @param {Object} params.operationSegment operation segment
* @param {Object} params.transaction active transaction
* @param {Object} params.args args key from resolver context
*
*/
function maybeCaptureFieldMetrics({ config, info, operationSegment, args }) {
function maybeCaptureFieldMetrics({ config, info, transaction, args }) {
if (config.captureFieldMetrics) {
const transaction = operationSegment.transaction
const fieldName = info.fieldName
const fieldType = info.parentType.toString()
captureFieldMetrics({ transaction, args, fieldName, fieldType })
Expand Down
3 changes: 1 addition & 2 deletions lib/error-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ class ErrorHelper {

noticeError(instrumentationApi, error) {
error = error.originalError || error
const activeSegment = instrumentationApi.getActiveSegment()
const transaction = activeSegment && activeSegment.transaction
const transaction = instrumentationApi.tracer.getTransaction()
instrumentationApi.agent.errors.add(transaction, error, error.extensions)
}
}
Expand Down
34 changes: 18 additions & 16 deletions lib/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ module.exports = function requestDidStart({ api, shim, logger, config }, request
logger.trace('Begin requestDidStart')

const requestParent = shim.getActiveSegment()
const transaction = shim.tracer.getTransaction()

if (!requestParent) {
logger.trace('No active segment found at query start. Not recording.')
Expand All @@ -60,10 +61,10 @@ module.exports = function requestDidStart({ api, shim, logger, config }, request

return {
didResolveOperation: didResolveOperation.bind(null, {
shim,
config,
logger,
operationSegment
operationSegment,
transaction
}),
didEncounterErrors: didEncounterErrors.bind(null, { shim, operationSegment }),
executionDidStart() {
Expand All @@ -79,7 +80,8 @@ module.exports = function requestDidStart({ api, shim, logger, config }, request
config,
logger,
operationSegment,
requestContext
requestContext,
transaction
})
}
},
Expand All @@ -88,7 +90,8 @@ module.exports = function requestDidStart({ api, shim, logger, config }, request
shim,
config,
logger,
operationSegment
operationSegment,
transaction
})
}
}
Expand All @@ -104,14 +107,10 @@ module.exports = function requestDidStart({ api, shim, logger, config }, request
* @param {TraceSegment} params.operationSegment the segment capturing the operation
* @param {GraphQLRequestContext} requestContext context on operation resolution
*/
function didResolveOperation({ shim, config, logger, operationSegment }, requestContext) {
updateOperationSegmentName(requestContext, operationSegment)
function didResolveOperation({ config, logger, operationSegment, transaction }, requestContext) {
updateOperationSegmentName(requestContext, operationSegment, transaction)
if (shouldIgnoreTransaction(requestContext.operation, config, logger)) {
const activeSegment = shim.getActiveSegment()
if (activeSegment) {
const transaction = activeSegment.transaction
transaction.setForceIgnore(true)
}
transaction.setForceIgnore(true)
}
}

Expand Down Expand Up @@ -146,15 +145,15 @@ function didEncounterErrors({ shim, operationSegment }, requestContext) {
* @returns {Function} handler once field resolves, used to handle errors
*/
function willResolveField(
{ api, shim, logger, config, operationSegment, requestContext },
{ api, shim, logger, config, operationSegment, requestContext, transaction },
resolverContext
) {
const { info, args } = resolverContext
const pathArray = flattenToArray(info.path)
const formattedPath = pathArray.reverse().join('.')
const flattenedArgs = flattenArgs({ obj: args })

maybeCaptureFieldMetrics({ operationSegment, info, args: flattenedArgs, config })
maybeCaptureFieldMetrics({ transaction, info, args: flattenedArgs, config })

if (!config.captureScalars && !isTopLevelField(info) && isScalar(info)) {
return null
Expand Down Expand Up @@ -237,13 +236,16 @@ function fieldResolver({ shim, requestContext, resolverSegment, currentSegment }
* @param {TraceSegment} params.operationSegment the segment capturing the operation
* @param {GraphQLRequestContext} requestContext context on operation resolution
*/
function willSendResponse({ api, shim, config, logger, operationSegment }, requestContext) {
function willSendResponse(
{ api, shim, config, logger, operationSegment, transaction },
requestContext
) {
// check if operation segment was never updated from default name
// If so, try to rename before setting the transaction name to `*`
if (operationSegment.name === DEFAULT_OPERATION_NAME) {
const updated = updateOperationSegmentName(requestContext, operationSegment)
const updated = updateOperationSegmentName(requestContext, operationSegment, transaction)
if (!updated) {
setTransactionName(operationSegment.transaction, '*')
setTransactionName(transaction, '*')
}
}

Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@
"husky": "^6.0.0",
"lint-staged": "^11.0.0",
"lockfile-lint": "^4.9.6",
"newrelic": "^11.15.0",
"newrelic": "^12.11.0",
"prettier": "^2.3.2",
"sinon": "^11.1.2",
"tsd": "^0.18.0"
},
"peerDependencies": {
"newrelic": ">=6.13.0"
"newrelic": ">=12.11.0"
},
"tsd": {
"directory": "./tests/types",
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/config-capture-scalars.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ tests.push({
}

// Exact match to ensure no extra fields snuck in
assertSegments(transaction.trace.root, expectedSegments, { exact: true })
assertSegments(transaction.trace, transaction.trace.root, expectedSegments, { exact: true })
})

executeQuery(serverUrl, query, (err) => {
Expand Down
79 changes: 72 additions & 7 deletions tests/lib/custom-assertions.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,64 @@ function assertMetrics(
}

/**
* This function is used to verify that a tree of trace segments matches an
* expected tree of segment names. For example, if the trace looks like (i.e
* the `parent` parameter):
*
* ```js
* {
* name: 'root-segment',
* children: [
* {
* name: 'child 1',
* children: [
* {
* name: 'grandchild 1',
* children: [
* {
* name: 'great-grandchild',
* children: []
* }
* ]
* },
* {
* name: 'grandchild 2',
* children: []
* }
* ]
* },
* {
* name: 'child 2',
* children: []
* }
* ]
* }
* ```
*
* Then the provided `expected` parameter should look like:
*
* ```js
* [
* 'root-segment',
* [
* 'child 1',
* [
* 'grandchild 1',
* ['great-grandchild],
* 'grandchild 2'
* ],
* 'child 2'
* ],
* ]
* ```
*
* Ordering of the elements in the `expected` parameter is significant when
* `options.exact = true`. Regardless of the `exact` value, ordering of elements
* is significant to indicate the nesting order. Any string immediately
* followed by an array of strings indicates that the first string is a parent
* element, and the subsequent array of strings is its child elements.
*
* @param {Trace} trace Transaction trace
* @param {TraceSegment} parent Parent segment
* @param {Array} expected Array of strings that represent segment names.
* If an item in the array is another array, it
Expand All @@ -78,7 +136,13 @@ function assertMetrics(
* @param {object} [deps] Injected dependencies.
* @param {object} [deps.assert] Assertion library to use.
*/
function assertSegments(parent, expected, options, { assert = require('node:assert') } = {}) {
function assertSegments(
trace,
parent,
expected,
options,
{ assert = require('node:assert') } = {}
) {
let child
let childCount = 0

Expand All @@ -91,7 +155,8 @@ function assertSegments(parent, expected, options, { assert = require('node:asse
}

function getChildren(_parent) {
return _parent.children.filter(function (item) {
const children = trace.getChildren(_parent.id)
return children.filter(function (item) {
if (exact && options && options.exclude) {
return options.exclude.indexOf(item.name) === -1
}
Expand Down Expand Up @@ -126,7 +191,7 @@ function assertSegments(parent, expected, options, { assert = require('node:asse
)
}
} else if (typeof sequenceItem === 'object') {
assertSegments(child, sequenceItem, options, { assert })
assertSegments(trace, child, sequenceItem, options, { assert })
}
}

Expand All @@ -138,14 +203,14 @@ function assertSegments(parent, expected, options, { assert = require('node:asse

if (typeof sequenceItem === 'string') {
// find corresponding child in parent
for (let j = 0; j < parent.children.length; j++) {
if (parent.children[j].name.startsWith(sequenceItem) === true) {
child = parent.children[j]
for (let j = 0; j < children.length; j++) {
if (children[j].name.startsWith(sequenceItem) === true) {
child = children[j]
}
}
assert.ok(child, 'segment "' + parent.name + '" should have child "' + sequenceItem + '"')
if (typeof expected[i + 1] === 'object') {
assertSegments(child, expected[i + 1], { exact }, { assert })
assertSegments(trace, child, expected[i + 1], { exact }, { assert })
}
}
}
Expand Down
21 changes: 21 additions & 0 deletions tests/lib/find-segment.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright 2025 New Relic Corporation. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

'use strict'
module.exports = function findSegmentByName(trace, root, name) {
const children = trace.getChildren(root.id)
if (root.name === name) {
return root
} else if (children.length) {
for (let i = 0; i < children.length; i++) {
const child = children[i]
const found = findSegmentByName(trace, child, name)
if (found) {
return found
}
}
}
return null
}
Loading
Loading