From f0c0626c8ce9b3d3e3492a93e74158e65212e4e5 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 31 Jul 2020 09:00:46 -0400 Subject: [PATCH] chore: clean up span naming (#162) * chore: clean up span naming * chore: _ name prefix for allowEventType * chore: remove dblclick from traced events --- .../src/userInteraction.ts | 30 +++++++++---------- .../test/helper.test.ts | 2 +- .../test/userInteraction.nozone.test.ts | 10 +++++++ 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/plugins/web/opentelemetry-plugin-user-interaction/src/userInteraction.ts b/plugins/web/opentelemetry-plugin-user-interaction/src/userInteraction.ts index cc5753e902..35d378d3f0 100644 --- a/plugins/web/opentelemetry-plugin-user-interaction/src/userInteraction.ts +++ b/plugins/web/opentelemetry-plugin-user-interaction/src/userInteraction.ts @@ -34,7 +34,6 @@ import { AttributeNames } from './enums/AttributeNames'; import { VERSION } from './version'; const ZONE_CONTEXT_KEY = 'OT_ZONE_CONTEXT'; -const EVENT_CLICK_NAME = 'event_click:'; const EVENT_NAVIGATION_NAME = 'Navigation:'; /** @@ -80,6 +79,12 @@ export class UserInteractionPlugin extends BasePlugin { } } + /** + * Controls whether or not to create a span, based on the event type. + */ + protected _allowEventType(eventType: string): boolean { + return eventType === 'click'; + } /** * Creates a new span * @param element @@ -95,9 +100,12 @@ export class UserInteractionPlugin extends BasePlugin { if (element.hasAttribute('disabled')) { return undefined; } + if (!this._allowEventType(eventName)) { + return undefined; + } const xpath = getElementXPath(element, true); try { - const span = this._tracer.startSpan(`${EVENT_CLICK_NAME} ${xpath}`, { + const span = this._tracer.startSpan(eventName, { attributes: { [AttributeNames.COMPONENT]: this.component, [AttributeNames.EVENT_TYPE]: eventName, @@ -147,17 +155,6 @@ export class UserInteractionPlugin extends BasePlugin { return context; } - /** - * It gets the element that has been clicked when zone tries to run a new task - * @param task - */ - private _getClickedElement(task: AsyncTask): HTMLElement | undefined { - if (task.eventName === 'click') { - return task.target; - } - return undefined; - } - /** * Increment number of tasks that are run within the same zone. * This is needed to be able to end span when no more tasks left @@ -245,7 +242,7 @@ export class UserInteractionPlugin extends BasePlugin { if (once) { plugin.removePatchedListener(this, type, listener); } - const span = plugin._createSpan(target, 'click'); + const span = plugin._createSpan(target, type); if (span) { return plugin._tracer.withSpan(span, () => { const result = listener.apply(target, args); @@ -405,11 +402,11 @@ export class UserInteractionPlugin extends BasePlugin { applyThis?: any, applyArgs?: any ): Zone { - const target: HTMLElement | undefined = plugin._getClickedElement(task); + const target: HTMLElement | undefined = task.target; let span: api.Span | undefined; const activeZone = this; if (target) { - span = plugin._createSpan(target, 'click'); + span = plugin._createSpan(target, task.eventName); if (span) { plugin._incrementTask(span); return activeZone.run(() => { @@ -568,6 +565,7 @@ export class UserInteractionPlugin extends BasePlugin { shimmer.unwrap(ZoneWithPrototype.prototype, 'cancelTask'); } else { shimmer.unwrap(HTMLElement.prototype, 'addEventListener'); + shimmer.unwrap(HTMLElement.prototype, 'removeEventListener'); } this._unpatchHistoryApi(); } diff --git a/plugins/web/opentelemetry-plugin-user-interaction/test/helper.test.ts b/plugins/web/opentelemetry-plugin-user-interaction/test/helper.test.ts index 61f1a9a963..a0260f12af 100644 --- a/plugins/web/opentelemetry-plugin-user-interaction/test/helper.test.ts +++ b/plugins/web/opentelemetry-plugin-user-interaction/test/helper.test.ts @@ -46,7 +46,7 @@ export function fakeInteraction( } export function assertClickSpan(span: tracing.ReadableSpan, id = 'testBtn') { - assert.equal(span.name, `event_click: //*[@id="${id}"]`); + assert.equal(span.name, 'click'); const attributes = span.attributes; assert.equal(attributes.component, 'user-interaction'); diff --git a/plugins/web/opentelemetry-plugin-user-interaction/test/userInteraction.nozone.test.ts b/plugins/web/opentelemetry-plugin-user-interaction/test/userInteraction.nozone.test.ts index 65a1e07966..7f3faea3ca 100644 --- a/plugins/web/opentelemetry-plugin-user-interaction/test/userInteraction.nozone.test.ts +++ b/plugins/web/opentelemetry-plugin-user-interaction/test/userInteraction.nozone.test.ts @@ -368,6 +368,11 @@ describe('UserInteractionPlugin', () => { true, 'addEventListener should be wrapped' ); + assert.strictEqual( + isWrapped(HTMLElement.prototype.removeEventListener), + true, + 'removeEventListener should be wrapped' + ); assert.strictEqual( isWrapped(history.replaceState), @@ -398,6 +403,11 @@ describe('UserInteractionPlugin', () => { false, 'addEventListener should be unwrapped' ); + assert.strictEqual( + isWrapped(HTMLElement.prototype.removeEventListener), + false, + 'removeEventListener should be unwrapped' + ); assert.strictEqual( isWrapped(history.replaceState),