From d80f37fa0e2b5a671c137443e71f39903bcb8d2a Mon Sep 17 00:00:00 2001 From: Tina Gao Date: Thu, 4 Jun 2020 13:57:20 -0400 Subject: [PATCH 01/10] test(opentelemetry-plugin-xml-http-request): added un/patching tests Signed-off-by: Tina Gao --- .../test/xhr.test.ts | 107 +++++++++++++++++- 1 file changed, 102 insertions(+), 5 deletions(-) diff --git a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts index 1a59f5c092c..a8397dc0913 100644 --- a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts +++ b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts @@ -21,6 +21,8 @@ import { X_B3_SAMPLED, X_B3_SPAN_ID, X_B3_TRACE_ID, + isWrapped, + NoopLogger, } from '@opentelemetry/core'; import { ZoneContextManager } from '@opentelemetry/context-zone'; import * as tracing from '@opentelemetry/tracing'; @@ -63,7 +65,6 @@ const getData = (url: string, callbackAfterSend: Function, async?: boolean) => { resolve(); }; req.send(); - callbackAfterSend(); }); }; @@ -132,6 +133,7 @@ describe('xhr', () => { let spyEntries: any; const url = `${window.location.origin}/xml-http-request.js`; let fakeNow = 0; + let xmlHttpRequestPlugin: XMLHttpRequestPlugin; clearData = () => { requests = []; @@ -163,13 +165,13 @@ describe('xhr', () => { spyEntries = sandbox.stub(performance, 'getEntriesByType'); spyEntries.withArgs('resource').returns(resources); - + xmlHttpRequestPlugin = new XMLHttpRequestPlugin({ + propagateTraceHeaderCorsUrls: propagateTraceHeaderCorsUrls, + }); webTracerProviderWithZone = new WebTracerProvider({ logLevel: LogLevel.ERROR, plugins: [ - new XMLHttpRequestPlugin({ - propagateTraceHeaderCorsUrls: propagateTraceHeaderCorsUrls, - }), + xmlHttpRequestPlugin, ], }); webTracerWithZone = webTracerProviderWithZone.getTracer('xhr-test'); @@ -211,6 +213,20 @@ describe('xhr', () => { clearData(); }); + it('should patch to wrap XML HTTP Requests when enabled', () => { + let xhttp = new XMLHttpRequest(); + assert.ok(isWrapped(xhttp.send)); + xmlHttpRequestPlugin.enable(XMLHttpRequest.prototype, new api.NoopTracerProvider(), new NoopLogger()); + assert.ok(isWrapped(xhttp.send)); + }); + + it('should unpatch to unwrap XML HTTP Requests when disabled', () => { + let xhttp = new XMLHttpRequest(); + assert.ok(isWrapped(xhttp.send)); + xmlHttpRequestPlugin.disable() + assert.ok(!isWrapped(xhttp.send)); + }); + it('should create a span with correct root span', () => { const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; assert.strictEqual( @@ -346,6 +362,87 @@ describe('xhr', () => { ); assert.strictEqual(events.length, 12, 'number of events is wrong'); + + // it('should create a span for preflight request', () => { + // const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + // const parentSpan: tracing.ReadableSpan = exportSpy.args[1][0][0]; + // assert.strictEqual( + // span.parentSpanId, + // parentSpan.spanContext.spanId, + // 'parent span is not root span' + // ); + // }); + + // it('preflight request span should have correct name', () => { + // const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + // assert.strictEqual( + // span.name, + // 'CORS Preflight', + // 'preflight request span has wrong name' + // ); + // }); + + // it('preflight request span should have correct kind', () => { + // const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + // assert.strictEqual( + // span.kind, + // api.SpanKind.INTERNAL, + // 'span has wrong kind' + // ); + // }); + + // it('preflight request span should have correct events', () => { + // const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + // const events = span.events; + // assert.strictEqual(events.length, 9, 'number of events is wrong'); + + // assert.strictEqual( + // events[0].name, + // PTN.FETCH_START, + // `event ${PTN.FETCH_START} is not defined` + // ); + // assert.strictEqual( + // events[1].name, + // PTN.DOMAIN_LOOKUP_START, + // `event ${PTN.DOMAIN_LOOKUP_START} is not defined` + // ); + // assert.strictEqual( + // events[2].name, + // PTN.DOMAIN_LOOKUP_END, + // `event ${PTN.DOMAIN_LOOKUP_END} is not defined` + // ); + // assert.strictEqual( + // events[3].name, + // PTN.CONNECT_START, + // `event ${PTN.CONNECT_START} is not defined` + // ); + // assert.strictEqual( + // events[4].name, + // PTN.SECURE_CONNECTION_START, + // `event ${PTN.SECURE_CONNECTION_START} is not defined` + // ); + // assert.strictEqual( + // events[5].name, + // PTN.CONNECT_END, + // `event ${PTN.CONNECT_END} is not defined` + // ); + // assert.strictEqual( + // events[6].name, + // PTN.REQUEST_START, + // `event ${PTN.REQUEST_START} is not defined` + // ); + // assert.strictEqual( + // events[7].name, + // PTN.RESPONSE_START, + // `event ${PTN.RESPONSE_START} is not defined` + // ); + // assert.strictEqual( + // events[8].name, + // PTN.RESPONSE_END, + // `event ${PTN.RESPONSE_END} is not defined` + // ); + // }); + }); describe('AND origin match with window.location', () => { From c7f8d330514f547824582a140297f065b3af9ea8 Mon Sep 17 00:00:00 2001 From: Tina Gao Date: Thu, 11 Jun 2020 13:57:26 -0400 Subject: [PATCH 02/10] test(opentelemetry-plugin-xml-http-request): added CORS preflight tests --- .../test/xhr.test.ts | 210 ++++++++++-------- 1 file changed, 114 insertions(+), 96 deletions(-) diff --git a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts index a8397dc0913..ed5e5635328 100644 --- a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts +++ b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts @@ -33,6 +33,7 @@ import { import { PerformanceTimingNames as PTN, WebTracerProvider, + parseUrl } from '@opentelemetry/web'; import * as assert from 'assert'; import * as sinon from 'sinon'; @@ -83,7 +84,7 @@ function createResource(resource = {}): PerformanceResourceTiming { redirectEnd: 0, redirectStart: 0, requestStart: 16, - responseEnd: 80.5, + responseEnd: 20.5, responseStart: 17, secureConnectionStart: 14, transferSize: 0, @@ -100,6 +101,16 @@ function createResource(resource = {}): PerformanceResourceTiming { ) as PerformanceResourceTiming; } +function createMasterResource(resource = {}): PerformanceResourceTiming { + const masterResource: any = createResource(resource); + Object.keys(masterResource).forEach((key: string) => { + if (typeof masterResource[key] === 'number') { + masterResource[key] = masterResource[key] + 30; + } + }); + return masterResource; +} + describe('xhr', () => { const asyncTests = [{ async: true }, { async: false }]; asyncTests.forEach(test => { @@ -131,7 +142,7 @@ describe('xhr', () => { let exportSpy: any; let rootSpan: api.Span; let spyEntries: any; - const url = `${window.location.origin}/xml-http-request.js`; + const url = 'http://localhost:8090/xml-http-request.js'; let fakeNow = 0; let xmlHttpRequestPlugin: XMLHttpRequestPlugin; @@ -159,15 +170,16 @@ describe('xhr', () => { const resources: PerformanceResourceTiming[] = []; resources.push( createResource({ - name: url, + name: fileUrl, + }), + createMasterResource({ + name: fileUrl, }) ); spyEntries = sandbox.stub(performance, 'getEntriesByType'); spyEntries.withArgs('resource').returns(resources); - xmlHttpRequestPlugin = new XMLHttpRequestPlugin({ - propagateTraceHeaderCorsUrls: propagateTraceHeaderCorsUrls, - }); + xmlHttpRequestPlugin = new XMLHttpRequestPlugin(propagateTraceHeaderCorsUrls); webTracerProviderWithZone = new WebTracerProvider({ logLevel: LogLevel.ERROR, plugins: [ @@ -206,7 +218,7 @@ describe('xhr', () => { beforeEach(done => { const propagateTraceHeaderCorsUrls = [window.location.origin]; - prepareData(done, url, propagateTraceHeaderCorsUrls); + prepareData(done, url, { propagateTraceHeaderCorsUrls }); }); afterEach(() => { @@ -228,7 +240,7 @@ describe('xhr', () => { }); it('should create a span with correct root span', () => { - const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; assert.strictEqual( span.parentSpanId, rootSpan.context().spanId, @@ -237,12 +249,12 @@ describe('xhr', () => { }); it('span should have correct name', () => { - const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; assert.strictEqual(span.name, url, 'span has wrong name'); }); it('span should have correct kind', () => { - const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; assert.strictEqual( span.kind, api.SpanKind.CLIENT, @@ -251,7 +263,7 @@ describe('xhr', () => { }); it('span should have correct attributes', () => { - const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; const attributes = span.attributes; const keys = Object.keys(attributes); @@ -281,7 +293,7 @@ describe('xhr', () => { ); assert.strictEqual( attributes[keys[5]], - window.location.host, + parseUrl(url).host, `attributes ${HttpAttribute.HTTP_HOST} is wrong` ); assert.ok( @@ -297,7 +309,7 @@ describe('xhr', () => { }); it('span should have correct events', () => { - const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; const events = span.events; assert.strictEqual( @@ -362,92 +374,98 @@ describe('xhr', () => { ); assert.strictEqual(events.length, 12, 'number of events is wrong'); - - // it('should create a span for preflight request', () => { - // const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; - // const parentSpan: tracing.ReadableSpan = exportSpy.args[1][0][0]; - // assert.strictEqual( - // span.parentSpanId, - // parentSpan.spanContext.spanId, - // 'parent span is not root span' - // ); - // }); - - // it('preflight request span should have correct name', () => { - // const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; - // assert.strictEqual( - // span.name, - // 'CORS Preflight', - // 'preflight request span has wrong name' - // ); - // }); - - // it('preflight request span should have correct kind', () => { - // const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; - // assert.strictEqual( - // span.kind, - // api.SpanKind.INTERNAL, - // 'span has wrong kind' - // ); - // }); - - // it('preflight request span should have correct events', () => { - // const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; - // const events = span.events; - // assert.strictEqual(events.length, 9, 'number of events is wrong'); - - // assert.strictEqual( - // events[0].name, - // PTN.FETCH_START, - // `event ${PTN.FETCH_START} is not defined` - // ); - // assert.strictEqual( - // events[1].name, - // PTN.DOMAIN_LOOKUP_START, - // `event ${PTN.DOMAIN_LOOKUP_START} is not defined` - // ); - // assert.strictEqual( - // events[2].name, - // PTN.DOMAIN_LOOKUP_END, - // `event ${PTN.DOMAIN_LOOKUP_END} is not defined` - // ); - // assert.strictEqual( - // events[3].name, - // PTN.CONNECT_START, - // `event ${PTN.CONNECT_START} is not defined` - // ); - // assert.strictEqual( - // events[4].name, - // PTN.SECURE_CONNECTION_START, - // `event ${PTN.SECURE_CONNECTION_START} is not defined` - // ); - // assert.strictEqual( - // events[5].name, - // PTN.CONNECT_END, - // `event ${PTN.CONNECT_END} is not defined` - // ); - // assert.strictEqual( - // events[6].name, - // PTN.REQUEST_START, - // `event ${PTN.REQUEST_START} is not defined` - // ); - // assert.strictEqual( - // events[7].name, - // PTN.RESPONSE_START, - // `event ${PTN.RESPONSE_START} is not defined` - // ); - // assert.strictEqual( - // events[8].name, - // PTN.RESPONSE_END, - // `event ${PTN.RESPONSE_END} is not defined` - // ); - // }); }); + it('should create a span for preflight request', () => { + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const parentSpan: tracing.ReadableSpan = exportSpy.args[1][0][0]; + assert.strictEqual( + span.parentSpanId, + parentSpan.spanContext.spanId, + 'parent span is not root span' + ); + }); + + it('preflight request span should have correct name', () => { + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + assert.strictEqual( + span.name, + 'CORS Preflight', + 'preflight request span has wrong name' + ); + }); + + it('preflight request span should have correct kind', () => { + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + assert.strictEqual( + span.kind, + api.SpanKind.INTERNAL, + 'span has wrong kind' + ); + }); + + it('preflight request span should have correct events', () => { + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const events = span.events; + assert.strictEqual(events.length, 9, 'number of events is wrong'); + + assert.strictEqual( + events[0].name, + PTN.FETCH_START, + `event ${PTN.FETCH_START} is not defined` + ); + assert.strictEqual( + events[1].name, + PTN.DOMAIN_LOOKUP_START, + `event ${PTN.DOMAIN_LOOKUP_START} is not defined` + ); + assert.strictEqual( + events[2].name, + PTN.DOMAIN_LOOKUP_END, + `event ${PTN.DOMAIN_LOOKUP_END} is not defined` + ); + assert.strictEqual( + events[3].name, + PTN.CONNECT_START, + `event ${PTN.CONNECT_START} is not defined` + ); + assert.strictEqual( + events[4].name, + PTN.SECURE_CONNECTION_START, + `event ${PTN.SECURE_CONNECTION_START} is not defined` + ); + assert.strictEqual( + events[5].name, + PTN.CONNECT_END, + `event ${PTN.CONNECT_END} is not defined` + ); + assert.strictEqual( + events[6].name, + PTN.REQUEST_START, + `event ${PTN.REQUEST_START} is not defined` + ); + assert.strictEqual( + events[7].name, + PTN.RESPONSE_START, + `event ${PTN.RESPONSE_START} is not defined` + ); + assert.strictEqual( + events[8].name, + PTN.RESPONSE_END, + `event ${PTN.RESPONSE_END} is not defined` + ); + }); + describe('AND origin match with window.location', () => { + beforeEach(done => { + clearData(); + const propagateTraceHeaderCorsUrls = [url]; + prepareData(done, url, { propagateTraceHeaderCorsUrls }); + }); + it('should set trace headers', () => { - const span: api.Span = exportSpy.args[0][0][0]; + const span: api.Span = exportSpy.args[1][0][0]; assert.strictEqual( requests[0].requestHeaders[X_B3_TRACE_ID], span.context().traceId, @@ -475,11 +493,11 @@ describe('xhr', () => { prepareData( done, 'https://raw.githubusercontent.com/open-telemetry/opentelemetry-js/master/package.json', - /raw\.githubusercontent\.com/ + { propagateTraceHeaderCorsUrls: /raw\.githubusercontent\.com/ } ); }); it('should set trace headers', () => { - const span: api.Span = exportSpy.args[0][0][0]; + const span: api.Span = exportSpy.args[1][0][0]; assert.strictEqual( requests[0].requestHeaders[X_B3_TRACE_ID], span.context().traceId, From 595ee8549a484f14381632481abe8a5e1a5148fa Mon Sep 17 00:00:00 2001 From: Tina Gao Date: Thu, 11 Jun 2020 14:30:34 -0400 Subject: [PATCH 03/10] style(opentelemetry-plugin-xml-http-request): ran lint:fix --- .../test/xhr.test.ts | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts index ed5e5635328..847fd0220a9 100644 --- a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts +++ b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts @@ -179,12 +179,12 @@ describe('xhr', () => { spyEntries = sandbox.stub(performance, 'getEntriesByType'); spyEntries.withArgs('resource').returns(resources); - xmlHttpRequestPlugin = new XMLHttpRequestPlugin(propagateTraceHeaderCorsUrls); + xmlHttpRequestPlugin = new XMLHttpRequestPlugin( + propagateTraceHeaderCorsUrls + ); webTracerProviderWithZone = new WebTracerProvider({ logLevel: LogLevel.ERROR, - plugins: [ - xmlHttpRequestPlugin, - ], + plugins: [xmlHttpRequestPlugin], }); webTracerWithZone = webTracerProviderWithZone.getTracer('xhr-test'); dummySpanExporter = new DummySpanExporter(); @@ -226,16 +226,20 @@ describe('xhr', () => { }); it('should patch to wrap XML HTTP Requests when enabled', () => { - let xhttp = new XMLHttpRequest(); + const xhttp = new XMLHttpRequest(); assert.ok(isWrapped(xhttp.send)); - xmlHttpRequestPlugin.enable(XMLHttpRequest.prototype, new api.NoopTracerProvider(), new NoopLogger()); + xmlHttpRequestPlugin.enable( + XMLHttpRequest.prototype, + new api.NoopTracerProvider(), + new NoopLogger() + ); assert.ok(isWrapped(xhttp.send)); }); - + it('should unpatch to unwrap XML HTTP Requests when disabled', () => { - let xhttp = new XMLHttpRequest(); + const xhttp = new XMLHttpRequest(); assert.ok(isWrapped(xhttp.send)); - xmlHttpRequestPlugin.disable() + xmlHttpRequestPlugin.disable(); assert.ok(!isWrapped(xhttp.send)); }); @@ -374,7 +378,6 @@ describe('xhr', () => { ); assert.strictEqual(events.length, 12, 'number of events is wrong'); - }); it('should create a span for preflight request', () => { @@ -386,7 +389,7 @@ describe('xhr', () => { 'parent span is not root span' ); }); - + it('preflight request span should have correct name', () => { const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; assert.strictEqual( @@ -395,7 +398,7 @@ describe('xhr', () => { 'preflight request span has wrong name' ); }); - + it('preflight request span should have correct kind', () => { const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; assert.strictEqual( @@ -404,12 +407,12 @@ describe('xhr', () => { 'span has wrong kind' ); }); - + it('preflight request span should have correct events', () => { const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; const events = span.events; assert.strictEqual(events.length, 9, 'number of events is wrong'); - + assert.strictEqual( events[0].name, PTN.FETCH_START, @@ -463,7 +466,7 @@ describe('xhr', () => { const propagateTraceHeaderCorsUrls = [url]; prepareData(done, url, { propagateTraceHeaderCorsUrls }); }); - + it('should set trace headers', () => { const span: api.Span = exportSpy.args[1][0][0]; assert.strictEqual( From 0898236f45d13d5b77aead0ccfdc3cc4bd796c38 Mon Sep 17 00:00:00 2001 From: Tina Gao Date: Thu, 11 Jun 2020 14:35:25 -0400 Subject: [PATCH 04/10] test(opentelemetry-plugin-xml-http-request): ignored urls added test for when request url is an ignored url from plugin config --- .../test/xhr.test.ts | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts index 847fd0220a9..f0cb6075ef0 100644 --- a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts +++ b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts @@ -152,11 +152,7 @@ describe('xhr', () => { spyEntries.restore(); }; - prepareData = ( - done: any, - fileUrl: string, - propagateTraceHeaderCorsUrls?: any - ) => { + prepareData = (done: any, fileUrl: string, config?: any) => { sandbox = sinon.createSandbox(); const fakeXhr = sandbox.useFakeXMLHttpRequest(); fakeXhr.onCreate = function (xhr: any) { @@ -179,9 +175,7 @@ describe('xhr', () => { spyEntries = sandbox.stub(performance, 'getEntriesByType'); spyEntries.withArgs('resource').returns(resources); - xmlHttpRequestPlugin = new XMLHttpRequestPlugin( - propagateTraceHeaderCorsUrls - ); + xmlHttpRequestPlugin = new XMLHttpRequestPlugin(config); webTracerProviderWithZone = new WebTracerProvider({ logLevel: LogLevel.ERROR, plugins: [xmlHttpRequestPlugin], @@ -460,6 +454,25 @@ describe('xhr', () => { ); }); + describe('when url is ignored', () => { + beforeEach(done => { + clearData(); + const propagateTraceHeaderCorsUrls = url; + prepareData(done, url, { + propagateTraceHeaderCorsUrls, + ignoreUrls: [propagateTraceHeaderCorsUrls], + }); + }); + + it('should NOT create any span', () => { + assert.strictEqual( + exportSpy.args.length, + 0, + "span shouldn't be exported" + ); + }); + }); + describe('AND origin match with window.location', () => { beforeEach(done => { clearData(); From 7a88ba55b3f315e127ca7acef4f800eae80d7a99 Mon Sep 17 00:00:00 2001 From: Tina Gao Date: Thu, 11 Jun 2020 15:09:06 -0400 Subject: [PATCH 05/10] test(opentelemetry-plugin-xml-http-request): header prop tests fix header propagation tests that were affected by added a preflight span --- .../test/xhr.test.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts index f0cb6075ef0..d498876c63b 100644 --- a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts +++ b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts @@ -476,12 +476,15 @@ describe('xhr', () => { describe('AND origin match with window.location', () => { beforeEach(done => { clearData(); + // this won't generate a preflight span const propagateTraceHeaderCorsUrls = [url]; - prepareData(done, url, { propagateTraceHeaderCorsUrls }); + prepareData(done, window.location.origin + '/xml-http-request.js', { + propagateTraceHeaderCorsUrls, + }); }); it('should set trace headers', () => { - const span: api.Span = exportSpy.args[1][0][0]; + const span: api.Span = exportSpy.args[0][0][0]; assert.strictEqual( requests[0].requestHeaders[X_B3_TRACE_ID], span.context().traceId, @@ -513,7 +516,8 @@ describe('xhr', () => { ); }); it('should set trace headers', () => { - const span: api.Span = exportSpy.args[1][0][0]; + // span at exportSpy.args[0][0][0] is the preflight span + const span: api.Span = exportSpy.args[1][0][0]; assert.strictEqual( requests[0].requestHeaders[X_B3_TRACE_ID], span.context().traceId, From 5d8c2c613249d9e6177eb2a26d395564f91094b1 Mon Sep 17 00:00:00 2001 From: Tina Gao Date: Fri, 12 Jun 2020 11:33:09 -0400 Subject: [PATCH 06/10] test(opentelemetry-plugin-xml-http-request): clearTimingResources config Added tests for clearTimingResources setting in the plugin's config for the default behaviour when unspecified (false) and for explicitly setting to true. --- .../test/xhr.test.ts | 68 ++++++++++++++----- 1 file changed, 50 insertions(+), 18 deletions(-) diff --git a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts index d498876c63b..5c7583f8da2 100644 --- a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts +++ b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts @@ -140,6 +140,7 @@ describe('xhr', () => { let webTracerProviderWithZone: WebTracerProvider; let dummySpanExporter: DummySpanExporter; let exportSpy: any; + let clearResourceTimingsSpy: any; let rootSpan: api.Span; let spyEntries: any; const url = 'http://localhost:8090/xml-http-request.js'; @@ -183,6 +184,10 @@ describe('xhr', () => { webTracerWithZone = webTracerProviderWithZone.getTracer('xhr-test'); dummySpanExporter = new DummySpanExporter(); exportSpy = sinon.stub(dummySpanExporter, 'export'); + clearResourceTimingsSpy = sandbox.stub( + (performance as unknown) as Performance, + 'clearResourceTimings' + ); webTracerProviderWithZone.addSpanProcessor( new tracing.SimpleSpanProcessor(dummySpanExporter) ); @@ -454,23 +459,12 @@ describe('xhr', () => { ); }); - describe('when url is ignored', () => { - beforeEach(done => { - clearData(); - const propagateTraceHeaderCorsUrls = url; - prepareData(done, url, { - propagateTraceHeaderCorsUrls, - ignoreUrls: [propagateTraceHeaderCorsUrls], - }); - }); - - it('should NOT create any span', () => { - assert.strictEqual( - exportSpy.args.length, - 0, - "span shouldn't be exported" - ); - }); + it('should NOT clear the resources', () => { + assert.strictEqual( + clearResourceTimingsSpy.args.length, + 0, + 'resources have been cleared' + ); }); describe('AND origin match with window.location', () => { @@ -517,7 +511,7 @@ describe('xhr', () => { }); it('should set trace headers', () => { // span at exportSpy.args[0][0][0] is the preflight span - const span: api.Span = exportSpy.args[1][0][0]; + const span: api.Span = exportSpy.args[1][0][0]; assert.strictEqual( requests[0].requestHeaders[X_B3_TRACE_ID], span.context().traceId, @@ -566,6 +560,44 @@ describe('xhr', () => { }); } ); + + describe('when url is ignored', () => { + beforeEach(done => { + clearData(); + const propagateTraceHeaderCorsUrls = url; + prepareData(done, url, { + propagateTraceHeaderCorsUrls, + ignoreUrls: [propagateTraceHeaderCorsUrls], + }); + }); + + it('should NOT create any span', () => { + assert.strictEqual( + exportSpy.args.length, + 0, + "span shouldn't be exported" + ); + }); + }); + + describe('when clearTimingResources is set', () => { + beforeEach(done => { + clearData(); + const propagateTraceHeaderCorsUrls = url; + prepareData(done, url, { + propagateTraceHeaderCorsUrls, + clearTimingResources: true, + }); + }); + + it('should clear the resources', () => { + assert.strictEqual( + clearResourceTimingsSpy.args.length, + 1, + "resources haven't been cleared" + ); + }); + }); }); describe('when request is NOT successful', () => { From b9fc81a532cc3de2d9b0768b3c7db63a91574e30 Mon Sep 17 00:00:00 2001 From: Tina Gao Date: Mon, 15 Jun 2020 13:07:41 -0400 Subject: [PATCH 07/10] test(opentelemetry-plugin-xml-http-request): added test for XHR reuse --- .../test/xhr.test.ts | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts index 5c7583f8da2..399bc8c633d 100644 --- a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts +++ b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts @@ -598,6 +598,102 @@ describe('xhr', () => { ); }); }); + + describe('when reusing the same XML Http request', () => { + let reusableReq: XMLHttpRequest; //= new XMLHttpRequest(); + const firstUrl = 'http://localhost:8090/get'; + const secondUrl = 'http://localhost:8099/get'; + const getDataReuseXHR = ( + url: string, + callbackAfterSend: Function, + async?: boolean + ) => { + // eslint-disable-next-line no-async-promise-executor + return new Promise(async (resolve, reject) => { + if (async === undefined) { + async = true; + } + reusableReq.open('GET', url, async); + reusableReq.onload = function () { + resolve(); + }; + + reusableReq.onerror = function () { + resolve(); + }; + + reusableReq.ontimeout = function () { + resolve(); + }; + reusableReq.send(); + callbackAfterSend(); + }); + }; + + beforeEach(done => { + requests = []; + const resources: PerformanceResourceTiming[] = []; + resources.push( + createResource({ + name: firstUrl, + }), + createResource({ + name: secondUrl, + }) + ); + reusableReq = new XMLHttpRequest(); + webTracerWithZone.withSpan(rootSpan, () => { + getDataReuseXHR( + firstUrl, + () => { + fakeNow = 100; + }, + testAsync + ).then(() => { + fakeNow = 0; + sandbox.clock.tick(1000); + }); + }); + + webTracerWithZone.withSpan(rootSpan, () => { + getDataReuseXHR( + secondUrl, + () => { + fakeNow = 100; + }, + testAsync + ).then(() => { + fakeNow = 0; + sandbox.clock.tick(1000); + done(); + }); + + assert.strictEqual( + requests.length, + 1, + 'first request not called' + ); + + requests[0].respond( + 200, + { 'Content-Type': 'application/json' }, + '{"foo":"bar"}' + ); + }); + }); + + it('should clear previous span information', () => { + const span: tracing.ReadableSpan = exportSpy.args[2][0][0]; + const attributes = span.attributes; + const keys = Object.keys(attributes); + + assert.strictEqual( + attributes[keys[2]], + secondUrl, + `attribute ${AttributeNames.HTTP_URL} is wrong` + ); + }); + }); }); describe('when request is NOT successful', () => { From 8f11a6375afc51ef6645c6407441b63541382c0e Mon Sep 17 00:00:00 2001 From: Tina Gao Date: Mon, 15 Jun 2020 17:03:40 -0400 Subject: [PATCH 08/10] test(opentelemetry-plugin-xml-http-request): added error handler tests --- .../test/xhr.test.ts | 530 +++++++++++++----- 1 file changed, 405 insertions(+), 125 deletions(-) diff --git a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts index 399bc8c633d..5d484f1d294 100644 --- a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts +++ b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts @@ -46,6 +46,8 @@ class DummySpanExporter implements tracing.SpanExporter { shutdown() {} } +const XHR_TIMEOUT = 2000; + const getData = (url: string, callbackAfterSend: Function, async?: boolean) => { // eslint-disable-next-line no-async-promise-executor return new Promise(async (resolve, reject) => { @@ -53,6 +55,8 @@ const getData = (url: string, callbackAfterSend: Function, async?: boolean) => { async = true; } const req = new XMLHttpRequest(); + req.timeout = XHR_TIMEOUT; + req.open('GET', url, async); req.onload = function () { resolve(); @@ -62,6 +66,10 @@ const getData = (url: string, callbackAfterSend: Function, async?: boolean) => { resolve(); }; + req.onabort = function () { + resolve(); + }; + req.ontimeout = function () { resolve(); }; @@ -690,7 +698,7 @@ describe('xhr', () => { assert.strictEqual( attributes[keys[2]], secondUrl, - `attribute ${AttributeNames.HTTP_URL} is wrong` + `attribute ${HttpAttribute.HTTP_URL} is wrong` ); }); }); @@ -707,7 +715,7 @@ describe('xhr', () => { 'https://raw.githubusercontent.com/open-telemetry/opentelemetry-js/master/package.json'; let fakeNow = 0; - beforeEach(done => { + beforeEach(() => { sandbox = sinon.createSandbox(); const fakeXhr = sandbox.useFakeXMLHttpRequest(); fakeXhr.onCreate = function (xhr: any) { @@ -741,144 +749,416 @@ describe('xhr', () => { webTracerWithZone = webTracerWithZoneProvider.getTracer('xhr-test'); rootSpan = webTracerWithZone.startSpan('root'); + }); - webTracerWithZone.withSpan(rootSpan, () => { - getData( - url, - () => { - fakeNow = 100; - }, - testAsync - ).then(() => { - fakeNow = 0; - sandbox.clock.tick(1000); - done(); + afterEach(() => { + clearData(); + }); + + describe('when request loads and receives an error code', () => { + beforeEach(done => { + webTracerWithZone.withSpan(rootSpan, () => { + getData( + url, + () => { + fakeNow = 100; + }, + testAsync + ).then(() => { + fakeNow = 0; + sandbox.clock.tick(1000); + done(); + }); + assert.strictEqual(requests.length, 1, 'request not called'); + requests[0].respond( + 400, + { 'Content-Type': 'text/plain' }, + 'Bad Request' + ); }); - assert.strictEqual(requests.length, 1, 'request not called'); - requests[0].respond( + }); + it('span should have correct attributes', () => { + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const attributes = span.attributes; + const keys = Object.keys(attributes); + + assert.ok( + attributes[keys[0]] !== '', + `attributes ${GeneralAttribute.COMPONENT} is not defined` + ); + assert.strictEqual( + attributes[keys[1]], + 'GET', + `attributes ${HttpAttribute.HTTP_METHOD} is wrong` + ); + assert.strictEqual( + attributes[keys[2]], + url, + `attributes ${HttpAttribute.HTTP_URL} is wrong` + ); + assert.strictEqual( + attributes[keys[3]], 400, - { 'Content-Type': 'text/plain' }, - 'Bad Request' + `attributes ${HttpAttribute.HTTP_STATUS_CODE} is wrong` + ); + assert.strictEqual( + attributes[keys[4]], + 'Bad Request', + `attributes ${HttpAttribute.HTTP_STATUS_TEXT} is wrong` + ); + assert.strictEqual( + attributes[keys[5]], + 'raw.githubusercontent.com', + `attributes ${HttpAttribute.HTTP_HOST} is wrong` + ); + assert.ok( + attributes[keys[6]] === 'http' || attributes[keys[6]] === 'https', + `attributes ${HttpAttribute.HTTP_SCHEME} is wrong` + ); + assert.ok( + attributes[keys[7]] !== '', + `attributes ${HttpAttribute.HTTP_USER_AGENT} is not defined` ); + + assert.strictEqual(keys.length, 8, 'number of attributes is wrong'); + }); + + it('span should have correct events', () => { + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const events = span.events; + + assert.strictEqual( + events[0].name, + EventNames.METHOD_OPEN, + `event ${EventNames.METHOD_OPEN} is not defined` + ); + assert.strictEqual( + events[1].name, + EventNames.METHOD_SEND, + `event ${EventNames.METHOD_SEND} is not defined` + ); + assert.strictEqual( + events[2].name, + PTN.FETCH_START, + `event ${PTN.FETCH_START} is not defined` + ); + assert.strictEqual( + events[3].name, + PTN.DOMAIN_LOOKUP_START, + `event ${PTN.DOMAIN_LOOKUP_START} is not defined` + ); + assert.strictEqual( + events[4].name, + PTN.DOMAIN_LOOKUP_END, + `event ${PTN.DOMAIN_LOOKUP_END} is not defined` + ); + assert.strictEqual( + events[5].name, + PTN.CONNECT_START, + `event ${PTN.CONNECT_START} is not defined` + ); + assert.strictEqual( + events[6].name, + PTN.SECURE_CONNECTION_START, + `event ${PTN.SECURE_CONNECTION_START} is not defined` + ); + assert.strictEqual( + events[7].name, + PTN.CONNECT_END, + `event ${PTN.CONNECT_END} is not defined` + ); + assert.strictEqual( + events[8].name, + PTN.REQUEST_START, + `event ${PTN.REQUEST_START} is not defined` + ); + assert.strictEqual( + events[9].name, + PTN.RESPONSE_START, + `event ${PTN.RESPONSE_START} is not defined` + ); + assert.strictEqual( + events[10].name, + PTN.RESPONSE_END, + `event ${PTN.RESPONSE_END} is not defined` + ); + assert.strictEqual( + events[11].name, + EventNames.EVENT_ERROR, + `event ${EventNames.EVENT_ERROR} is not defined` + ); + + assert.strictEqual(events.length, 12, 'number of events is wrong'); }); }); - afterEach(() => { - clearData(); + describe('when request encounters a network error', () => { + beforeEach(done => { + webTracerWithZone.withSpan(rootSpan, () => { + getData(url, () => {}, testAsync).then(() => { + fakeNow = 0; + sandbox.clock.tick(1000); + done(); + }); + + assert.strictEqual(requests.length, 1, 'request not called'); + requests[0].error(); + }); + }); + + it('span should have correct attributes', () => { + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const attributes = span.attributes; + const keys = Object.keys(attributes); + + assert.ok( + attributes[keys[0]] !== '', + `attributes ${GeneralAttribute.COMPONENT} is not defined` + ); + assert.strictEqual( + attributes[keys[1]], + 'GET', + `attributes ${HttpAttribute.HTTP_METHOD} is wrong` + ); + assert.strictEqual( + attributes[keys[2]], + url, + `attributes ${HttpAttribute.HTTP_URL} is wrong` + ); + assert.strictEqual( + attributes[keys[3]], + 0, + `attributes ${HttpAttribute.HTTP_STATUS_CODE} is wrong` + ); + assert.strictEqual( + attributes[keys[4]], + '', + `attributes ${HttpAttribute.HTTP_STATUS_TEXT} is wrong` + ); + assert.strictEqual( + attributes[keys[5]], + 'raw.githubusercontent.com', + `attributes ${HttpAttribute.HTTP_HOST} is wrong` + ); + assert.ok( + attributes[keys[6]] === 'http' || attributes[keys[6]] === 'https', + `attributes ${HttpAttribute.HTTP_SCHEME} is wrong` + ); + assert.ok( + attributes[keys[7]] !== '', + `attributes ${HttpAttribute.HTTP_USER_AGENT} is not defined` + ); + + assert.strictEqual(keys.length, 8, 'number of attributes is wrong'); + }); + + it('span should have correct events', () => { + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const events = span.events; + + assert.strictEqual( + events[0].name, + EventNames.METHOD_OPEN, + `event ${EventNames.METHOD_OPEN} is not defined` + ); + assert.strictEqual( + events[1].name, + EventNames.METHOD_SEND, + `event ${EventNames.METHOD_SEND} is not defined` + ); + assert.strictEqual( + events[2].name, + EventNames.EVENT_ERROR, + `event ${EventNames.EVENT_ERROR} is not defined` + ); + + assert.strictEqual(events.length, 3, 'number of events is wrong'); + }); }); - it('span should have correct attributes', () => { - const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; - const attributes = span.attributes; - const keys = Object.keys(attributes); + describe('when request is aborted', () => { + before(function () { + // Can only abort Async requests + if (!testAsync) { + this.skip(); + } + }); - assert.ok( - attributes[keys[0]] !== '', - `attributes ${GeneralAttribute.COMPONENT} is not defined` - ); - assert.strictEqual( - attributes[keys[1]], - 'GET', - `attributes ${HttpAttribute.HTTP_METHOD} is wrong` - ); - assert.strictEqual( - attributes[keys[2]], - url, - `attributes ${HttpAttribute.HTTP_URL} is wrong` - ); - assert.strictEqual( - attributes[keys[3]], - 400, - `attributes ${HttpAttribute.HTTP_STATUS_CODE} is wrong` - ); - assert.strictEqual( - attributes[keys[4]], - 'Bad Request', - `attributes ${HttpAttribute.HTTP_STATUS_TEXT} is wrong` - ); - assert.strictEqual( - attributes[keys[5]], - 'raw.githubusercontent.com', - `attributes ${HttpAttribute.HTTP_HOST} is wrong` - ); - assert.ok( - attributes[keys[6]] === 'http' || attributes[keys[6]] === 'https', - `attributes ${HttpAttribute.HTTP_SCHEME} is wrong` - ); - assert.ok( - attributes[keys[7]] !== '', - `attributes ${HttpAttribute.HTTP_USER_AGENT} is not defined` - ); + beforeEach(done => { + webTracerWithZone.withSpan(rootSpan, () => { + getData(url, () => {}, testAsync).then(() => { + fakeNow = 0; + sandbox.clock.tick(1000); + done(); + }); - assert.strictEqual(keys.length, 8, 'number of attributes is wrong'); + assert.strictEqual(requests.length, 1, 'request not called'); + requests[0].abort(); + }); + }); + + it('span should have correct attributes', () => { + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const attributes = span.attributes; + const keys = Object.keys(attributes); + + assert.ok( + attributes[keys[0]] !== '', + `attributes ${GeneralAttribute.COMPONENT} is not defined` + ); + assert.strictEqual( + attributes[keys[1]], + 'GET', + `attributes ${HttpAttribute.HTTP_METHOD} is wrong` + ); + assert.strictEqual( + attributes[keys[2]], + url, + `attributes ${HttpAttribute.HTTP_URL} is wrong` + ); + assert.strictEqual( + attributes[keys[3]], + 0, + `attributes ${HttpAttribute.HTTP_STATUS_CODE} is wrong` + ); + assert.strictEqual( + attributes[keys[4]], + '', + `attributes ${HttpAttribute.HTTP_STATUS_TEXT} is wrong` + ); + assert.strictEqual( + attributes[keys[5]], + 'raw.githubusercontent.com', + `attributes ${HttpAttribute.HTTP_HOST} is wrong` + ); + assert.ok( + attributes[keys[6]] === 'http' || attributes[keys[6]] === 'https', + `attributes ${HttpAttribute.HTTP_SCHEME} is wrong` + ); + assert.ok( + attributes[keys[7]] !== '', + `attributes ${HttpAttribute.HTTP_USER_AGENT} is not defined` + ); + + assert.strictEqual(keys.length, 8, 'number of attributes is wrong'); + }); + + it('span should have correct events', () => { + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const events = span.events; + + assert.strictEqual( + events[0].name, + EventNames.METHOD_OPEN, + `event ${EventNames.METHOD_OPEN} is not defined` + ); + assert.strictEqual( + events[1].name, + EventNames.METHOD_SEND, + `event ${EventNames.METHOD_SEND} is not defined` + ); + assert.strictEqual( + events[2].name, + EventNames.EVENT_ABORT, + `event ${EventNames.EVENT_ABORT} is not defined` + ); + + assert.strictEqual(events.length, 3, 'number of events is wrong'); + }); }); - it('span should have correct events', () => { - const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; - const events = span.events; + describe('when request times out', () => { + before(function () { + // Can only set timeout for Async requests + if (!testAsync) { + this.skip(); + } + }); - assert.strictEqual( - events[0].name, - EventNames.METHOD_OPEN, - `event ${EventNames.METHOD_OPEN} is not defined` - ); - assert.strictEqual( - events[1].name, - EventNames.METHOD_SEND, - `event ${EventNames.METHOD_SEND} is not defined` - ); - assert.strictEqual( - events[2].name, - PTN.FETCH_START, - `event ${PTN.FETCH_START} is not defined` - ); - assert.strictEqual( - events[3].name, - PTN.DOMAIN_LOOKUP_START, - `event ${PTN.DOMAIN_LOOKUP_START} is not defined` - ); - assert.strictEqual( - events[4].name, - PTN.DOMAIN_LOOKUP_END, - `event ${PTN.DOMAIN_LOOKUP_END} is not defined` - ); - assert.strictEqual( - events[5].name, - PTN.CONNECT_START, - `event ${PTN.CONNECT_START} is not defined` - ); - assert.strictEqual( - events[6].name, - PTN.SECURE_CONNECTION_START, - `event ${PTN.SECURE_CONNECTION_START} is not defined` - ); - assert.strictEqual( - events[7].name, - PTN.CONNECT_END, - `event ${PTN.CONNECT_END} is not defined` - ); - assert.strictEqual( - events[8].name, - PTN.REQUEST_START, - `event ${PTN.REQUEST_START} is not defined` - ); - assert.strictEqual( - events[9].name, - PTN.RESPONSE_START, - `event ${PTN.RESPONSE_START} is not defined` - ); - assert.strictEqual( - events[10].name, - PTN.RESPONSE_END, - `event ${PTN.RESPONSE_END} is not defined` - ); - assert.strictEqual( - events[11].name, - EventNames.EVENT_ERROR, - `event ${EventNames.EVENT_ERROR} is not defined` - ); + beforeEach(done => { + webTracerWithZone.withSpan(rootSpan, () => { + getData( + url, + () => { + sandbox.clock.tick(XHR_TIMEOUT); + }, + testAsync + ).then(() => { + fakeNow = 0; + sandbox.clock.tick(1000); + done(); + }); + }); + }); - assert.strictEqual(events.length, 12, 'number of events is wrong'); + it('span should have correct attributes', () => { + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const attributes = span.attributes; + const keys = Object.keys(attributes); + + assert.ok( + attributes[keys[0]] !== '', + `attributes ${GeneralAttribute.COMPONENT} is not defined` + ); + assert.strictEqual( + attributes[keys[1]], + 'GET', + `attributes ${HttpAttribute.HTTP_METHOD} is wrong` + ); + assert.strictEqual( + attributes[keys[2]], + url, + `attributes ${HttpAttribute.HTTP_URL} is wrong` + ); + assert.strictEqual( + attributes[keys[3]], + 0, + `attributes ${HttpAttribute.HTTP_STATUS_CODE} is wrong` + ); + assert.strictEqual( + attributes[keys[4]], + '', + `attributes ${HttpAttribute.HTTP_STATUS_TEXT} is wrong` + ); + assert.strictEqual( + attributes[keys[5]], + 'raw.githubusercontent.com', + `attributes ${HttpAttribute.HTTP_HOST} is wrong` + ); + assert.ok( + attributes[keys[6]] === 'http' || attributes[keys[6]] === 'https', + `attributes ${HttpAttribute.HTTP_SCHEME} is wrong` + ); + assert.ok( + attributes[keys[7]] !== '', + `attributes ${HttpAttribute.HTTP_USER_AGENT} is not defined` + ); + + assert.strictEqual(keys.length, 8, 'number of attributes is wrong'); + }); + + it('span should have correct events', () => { + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; + const events = span.events; + + assert.strictEqual( + events[0].name, + EventNames.METHOD_OPEN, + `event ${EventNames.METHOD_OPEN} is not defined` + ); + assert.strictEqual( + events[1].name, + EventNames.METHOD_SEND, + `event ${EventNames.METHOD_SEND} is not defined` + ); + assert.strictEqual( + events[2].name, + EventNames.EVENT_TIMEOUT, + `event ${EventNames.EVENT_TIMEOUT} is not defined` + ); + + assert.strictEqual(events.length, 3, 'number of events is wrong'); + }); }); }); }); From b0fedd7c3f927b3329f57ac0f1e49c984dcc5f4c Mon Sep 17 00:00:00 2001 From: Tina Gao Date: Wed, 17 Jun 2020 14:07:21 -0400 Subject: [PATCH 09/10] style(opentelemetry-plugin-xml-http-request): cleaned up --- .../test/xhr.test.ts | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts index 5d484f1d294..f7416b28004 100644 --- a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts +++ b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts @@ -33,7 +33,7 @@ import { import { PerformanceTimingNames as PTN, WebTracerProvider, - parseUrl + parseUrl, } from '@opentelemetry/web'; import * as assert from 'assert'; import * as sinon from 'sinon'; @@ -468,9 +468,8 @@ describe('xhr', () => { }); it('should NOT clear the resources', () => { - assert.strictEqual( - clearResourceTimingsSpy.args.length, - 0, + assert.ok( + clearResourceTimingsSpy.notCalled, 'resources have been cleared' ); }); @@ -580,11 +579,7 @@ describe('xhr', () => { }); it('should NOT create any span', () => { - assert.strictEqual( - exportSpy.args.length, - 0, - "span shouldn't be exported" - ); + assert.ok(exportSpy.notCalled, "span shouldn't be exported"); }); }); @@ -599,16 +594,15 @@ describe('xhr', () => { }); it('should clear the resources', () => { - assert.strictEqual( - clearResourceTimingsSpy.args.length, - 1, + assert.ok( + clearResourceTimingsSpy.calledOnce, "resources haven't been cleared" ); }); }); describe('when reusing the same XML Http request', () => { - let reusableReq: XMLHttpRequest; //= new XMLHttpRequest(); + let reusableReq: XMLHttpRequest; const firstUrl = 'http://localhost:8090/get'; const secondUrl = 'http://localhost:8099/get'; const getDataReuseXHR = ( From 0ca3f9f5dcb141710f5eede2596273470c4fdf5b Mon Sep 17 00:00:00 2001 From: Tina Gao Date: Wed, 17 Jun 2020 14:11:33 -0400 Subject: [PATCH 10/10] refactor(opentelemetry-plugin-xml-http-request): use common getData --- .../test/xhr.test.ts | 70 ++++++++----------- 1 file changed, 28 insertions(+), 42 deletions(-) diff --git a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts index f7416b28004..339499f9f28 100644 --- a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts +++ b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts @@ -48,13 +48,17 @@ class DummySpanExporter implements tracing.SpanExporter { const XHR_TIMEOUT = 2000; -const getData = (url: string, callbackAfterSend: Function, async?: boolean) => { +const getData = ( + req: XMLHttpRequest, + url: string, + callbackAfterSend: Function, + async?: boolean +) => { // eslint-disable-next-line no-async-promise-executor return new Promise(async (resolve, reject) => { if (async === undefined) { async = true; } - const req = new XMLHttpRequest(); req.timeout = XHR_TIMEOUT; req.open('GET', url, async); @@ -203,6 +207,7 @@ describe('xhr', () => { rootSpan = webTracerWithZone.startSpan('root'); webTracerWithZone.withSpan(rootSpan, () => { getData( + new XMLHttpRequest(), fileUrl, () => { fakeNow = 100; @@ -602,35 +607,8 @@ describe('xhr', () => { }); describe('when reusing the same XML Http request', () => { - let reusableReq: XMLHttpRequest; const firstUrl = 'http://localhost:8090/get'; const secondUrl = 'http://localhost:8099/get'; - const getDataReuseXHR = ( - url: string, - callbackAfterSend: Function, - async?: boolean - ) => { - // eslint-disable-next-line no-async-promise-executor - return new Promise(async (resolve, reject) => { - if (async === undefined) { - async = true; - } - reusableReq.open('GET', url, async); - reusableReq.onload = function () { - resolve(); - }; - - reusableReq.onerror = function () { - resolve(); - }; - - reusableReq.ontimeout = function () { - resolve(); - }; - reusableReq.send(); - callbackAfterSend(); - }); - }; beforeEach(done => { requests = []; @@ -643,9 +621,10 @@ describe('xhr', () => { name: secondUrl, }) ); - reusableReq = new XMLHttpRequest(); + const reusableReq = new XMLHttpRequest(); webTracerWithZone.withSpan(rootSpan, () => { - getDataReuseXHR( + getData( + reusableReq, firstUrl, () => { fakeNow = 100; @@ -658,7 +637,8 @@ describe('xhr', () => { }); webTracerWithZone.withSpan(rootSpan, () => { - getDataReuseXHR( + getData( + reusableReq, secondUrl, () => { fakeNow = 100; @@ -753,6 +733,7 @@ describe('xhr', () => { beforeEach(done => { webTracerWithZone.withSpan(rootSpan, () => { getData( + new XMLHttpRequest(), url, () => { fakeNow = 100; @@ -889,11 +870,13 @@ describe('xhr', () => { describe('when request encounters a network error', () => { beforeEach(done => { webTracerWithZone.withSpan(rootSpan, () => { - getData(url, () => {}, testAsync).then(() => { - fakeNow = 0; - sandbox.clock.tick(1000); - done(); - }); + getData(new XMLHttpRequest(), url, () => {}, testAsync).then( + () => { + fakeNow = 0; + sandbox.clock.tick(1000); + done(); + } + ); assert.strictEqual(requests.length, 1, 'request not called'); requests[0].error(); @@ -980,11 +963,13 @@ describe('xhr', () => { beforeEach(done => { webTracerWithZone.withSpan(rootSpan, () => { - getData(url, () => {}, testAsync).then(() => { - fakeNow = 0; - sandbox.clock.tick(1000); - done(); - }); + getData(new XMLHttpRequest(), url, () => {}, testAsync).then( + () => { + fakeNow = 0; + sandbox.clock.tick(1000); + done(); + } + ); assert.strictEqual(requests.length, 1, 'request not called'); requests[0].abort(); @@ -1072,6 +1057,7 @@ describe('xhr', () => { beforeEach(done => { webTracerWithZone.withSpan(rootSpan, () => { getData( + new XMLHttpRequest(), url, () => { sandbox.clock.tick(XHR_TIMEOUT);