From 8e999d7e74c6b43ceec035fb9734871b97ce8494 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Mon, 9 Mar 2020 11:55:14 +0100 Subject: [PATCH 1/6] ref: Setup code --- packages/apm/src/integrations/tracing.ts | 130 ++++++++++++++--------- 1 file changed, 82 insertions(+), 48 deletions(-) diff --git a/packages/apm/src/integrations/tracing.ts b/packages/apm/src/integrations/tracing.ts index d074a1975e50..551ea8494f60 100644 --- a/packages/apm/src/integrations/tracing.ts +++ b/packages/apm/src/integrations/tracing.ts @@ -187,27 +187,7 @@ export class Tracing implements Integration { return; } - if (Tracing.options.traceXHR) { - addInstrumentationHandler({ - callback: xhrCallback, - type: 'xhr', - }); - } - - if (Tracing.options.traceFetch && supportsNativeFetch()) { - addInstrumentationHandler({ - callback: fetchCallback, - type: 'fetch', - }); - } - - if (Tracing.options.startTransactionOnLocationChange) { - addInstrumentationHandler({ - callback: historyCallback, - type: 'history', - }); - } - + // Starting our inital pageload transaction if (global.location && global.location.href) { // `${global.location.href}` will be used a temp transaction name Tracing.startIdleTransaction(global.location.href, { @@ -216,36 +196,15 @@ export class Tracing implements Integration { }); } - /** - * If an error or unhandled promise occurs, we mark the active transaction as failed - */ - // tslint:disable-next-line: completed-docs - function errorCallback(): void { - if (Tracing._activeTransaction) { - logger.log(`[Tracing] Global error occured, setting status in transaction: ${SpanStatus.InternalError}`); - (Tracing._activeTransaction as SpanClass).setStatus(SpanStatus.InternalError); - } - } + this._setupXHRTracing(); - addInstrumentationHandler({ - callback: errorCallback, - type: 'error', - }); + this._setupFetchTracing(); - addInstrumentationHandler({ - callback: errorCallback, - type: 'unhandledrejection', - }); + this._setupHistory(); - if (Tracing.options.discardBackgroundSpans && global.document) { - document.addEventListener('visibilitychange', () => { - if (document.hidden && Tracing._activeTransaction) { - logger.log('[Tracing] Discarded active transaction incl. activities since tab moved to the background'); - Tracing._activeTransaction = undefined; - Tracing._activities = {}; - } - }); - } + this._setupErrorHandling(); + + this._setupBackgroundTabDetection(); // This EventProcessor makes sure that the transaction is not longer than maxTransactionDuration addGlobalEventProcessor((event: Event) => { @@ -271,6 +230,81 @@ export class Tracing implements Integration { }); } + /** + * Discards active transactions if tab moves to background + */ + private _setupBackgroundTabDetection(): void { + if (Tracing.options.discardBackgroundSpans && global.document) { + document.addEventListener('visibilitychange', () => { + if (document.hidden && Tracing._activeTransaction) { + logger.log('[Tracing] Discarded active transaction incl. activities since tab moved to the background'); + Tracing._activeTransaction = undefined; + Tracing._activities = {}; + } + }); + } + } + + /** + * Registers to History API to detect navigation changes + */ + private _setupHistory(): void { + if (Tracing.options.startTransactionOnLocationChange) { + addInstrumentationHandler({ + callback: historyCallback, + type: 'history', + }); + } + } + + /** + * Attaches to fetch to add sentry-trace header + creating spans + */ + private _setupFetchTracing(): void { + if (Tracing.options.traceFetch && supportsNativeFetch()) { + addInstrumentationHandler({ + callback: fetchCallback, + type: 'fetch', + }); + } + } + + /** + * Attaches to XHR to add sentry-trace header + creating spans + */ + private _setupXHRTracing(): void { + if (Tracing.options.traceXHR) { + addInstrumentationHandler({ + callback: xhrCallback, + type: 'xhr', + }); + } + } + + /** + * Configures global error listeners + */ + private _setupErrorHandling(): void { + // tslint:disable-next-line: completed-docs + function errorCallback(): void { + if (Tracing._activeTransaction) { + /** + * If an error or unhandled promise occurs, we mark the active transaction as failed + */ + logger.log(`[Tracing] Global error occured, setting status in transaction: ${SpanStatus.InternalError}`); + (Tracing._activeTransaction as SpanClass).setStatus(SpanStatus.InternalError); + } + } + addInstrumentationHandler({ + callback: errorCallback, + type: 'error', + }); + addInstrumentationHandler({ + callback: errorCallback, + type: 'unhandledrejection', + }); + } + /** * Is tracing enabled */ From 2de233ccafd9bad3062a1e4a851b99e0a4495d09 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Mon, 9 Mar 2020 12:24:30 +0100 Subject: [PATCH 2/6] feat: Add heartbeat to finish a transaction --- packages/apm/src/integrations/tracing.ts | 48 +++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/packages/apm/src/integrations/tracing.ts b/packages/apm/src/integrations/tracing.ts index 551ea8494f60..828c7968581f 100644 --- a/packages/apm/src/integrations/tracing.ts +++ b/packages/apm/src/integrations/tracing.ts @@ -134,6 +134,12 @@ export class Tracing implements Integration { private static _performanceCursor: number = 0; + private static _heartbeat: number = 0; + + private static _prevHeartbeatString: string | undefined; + + private static _heartbeatCounter: number = 0; + /** * Constructor for Tracing * @@ -206,6 +212,8 @@ export class Tracing implements Integration { this._setupBackgroundTabDetection(); + Tracing._pingHeartbeat(); + // This EventProcessor makes sure that the transaction is not longer than maxTransactionDuration addGlobalEventProcessor((event: Event) => { const self = getCurrentHub().getIntegration(Tracing); @@ -230,6 +238,44 @@ export class Tracing implements Integration { }); } + /** + * Pings the heartbeat + */ + private static _pingHeartbeat(): void { + Tracing._heartbeat = (setTimeout(() => { + Tracing._beat(); + }, 5000) as any) as number; + } + + /** + * Checks when entries of Tracing._activities are not changing for 3 beats. If this occurs we finish the transaction + * + */ + private static _beat(): void { + clearTimeout(Tracing._heartbeat); + const keys = Object.keys(Tracing._activities); + if (keys.length) { + const heartbeatString = keys.reduce((prev: string, current: string) => prev + current); + if (heartbeatString === Tracing._prevHeartbeatString) { + Tracing._heartbeatCounter++; + } else { + Tracing._heartbeatCounter = 0; + } + if (Tracing._heartbeatCounter >= 3) { + if (Tracing._activeTransaction) { + logger.log( + "[Tracing] Heartbeat safeguard kicked in, finishing transaction since activies content hasn't changed for 3 beats", + ); + Tracing._activeTransaction.setStatus(SpanStatus.DeadlineExceeded); + Tracing._activeTransaction.setData('heartbeat', 'failed'); + Tracing.finishIdleTransaction(); + } + } + Tracing._prevHeartbeatString = heartbeatString; + } + Tracing._pingHeartbeat(); + } + /** * Discards active transactions if tab moves to background */ @@ -397,8 +443,8 @@ export class Tracing implements Integration { public static finishIdleTransaction(): void { const active = Tracing._activeTransaction as SpanClass; if (active) { - logger.log('[Tracing] finishIdleTransaction', active.transaction); Tracing._addPerformanceEntries(active); + logger.log('[Tracing] finishIdleTransaction', active.transaction); // true = use timestamp of last span active.finish(true); } From ba471209f3f2b37392c01bf2c8b9d83878597629 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Mon, 9 Mar 2020 12:26:16 +0100 Subject: [PATCH 3/6] meta: Changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 59ab09df33b7..1dc1a62b0095 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,9 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +- [apm] feat: Add a simple heartbeat check, if activities don't change in 3 beats, finish the transaction (#2478) - [apm] feat: Make use of the `performance` browser API to provide better instrumentation (#2474) -- [browser] ref: Move global error handler + unhandled promise rejection to instrument +- [browser] ref: Move global error handler + unhandled promise rejection to instrument (#2475) ## 5.13.2 From f5918311bb0c2944d97e0ca5d10156302fb25d13 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Mon, 9 Mar 2020 13:24:08 +0100 Subject: [PATCH 4/6] chore: CodeReview --- packages/apm/src/integrations/tracing.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/apm/src/integrations/tracing.ts b/packages/apm/src/integrations/tracing.ts index 828c7968581f..fbdd11c7eb13 100644 --- a/packages/apm/src/integrations/tracing.ts +++ b/packages/apm/src/integrations/tracing.ts @@ -134,7 +134,7 @@ export class Tracing implements Integration { private static _performanceCursor: number = 0; - private static _heartbeat: number = 0; + private static _heartbeatTimer: number = 0; private static _prevHeartbeatString: string | undefined; @@ -242,7 +242,7 @@ export class Tracing implements Integration { * Pings the heartbeat */ private static _pingHeartbeat(): void { - Tracing._heartbeat = (setTimeout(() => { + Tracing._heartbeatTimer = (setTimeout(() => { Tracing._beat(); }, 5000) as any) as number; } @@ -252,7 +252,7 @@ export class Tracing implements Integration { * */ private static _beat(): void { - clearTimeout(Tracing._heartbeat); + clearTimeout(Tracing._heartbeatTimer); const keys = Object.keys(Tracing._activities); if (keys.length) { const heartbeatString = keys.reduce((prev: string, current: string) => prev + current); @@ -597,7 +597,8 @@ export class Tracing implements Integration { addSpan(evaluation); } - logger.log('[Tracing] Clearing most performance marks'); + logger.log(`[Tracing] Clearing most performance marks`); + performance.clearMarks(); performance.clearMeasures(); performance.clearResourceTimings(); From 6bdd6d9422b2f3191af497a19dfb61558736ef29 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Mon, 9 Mar 2020 13:25:31 +0100 Subject: [PATCH 5/6] Update packages/apm/src/integrations/tracing.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Kamil Ogórek --- packages/apm/src/integrations/tracing.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/apm/src/integrations/tracing.ts b/packages/apm/src/integrations/tracing.ts index fbdd11c7eb13..e1ec9eb1ded5 100644 --- a/packages/apm/src/integrations/tracing.ts +++ b/packages/apm/src/integrations/tracing.ts @@ -264,7 +264,7 @@ export class Tracing implements Integration { if (Tracing._heartbeatCounter >= 3) { if (Tracing._activeTransaction) { logger.log( - "[Tracing] Heartbeat safeguard kicked in, finishing transaction since activies content hasn't changed for 3 beats", + "[Tracing] Heartbeat safeguard kicked in, finishing transaction since activities content hasn't changed for 3 beats", ); Tracing._activeTransaction.setStatus(SpanStatus.DeadlineExceeded); Tracing._activeTransaction.setData('heartbeat', 'failed'); From 31611083fb982337339fa31203d8378f6b53eb1a Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Mon, 9 Mar 2020 16:44:24 +0100 Subject: [PATCH 6/6] Update packages/apm/src/integrations/tracing.ts Co-Authored-By: Rodolfo Carvalho --- packages/apm/src/integrations/tracing.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/apm/src/integrations/tracing.ts b/packages/apm/src/integrations/tracing.ts index e1ec9eb1ded5..4d31bd7e0bab 100644 --- a/packages/apm/src/integrations/tracing.ts +++ b/packages/apm/src/integrations/tracing.ts @@ -267,7 +267,7 @@ export class Tracing implements Integration { "[Tracing] Heartbeat safeguard kicked in, finishing transaction since activities content hasn't changed for 3 beats", ); Tracing._activeTransaction.setStatus(SpanStatus.DeadlineExceeded); - Tracing._activeTransaction.setData('heartbeat', 'failed'); + Tracing._activeTransaction.setTag('heartbeat', 'failed'); Tracing.finishIdleTransaction(); } }