Refactoring

Author: crdgonzalezca

packages/opencensus-web-instrumentation-zone/src/monkey-patching.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { XHRWithUrl } from './zone-types';
+import { XhrWithUrl } from './zone-types';
 
 export function doPatching() {
   patchXmlHttpRequestOpen();
@@ -38,6 +38,6 @@ function patchXmlHttpRequestOpen() {
     } else {
       open.call(this, method, url, true, null, null);
     }
-    (this as XHRWithUrl)._ocweb_method = method;
+    (this as XhrWithUrl)._ocweb_method = method;
   };
 }

packages/opencensus-web-instrumentation-zone/src/perf-resource-timing-selector.ts

@@ -19,37 +19,12 @@ import { XhrPerformanceResourceTiming } from './zone-types';
 
 /**
  * Get Browser's performance resource timing data associated to a XHR.
- * For this case, some XHR might have two or one performance resource
- * timings as one of them is CORS pre-flight request and the second is related
- * to the actual HTTP request.
- * The algorithm to select performance resource timings related to that xhr is
- * is composed in general by three steps:
- *
- * 1. Filter the Performance Resource Timings by the name (it should match the
- * XHR URL), additionally, the start/end timings of every performance entry
- * should fit within the span start/end timings. These filtered performance
- * resource entries are considered as possible entries associated to the xhr.
- * Those are possible as there might be more than two entries that pass the
- * filter.
- *
- * 2. As the XHR could cause a CORS pre-flight, we have to look for either
- * possible pairs of performance resource timings or a single performance
- * resource entry (a possible pair is when a resource timing entry does not
- * overlap timings with other resource timing entry. Also, a possible single
- * resource timing is when that resource timing entry is not paired with any
- * other entry). Thus, for this step traverse the array of possible resource
- * entries and for every entry try to pair it with the other possible entries.
- *
- * 3. Pick the best performance resource timing for the XHR: Using the possible
- * performance resource timing entries from previous step, the best entry will
- * be the one with the minimum gap to the span start/end timings. That is the
- * substraction between the entry `respondeEnd` value and the span
- * `endPerfTime` plus the substraction between the entry `startTime` and span
- * `startPerfTime`. In case it is a tuple, the `startTime` corresponds to the
- * first entry and the `responseEnd` is from second entry.
- * The performance resource timing entry with the minimum gap to the span
- * start/end timings points out that entry is the best fit for the span.
- *
+ * Some XHRs might have two or one performance resource timings as one of them
+ * is the CORS pre-flight request and the second is related to the actual HTTP
+ * request.
+ * In overall, the algorithm to get this data takes the best fit for the span,
+ * this means the closest performance resource timing to the span start/end
+ * performance times is the returned value.
  * @param xhrUrl
  * @param span
  */
@@ -63,9 +38,17 @@ export function getXhrPerfomanceData(
   return bestEntry;
 }
 
-// Get Performance Resource Timings and filter them by matching the XHR url
-// with perfomance entry name. Additionally, the entry's start/end
-// timings must fit with in the span's start/end timings.
+/**
+ * First step for the algorithm. Filter the Performance Resource Timings by the
+ * name (it should match the XHR URL), additionally, the start/end timings of
+ * every performance entry should fit within the span start/end timings.
+ * These filtered performance resource entries are considered as possible
+ * entries associated to the xhr.
+ * Those are possible because there might be more than two entries that pass the
+ * filter.
+ * @param xhrUrl
+ * @param span
+ */
 export function getPerfResourceEntries(
   xhrUrl: string,
   span: Span
@@ -77,64 +60,86 @@ export function getPerfResourceEntries(
     ) as PerformanceResourceTiming[];
 }
 
