From 73e522e4d9cdee1ccd156c2fd3913a7f5ac96d0f Mon Sep 17 00:00:00 2001 From: Krystian Kruk <58699793+kkruk-sumo@users.noreply.github.com> Date: Mon, 21 Jun 2021 21:41:38 +0200 Subject: [PATCH] fix(instrumentation-user-interaction): support clicks in React apps (#537) * fix(instrumentation-user-interaction): support clicks in React apps * chore: code maintenance --- .../src/instrumentation.ts | 29 ++-- .../src/types.ts | 2 +- .../test/helper.test.ts | 4 +- .../test/userInteraction.nozone.test.ts | 129 +++++++++++++++++- .../test/userInteraction.test.ts | 123 +++++++++++++++++ 5 files changed, 268 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 c9b3f5393e..2f5663f36c 100644 --- a/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts +++ b/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts @@ -98,10 +98,13 @@ export class UserInteractionInstrumentation extends InstrumentationBase * @param eventName */ private _createSpan( - element: HTMLElement, + element: EventTarget | null | undefined, eventName: string, parentSpan?: api.Span | undefined ): api.Span | undefined { + if (!(element instanceof HTMLElement)) { + return undefined; + } if (!element.getAttribute) { return undefined; } @@ -254,7 +257,7 @@ export class UserInteractionInstrumentation extends InstrumentationBase * auto instrument the click events * This is done when zone is not available */ - private _patchElement() { + private _patchAddEventListener() { const plugin = this; return (original: Function) => { return function addEventListenerPatched( @@ -265,9 +268,9 @@ export class UserInteractionInstrumentation extends InstrumentationBase ) { const once = useCapture && useCapture.once; const patchedListener = (...args: any[]) => { - const target = this; let parentSpan: api.Span | undefined; const event: Event | undefined = args[0]; + const target = event?.target; if (event) { parentSpan = plugin._eventsSpanMap.get(event); } @@ -439,7 +442,11 @@ export class UserInteractionInstrumentation extends InstrumentationBase applyThis?: any, applyArgs?: any ): Zone { - const target: HTMLElement | undefined = task.target; + const event = + Array.isArray(applyArgs) && applyArgs[0] instanceof Event + ? applyArgs[0] + : undefined; + const target = event?.target; let span: api.Span | undefined; const activeZone = this; if (target) { @@ -564,23 +571,23 @@ export class UserInteractionInstrumentation extends InstrumentationBase ); } else { this._zonePatched = false; - if (isWrapped(HTMLElement.prototype.addEventListener)) { - this._unwrap(HTMLElement.prototype, 'addEventListener'); + if (isWrapped(EventTarget.prototype.addEventListener)) { + this._unwrap(EventTarget.prototype, 'addEventListener'); api.diag.debug('removing previous patch from method addEventListener'); } - if (isWrapped(HTMLElement.prototype.removeEventListener)) { - this._unwrap(HTMLElement.prototype, 'removeEventListener'); + if (isWrapped(EventTarget.prototype.removeEventListener)) { + this._unwrap(EventTarget.prototype, 'removeEventListener'); api.diag.debug( 'removing previous patch from method removeEventListener' ); } this._wrap( - HTMLElement.prototype, + EventTarget.prototype, 'addEventListener', - this._patchElement() + this._patchAddEventListener() ); this._wrap( - HTMLElement.prototype, + EventTarget.prototype, 'removeEventListener', this._patchRemoveEventListener() ); diff --git a/plugins/web/opentelemetry-instrumentation-user-interaction/src/types.ts b/plugins/web/opentelemetry-instrumentation-user-interaction/src/types.ts index a400810dd0..1de8cd3db7 100644 --- a/plugins/web/opentelemetry-instrumentation-user-interaction/src/types.ts +++ b/plugins/web/opentelemetry-instrumentation-user-interaction/src/types.ts @@ -21,7 +21,7 @@ import * as types from '@opentelemetry/api'; */ export type AsyncTask = Task & { eventName: string; - target: HTMLElement; + target: EventTarget; // Allows access to the private `_zone` property of a Zone.js Task. _zone: Zone; }; diff --git a/plugins/web/opentelemetry-instrumentation-user-interaction/test/helper.test.ts b/plugins/web/opentelemetry-instrumentation-user-interaction/test/helper.test.ts index 385b9d0ecf..863e9c9e7d 100644 --- a/plugins/web/opentelemetry-instrumentation-user-interaction/test/helper.test.ts +++ b/plugins/web/opentelemetry-instrumentation-user-interaction/test/helper.test.ts @@ -36,10 +36,8 @@ export function createButton(disabled?: boolean): HTMLElement { export function fakeInteraction( callback: Function = function () {}, - elem?: HTMLElement + element: HTMLElement = createButton() ) { - const element: HTMLElement = elem || createButton(); - element.addEventListener('click', () => { callback(); }); 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 dfba789dd9..0a48d71e71 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 @@ -337,10 +337,15 @@ describe('UserInteractionInstrumentation', () => { callCount++; }; document.body.addEventListener('click', listener1); - document.body.firstElementChild?.addEventListener('click', listener2); - document.body.firstElementChild?.dispatchEvent( - new MouseEvent('click', { bubbles: true }) - ); + try { + document.body.firstElementChild?.addEventListener('click', listener2); + document.body.firstElementChild?.dispatchEvent( + new MouseEvent('click', { bubbles: true }) + ); + } finally { + // remove added listener so we don't pollute other tests + document.body.removeEventListener('click', listener1); + } assert.strictEqual(callCount, 2); assert.strictEqual(exportSpy.args.length, 2); assert.strictEqual( @@ -410,6 +415,122 @@ describe('UserInteractionInstrumentation', () => { }); }); + it('should handle interactions listened on document - react < 17', done => { + const btn1 = document.createElement('button'); + btn1.setAttribute('id', 'btn1'); + document.body.appendChild(btn1); + const btn2 = document.createElement('button'); + btn2.setAttribute('id', 'btn2'); + document.body.appendChild(btn2); + + const listener = (event: MouseEvent) => { + switch (event.target) { + case btn1: + getData(FILE_URL, () => { + sandbox.clock.tick(10); + }).then(() => {}); + break; + case btn2: + getData(FILE_URL, () => { + sandbox.clock.tick(10); + }).then(() => {}); + break; + } + }; + + document.addEventListener('click', listener); + + try { + btn1.click(); + btn2.click(); + } finally { + // remove added listener so we don't pollute other tests + document.removeEventListener('click', listener); + } + + sandbox.clock.tick(1000); + originalSetTimeout(() => { + assert.equal(exportSpy.args.length, 4, 'should export 4 spans'); + + const span1: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const span2: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span3: tracing.ReadableSpan = exportSpy.args[2][0][0]; + const span4: tracing.ReadableSpan = exportSpy.args[3][0][0]; + + assertClickSpan(span1, 'btn1'); + assertClickSpan(span2, 'btn2'); + + assert.strictEqual( + span1.spanContext().spanId, + span3.parentSpanId, + 'span3 has wrong parent' + ); + assert.strictEqual( + span2.spanContext().spanId, + span4.parentSpanId, + 'span4 has wrong parent' + ); + + done(); + }); + }); + + it('should handle interactions listened on a parent element (bubbled events) - react >= 17', done => { + const root = document.createElement('div'); + document.body.appendChild(root); + + const btn1 = document.createElement('button'); + btn1.setAttribute('id', 'btn1'); + root.appendChild(btn1); + const btn2 = document.createElement('button'); + btn2.setAttribute('id', 'btn2'); + root.appendChild(btn2); + + root.addEventListener('click', event => { + switch (event.target) { + case btn1: + getData(FILE_URL, () => { + sandbox.clock.tick(10); + }).then(() => {}); + break; + case btn2: + getData(FILE_URL, () => { + sandbox.clock.tick(10); + }).then(() => {}); + break; + } + }); + + btn1.click(); + btn2.click(); + + sandbox.clock.tick(1000); + originalSetTimeout(() => { + assert.equal(exportSpy.args.length, 4, 'should export 4 spans'); + + const span1: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const span2: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span3: tracing.ReadableSpan = exportSpy.args[2][0][0]; + const span4: tracing.ReadableSpan = exportSpy.args[3][0][0]; + + assertClickSpan(span1, 'btn1'); + assertClickSpan(span2, 'btn2'); + + assert.strictEqual( + span1.spanContext().spanId, + span3.parentSpanId, + 'span3 has wrong parent' + ); + assert.strictEqual( + span2.spanContext().spanId, + span4.parentSpanId, + 'span4 has wrong parent' + ); + + done(); + }); + }); + it('should handle disable', () => { assert.strictEqual( isWrapped(HTMLElement.prototype.addEventListener), diff --git a/plugins/web/opentelemetry-instrumentation-user-interaction/test/userInteraction.test.ts b/plugins/web/opentelemetry-instrumentation-user-interaction/test/userInteraction.test.ts index bcdf9efd89..cf83d3045d 100644 --- a/plugins/web/opentelemetry-instrumentation-user-interaction/test/userInteraction.test.ts +++ b/plugins/web/opentelemetry-instrumentation-user-interaction/test/userInteraction.test.ts @@ -40,6 +40,13 @@ const FILE_URL = 'https://raw.githubusercontent.com/open-telemetry/opentelemetry-js/main/package.json'; describe('UserInteractionInstrumentation', () => { + afterEach(() => { + // clear body from elements created by some tests to make sure they are independent + while (document.body.lastChild) { + document.body.removeChild(document.body.lastChild); + } + }); + describe('when zone.js is available', () => { let contextManager: ZoneContextManager; let userInteractionInstrumentation: UserInteractionInstrumentation; @@ -313,6 +320,122 @@ describe('UserInteractionInstrumentation', () => { }); }); + it('should handle interactions listened on document - react < 17', done => { + const btn1 = document.createElement('button'); + btn1.setAttribute('id', 'btn1'); + document.body.appendChild(btn1); + const btn2 = document.createElement('button'); + btn2.setAttribute('id', 'btn2'); + document.body.appendChild(btn2); + + const listener = (event: MouseEvent) => { + switch (event.target) { + case btn1: + getData(FILE_URL, () => { + sandbox.clock.tick(10); + }).then(() => {}); + break; + case btn2: + getData(FILE_URL, () => { + sandbox.clock.tick(10); + }).then(() => {}); + break; + } + }; + + document.addEventListener('click', listener); + + try { + btn1.click(); + btn2.click(); + } finally { + // remove added listener so we don't pollute other tests + document.removeEventListener('click', listener); + } + + sandbox.clock.tick(1000); + originalSetTimeout(() => { + assert.equal(exportSpy.args.length, 4, 'should export 4 spans'); + + const span1: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const span2: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span3: tracing.ReadableSpan = exportSpy.args[2][0][0]; + const span4: tracing.ReadableSpan = exportSpy.args[3][0][0]; + + assertClickSpan(span1, 'btn1'); + assertClickSpan(span2, 'btn2'); + + assert.strictEqual( + span1.spanContext().spanId, + span3.parentSpanId, + 'span3 has wrong parent' + ); + assert.strictEqual( + span2.spanContext().spanId, + span4.parentSpanId, + 'span4 has wrong parent' + ); + + done(); + }); + }); + + it('should handle interactions listened on a parent element (bubbled events) - react >= 17', done => { + const root = document.createElement('div'); + document.body.appendChild(root); + + const btn1 = document.createElement('button'); + btn1.setAttribute('id', 'btn1'); + root.appendChild(btn1); + const btn2 = document.createElement('button'); + btn2.setAttribute('id', 'btn2'); + root.appendChild(btn2); + + root.addEventListener('click', event => { + switch (event.target) { + case btn1: + getData(FILE_URL, () => { + sandbox.clock.tick(10); + }).then(() => {}); + break; + case btn2: + getData(FILE_URL, () => { + sandbox.clock.tick(10); + }).then(() => {}); + break; + } + }); + + btn1.click(); + btn2.click(); + + sandbox.clock.tick(1000); + originalSetTimeout(() => { + assert.equal(exportSpy.args.length, 4, 'should export 4 spans'); + + const span1: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const span2: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span3: tracing.ReadableSpan = exportSpy.args[2][0][0]; + const span4: tracing.ReadableSpan = exportSpy.args[3][0][0]; + + assertClickSpan(span1, 'btn1'); + assertClickSpan(span2, 'btn2'); + + assert.strictEqual( + span1.spanContext().spanId, + span3.parentSpanId, + 'span3 has wrong parent' + ); + assert.strictEqual( + span2.spanContext().spanId, + span4.parentSpanId, + 'span4 has wrong parent' + ); + + done(); + }); + }); + it('should handle unpatch', () => { const _window: WindowWithZone = window as unknown as WindowWithZone; const ZoneWithPrototype = _window.Zone;