From 1cd6d6f8e15c55516d1cab51b3560e1b0aea58ca Mon Sep 17 00:00:00 2001 From: t2t2 Date: Wed, 18 Aug 2021 16:17:42 +0300 Subject: [PATCH 1/7] fix(user-interaction): EventTarget is undefined in IE --- .../src/instrumentation.ts | 68 ++++++++++++------- 1 file changed, 43 insertions(+), 25 deletions(-) diff --git a/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts b/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts index 6e0cbb9283..fb15bd5072 100644 --- a/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts +++ b/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts @@ -36,6 +36,20 @@ import { VERSION } from './version'; const ZONE_CONTEXT_KEY = 'OT_ZONE_CONTEXT'; const EVENT_NAVIGATION_NAME = 'Navigation:'; +/** + * Most browser provide event listener api via EventTarget in prototype chain. + * Exception to this is IE 11 which has it on the prototypes closest to EventTarget: + * + * * - has addEventListener in IE + * ** - has addEventListener in all other browsers + * ! - missing in IE + * + * HTMLElement -> Element -> Node * -> EventTarget **! -> Object + * Document -> Node * -> EventTarget **! -> Object + * Window * -> WindowProperties ! -> EventTarget **! -> Object + */ +const EVENT_TARGETS = window.EventTarget ? [EventTarget.prototype] : [Node.prototype, Window.prototype]; + /** * This class represents a UserInteraction plugin for auto instrumentation. * If zone.js is available then it patches the zone otherwise it patches @@ -571,26 +585,28 @@ export class UserInteractionInstrumentation extends InstrumentationBase ); } else { this._zonePatched = false; - if (isWrapped(EventTarget.prototype.addEventListener)) { - this._unwrap(EventTarget.prototype, 'addEventListener'); - api.diag.debug('removing previous patch from method addEventListener'); - } - if (isWrapped(EventTarget.prototype.removeEventListener)) { - this._unwrap(EventTarget.prototype, 'removeEventListener'); - api.diag.debug( - 'removing previous patch from method removeEventListener' + EVENT_TARGETS.forEach(target => { + if (isWrapped(target.addEventListener)) { + this._unwrap(target, 'addEventListener'); + api.diag.debug('removing previous patch from method addEventListener'); + } + if (isWrapped(target.removeEventListener)) { + this._unwrap(target, 'removeEventListener'); + api.diag.debug( + 'removing previous patch from method removeEventListener' + ); + } + this._wrap( + target, + 'addEventListener', + this._patchAddEventListener() ); - } - this._wrap( - EventTarget.prototype, - 'addEventListener', - this._patchAddEventListener() - ); - this._wrap( - EventTarget.prototype, - 'removeEventListener', - this._patchRemoveEventListener() - ); + this._wrap( + target, + 'removeEventListener', + this._patchRemoveEventListener() + ); + }) } this._patchHistoryApi(); @@ -619,12 +635,14 @@ export class UserInteractionInstrumentation extends InstrumentationBase this._unwrap(ZoneWithPrototype.prototype, 'cancelTask'); } } else { - if (isWrapped(HTMLElement.prototype.addEventListener)) { - this._unwrap(HTMLElement.prototype, 'addEventListener'); - } - if (isWrapped(HTMLElement.prototype.removeEventListener)) { - this._unwrap(HTMLElement.prototype, 'removeEventListener'); - } + EVENT_TARGETS.forEach(target => { + if (isWrapped(target.addEventListener)) { + this._unwrap(target, 'addEventListener'); + } + if (isWrapped(target.removeEventListener)) { + this._unwrap(target, 'removeEventListener'); + } + }); } this._unpatchHistoryApi(); } From d7d2bb26872b4b6277a6f7d557313f8619496908 Mon Sep 17 00:00:00 2001 From: t2t2 Date: Wed, 18 Aug 2021 18:00:23 +0300 Subject: [PATCH 2/7] fix: lint --- .../src/instrumentation.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts b/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts index fb15bd5072..f4315db53e 100644 --- a/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts +++ b/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts @@ -48,7 +48,8 @@ const EVENT_NAVIGATION_NAME = 'Navigation:'; * Document -> Node * -> EventTarget **! -> Object * Window * -> WindowProperties ! -> EventTarget **! -> Object */ -const EVENT_TARGETS = window.EventTarget ? [EventTarget.prototype] : [Node.prototype, Window.prototype]; +const EVENT_TARGETS = window.EventTarget + ? [EventTarget.prototype] : [Node.prototype, Window.prototype]; /** * This class represents a UserInteraction plugin for auto instrumentation. @@ -588,7 +589,9 @@ export class UserInteractionInstrumentation extends InstrumentationBase EVENT_TARGETS.forEach(target => { if (isWrapped(target.addEventListener)) { this._unwrap(target, 'addEventListener'); - api.diag.debug('removing previous patch from method addEventListener'); + api.diag.debug( + 'removing previous patch from method addEventListener' + ); } if (isWrapped(target.removeEventListener)) { this._unwrap(target, 'removeEventListener'); @@ -596,17 +599,13 @@ export class UserInteractionInstrumentation extends InstrumentationBase 'removing previous patch from method removeEventListener' ); } - this._wrap( - target, - 'addEventListener', - this._patchAddEventListener() - ); + this._wrap(target, 'addEventListener', this._patchAddEventListener()); this._wrap( target, 'removeEventListener', this._patchRemoveEventListener() ); - }) + }); } this._patchHistoryApi(); From 92868c3867e5313d2f15d8cc177499a2fc9e1f5e Mon Sep 17 00:00:00 2001 From: t2t2 Date: Wed, 18 Aug 2021 18:05:37 +0300 Subject: [PATCH 3/7] fix: lint 2 --- .../src/instrumentation.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts b/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts index f4315db53e..a6792217bf 100644 --- a/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts +++ b/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts @@ -49,7 +49,8 @@ const EVENT_NAVIGATION_NAME = 'Navigation:'; * Window * -> WindowProperties ! -> EventTarget **! -> Object */ const EVENT_TARGETS = window.EventTarget - ? [EventTarget.prototype] : [Node.prototype, Window.prototype]; + ? [EventTarget.prototype] + : [Node.prototype, Window.prototype]; /** * This class represents a UserInteraction plugin for auto instrumentation. From aadb6c1c412c7f44e80916000d7187d660d3ac96 Mon Sep 17 00:00:00 2001 From: t2t2 Date: Thu, 26 Aug 2021 14:11:40 +0300 Subject: [PATCH 4/7] simulate IE in test --- .../src/instrumentation.ts | 37 ++++++++++--------- .../test/userInteraction.nozone.test.ts | 37 +++++++++++++++++++ 2 files changed, 56 insertions(+), 18 deletions(-) diff --git a/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts b/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts index a6792217bf..935e1829be 100644 --- a/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts +++ b/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts @@ -36,22 +36,6 @@ import { VERSION } from './version'; const ZONE_CONTEXT_KEY = 'OT_ZONE_CONTEXT'; const EVENT_NAVIGATION_NAME = 'Navigation:'; -/** - * Most browser provide event listener api via EventTarget in prototype chain. - * Exception to this is IE 11 which has it on the prototypes closest to EventTarget: - * - * * - has addEventListener in IE - * ** - has addEventListener in all other browsers - * ! - missing in IE - * - * HTMLElement -> Element -> Node * -> EventTarget **! -> Object - * Document -> Node * -> EventTarget **! -> Object - * Window * -> WindowProperties ! -> EventTarget **! -> Object - */ -const EVENT_TARGETS = window.EventTarget - ? [EventTarget.prototype] - : [Node.prototype, Window.prototype]; - /** * This class represents a UserInteraction plugin for auto instrumentation. * If zone.js is available then it patches the zone otherwise it patches @@ -63,6 +47,7 @@ export class UserInteractionInstrumentation extends InstrumentationBase moduleName = this.component; private _spansData = new WeakMap(); private _zonePatched = false; + private _eventTargets: EventTarget[]; // for addEventListener/removeEventListener state private _wrappedListeners = new WeakMap< Function | EventListenerObject, @@ -76,6 +61,22 @@ export class UserInteractionInstrumentation extends InstrumentationBase constructor(config?: InstrumentationConfig) { super('@opentelemetry/instrumentation-user-interaction', VERSION, config); + + /** + * Most browser provide event listener api via EventTarget in prototype chain. + * Exception to this is IE 11 which has it on the prototypes closest to EventTarget: + * + * * - has addEventListener in IE + * ** - has addEventListener in all other browsers + * ! - missing in IE + * + * HTMLElement -> Element -> Node * -> EventTarget **! -> Object + * Document -> Node * -> EventTarget **! -> Object + * Window * -> WindowProperties ! -> EventTarget **! -> Object + */ + this._eventTargets = window.EventTarget + ? [EventTarget.prototype] + : [Node.prototype, Window.prototype]; } init() {} @@ -587,7 +588,7 @@ export class UserInteractionInstrumentation extends InstrumentationBase ); } else { this._zonePatched = false; - EVENT_TARGETS.forEach(target => { + this._eventTargets.forEach(target => { if (isWrapped(target.addEventListener)) { this._unwrap(target, 'addEventListener'); api.diag.debug( @@ -635,7 +636,7 @@ export class UserInteractionInstrumentation extends InstrumentationBase this._unwrap(ZoneWithPrototype.prototype, 'cancelTask'); } } else { - EVENT_TARGETS.forEach(target => { + this._eventTargets.forEach(target => { if (isWrapped(target.addEventListener)) { this._unwrap(target, 'addEventListener'); } diff --git a/plugins/web/opentelemetry-instrumentation-user-interaction/test/userInteraction.nozone.test.ts b/plugins/web/opentelemetry-instrumentation-user-interaction/test/userInteraction.nozone.test.ts index 0a48d71e71..1d281bd2e7 100644 --- a/plugins/web/opentelemetry-instrumentation-user-interaction/test/userInteraction.nozone.test.ts +++ b/plugins/web/opentelemetry-instrumentation-user-interaction/test/userInteraction.nozone.test.ts @@ -604,5 +604,42 @@ describe('UserInteractionInstrumentation', () => { 'go should be unwrapped' ); }); + + describe('simulate IE', () => { + // Save window.EventTarget reference (including enumerable state) + const EventTargetDesc = Object.getOwnPropertyDescriptor( + window, + 'EventTarget' + )!; + before(() => { + // @ts-expect-error window.EventTarget not optional + delete window.EventTarget; + }); + after(() => { + Object.defineProperty(window, 'EventTarget', EventTargetDesc); + // Undo unwrap putting originals back on it's targets + // @ts-expect-error + delete Node.prototype.addEventListener; + // @ts-expect-error + delete Node.prototype.removeEventListener; + // @ts-expect-error + delete Window.prototype.addEventListener; + // @ts-expect-error + delete Window.prototype.removeEventListener; + }); + + it('works with missing EventTarget', () => { + /* + * Would already error out with: + * "before each" hook for "works with missing EventTarget" + * ReferenceError: EventTarget is not defined + */ + + fakeInteraction(); + assert.equal(exportSpy.args.length, 1, 'should export one span'); + const spanClick = exportSpy.args[0][0][0]; + assertClickSpan(spanClick); + }); + }); }); }); From 34b98dc6f205d90ed3f22da6142fab793b58de6d Mon Sep 17 00:00:00 2001 From: t2t2 Date: Thu, 26 Aug 2021 14:21:20 +0300 Subject: [PATCH 5/7] lint --- .../test/userInteraction.nozone.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/web/opentelemetry-instrumentation-user-interaction/test/userInteraction.nozone.test.ts b/plugins/web/opentelemetry-instrumentation-user-interaction/test/userInteraction.nozone.test.ts index 1d281bd2e7..1cbde268fe 100644 --- a/plugins/web/opentelemetry-instrumentation-user-interaction/test/userInteraction.nozone.test.ts +++ b/plugins/web/opentelemetry-instrumentation-user-interaction/test/userInteraction.nozone.test.ts @@ -618,13 +618,13 @@ describe('UserInteractionInstrumentation', () => { after(() => { Object.defineProperty(window, 'EventTarget', EventTargetDesc); // Undo unwrap putting originals back on it's targets - // @ts-expect-error + // @ts-expect-error event listener API not optional delete Node.prototype.addEventListener; - // @ts-expect-error + // @ts-expect-error copy delete Node.prototype.removeEventListener; - // @ts-expect-error + // @ts-expect-error copy delete Window.prototype.addEventListener; - // @ts-expect-error + // @ts-expect-error copy delete Window.prototype.removeEventListener; }); From b4d2d4cafc2fd8b16e29f10ca28a4f9553861777 Mon Sep 17 00:00:00 2001 From: t2t2 Date: Thu, 26 Aug 2021 14:48:56 +0300 Subject: [PATCH 6/7] Move code around until it's no longer affected by double enable call --- .../src/instrumentation.ts | 41 ++++++++++--------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts b/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts index 935e1829be..40831b4444 100644 --- a/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts +++ b/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts @@ -47,7 +47,6 @@ export class UserInteractionInstrumentation extends InstrumentationBase moduleName = this.component; private _spansData = new WeakMap(); private _zonePatched = false; - private _eventTargets: EventTarget[]; // for addEventListener/removeEventListener state private _wrappedListeners = new WeakMap< Function | EventListenerObject, @@ -61,22 +60,6 @@ export class UserInteractionInstrumentation extends InstrumentationBase constructor(config?: InstrumentationConfig) { super('@opentelemetry/instrumentation-user-interaction', VERSION, config); - - /** - * Most browser provide event listener api via EventTarget in prototype chain. - * Exception to this is IE 11 which has it on the prototypes closest to EventTarget: - * - * * - has addEventListener in IE - * ** - has addEventListener in all other browsers - * ! - missing in IE - * - * HTMLElement -> Element -> Node * -> EventTarget **! -> Object - * Document -> Node * -> EventTarget **! -> Object - * Window * -> WindowProperties ! -> EventTarget **! -> Object - */ - this._eventTargets = window.EventTarget - ? [EventTarget.prototype] - : [Node.prototype, Window.prototype]; } init() {} @@ -347,6 +330,24 @@ export class UserInteractionInstrumentation extends InstrumentationBase }; } + /** + * Most browser provide event listener api via EventTarget in prototype chain. + * Exception to this is IE 11 which has it on the prototypes closest to EventTarget: + * + * * - has addEventListener in IE + * ** - has addEventListener in all other browsers + * ! - missing in IE + * + * HTMLElement -> Element -> Node * -> EventTarget **! -> Object + * Document -> Node * -> EventTarget **! -> Object + * Window * -> WindowProperties ! -> EventTarget **! -> Object + */ + private _getPatchableEventTargets(): EventTarget[] { + return window.EventTarget + ? [EventTarget.prototype] + : [Node.prototype, Window.prototype]; + } + /** * Patches the history api */ @@ -588,7 +589,8 @@ export class UserInteractionInstrumentation extends InstrumentationBase ); } else { this._zonePatched = false; - this._eventTargets.forEach(target => { + const targets = this._getPatchableEventTargets(); + targets.forEach(target => { if (isWrapped(target.addEventListener)) { this._unwrap(target, 'addEventListener'); api.diag.debug( @@ -636,7 +638,8 @@ export class UserInteractionInstrumentation extends InstrumentationBase this._unwrap(ZoneWithPrototype.prototype, 'cancelTask'); } } else { - this._eventTargets.forEach(target => { + const targets = this._getPatchableEventTargets(); + targets.forEach(target => { if (isWrapped(target.addEventListener)) { this._unwrap(target, 'addEventListener'); } From 6d7142cf863199e721815343c35e7ea9f063e87c Mon Sep 17 00:00:00 2001 From: t2t2 Date: Thu, 26 Aug 2021 14:58:17 +0300 Subject: [PATCH 7/7] lint would be nice if local errors were anywhere close to CI-s output but no --- .../src/instrumentation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts b/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts index 40831b4444..2d2913379b 100644 --- a/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts +++ b/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts @@ -330,7 +330,7 @@ export class UserInteractionInstrumentation extends InstrumentationBase }; } - /** + /** * Most browser provide event listener api via EventTarget in prototype chain. * Exception to this is IE 11 which has it on the prototypes closest to EventTarget: *