Skip to content

Commit

Permalink
cat: use full transaction name for path hash generation
Browse files Browse the repository at this point in the history
  • Loading branch information
@NatalieWolfe
NatalieWolfe committed May 17, 2017
1 parent 9ca6206 commit 8133031
Show file tree
Hide file tree
Showing 34 changed files with 299 additions and 231 deletions.
2 changes: 1 addition & 1 deletion api.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ API.prototype.getBrowserTimingHeader = function getBrowserTimingHeader() {
// bail gracefully outside a transaction
if (!trans) return _gracefail(1)

var name = trans.getName()
var name = trans.getFullName()

/* If we're in an unnamed transaction, add a friendly warning this is to
* avoid people going crazy, trying to figure out why browser monitoring is
Expand Down
19 changes: 9 additions & 10 deletions lib/agent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -887,17 +887,17 @@ function _addIntrinsicAttrsFromTransaction(transaction) {

// FLAG: cat
if (this.config.feature_flag.cat) {
if (!transaction.invalidIncomingExternalTransaction &&
(
transaction.referringTransactionGuid ||
transaction.includesOutboundRequests()
)
) {
if (
!transaction.invalidIncomingExternalTransaction && (
transaction.referringTransactionGuid ||
transaction.includesOutboundRequests()
)
) {
intrinsicAttributes['nr.guid'] = transaction.id
intrinsicAttributes['nr.tripId'] = transaction.tripId || transaction.id
intrinsicAttributes['nr.pathHash'] = hashes.calculatePathHash(
this.config.applications()[0],
transaction.name || transaction.nameState.getName(),
transaction.getFullName(),
transaction.referringPathHash
)
if (transaction.referringPathHash) {
Expand Down Expand Up @@ -971,9 +971,8 @@ Agent.prototype._transactionFinished = function _transactionFinished(transaction
logger.trace({trace_dump: transaction.describer.verbose}, 'Dumped transaction state.')
}

// Allow the API to explicitly set the ignored status on bg-tx.
// This is handled for web-tx when finalizeNameFromUri is called on the tx.
if (!transaction.isWeb() && transaction.forceIgnore !== null) {
// Allow the API to explicitly set the ignored status.
if (transaction.forceIgnore !== null) {
transaction.ignore = transaction.forceIgnore
}

Expand Down
5 changes: 2 additions & 3 deletions lib/errors/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,8 @@ function createError(transaction, exception, customParameters, config) {

if (transaction) {
// transaction.getName is expensive due to running normalizers and ignore
// rules if a name hasn't been assigned yet. Also has the side effect of
// changing the transaction's url property or ignore status.
var txName = transaction.getName()
// rules if a name hasn't been assigned yet.
var txName = transaction.getFullName()
if (txName) {
name = txName
}
Expand Down
10 changes: 3 additions & 7 deletions lib/instrumentation/core/http.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,14 +393,10 @@ function wrapWriteHead(agent, writeHead) {
// actual duration.
transaction.catResponseTime = transaction.timer.getDurationInMillis()

var appData
var txName = transaction.name || transaction.nameState.getName() || ''
var appData = null
var txName = transaction.getFullName() || ''

try {
if (txName) {
txName = NAMES.WEB.RESPONSE_TIME + '/' + txName
}

appData = JSON.stringify([
agent.config.cross_process_id, // cross_process_id
txName, // transaction name
Expand Down Expand Up @@ -455,7 +451,7 @@ function wrapRequest(agent, request) {

var pathHash = hashes.calculatePathHash(
agent.config.applications()[0],
transaction.name || transaction.nameState.getName() || '',
transaction.getFullName() || '',
transaction.referringPathHash
)
transaction.pushPathHash(pathHash)
Expand Down
143 changes: 88 additions & 55 deletions lib/transaction/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ var logger = require('../logger').child({component: 'transaction'})
*
*/
var FROM_MILLIS = 1e-3
var TYPE_NAMES = {
var TYPES = {
WEB: 'web',
BG: 'bg',
MESSAGE: 'message'
}
var TYPE_METRICS = {
web: NAMES.WEB.RESPONSE_TIME,
bg: NAMES.OTHER_TRANSACTION.RESPONSE_TIME,
message: NAMES.OTHER_TRANSACTION.MESSAGE
Expand Down Expand Up @@ -93,11 +98,13 @@ function Transaction(agent) {
this.parsedUrl = null
this.verb = null
this.baseSegment = null
this.type = 'web'
this.type = TYPES.WEB

this.probe('Transaction created', {id: this.id})
}

Transaction.TYPES = TYPES

Transaction.prototype.probe = function probe(action, extra) {
if (this.traceStacks) {
this.traceStacks.push({
Expand All @@ -114,7 +121,7 @@ Transaction.prototype.probe = function probe(action, extra) {
* @returns {boolean} Whether this transaction has a URL.
*/
Transaction.prototype.isWeb = function isWeb() {
return this.url ? true : false
return this.type === TYPES.WEB
}

/**
Expand Down Expand Up @@ -179,21 +186,18 @@ Transaction.prototype.getResponseTimeInMillis = function getResponseTimeInMillis
Transaction.prototype._runUserNamingRules = function _runUserNamingRules(requestUrl) {
// 1. user normalization rules (set in configuration)
var normalized = this.agent.userNormalizer.normalize(requestUrl)
if (normalized.ignore) {
this.ignore = true
}
if (normalized.matched) {
// After applying user naming rule, apply server-side sent rules to
// further squash possible MGIs
var serverNormalized = this.agent.urlNormalizer.normalize(normalized.value)
if (serverNormalized.ignore) {
this.ignore = true
normalized.ignore = true
}
if (serverNormalized.matched) {
// NAMES.NORMALIZED is prepended by the sever rule normalizer
this._partialName = serverNormalized.value
normalized.value = serverNormalized.value
} else {
this._partialName = NAMES.NORMALIZED + normalized.value
normalized.value = NAMES.NORMALIZED + normalized.value
}
}
return normalized
Expand Down Expand Up @@ -225,6 +229,49 @@ Transaction.prototype.setPartialName = function setPartialName(name) {
this._partialName = name
}

Transaction.prototype._partialNameFromUri = function _nameFromUri(requestUrl, status) {
var scrubbedUrl = urltils.scrub(requestUrl)

// 0. If there is a name in the name-state stack, use it.
var partialName = this._partialName
var ignore = false
if (!this.nameState.isEmpty()) {
partialName = this.nameState.getFullName()
}

// 1. name set by the api
if (this.forceName !== null) {
partialName = this.forceName
}

// 2. user normalization rules (set in configuration) can override transaction
// naming from API
var userNormalized = this._applyUserNamingRules(scrubbedUrl)
ignore = ignore || userNormalized.ignore
if (userNormalized.matched) {
partialName = userNormalized.value
}

// 3. URL normalization rules (sent by server).
// Nothing has already set a name for this transaction, so normalize and
// potentially apply the URL backstop now. Only do so if no user rules matched.
if (!partialName) {
// avoid polluting root path when 404
if (status === 404) {
partialName = this.nameState.getNameNotFound()
} else {
var normalized = this.agent.urlNormalizer.normalize(scrubbedUrl)
ignore = ignore || normalized.ignore
partialName = normalized.value
}
}

return {
ignore: ignore,
value: partialName
}
}

/**
* Derive the transaction partial name from the given url and status code.
*
Expand Down Expand Up @@ -311,45 +358,23 @@ function finalizeNameFromUri(requestURL, statusCode) {
this.url = urltils.scrub(requestURL)
this.statusCode = statusCode

// 0. If there is a name in the name-state stack, use it.
if (!this.nameState.isEmpty()) {
this._partialName = this.nameState.getFullName()
if (this.baseSegment) {
this.nameState.forEachParams(function forEachRouteParams(params) {
urltils.copyParameters(this.agent.config, params, this.baseSegment.parameters)
}, this)
}
}

// 1. name set by the api
if (this.forceName !== null) {
this._partialName = this.forceName
// Derive the name from the request URL.
var partialName = this._partialNameFromUri(requestURL, statusCode)
this._partialName = partialName.value
if (partialName.ignore) {
this.ignore = true
}

// 2. user normalization rules (set in configuration)
// can override transaction naming from API
this.applyUserNamingRules(this.url)

// 3. URL normalization rules (sent by server)
// Nothing has already set a name for this transaction, so normalize and
// potentially apply the URL backstop now. Only do so if no user rules
// matched.
if (!this._partialName) {
// avoid polluting root path when 404
if (statusCode === 404) {
this._partialName = this.nameState.getNameNotFound()
} else {
var normalized = this.agent.urlNormalizer.normalize(this.url)
if (normalized.ignore) {
this.ignore = true
}
this._partialName = normalized.value
}
// If a namestate stack exists, copy route parameters over to the trace.
if (!this.nameState.isEmpty() && this.baseSegment) {
this.nameState.forEachParams(function forEachRouteParams(params) {
urltils.copyParameters(this.agent.config, params, this.baseSegment.parameters)
}, this)
}

// 4. transaction name normalization rules (sent by server)
var fullName = TYPE_NAMES[this.type] + '/' + this._partialName
normalized = this.agent.transactionNameNormalizer.normalize(fullName)
// Apply transaction name normalization rules (sent by server) to full name.
var fullName = TYPE_METRICS[this.type] + '/' + this._partialName
var normalized = this.agent.transactionNameNormalizer.normalize(fullName)
if (normalized.ignore) {
this.ignore = true
}
Expand All @@ -376,7 +401,7 @@ function finalizeNameFromUri(requestURL, statusCode) {
Transaction.prototype.finalizeName = function finalizeName(name) {
this._partialName = name

var fullName = TYPE_NAMES[this.type] + '/' + this._partialName
var fullName = TYPE_METRICS[this.type] + '/' + this._partialName

// Transaction normalizers run on the full metric name, not the user facing
// transaction name.
Expand Down Expand Up @@ -404,15 +429,23 @@ Transaction.prototype.finalizeName = function finalizeName(name) {
* is called.
*/
Transaction.prototype.getName = function getName() {
// Detect web transactions as they are more complex.
if (this.isWeb() && !this.name) {
// Save and restore the partial name, as some instrumentation relies on it
// not being set to detect if it should set the transaction name.
var tempPartialName = this._partialName
this.finalizeNameFromUri(this.url, this.statusCode)
this._partialName = tempPartialName
}
return this.name
if (this.isWeb() && this.url) {
return this._partialNameFromUri(this.url, this.statusCode).value
}
return this._partialName
}

Transaction.prototype.getFullName = function getFullName() {
if (this.name) {
return this.name
}

var name = this.getName()
if (!name) {
return null
}
var fullName = TYPE_METRICS[this.type] + '/' + name
return this.agent.transactionNameNormalizer.normalize(fullName).value
}

/**
Expand Down Expand Up @@ -536,7 +569,7 @@ Transaction.prototype.includesOutboundRequests = function includesOutboundReques
Transaction.prototype.alternatePathHashes = function alternatePathHashes() {
var curHash = hashes.calculatePathHash(
this.agent.config.applications()[0],
this.name || this._partialName || this.nameState.getName(),
this.getFullName(),
this.referringPathHash
)
var altHashes = this.pathHashes.slice()
Expand Down
26 changes: 17 additions & 9 deletions test/integration/cat/cat.tap.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ var END_PORT = 10002
var CROSS_PROCESS_ID = '1337#7331'


test('cross application tracing full integration', function (t) {
test('cross application tracing full integration', function(t) {
t.plan(57)
var feature_flag = {
cat: true
Expand All @@ -40,34 +40,41 @@ test('cross application tracing full integration', function (t) {

// Naming is how the requests will flow through the system, to test that all
// metrics are generated as expected as well as the dirac events.
var start = generateServer(http, api, START_PORT, started, function (req, res) {
http.get(generateUrl(MIDDLE_PORT, 'start/middle'), function (externRes) {
var start = generateServer(http, api, START_PORT, started, function(req, res) {
var tx = agent.tracer.getTransaction()
tx.nameState.appendPath('foobar')
http.get(generateUrl(MIDDLE_PORT, 'start/middle'), function(externRes) {
externRes.resume()
externRes.on('end', function() {
tx.nameState.popPath('foobar')
res.end()
})
})
})

var middle = generateServer(http, api, MIDDLE_PORT, started, function (req, res) {
var middle = generateServer(http, api, MIDDLE_PORT, started, function(req, res) {
t.ok(req.headers['x-newrelic-id'], 'middle received x-newrelic-id from start')
t.ok(req.headers['x-newrelic-transaction'], 'middle received x-newrelic-transaction from start')
http.get(generateUrl(END_PORT, 'middle/end'), function (externRes) {

var tx = agent.tracer.getTransaction()
tx.nameState.appendPath('foobar')
http.get(generateUrl(END_PORT, 'middle/end'), function(externRes) {
externRes.resume()
externRes.on('end', function() {
tx.nameState.popPath('foobar')
res.end()
})
})
})

var end = generateServer(http, api, END_PORT, started, function (req, res) {
var end = generateServer(http, api, END_PORT, started, function(req, res) {
t.ok(req.headers['x-newrelic-id'], 'end received x-newrelic-id from middle')
t.ok(req.headers['x-newrelic-transaction'], 'end received x-newrelic-transaction from middle')
res.end()
})

function runTest() {
http.get(generateUrl(START_PORT, 'start'), function (res) {
http.get(generateUrl(START_PORT, 'start'), function(res) {
res.resume()
start.close()
middle.close()
Expand Down Expand Up @@ -209,8 +216,9 @@ test('cross application tracing full integration', function (t) {
})

function generateServer(http, api, port, started, responseHandler) {
var server = http.createServer(function (req, res) {
api.agent.getTransaction().nameState.appendPath(req.url)
var server = http.createServer(function(req, res) {
var tx = api.agent.getTransaction()
tx.nameState.appendPath(req.url)
req.resume()
responseHandler(req, res)
})
Expand Down
Loading

0 comments on commit 8133031

Please sign in to comment.