Skip to content

Commit

Permalink
Check if span is active when set as root
Browse files Browse the repository at this point in the history
Use the `setRoot` function to set the root span. Make sure that when the
root span is set, it's still open. If it's not open, don't set it.
Setting a closed root span is a worse experience and may break things,
because it introduces a broken state.
  • Loading branch information
tombruijn committed Jun 3, 2022
1 parent bb2ea7e commit 8a32a21
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: "patch"
type: "fix"
---

Only allow open root spans to be set as root. This avoids closed root spans to be reused in child contexts.
12 changes: 12 additions & 0 deletions packages/nodejs/src/__tests__/scope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,18 @@ describe("ScopeManager", () => {
expect(scopeManager.root()).toBe(span)
expect(scopeManager.active()).toBe(span)
})

it("when root span is closed, it doesn't overwrite the root span", () => {
const span1 = new RootSpan()
scopeManager.setRoot(span1)

const span2 = new RootSpan()
span2.close()
scopeManager.setRoot(span2)

expect(scopeManager.root()).toBe(span1)
expect(scopeManager.active()).toBe(span1)
})
})

describe(".root()", () => {
Expand Down
8 changes: 5 additions & 3 deletions packages/nodejs/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,11 @@ export class ScopeManager {
* Sets the root `Span`
*/
public setRoot(rootSpan: Span) {
const uid = asyncHooks.executionAsyncId()
this.#roots.set(uid, rootSpan)
this.#scopes.set(uid, rootSpan)
if (rootSpan.open) {
const uid = asyncHooks.executionAsyncId()
this.#roots.set(uid, rootSpan)
this.#scopes.set(uid, rootSpan)
}
}

/*
Expand Down

0 comments on commit 8a32a21

Please sign in to comment.