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.

Sign up for GitHub

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

feat: Allow starting a span on a transaction that has already ended #2653

Merged
merged 2 commits into from
Apr 21, 2022
Merged

feat: Allow starting a span on a transaction that has already ended #2653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
445338a
feat: Allow starting a span on a transaction that has already ended
trentm Apr 20, 2022
3275afb
changelog entry
trentm Apr 20, 2022
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
3 changes: 3 additions & 0 deletions CHANGELOG.asciidoc
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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