Skip to content

Commit

Permalink
feat: Allow starting a span on a transaction that has already ended (#…
Browse files Browse the repository at this point in the history
…2653)

This was a restriction that dated back to the v1 APM Server intake
API that required all a transaction's spans to be sent along with the
transaction. This will allow for cases where transaction.span_count
reported values will *under* count.

The Java APM agent, FWIW, allows this.
  • Loading branch information
trentm authored Apr 21, 2022
1 parent 75c00b6 commit 9bdbd07
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 6 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ Notes:
child spans from being added to the exit spans. See issue for a full list of
spans types that will be treated as exit spans. ({issues}2601[#2601])
* Allow a new span to be created/started even if its transaction has ended.
This is expected to be a very rare use case. ({pull}2653[#2653])
[float]
===== Bug fixes
Expand Down
4 changes: 0 additions & 4 deletions lib/instrumentation/transaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,6 @@ Transaction.prototype.createSpan = function (...args) {
if (!this.sampled) {
return null
}
if (this.ended) {
this._agent.logger.debug('transaction already ended - cannot build new span %o', { trans: this.id, parent: this.parentId, trace: this.traceId }) // TODO: Should this be supported in the new API?
return null
}
if (this._builtSpans >= this._agent._conf.transactionMaxSpans) {
this._droppedSpans++
return null
Expand Down
7 changes: 5 additions & 2 deletions test/instrumentation/transaction.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const agent = require('../..').start({
var test = require('tape')

var Transaction = require('../../lib/instrumentation/transaction')
const Span = require('../../lib/instrumentation/span')

test('init', function (t) {
t.test('name and type', function (t) {
Expand Down Expand Up @@ -565,8 +566,10 @@ test('Transaction API on ended transaction', function (t) {
trans.ensureParentId()
t.pass('trans.ensureParentId(...) does not blow up')

const newSpan = trans.startSpan('aNewSpanName')
t.equal(newSpan, null, 'trans.startSpan(...) returns null')
// Starting a span after the transaction has ended is allowed.
const spanStartedAfterTransEnd = trans.startSpan('aNewSpanName')
t.ok(spanStartedAfterTransEnd instanceof Span, 'trans.startSpan(...) works')
spanStartedAfterTransEnd.end()

// Ending a span that is a child of the transaction uses some of the
// transaction fields for breakdown metrics calculation. Ensure that works.
Expand Down

0 comments on commit 9bdbd07

Please sign in to comment.