+/**
+ * Second step for the 'Performance resource timings selector algorithm'.
+ * As the XHR could cause a CORS pre-flight request, we have to look for
+ * possible performance entries either containing cors preflight timings or not.
+ * A possible entry with cors data is when a resource timing entry does not
+ * overlap timings with other resource timing entry. Also, a possible entry
+ * without cors resource timing is when that resource timing entry is not
+ * 'paired' with any other entry.
+ * Thus, for this step traverse the array of resource entries and for every
+ * entry check if it is a possible performance resource entry.
+ * @param perfEntries
+ */
 export function getPossiblePerfResourceEntries(
   perfEntries: PerformanceResourceTiming[]
 ): XhrPerformanceResourceTiming[] {
   const possiblePerfEntries = new Array<XhrPerformanceResourceTiming>();
   const pairedEntries = new Set<PerformanceResourceTiming>();
-  let perfEntry1: PerformanceResourceTiming;
-  let perfEntry2: PerformanceResourceTiming;
+  let entryI: PerformanceResourceTiming;
+  let entryJ: PerformanceResourceTiming;
   // As this part of the algorithm traverses the array twice, although,
-  // this array is not big as the performance resource entries is cleared
-  // when there are no more running XHRs.
+  // this array is not large as the performance resource entries buffer is
+  // cleared when there are no more running XHRs.
   for (let i = 0; i < perfEntries.length; i++) {
-    perfEntry1 = perfEntries[i];
+    entryI = perfEntries[i];
     // Compare every performance entry with its consecutive perfomance entries.
     // That way to avoid comparing twice the entries.
     for (let j = i + 1; j < perfEntries.length; j++) {
-      perfEntry2 = perfEntries[j];
-      if (!overlappingPerfResourceTimings(perfEntry1, perfEntry2)) {
+      entryJ = perfEntries[j];
+      if (!overlappingPerfResourceTimings(entryI, entryJ)) {
         // As the entries are not overlapping, that means those timings
         // are possible perfomance timings related to the XHR.
-        possiblePerfEntries.push([perfEntry1, perfEntry2]);
-        pairedEntries.add(perfEntry1);
-        pairedEntries.add(perfEntry2);
+        possiblePerfEntries.push({
+          corsPreFlightRequest: entryI,
+          mainRequest: entryJ,
+        });
+        pairedEntries.add(entryI);
+        pairedEntries.add(entryJ);
       }
     }
-    // If the entry1 couldn't be paired with any other resource timing,
-    // add it as a single resource timing. This is possible because this
-    // single entry might be better that the other possible entries.
-    if (!pairedEntries.has(perfEntry1)) {
-      possiblePerfEntries.push(perfEntry1 as PerformanceResourceTiming);
+    // If the entry couldn't be paired with any other resource timing,
+    // add it as a possible resource timing without cors preflight data.
+    // This is possible because this entry might be better than the other
+    // possible entries.
+    if (!pairedEntries.has(entryI)) {
+      possiblePerfEntries.push({ mainRequest: entryI });
     }
   }
   return possiblePerfEntries;
 }
 
-// The best Performance Resource Timing Entry is considered the one with the
-// minimum gap the span end/start timings. That way we think that it fits
-// better to the XHR as it is the closest data to the actual XHR.
+// Pick the best performance resource timing for the XHR: Using the possible
+// performance resource timing entries from previous step, the best entry will
+// be the one with the minimum gap to the span start/end timings.
+// The performance resource timing entry with the minimum gap to the span
+// start/end timings points out that entry is the best fit for the span.
 function getBestPerfResourceTiming(
   perfEntries: XhrPerformanceResourceTiming[],
   span: Span
 ): XhrPerformanceResourceTiming | undefined {
   let minimumGapToSpan = Number.MAX_VALUE;
   let bestPerfEntry: XhrPerformanceResourceTiming | undefined = undefined;
-  let sumGapsToSpan: number;
   for (const perfEntry of perfEntries) {
-    // As a Tuple is in the end an Array, check that perfEntry is instance of
-    // Array is enough to know if this value refers to a Tuple.
-    if (perfEntry instanceof Array) {
-      sumGapsToSpan = Math.abs(perfEntry[0].startTime - span.startPerfTime);
-      sumGapsToSpan += Math.abs(perfEntry[1].responseEnd - span.endPerfTime);
+    let gapToSpan = Math.abs(
+      perfEntry.mainRequest.responseEnd - span.endPerfTime
+    );
+    // If the current entry has cors preflight data use its `startTime` to
+    // calculate the gap to the span.
+    if (perfEntry.corsPreFlightRequest) {
+      gapToSpan += Math.abs(
+        perfEntry.corsPreFlightRequest.startTime - span.startPerfTime
+      );
     } else {
-      sumGapsToSpan = Math.abs(perfEntry.responseEnd - span.endPerfTime);
-      sumGapsToSpan += Math.abs(perfEntry.startTime - span.startPerfTime);
+      gapToSpan += Math.abs(
+        perfEntry.mainRequest.startTime - span.startPerfTime
+      );
     }
     // If there is a new minimum gap to the span, update the minimum and pick
     // the current performance entry as the best at this point.
-    if (sumGapsToSpan < minimumGapToSpan) {
-      minimumGapToSpan = sumGapsToSpan;
+    if (gapToSpan < minimumGapToSpan) {
+      minimumGapToSpan = gapToSpan;
       bestPerfEntry = perfEntry;
     }
   }

packages/opencensus-web-instrumentation-zone/src/xhr-interceptor.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { AsyncTask, XHRWithUrl } from './zone-types';
+import { AsyncTask, XhrWithUrl } from './zone-types';
 import {
   RootSpan,
   Span,
@@ -33,9 +33,11 @@ import {
   getResourceSpan,
 } from '@opencensus/web-instrumentation-perf';
 
+const TRACEPARENT_HEADER = 'traceparent';
+
 // Map intended to keep track of current XHR objects
 // associated to a span.
-const xhrSpans = new Map<XHRWithUrl, Span>();
+const xhrSpans = new Map<XhrWithUrl, Span>();
 
 // Keeps track of the current xhr tasks that are running. This is
 // useful to clear the Performance Resource Timing entries when no
@@ -54,7 +56,7 @@ export function interceptXhrTask(task: AsyncTask) {
   if (!isTrackedTask(task)) return;
   if (!(task.target instanceof XMLHttpRequest)) return;
 
-  const xhr = task.target as XHRWithUrl;
+  const xhr = task.target as XhrWithUrl;
   if (xhr.readyState === XMLHttpRequest.OPENED) {
     incrementXhrTaskCount();
     const rootSpan: RootSpan = task.zone.get('data').rootSpan;
@@ -66,7 +68,7 @@ export function interceptXhrTask(task: AsyncTask) {
 }
 
 function setTraceparentContextHeader(
-  xhr: XHRWithUrl,
+  xhr: XhrWithUrl,
   rootSpan: RootSpan
 ): void {
   // `__zone_symbol__xhrURL` is set by the Zone monkey-path.
@@ -80,7 +82,7 @@ function setTraceparentContextHeader(
   xhrSpans.set(xhr, childSpan);
   if (traceOriginMatchesOrSameOrigin(xhrUrl)) {
     xhr.setRequestHeader(
-      'traceparent',
+      TRACEPARENT_HEADER,
       spanContextToTraceParent({
         traceId: rootSpan.traceId,
         spanId: childSpan.id,
@@ -89,7 +91,7 @@ function setTraceparentContextHeader(
   }
 }
 
-function endXhrSpan(xhr: XHRWithUrl): void {
+function endXhrSpan(xhr: XhrWithUrl): void {
   const span = xhrSpans.get(xhr);
   if (span) {
     // TODO: Investigate more to send the the status code a `number` rather
@@ -109,24 +111,15 @@ function maybeClearPerfResourceBuffer(): void {
   if (xhrTasksCount === 0) performance.clearResourceTimings();
 }
 
-function joinPerfResourceDataToSpan(xhr: XHRWithUrl, span: Span) {
-  const perfResourceTimings = getXhrPerfomanceData(xhr.responseURL, span);
-  if (perfResourceTimings instanceof Array) {
-    // This case is true when the resource timings data associates two entries
-    // to the span, where the first entry is the CORS pre-flight request and
-    // the second is the actual HTTP request. Create a child span which is
-    // related to the CORS pre-flight and use the second entry to add
-    // annotations to the span.
-    const corsPerfTiming = perfResourceTimings[0] as PerformanceResourceTimingExtended;
-    const actualXhrPerfTiming = perfResourceTimings[1] as PerformanceResourceTimingExtended;
-    setCorsPerfTimingAsChildSpan(corsPerfTiming, span);
-    span.annotations = annotationsForPerfTimeFields(
-      actualXhrPerfTiming,
-      PERFORMANCE_ENTRY_EVENTS
-    );
-  } else if (perfResourceTimings) {
+function joinPerfResourceDataToSpan(xhr: XhrWithUrl, span: Span) {
+  const xhrPerfResourceTiming = getXhrPerfomanceData(xhr.responseURL, span);
+  if (xhrPerfResourceTiming) {
+    if (xhrPerfResourceTiming.corsPreFlightRequest) {
+      const corsPerfTiming = xhrPerfResourceTiming.corsPreFlightRequest as PerformanceResourceTimingExtended;
+      setCorsPerfTimingAsChildSpan(corsPerfTiming, span);
+    }
     span.annotations = annotationsForPerfTimeFields(
-      perfResourceTimings as PerformanceResourceTimingExtended,
+      xhrPerfResourceTiming.mainRequest as PerformanceResourceTimingExtended,
       PERFORMANCE_ENTRY_EVENTS
     );
   }
@@ -137,7 +130,7 @@ function setCorsPerfTimingAsChildSpan(
   span: Span
 ): void {
   const corsSpan = getResourceSpan(performanceTiming, span.traceId, span.id);
-  corsSpan.name = 'CORS';
+  corsSpan.name = 'CORS Preflight';
   span.spans.push(corsSpan);
 }
 

packages/opencensus-web-instrumentation-zone/src/zone-types.ts

@@ -43,7 +43,7 @@ export interface OnPageInteractionData {
  * `HTMLElement` is necessary when the xhr is captured from the tasks target
  *  as the Zone monkey-patch parses xhrs as `HTMLElement & XMLHttpRequest`.
  */
-export type XHRWithUrl = HTMLElement &
+export type XhrWithUrl = HTMLElement &
   XMLHttpRequest & {
     __zone_symbol__xhrURL: string;
     _ocweb_method: string;
@@ -71,20 +71,10 @@ export declare interface WindowWithOcwGlobals extends Window {
 /**
  * Allows to keep track of performance entries related to a XHR.
  * As some XHRs might generate a CORS pre-flight request, the XHR
- * might have either two performance resource entries or a single
- * performance resource entry.
+ * might have a cors preflight performance resource timing entry or only the
+ * main request performance resource timing.
  */
-export type XhrPerformanceResourceTiming =
-  | PerformanceResourceTimingTuple
-  | PerformanceResourceTiming;
-
-/**
- * Tuple type to associate two `PerformanceResourceTiming` objects as a pair.
- * Used to select performance resource timing data associated to an XHR. In
- * general, the first value points out it is a CORS pre-flight request data and
- * the second value corresponds to the actual HTTP request.
- */
-type PerformanceResourceTimingTuple = [
-  PerformanceResourceTiming,
-  PerformanceResourceTiming
-];
+export interface XhrPerformanceResourceTiming {
+  corsPreFlightRequest?: PerformanceResourceTiming;
+  mainRequest: PerformanceResourceTiming;
+}