From 27e37e38f983eabdae4f2cfe859d156440378e08 Mon Sep 17 00:00:00 2001 From: Krystian Kruk <58699793+kkruk-sumo@users.noreply.github.com> Date: Sat, 11 Dec 2021 13:00:02 +0100 Subject: [PATCH] feat(user-interaction): support for custom events and span enhancement (#653) Co-authored-by: Bartlomiej Obecny Co-authored-by: Valentin Marchaud --- .../src/instrumentation.ts | 38 +++-- .../src/types.ts | 31 +++- .../test/helper.test.ts | 39 ++++- .../test/userInteraction.nozone.test.ts | 156 ++++++++++++++---- .../test/userInteraction.test.ts | 141 +++++++++++++--- 5 files changed, 338 insertions(+), 67 deletions(-) diff --git a/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts b/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts index 8317eef58e9..69277da6bdc 100644 --- a/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts +++ b/plugins/web/opentelemetry-instrumentation-user-interaction/src/instrumentation.ts @@ -14,11 +14,7 @@ * limitations under the License. */ -import { - isWrapped, - InstrumentationBase, - InstrumentationConfig, -} from '@opentelemetry/instrumentation'; +import { isWrapped, InstrumentationBase } from '@opentelemetry/instrumentation'; import * as api from '@opentelemetry/api'; import { hrTime } from '@opentelemetry/core'; @@ -26,8 +22,11 @@ import { getElementXPath } from '@opentelemetry/sdk-trace-web'; import { AttributeNames } from './enums/AttributeNames'; import { AsyncTask, + EventName, RunTaskFunction, + ShouldPreventSpanCreation, SpanData, + UserInteractionInstrumentationConfig, WindowWithZone, ZoneTypeWithPrototype, } from './types'; @@ -35,6 +34,11 @@ import { VERSION } from './version'; const ZONE_CONTEXT_KEY = 'OT_ZONE_CONTEXT'; const EVENT_NAVIGATION_NAME = 'Navigation:'; +const DEFAULT_EVENT_NAMES: EventName[] = ['click']; + +function defaultShouldPreventSpanCreation() { + return false; +} /** * This class represents a UserInteraction plugin for auto instrumentation. @@ -57,9 +61,16 @@ export class UserInteractionInstrumentation extends InstrumentationBase Event, api.Span >(); + private _eventNames: Set; + private _shouldPreventSpanCreation: ShouldPreventSpanCreation; - constructor(config?: InstrumentationConfig) { + constructor(config?: UserInteractionInstrumentationConfig) { super('@opentelemetry/instrumentation-user-interaction', VERSION, config); + this._eventNames = new Set(config?.eventNames ?? DEFAULT_EVENT_NAMES); + this._shouldPreventSpanCreation = + typeof config?.shouldPreventSpanCreation === 'function' + ? config.shouldPreventSpanCreation + : defaultShouldPreventSpanCreation; } init() {} @@ -89,9 +100,10 @@ export class UserInteractionInstrumentation extends InstrumentationBase /** * Controls whether or not to create a span, based on the event type. */ - protected _allowEventType(eventType: string): boolean { - return eventType === 'click'; + protected _allowEventName(eventName: EventName): boolean { + return this._eventNames.has(eventName); } + /** * Creates a new span * @param element @@ -99,7 +111,7 @@ export class UserInteractionInstrumentation extends InstrumentationBase */ private _createSpan( element: EventTarget | null | undefined, - eventName: string, + eventName: EventName, parentSpan?: api.Span | undefined ): api.Span | undefined { if (!(element instanceof HTMLElement)) { @@ -111,7 +123,7 @@ export class UserInteractionInstrumentation extends InstrumentationBase if (element.hasAttribute('disabled')) { return undefined; } - if (!this._allowEventType(eventName)) { + if (!this._allowEventName(eventName)) { return undefined; } const xpath = getElementXPath(element, true); @@ -133,6 +145,10 @@ export class UserInteractionInstrumentation extends InstrumentationBase : undefined ); + if (this._shouldPreventSpanCreation(eventName, element, span) === true) { + return undefined; + } + this._spansData.set(span, { taskCount: 0, }); @@ -262,7 +278,7 @@ export class UserInteractionInstrumentation extends InstrumentationBase return (original: EventTarget['addEventListener']) => { return function addEventListenerPatched( this: HTMLElement, - type: string, + type: keyof HTMLElementEventMap, listener: EventListenerOrEventListenerObject | null, useCapture?: boolean | AddEventListenerOptions ) { diff --git a/plugins/web/opentelemetry-instrumentation-user-interaction/src/types.ts b/plugins/web/opentelemetry-instrumentation-user-interaction/src/types.ts index 1de8cd3db73..665a68ba6a7 100644 --- a/plugins/web/opentelemetry-instrumentation-user-interaction/src/types.ts +++ b/plugins/web/opentelemetry-instrumentation-user-interaction/src/types.ts @@ -14,13 +14,38 @@ * limitations under the License. */ -import * as types from '@opentelemetry/api'; +import { HrTime, Span } from '@opentelemetry/api'; +import { InstrumentationConfig } from '@opentelemetry/instrumentation'; + +export type EventName = keyof HTMLElementEventMap; + +export type ShouldPreventSpanCreation = ( + eventType: EventName, + element: HTMLElement, + span: Span +) => boolean | void; + +export interface UserInteractionInstrumentationConfig + extends InstrumentationConfig { + /** + * List of events to instrument (like 'mousedown', 'touchend', 'play' etc). + * By default only 'click' event is instrumented. + */ + eventNames?: EventName[]; + + /** + * Callback function called each time new span is being created. + * Return `true` to prevent span recording. + * You can also use this handler to enhance created span with extra attributes. + */ + shouldPreventSpanCreation?: ShouldPreventSpanCreation; +} /** * Async Zone task */ export type AsyncTask = Task & { - eventName: string; + eventName: EventName; target: EventTarget; // Allows access to the private `_zone` property of a Zone.js Task. _zone: Zone; @@ -39,7 +64,7 @@ export type RunTaskFunction = ( * interface to store information in weak map per span */ export interface SpanData { - hrTimeLastTimeout?: types.HrTime; + hrTimeLastTimeout?: HrTime; taskCount: number; } 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 0191da7a390..bd669db29cb 100644 --- a/plugins/web/opentelemetry-instrumentation-user-interaction/test/helper.test.ts +++ b/plugins/web/opentelemetry-instrumentation-user-interaction/test/helper.test.ts @@ -34,7 +34,7 @@ export function createButton(disabled?: boolean): HTMLElement { return button; } -export function fakeInteraction( +export function fakeClickInteraction( callback: Function = function () {}, element: HTMLElement = createButton() ) { @@ -45,14 +45,41 @@ export function fakeInteraction( element.click(); } +export function fakeEventInteraction( + eventType: string, + callback: Function = function () {}, + elem?: HTMLElement +) { + const element: HTMLElement = elem || createButton(); + const event = document.createEvent('Event'); + event.initEvent(eventType, true, true); + + element.addEventListener(eventType, () => { + callback(); + }); + + element.dispatchEvent(event); +} + export function assertClickSpan(span: tracing.ReadableSpan, id = 'testBtn') { - assert.equal(span.name, 'click'); + assertInteractionSpan(span, { name: 'click', elementId: id }); +} + +export function assertInteractionSpan( + span: tracing.ReadableSpan, + { + name, + eventType = name, + elementId = 'testBtn', + }: { name: string; eventType?: string; elementId?: string } +) { + assert.strictEqual(span.name, name); const attributes = span.attributes; - assert.equal(attributes.component, 'user-interaction'); - assert.equal(attributes.event_type, 'click'); - assert.equal(attributes.target_element, 'BUTTON'); - assert.equal(attributes.target_xpath, `//*[@id="${id}"]`); + assert.strictEqual(attributes.component, 'user-interaction'); + assert.strictEqual(attributes.event_type, eventType); + assert.strictEqual(attributes.target_element, 'BUTTON'); + assert.strictEqual(attributes.target_xpath, `//*[@id="${elementId}"]`); assert.ok(attributes['http.url'] !== ''); assert.ok(attributes['user_agent'] !== ''); } 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 155a913c023..9cfeffe706c 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 @@ -24,10 +24,14 @@ import { WebTracerProvider } from '@opentelemetry/sdk-trace-web'; import * as assert from 'assert'; import * as sinon from 'sinon'; import { UserInteractionInstrumentation } from '../src'; +import { UserInteractionInstrumentationConfig } from '../src/types'; import { assertClickSpan, + assertInteractionSpan, + createButton, DummySpanExporter, - fakeInteraction, + fakeClickInteraction, + fakeEventInteraction, getData, } from './helper.test'; @@ -42,6 +46,32 @@ describe('UserInteractionInstrumentation', () => { let dummySpanExporter: DummySpanExporter; let exportSpy: sinon.SinonSpy; let requests: sinon.SinonFakeXMLHttpRequest[] = []; + + const registerTestInstrumentations = ( + config?: UserInteractionInstrumentationConfig + ) => { + userInteractionInstrumentation?.disable(); + + userInteractionInstrumentation = new UserInteractionInstrumentation({ + enabled: false, + ...config, + }); + + sandbox + .stub(userInteractionInstrumentation, 'getZoneWithPrototype') + .callsFake(() => { + return false as any; + }); + + registerInstrumentations({ + tracerProvider: webTracerProvider, + instrumentations: [ + userInteractionInstrumentation, + new XMLHttpRequestInstrumentation(), + ], + }); + }; + beforeEach(() => { sandbox = sinon.createSandbox(); const fakeXhr = sandbox.useFakeXMLHttpRequest(); @@ -58,16 +88,6 @@ describe('UserInteractionInstrumentation', () => { sandbox.useFakeTimers(); - userInteractionInstrumentation = new UserInteractionInstrumentation({ - enabled: false, - }); - - sandbox - .stub(userInteractionInstrumentation, 'getZoneWithPrototype') - .callsFake(() => { - return false as any; - }); - webTracerProvider = new WebTracerProvider(); dummySpanExporter = new DummySpanExporter(); @@ -77,22 +97,19 @@ describe('UserInteractionInstrumentation', () => { ); webTracerProvider.register(); - registerInstrumentations({ - instrumentations: [ - userInteractionInstrumentation, - new XMLHttpRequestInstrumentation(), - ], - }); + registerTestInstrumentations(); // this is needed as window is treated as context and karma is adding // context which is then detected as spanContext (window as { context?: {} }).context = undefined; }); + afterEach(() => { requests = []; sandbox.restore(); exportSpy.restore(); trace.disable(); + userInteractionInstrumentation.disable(); }); it('should not break removeEventListener', () => { @@ -186,14 +203,14 @@ describe('UserInteractionInstrumentation', () => { }); it('should handle task without async operation', () => { - fakeInteraction(); + fakeClickInteraction(); assert.equal(exportSpy.args.length, 1, 'should export one span'); const spanClick = exportSpy.args[0][0][0]; assertClickSpan(spanClick); }); it('should handle timeout', done => { - fakeInteraction(() => { + fakeClickInteraction(() => { originalSetTimeout(() => { const spanClick: tracing.ReadableSpan = exportSpy.args[0][0][0]; @@ -215,7 +232,7 @@ describe('UserInteractionInstrumentation', () => { callback(); }, }; - fakeInteraction(() => { + fakeClickInteraction(() => { originalSetTimeout(() => { assert.equal(exportSpy.args.length, 0, 'should NOT export any span'); done(); @@ -238,7 +255,7 @@ describe('UserInteractionInstrumentation', () => { return name === 'disabled' ? true : false; }, }; - fakeInteraction(() => { + fakeClickInteraction(() => { originalSetTimeout(() => { assert.equal(exportSpy.args.length, 0, 'should NOT export any span'); done(); @@ -252,7 +269,7 @@ describe('UserInteractionInstrumentation', () => { throw 'foo'; }; - fakeInteraction(() => { + fakeClickInteraction(() => { originalSetTimeout(() => { assert.equal(exportSpy.args.length, 0, 'should NOT export any span'); done(); @@ -262,7 +279,7 @@ describe('UserInteractionInstrumentation', () => { }); it('should handle task with navigation change', done => { - fakeInteraction(() => { + fakeClickInteraction(() => { history.pushState( { test: 'testing' }, '', @@ -299,7 +316,7 @@ describe('UserInteractionInstrumentation', () => { }); it('should handle task with timeout and async operation', done => { - fakeInteraction(() => { + fakeClickInteraction(() => { getData(FILE_URL, () => { sandbox.clock.tick(1000); }).then(() => { @@ -365,17 +382,17 @@ describe('UserInteractionInstrumentation', () => { btn2.setAttribute('id', 'btn2'); const btn3 = document.createElement('button'); btn3.setAttribute('id', 'btn3'); - fakeInteraction(() => { + fakeClickInteraction(() => { getData(FILE_URL, () => { sandbox.clock.tick(10); }).then(() => {}); }, btn1); - fakeInteraction(() => { + fakeClickInteraction(() => { getData(FILE_URL, () => { sandbox.clock.tick(10); }).then(() => {}); }, btn2); - fakeInteraction(() => { + fakeClickInteraction(() => { getData(FILE_URL, () => { sandbox.clock.tick(10); }).then(() => {}); @@ -546,6 +563,89 @@ describe('UserInteractionInstrumentation', () => { }); }); + it('should not create spans from unknown events', () => { + fakeEventInteraction('play'); + assert.strictEqual( + exportSpy.args.length, + 0, + 'should not export any spans' + ); + }); + + it('should export spans for configured event types', () => { + registerTestInstrumentations({ + eventNames: ['play'], + }); + + fakeEventInteraction('play'); + assert.strictEqual(exportSpy.args.length, 1, 'should export one span'); + const span = exportSpy.args[0][0][0]; + assertInteractionSpan(span, { name: 'play' }); + }); + + it('should not be exported not configured spans', () => { + registerTestInstrumentations({ + eventNames: ['play'], + }); + + fakeClickInteraction(); + assert.strictEqual( + exportSpy.args.length, + 0, + 'should not export any spans' + ); + }); + + it('should call shouldPreventSpanCreation with proper arguments', () => { + const shouldPreventSpanCreation = sinon.stub(); + registerTestInstrumentations({ + shouldPreventSpanCreation, + }); + + const element = createButton(); + element.addEventListener('click', () => {}); + element.click(); + + const span = exportSpy.args[0][0][0]; + assert.deepStrictEqual(shouldPreventSpanCreation.args, [ + ['click', element, span], + ]); + }); + + describe('when shouldPreventSpanCreation returns true', () => { + it('should not record span', () => { + const shouldPreventSpanCreation = () => true; + registerTestInstrumentations({ + shouldPreventSpanCreation, + }); + + const element = createButton(); + element.addEventListener('click', () => {}); + element.click(); + + assert.strictEqual( + exportSpy.args.length, + 0, + 'should not export any spans' + ); + }); + }); + + describe('when shouldPreventSpanCreation returns false', () => { + it('should record span', () => { + const shouldPreventSpanCreation = () => false; + registerTestInstrumentations({ + shouldPreventSpanCreation, + }); + + const element = createButton(); + element.addEventListener('click', () => {}); + element.click(); + + assert.strictEqual(exportSpy.args.length, 1, 'should export one span'); + }); + }); + it('should handle null event listener argument', () => { // @ts-expect-error Typescript typings report null listener as error // while allowed by EventTarget['addEventListener'] and js engines @@ -658,7 +758,7 @@ describe('UserInteractionInstrumentation', () => { * ReferenceError: EventTarget is not defined */ - fakeInteraction(); + fakeClickInteraction(); assert.equal(exportSpy.args.length, 1, 'should export one span'); const spanClick = exportSpy.args[0][0][0]; assertClickSpan(spanClick); 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 716dd78ab1e..5cfa3b4f7df 100644 --- a/plugins/web/opentelemetry-instrumentation-user-interaction/test/userInteraction.test.ts +++ b/plugins/web/opentelemetry-instrumentation-user-interaction/test/userInteraction.test.ts @@ -27,12 +27,17 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import 'zone.js'; import { UserInteractionInstrumentation } from '../src'; -import { WindowWithZone } from '../src/types'; +import { + UserInteractionInstrumentationConfig, + WindowWithZone, +} from '../src/types'; import { assertClickSpan, + assertInteractionSpan, createButton, DummySpanExporter, - fakeInteraction, + fakeClickInteraction, + fakeEventInteraction, getData, } from './helper.test'; @@ -55,6 +60,25 @@ describe('UserInteractionInstrumentation', () => { let dummySpanExporter: DummySpanExporter; let exportSpy: sinon.SinonSpy; let requests: sinon.SinonFakeXMLHttpRequest[] = []; + + const registerTestInstrumentations = ( + config?: UserInteractionInstrumentationConfig + ) => { + userInteractionInstrumentation?.disable(); + + userInteractionInstrumentation = new UserInteractionInstrumentation( + config + ); + + registerInstrumentations({ + tracerProvider: webTracerProvider, + instrumentations: [ + userInteractionInstrumentation, + new XMLHttpRequestInstrumentation(), + ], + }); + }; + beforeEach(() => { contextManager = new ZoneContextManager().enable(); sandbox = sinon.createSandbox(); @@ -83,36 +107,32 @@ describe('UserInteractionInstrumentation', () => { webTracerProvider.register({ contextManager, }); - userInteractionInstrumentation = new UserInteractionInstrumentation(); - registerInstrumentations({ - instrumentations: [ - userInteractionInstrumentation, - new XMLHttpRequestInstrumentation(), - ], - }); + registerTestInstrumentations(); // this is needed as window is treated as context and karma is adding // context which is then detected as spanContext (window as { context?: {} }).context = undefined; }); + afterEach(() => { requests = []; sandbox.restore(); exportSpy.restore(); context.disable(); trace.disable(); + userInteractionInstrumentation.disable(); }); it('should handle task without async operation', () => { - fakeInteraction(); + fakeClickInteraction(); assert.equal(exportSpy.args.length, 1, 'should export one span'); const spanClick = exportSpy.args[0][0][0]; assertClickSpan(spanClick); }); it('should ignore timeout when nothing happens afterwards', done => { - fakeInteraction(() => { + fakeClickInteraction(() => { originalSetTimeout(() => { const spanClick: tracing.ReadableSpan = exportSpy.args[0][0][0]; @@ -125,7 +145,7 @@ describe('UserInteractionInstrumentation', () => { }); it('should ignore periodic tasks', done => { - fakeInteraction(() => { + fakeClickInteraction(() => { const interval = setInterval(() => {}, 1); originalSetTimeout(() => { assert.equal( @@ -145,7 +165,7 @@ describe('UserInteractionInstrumentation', () => { }); it('should handle task with navigation change', done => { - fakeInteraction(() => { + fakeClickInteraction(() => { history.pushState( { test: 'testing' }, '', @@ -182,7 +202,7 @@ describe('UserInteractionInstrumentation', () => { }); it('should handle task with timeout and async operation', done => { - fakeInteraction(() => { + fakeClickInteraction(() => { getData(FILE_URL, () => { sandbox.clock.tick(1000); }).then(() => { @@ -211,6 +231,89 @@ describe('UserInteractionInstrumentation', () => { }); }); + it('should not create spans from unknown events', () => { + fakeEventInteraction('play'); + assert.strictEqual( + exportSpy.args.length, + 0, + 'should not export any spans' + ); + }); + + it('should export spans for configured event types', () => { + registerTestInstrumentations({ + eventNames: ['play'], + }); + + fakeEventInteraction('play'); + assert.strictEqual(exportSpy.args.length, 1, 'should export one span'); + const span = exportSpy.args[0][0][0]; + assertInteractionSpan(span, { name: 'play' }); + }); + + it('not configured spans should not be exported', () => { + registerTestInstrumentations({ + eventNames: ['play'], + }); + + fakeClickInteraction(); + assert.strictEqual( + exportSpy.args.length, + 0, + 'should not export any spans' + ); + }); + + it('should call shouldPreventSpanCreation with proper arguments', () => { + const shouldPreventSpanCreation = sinon.stub(); + registerTestInstrumentations({ + shouldPreventSpanCreation, + }); + + const element = createButton(); + element.addEventListener('click', () => {}); + element.click(); + + const span = exportSpy.args[0][0][0]; + assert.deepStrictEqual(shouldPreventSpanCreation.args, [ + ['click', element, span], + ]); + }); + + describe('when shouldPreventSpanCreation returns true', () => { + it('should not record span', () => { + const shouldPreventSpanCreation = () => true; + registerTestInstrumentations({ + shouldPreventSpanCreation, + }); + + const element = createButton(); + element.addEventListener('click', () => {}); + element.click(); + + assert.strictEqual( + exportSpy.args.length, + 0, + 'should not export any spans' + ); + }); + }); + + describe('when shouldPreventSpanCreation returns false', () => { + it('should record span', () => { + const shouldPreventSpanCreation = () => false; + registerTestInstrumentations({ + shouldPreventSpanCreation, + }); + + const element = createButton(); + element.addEventListener('click', () => {}); + element.click(); + + assert.strictEqual(exportSpy.args.length, 1, 'should export one span'); + }); + }); + it('should run task from different zone - angular test', done => { const context = ROOT_CONTEXT; const rootZone = Zone.current; @@ -236,7 +339,7 @@ describe('UserInteractionInstrumentation', () => { newZone.run(() => { assert.ok(Zone.current === newZone, 'New zone is wrong'); - fakeInteraction(() => { + fakeClickInteraction(() => { assert.ok( Zone.current.parent === newZone, 'Parent zone for click is wrong' @@ -255,7 +358,7 @@ describe('UserInteractionInstrumentation', () => { const callback = function () { called = true; }; - fakeInteraction(callback, btn); + fakeClickInteraction(callback, btn); sandbox.clock.tick(1000); originalSetTimeout(() => { assert.equal(called, false, 'callback should not be called'); @@ -270,17 +373,17 @@ describe('UserInteractionInstrumentation', () => { btn2.setAttribute('id', 'btn2'); const btn3 = document.createElement('button'); btn3.setAttribute('id', 'btn3'); - fakeInteraction(() => { + fakeClickInteraction(() => { getData(FILE_URL, () => { sandbox.clock.tick(10); }).then(() => {}); }, btn1); - fakeInteraction(() => { + fakeClickInteraction(() => { getData(FILE_URL, () => { sandbox.clock.tick(10); }).then(() => {}); }, btn2); - fakeInteraction(() => { + fakeClickInteraction(() => { getData(FILE_URL, () => { sandbox.clock.tick(10); }).then(() => {});