From 2e521c205933a585728f40c9dbc617f1d2372d0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Wed, 11 Dec 2019 16:15:33 -0800 Subject: [PATCH 1/2] fix: Dont require transaction to be on the scope to be delivered correctly --- CHANGELOG.md | 2 +- packages/apm/src/span.ts | 3 +++ packages/hub/src/scope.ts | 4 ---- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b77274f896d2..6ff4559c4402 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -Coming soon... +- [apm] fix: Always attach `contexts.trace` to finished transaction (#2353) ## 5.10.2 diff --git a/packages/apm/src/span.ts b/packages/apm/src/span.ts index dfa00e97072a..2ea97b86ed4d 100644 --- a/packages/apm/src/span.ts +++ b/packages/apm/src/span.ts @@ -287,6 +287,9 @@ export class Span implements SpanInterface, SpanContext { } return this._hub.captureEvent({ + contexts: { + trace: this.getTraceContext(), + }, spans: finishedSpans, start_timestamp: this.startTimestamp, tags: this.tags, diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index fd3cd6ef331f..3867f04e9eb5 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -327,10 +327,6 @@ export class Scope implements ScopeInterface { if (this._transaction) { event.transaction = this._transaction; } - if (this._span) { - event.contexts = event.contexts || {}; - event.contexts.trace = this._span.getTraceContext(); - } this._applyFingerprint(event); From 3541a04658645771edec1b2c55d73d790caf7af6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Tue, 17 Dec 2019 14:05:04 +0100 Subject: [PATCH 2/2] test: Added tests for contexts.trace --- packages/apm/test/span.test.ts | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/packages/apm/test/span.test.ts b/packages/apm/test/span.test.ts index 4c4313a998b6..d7be6241aae2 100644 --- a/packages/apm/test/span.test.ts +++ b/packages/apm/test/span.test.ts @@ -164,31 +164,48 @@ describe('Span', () => { expect(span.timestamp).toBeGreaterThan(1); }); - test('finish a scope span without transaction', () => { + test('finish a span without transaction', () => { const spy = jest.spyOn(hub as any, 'captureEvent'); const span = new Span({}, hub); span.finish(); expect(spy).not.toHaveBeenCalled(); }); - test('finish a scope span with transaction', () => { + test('finish a span with transaction', () => { const spy = jest.spyOn(hub as any, 'captureEvent') as any; const span = new Span({ transaction: 'test', sampled: false }, hub); span.initFinishedSpans(); span.finish(); - expect(spy).toHaveBeenCalled(); expect(spy.mock.calls[0][0].spans).toHaveLength(0); + expect(spy.mock.calls[0][0].contexts.trace).toEqual(span.getTraceContext()); }); - test('finish a scope span with transaction + child span', () => { + test('finish a span with transaction + child span', () => { const spy = jest.spyOn(hub as any, 'captureEvent') as any; const parentSpan = new Span({ transaction: 'test', sampled: false }, hub); parentSpan.initFinishedSpans(); const childSpan = parentSpan.child(); childSpan.finish(); parentSpan.finish(); - expect(spy).toHaveBeenCalled(); expect(spy.mock.calls[0][0].spans).toHaveLength(1); + expect(spy.mock.calls[0][0].contexts.trace).toEqual(parentSpan.getTraceContext()); + }); + + test('finish a span with another one on the scope shouldnt override contexts.trace', () => { + const spy = jest.spyOn(hub as any, 'captureEvent') as any; + + const spanOne = new Span({ transaction: 'testOne', sampled: false }, hub); + spanOne.initFinishedSpans(); + const childSpanOne = spanOne.child(); + childSpanOne.finish(); + hub.configureScope((scope) => { scope.setSpan(spanOne) }) + + const spanTwo = new Span({ transaction: 'testTwo', sampled: false }, hub); + spanTwo.initFinishedSpans(); + spanTwo.finish(); + + expect(spy.mock.calls[0][0].spans).toHaveLength(0); + expect(spy.mock.calls[0][0].contexts.trace).toEqual(spanTwo.getTraceContext()); }); });