From 570702b2e2790001917f33b6b00176c9cb638c93 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Thu, 18 Jun 2020 01:29:42 +0200 Subject: [PATCH 1/3] chore: fixing running task in a proper zone preserving the active zone --- .../package.json | 1 + .../src/userInteraction.ts | 27 +++++----- .../test/userInteraction.test.ts | 51 ++++++++++++++++++- 3 files changed, 66 insertions(+), 13 deletions(-) diff --git a/plugins/web/opentelemetry-plugin-user-interaction/package.json b/plugins/web/opentelemetry-plugin-user-interaction/package.json index 09e782c3c3..00cd39c4a7 100644 --- a/plugins/web/opentelemetry-plugin-user-interaction/package.json +++ b/plugins/web/opentelemetry-plugin-user-interaction/package.json @@ -46,6 +46,7 @@ "@opentelemetry/context-zone-peer-dep": "0.8.3", "@opentelemetry/plugin-xml-http-request": "0.8.3", "@opentelemetry/tracing": "0.8.3", + "@opentelemetry/context-base": "0.8.3", "@types/jquery": "3.3.38", "@types/mocha": "7.0.2", "@types/node": "12.12.47", diff --git a/plugins/web/opentelemetry-plugin-user-interaction/src/userInteraction.ts b/plugins/web/opentelemetry-plugin-user-interaction/src/userInteraction.ts index 6e2a134988..c187998b13 100644 --- a/plugins/web/opentelemetry-plugin-user-interaction/src/userInteraction.ts +++ b/plugins/web/opentelemetry-plugin-user-interaction/src/userInteraction.ts @@ -313,28 +313,31 @@ export class UserInteractionPlugin extends BasePlugin { ): Zone { const target: HTMLElement | undefined = plugin._getClickedElement(task); let span: api.Span | undefined; + const activeZone = this; if (target) { span = plugin._createSpan(target, 'click'); if (span) { plugin._incrementTask(span); - try { - return plugin._tracer.withSpan(span, () => { - const currentZone = Zone.current; - task._zone = currentZone; - return original.call(currentZone, task, applyThis, applyArgs); - }); - } finally { - plugin._decrementTask(span); - } + return activeZone.run(() => { + try { + return plugin._tracer.withSpan(span as api.Span, () => { + const currentZone = Zone.current; + task._zone = currentZone; + return original.call(currentZone, task, applyThis, applyArgs); + }); + } finally { + plugin._decrementTask(span as api.Span); + } + }); } } else { - span = plugin._getCurrentSpan(this); + span = plugin._getCurrentSpan(activeZone); } try { - return original.call(this, task, applyThis, applyArgs); + return original.call(activeZone, task, applyThis, applyArgs); } finally { - if (span && plugin._shouldCountTask(task, Zone.current)) { + if (span && plugin._shouldCountTask(task, activeZone)) { plugin._decrementTask(span); } } diff --git a/plugins/web/opentelemetry-plugin-user-interaction/test/userInteraction.test.ts b/plugins/web/opentelemetry-plugin-user-interaction/test/userInteraction.test.ts index c96321e9f0..2ebd7bda28 100644 --- a/plugins/web/opentelemetry-plugin-user-interaction/test/userInteraction.test.ts +++ b/plugins/web/opentelemetry-plugin-user-interaction/test/userInteraction.test.ts @@ -17,7 +17,7 @@ // because of zone original timeout needs to be patched to be able to run // code outside zone.js. This needs to be done before all const originalSetTimeout = window.setTimeout; - +import { Context } from '@opentelemetry/context-base'; import { context } from '@opentelemetry/api'; import { isWrapped, LogLevel } from '@opentelemetry/core'; import { XMLHttpRequestPlugin } from '@opentelemetry/plugin-xml-http-request'; @@ -40,6 +40,22 @@ import { const FILE_URL = 'https://raw.githubusercontent.com/open-telemetry/opentelemetry-js/master/package.json'; +// this is needed until this is merged and released +// https://github.com/open-telemetry/opentelemetry-js/pull/1209 +const ZONE_CONTEXT_KEY = 'OT_ZONE_CONTEXT'; +function createZone(zoneName: string, context: unknown): Zone { + return Zone.current.fork({ + name: zoneName, + properties: { + [ZONE_CONTEXT_KEY]: context, + }, + }); +} +interface ContextManagerWithPrivate { + _createZone: Function; +} + + describe('UserInteractionPlugin', () => { describe('when zone.js is available', () => { let contextManager: ZoneContextManager; @@ -51,6 +67,12 @@ describe('UserInteractionPlugin', () => { let requests: sinon.SinonFakeXMLHttpRequest[] = []; beforeEach(() => { contextManager = new ZoneContextManager().enable(); + + // this is needed until this is merged and released + // https://github.com/open-telemetry/opentelemetry-js/pull/1209 + const contextManagerWithPrivate = ((contextManager as unknown) as ContextManagerWithPrivate); + contextManagerWithPrivate._createZone = createZone; + context.setGlobalContextManager(contextManager); sandbox = sinon.createSandbox(); history.pushState({ test: 'testing' }, '', `${location.pathname}`); @@ -200,6 +222,33 @@ describe('UserInteractionPlugin', () => { }); }); + it('should handle task with within different zone - angular test', done => { + const context = Context.ROOT_CONTEXT; + const rootZone = Zone.current; + const newZone = createZone('test', context); + + const element = createButton(); + element.addEventListener('click', () => { + assert.ok(Zone.current !== newZone, 'Current zone for 2nd' + + ' listener click is wrong'); + assert.ok(Zone.current.parent === rootZone, 'Parent Zone for 2nd' + + ' listener click is wrong'); + }); + + newZone.run(()=> { + assert.ok(Zone.current === newZone, 'New zone is wrong'); + fakeInteraction(() => { + assert.ok(Zone.current.parent === newZone, 'Parent zone for click' + + ' is wrong'); + const spanClick: tracing.ReadableSpan = exportSpy.args[0][0][0]; + assertClickSpan(spanClick); + + done(); + }, element); + + }); + }); + it('should ignore interaction when element is disabled', done => { const btn = createButton(true); let called = false; From 18dddf8449027103cc4bb02389af6af05117215e Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Thu, 18 Jun 2020 01:37:54 +0200 Subject: [PATCH 2/3] chore: linting --- .../test/userInteraction.test.ts | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/plugins/web/opentelemetry-plugin-user-interaction/test/userInteraction.test.ts b/plugins/web/opentelemetry-plugin-user-interaction/test/userInteraction.test.ts index 2ebd7bda28..b390f47993 100644 --- a/plugins/web/opentelemetry-plugin-user-interaction/test/userInteraction.test.ts +++ b/plugins/web/opentelemetry-plugin-user-interaction/test/userInteraction.test.ts @@ -55,7 +55,6 @@ interface ContextManagerWithPrivate { _createZone: Function; } - describe('UserInteractionPlugin', () => { describe('when zone.js is available', () => { let contextManager: ZoneContextManager; @@ -70,7 +69,7 @@ describe('UserInteractionPlugin', () => { // this is needed until this is merged and released // https://github.com/open-telemetry/opentelemetry-js/pull/1209 - const contextManagerWithPrivate = ((contextManager as unknown) as ContextManagerWithPrivate); + const contextManagerWithPrivate = (contextManager as unknown) as ContextManagerWithPrivate; contextManagerWithPrivate._createZone = createZone; context.setGlobalContextManager(contextManager); @@ -222,30 +221,35 @@ describe('UserInteractionPlugin', () => { }); }); - it('should handle task with within different zone - angular test', done => { + it('should run task from different zone - angular test', done => { const context = Context.ROOT_CONTEXT; const rootZone = Zone.current; const newZone = createZone('test', context); const element = createButton(); element.addEventListener('click', () => { - assert.ok(Zone.current !== newZone, 'Current zone for 2nd' + - ' listener click is wrong'); - assert.ok(Zone.current.parent === rootZone, 'Parent Zone for 2nd' + - ' listener click is wrong'); + assert.ok( + Zone.current !== newZone, + 'Current zone for 2nd listener click is wrong' + ); + assert.ok( + Zone.current.parent === rootZone, + 'Parent Zone for 2nd listener click is wrong' + ); }); - newZone.run(()=> { + newZone.run(() => { assert.ok(Zone.current === newZone, 'New zone is wrong'); fakeInteraction(() => { - assert.ok(Zone.current.parent === newZone, 'Parent zone for click' + - ' is wrong'); + assert.ok( + Zone.current.parent === newZone, + 'Parent zone for click is wrong' + ); const spanClick: tracing.ReadableSpan = exportSpy.args[0][0][0]; assertClickSpan(spanClick); done(); }, element); - }); }); From 927fa113a5bf39c6a63d1d7aea4c81548d62a39e Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Wed, 24 Jun 2020 21:27:20 +0200 Subject: [PATCH 3/3] chore: changes after review and merge with latest api 0.9 --- .../test/userInteraction.test.ts | 29 +++++-------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/plugins/web/opentelemetry-plugin-user-interaction/test/userInteraction.test.ts b/plugins/web/opentelemetry-plugin-user-interaction/test/userInteraction.test.ts index b390f47993..14e1de1e02 100644 --- a/plugins/web/opentelemetry-plugin-user-interaction/test/userInteraction.test.ts +++ b/plugins/web/opentelemetry-plugin-user-interaction/test/userInteraction.test.ts @@ -40,21 +40,6 @@ import { const FILE_URL = 'https://raw.githubusercontent.com/open-telemetry/opentelemetry-js/master/package.json'; -// this is needed until this is merged and released -// https://github.com/open-telemetry/opentelemetry-js/pull/1209 -const ZONE_CONTEXT_KEY = 'OT_ZONE_CONTEXT'; -function createZone(zoneName: string, context: unknown): Zone { - return Zone.current.fork({ - name: zoneName, - properties: { - [ZONE_CONTEXT_KEY]: context, - }, - }); -} -interface ContextManagerWithPrivate { - _createZone: Function; -} - describe('UserInteractionPlugin', () => { describe('when zone.js is available', () => { let contextManager: ZoneContextManager; @@ -66,12 +51,6 @@ describe('UserInteractionPlugin', () => { let requests: sinon.SinonFakeXMLHttpRequest[] = []; beforeEach(() => { contextManager = new ZoneContextManager().enable(); - - // this is needed until this is merged and released - // https://github.com/open-telemetry/opentelemetry-js/pull/1209 - const contextManagerWithPrivate = (contextManager as unknown) as ContextManagerWithPrivate; - contextManagerWithPrivate._createZone = createZone; - context.setGlobalContextManager(contextManager); sandbox = sinon.createSandbox(); history.pushState({ test: 'testing' }, '', `${location.pathname}`); @@ -224,7 +203,13 @@ describe('UserInteractionPlugin', () => { it('should run task from different zone - angular test', done => { const context = Context.ROOT_CONTEXT; const rootZone = Zone.current; - const newZone = createZone('test', context); + + interface CtxMngrWithPrv { + _createZone: Function; + } + + const ctxMngrWithPrv = (contextManager as unknown) as CtxMngrWithPrv; + const newZone = ctxMngrWithPrv._createZone('test', context); const element = createButton(); element.addEventListener('click', () => {