diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 2dc5f91cbfb..9dadb08560c 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -129,6 +129,7 @@ All notable changes to experimental packages in this project will be documented ### :bug: (Bug Fix) +* fix(instrumentation-fetch): removes CORS sub-span creation as it was not compliant with ResourceTiming spec @mariomc * fix(sampler-jaeger-remote): fixes an issue where package could emit unhandled promise rejections @Just-Sieb * fix(otlp-grpc-exporter-base): default compression to `'none'` if env vars `OTEL_EXPORTER_OTLP_TRACES_COMPRESSION` and `OTEL_EXPORTER_OTLP_COMPRESSION` are falsy @sjvans * fix(sdk-events): remove devDependencies to old `@opentelemetry/api-logs@0.52.0`, `@opentelemetry/api-events@0.52.0` packages [#5013](https://github.com/open-telemetry/opentelemetry-js/pull/5013) @pichlermarc diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts index e5d9a84bdeb..ee86f934707 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts @@ -96,30 +96,6 @@ export class FetchInstrumentation extends InstrumentationBase { const resources: PerformanceResourceTiming[] = []; resources.push( - createResource({ - name: fileUrl, - }), createMainResource({ name: fileUrl, }) @@ -389,7 +385,7 @@ describe('fetch', () => { }); it('should create a span with correct root span', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; assert.strictEqual( span.parentSpanId, rootSpan.spanContext().spanId, @@ -398,17 +394,17 @@ describe('fetch', () => { }); it('span should have correct name', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; assert.strictEqual(span.name, 'HTTP GET', 'span has wrong name'); }); it('span should have correct kind', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; assert.strictEqual(span.kind, api.SpanKind.CLIENT, 'span has wrong kind'); }); it('span should have correct attributes', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; const attributes = span.attributes; const keys = Object.keys(attributes); assert.notStrictEqual( @@ -473,50 +469,6 @@ describe('fetch', () => { }); it('span should have correct events', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; - const events = span.events; - assert.strictEqual(events.length, 8, 'number of events is wrong'); - testForCorrectEvents(events, [ - PTN.FETCH_START, - PTN.DOMAIN_LOOKUP_START, - PTN.DOMAIN_LOOKUP_END, - PTN.CONNECT_START, - PTN.CONNECT_END, - PTN.REQUEST_START, - PTN.RESPONSE_START, - PTN.RESPONSE_END, - ]); - }); - - 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, 8, 'number of events is wrong'); @@ -533,7 +485,7 @@ describe('fetch', () => { }); 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( lastResponse.headers[X_B3_TRACE_ID], span.spanContext().traceId, @@ -858,7 +810,7 @@ describe('fetch', () => { }); it('span should have correct events', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; const events = span.events; assert.strictEqual(events.length, 9, 'number of events is wrong'); testForCorrectEvents(events, [ @@ -913,7 +865,7 @@ describe('fetch', () => { await prepare(url, span => { span.setAttribute(CUSTOM_ATTRIBUTE_KEY, 'custom value'); }); - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; const attributes = span.attributes; assert.ok(attributes[CUSTOM_ATTRIBUTE_KEY] === 'custom value'); @@ -923,7 +875,7 @@ describe('fetch', () => { await prepare(badUrl, span => { span.setAttribute(CUSTOM_ATTRIBUTE_KEY, 'custom value'); }); - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; const attributes = span.attributes; assert.ok(attributes[CUSTOM_ATTRIBUTE_KEY] === 'custom value'); @@ -1022,7 +974,7 @@ describe('fetch', () => { clearData(); }); it('should create a span with correct root span', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; assert.strictEqual( span.parentSpanId, rootSpan.spanContext().spanId, @@ -1043,7 +995,7 @@ describe('fetch', () => { }); it('should create a span with correct root span', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; assert.strictEqual( span.parentSpanId, rootSpan.spanContext().spanId, @@ -1068,7 +1020,7 @@ describe('fetch', () => { assert.strictEqual( exportSpy.args.length, - 2, + 1, `Wrong number of spans: ${exportSpy.args.length}` ); @@ -1146,7 +1098,7 @@ describe('fetch', () => { assert.strictEqual( exportSpy.args.length, - 2, + 1, `Wrong number of spans: ${exportSpy.args.length}` ); assert.strictEqual(events.length, 8, 'number of events is wrong'); @@ -1208,7 +1160,7 @@ describe('fetch', () => { clearData(); }); it('should NOT add network events', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; const events = span.events; assert.strictEqual(events.length, 0, 'number of events is wrong'); }); diff --git a/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts b/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts index 6d2eafa054c..bcb9b413b05 100644 --- a/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts +++ b/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts @@ -34,7 +34,6 @@ import { import { addSpanNetworkEvents, getResource, - PerformanceTimingNames as PTN, shouldPropagateTraceHeaders, parseUrl, } from '@opentelemetry/sdk-trace-web'; @@ -135,27 +134,6 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase { - const childSpan = this.tracer.startSpan('CORS Preflight', { - startTime: corsPreFlightRequest[PTN.FETCH_START], - }); - if (!this.getConfig().ignoreNetworkEvents) { - addSpanNetworkEvents(childSpan, corsPreFlightRequest); - } - childSpan.end(corsPreFlightRequest[PTN.RESPONSE_END]); - }); - } - /** * Add attributes when span is going to end * @param span @@ -295,11 +273,6 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase { }); it('should create a span with correct root span', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; assert.strictEqual( span.parentSpanId, rootSpan.spanContext().spanId, @@ -381,12 +381,12 @@ describe('xhr', () => { }); it('span should have correct name', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; assert.strictEqual(span.name, 'GET', 'span has wrong name'); }); it('span should have correct kind', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; assert.strictEqual( span.kind, api.SpanKind.CLIENT, @@ -395,7 +395,7 @@ describe('xhr', () => { }); it('span should have correct attributes', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; const attributes = span.attributes; const keys = Object.keys(attributes); @@ -456,7 +456,7 @@ describe('xhr', () => { }); it('span should have correct events', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; const events = span.events; testForCorrectEvents(events, [ EventNames.METHOD_OPEN, @@ -474,50 +474,6 @@ describe('xhr', () => { assert.strictEqual(events.length, 11, '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, 8, 'number of events is wrong'); - testForCorrectEvents(events, [ - PTN.FETCH_START, - PTN.DOMAIN_LOOKUP_START, - PTN.DOMAIN_LOOKUP_END, - PTN.CONNECT_START, - PTN.CONNECT_END, - PTN.REQUEST_START, - PTN.RESPONSE_START, - PTN.RESPONSE_END, - ]); - }); - it('should NOT clear the resources', () => { assert.ok( clearResourceTimingsSpy.notCalled, @@ -535,7 +491,7 @@ describe('xhr', () => { }); it('span should have correct events', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; const events = span.events; testForCorrectEvents(events, [ EventNames.METHOD_OPEN, @@ -553,23 +509,6 @@ describe('xhr', () => { ]); assert.strictEqual(events.length, 12, 'number of events is wrong'); }); - - 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'); - testForCorrectEvents(events, [ - PTN.FETCH_START, - PTN.DOMAIN_LOOKUP_START, - PTN.DOMAIN_LOOKUP_END, - PTN.CONNECT_START, - PTN.SECURE_CONNECTION_START, - PTN.CONNECT_END, - PTN.REQUEST_START, - PTN.RESPONSE_START, - PTN.RESPONSE_END, - ]); - }); }); describe('AND origin match with window.location', () => { @@ -615,8 +554,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[0][0][0]; assert.strictEqual( requests[0].requestHeaders[X_B3_TRACE_ID], span.spanContext().traceId, @@ -767,7 +705,7 @@ describe('xhr', () => { }); it('should clear previous span information', () => { - const span: tracing.ReadableSpan = exportSpy.args[2][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; const attributes = span.attributes; const keys = Object.keys(attributes); @@ -796,7 +734,7 @@ describe('xhr', () => { }); it('span should have custom attribute', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; const attributes = span.attributes; assert.ok(attributes['xhr-custom-attribute'] === 'bar'); }); @@ -853,7 +791,7 @@ describe('xhr', () => { }); }); it('should NOT add network events', () => { - const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const span: tracing.ReadableSpan = exportSpy.args[0][0][0]; const events = span.events; assert.strictEqual(events.length, 3, 'number of events is wrong'); }); diff --git a/packages/opentelemetry-sdk-trace-web/src/types.ts b/packages/opentelemetry-sdk-trace-web/src/types.ts index 076665cc3f6..67dd66e9f20 100644 --- a/packages/opentelemetry-sdk-trace-web/src/types.ts +++ b/packages/opentelemetry-sdk-trace-web/src/types.ts @@ -49,10 +49,9 @@ export interface PerformanceLegacy { /** * This interface is used in {@link getResource} function to return - * main request and it's corresponding PreFlight request + * main request */ export interface PerformanceResourceTimingInfo { - corsPreFlightRequest?: PerformanceResourceTiming; mainRequest?: PerformanceResourceTiming; } diff --git a/packages/opentelemetry-sdk-trace-web/src/utils.ts b/packages/opentelemetry-sdk-trace-web/src/utils.ts index 5bcc57b72e9..cf92a6db9a8 100644 --- a/packages/opentelemetry-sdk-trace-web/src/utils.ts +++ b/packages/opentelemetry-sdk-trace-web/src/utils.ts @@ -180,84 +180,9 @@ export function getResource( initiatorType ); - if (filteredResources.length === 0) { - return { - mainRequest: undefined, - }; + return { + mainRequest: filteredResources?.[0] } - if (filteredResources.length === 1) { - return { - mainRequest: filteredResources[0], - }; - } - const sorted = sortResources(filteredResources); - - if (parsedSpanUrl.origin !== getOrigin() && sorted.length > 1) { - let corsPreFlightRequest: PerformanceResourceTiming | undefined = sorted[0]; - let mainRequest: PerformanceResourceTiming = findMainRequest( - sorted, - corsPreFlightRequest[PTN.RESPONSE_END], - endTimeHR - ); - - const responseEnd = corsPreFlightRequest[PTN.RESPONSE_END]; - const fetchStart = mainRequest[PTN.FETCH_START]; - - // no corsPreFlightRequest - if (fetchStart < responseEnd) { - mainRequest = corsPreFlightRequest; - corsPreFlightRequest = undefined; - } - - return { - corsPreFlightRequest, - mainRequest, - }; - } else { - return { - mainRequest: filteredResources[0], - }; - } -} - -/** - * Will find the main request skipping the cors pre flight requests - * @param resources - * @param corsPreFlightRequestEndTime - * @param spanEndTimeHR - */ -function findMainRequest( - resources: PerformanceResourceTiming[], - corsPreFlightRequestEndTime: number, - spanEndTimeHR: api.HrTime -): PerformanceResourceTiming { - const spanEndTime = hrTimeToNanoseconds(spanEndTimeHR); - const minTime = hrTimeToNanoseconds( - timeInputToHrTime(corsPreFlightRequestEndTime) - ); - - let mainRequest: PerformanceResourceTiming = resources[1]; - let bestGap; - - const length = resources.length; - for (let i = 1; i < length; i++) { - const resource = resources[i]; - const resourceStartTime = hrTimeToNanoseconds( - timeInputToHrTime(resource[PTN.FETCH_START]) - ); - - const resourceEndTime = hrTimeToNanoseconds( - timeInputToHrTime(resource[PTN.RESPONSE_END]) - ); - - const currentGap = spanEndTime - resourceEndTime; - - if (resourceStartTime >= minTime && (!bestGap || currentGap < bestGap)) { - bestGap = currentGap; - mainRequest = resource; - } - } - return mainRequest; } /**