add tests for sampling decision propagation for XHR requests

Author: lobsterkatie

packages/tracing/test/hub.test.ts

@@ -1,15 +1,24 @@
 /* eslint-disable @typescript-eslint/unbound-method */
 import { BrowserClient } from '@sentry/browser';
 import { getMainCarrier, Hub } from '@sentry/hub';
+import * as hubModule from '@sentry/hub';
 import * as utilsModule from '@sentry/utils'; // for mocking
 import { getGlobalObject, isNodeEnv, logger } from '@sentry/utils';
 import * as nodeHttpModule from 'http';
 
+import { BrowserTracing } from '../src/browser/browsertracing';
 import { addExtensionMethods } from '../src/hubextensions';
+import { extractTraceparentData, TRACEPARENT_REGEXP } from '../src/utils';
+import { addDOMPropertiesToGlobal, getSymbolObjectKeyByName } from './testutils';
 
 addExtensionMethods();
 
 const mathRandom = jest.spyOn(Math, 'random');
+
+// we have to add things into the real global object (rather than mocking the return value of getGlobalObject) because
+// there are modules which call getGlobalObject as they load, which is seemingly too early for jest to intervene
+addDOMPropertiesToGlobal(['XMLHttpRequest', 'Event', 'location', 'document']);
+
 describe('Hub', () => {
   beforeEach(() => {
     jest.spyOn(logger, 'warn');
@@ -339,12 +348,84 @@ describe('Hub', () => {
         expect(child.sampled).toBe(transaction.sampled);
       });
 
-      it('should propagate sampling decision to child transactions in XHR header', () => {
-        // TODO fix this and write the test
+      it('should propagate positive sampling decision to child transactions in XHR header', () => {
+        const hub = new Hub(
+          new BrowserClient({
+            dsn: 'https://[email protected]/1121',
+            tracesSampleRate: 1,
+            integrations: [new BrowserTracing()],
+          }),
+        );
+        jest.spyOn(hubModule, 'getCurrentHub').mockReturnValue(hub);
+
+        const transaction = hub.startTransaction({ name: 'dogpark' });
+        hub.configureScope(scope => {
+          scope.setSpan(transaction);
+        });
+
+        const request = new XMLHttpRequest();
+        request.open('GET', '/chase-partners');
+
+        // mock a response having been received successfully (we have to do it in this roundabout way because readyState
+        // is readonly and changing it doesn't trigger a readystatechange event)
+        Object.defineProperty(request, 'readyState', { value: 4 });
+        request.dispatchEvent(new Event('readystatechange'));
+
+        // this looks weird, it's true, but it's really just `request.impl.flag.requestHeaders` - it's just that the
+        // `impl` key is a symbol rather than a string, and therefore needs to be referred to by reference rather than
+        // value
+        const headers = (request as any)[getSymbolObjectKeyByName(request, 'impl') as symbol].flag.requestHeaders;
+
+        // check that sentry-trace header is added to request
+        expect(headers).toEqual(expect.objectContaining({ 'sentry-trace': expect.stringMatching(TRACEPARENT_REGEXP) }));
+
+        // check that sampling decision is passed down correctly
+        expect(transaction.sampled).toBe(true);
+        expect(extractTraceparentData(headers['sentry-trace'])!.parentSampled).toBe(true);
+      });
+
+      it('should propagate negative sampling decision to child transactions in XHR header', () => {
+        const hub = new Hub(
+          new BrowserClient({
+            dsn: 'https://[email protected]/1121',
+            tracesSampleRate: 1,
+            integrations: [new BrowserTracing()],
+          }),
+        );
+        jest.spyOn(hubModule, 'getCurrentHub').mockReturnValue(hub);
+
+        const transaction = hub.startTransaction({ name: 'dogpark', sampled: false });
+        hub.configureScope(scope => {
+          scope.setSpan(transaction);
+        });
+
+        const request = new XMLHttpRequest();
+        request.open('GET', '/chase-partners');
+
+        // mock a response having been received successfully (we have to do it in this roundabout way because readyState
+        // is readonly and changing it doesn't trigger a readystatechange event)
+        Object.defineProperty(request, 'readyState', { value: 4 });
+        request.dispatchEvent(new Event('readystatechange'));
+
+        // this looks weird, it's true, but it's really just `request.impl.flag.requestHeaders` - it's just that the
+        // `impl` key is a symbol rather than a string, and therefore needs to be referred to by reference rather than
+        // value
+        const headers = (request as any)[getSymbolObjectKeyByName(request, 'impl') as symbol].flag.requestHeaders;
+
+        // check that sentry-trace header is added to request
+        expect(headers).toEqual(expect.objectContaining({ 'sentry-trace': expect.stringMatching(TRACEPARENT_REGEXP) }));
+
+        // check that sampling decision is passed down correctly
+        expect(transaction.sampled).toBe(false);
+        expect(extractTraceparentData(headers['sentry-trace'])!.parentSampled).toBe(false);
+      });
+
+      it('should propagate positive sampling decision to child transactions in fetch header', () => {
+        // TODO
       });
 
-      it('should propagate sampling decision to child transactions in fetch header', () => {
-        // TODO fix this and write the test
+      it('should propagate negative sampling decision to child transactions in fetch header', () => {
+        // TODO
       });
 
       it("should inherit parent's positive sampling decision if tracesSampler is undefined", () => {

packages/tracing/test/testutils.ts

@@ -0,0 +1,44 @@
+import { getGlobalObject } from '@sentry/utils';
+import { JSDOM } from 'jsdom';
+
+/**
+ * Injects DOM properties into node global object.
+ *
+ * Useful for running tests where some of the tested code applies to @sentry/node and some applies to @sentry/browser
+ * (e.g. tests in @sentry/tracing or @sentry/hub). Note that not all properties from the browser `window` object are
+ * available.
+ *
+ * @param properties The names of the properties to add
+ */
+export function addDOMPropertiesToGlobal(properties: string[]): void {
+  // we have to add things into the real global object (rather than mocking the return value of getGlobalObject)
+  // because there are modules which call getGlobalObject as they load, which is too early for jest to intervene
+  const { window } = new JSDOM('', { url: 'http://dogs.are.great/' });
+  const global = getGlobalObject<NodeJS.Global & Window>();
+
+  properties.forEach(prop => {
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
+    (global as any)[prop] = window[prop];
+  });
+}
+
+/**
+ * Returns the symbol with the given description being used as a key in the given object.
+ *
+ * In the case where there are multiple symbols in the object with the same description, it throws an error.
+ *
+ * @param obj The object whose symbol-type key you want
+ * @param description The symbol's descriptor
+ * @returns The first symbol found in the object with the given description, or undefined if not found.
+ */
+export function getSymbolObjectKeyByName(obj: Record<string | symbol, any>, description: string): symbol | undefined {
+  const symbols = Object.getOwnPropertySymbols(obj);
+
+  const matches = symbols.filter(sym => sym.toString() === `Symbol(${description})`);
+
+  if (matches.length > 1) {
+    throw new Error(`More than one symbol key found with description '${description}'.`);
+  }
+
+  return matches[0] || undefined;
+}