test(node): Node HTTP instrumentation integration tests (#11335)

This PR adds some utilities to aid in testing Node HTTP instrumentation
and then updates the tests to use the new runner

Author: timfish

dev-packages/node-integration-tests/suites/tracing/spans/scenario.ts

@@ -1,17 +1,19 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
 import * as Sentry from '@sentry/node';
 
 Sentry.init({
   dsn: 'https://[email protected]/1337',
   release: '1.0',
   tracesSampleRate: 1.0,
+  transport: loggingTransport,
 });
 
 import * as http from 'http';
 
 // eslint-disable-next-line @typescript-eslint/no-floating-promises
 Sentry.startSpan({ name: 'test_transaction' }, async () => {
-  http.get('http://match-this-url.com/api/v0');
-  http.get('http://match-this-url.com/api/v1');
+  http.get(`${process.env.SERVER_URL}/api/v0`);
+  http.get(`${process.env.SERVER_URL}/api/v1`);
 
   // Give it a tick to resolve...
   await new Promise(resolve => setTimeout(resolve, 100));

dev-packages/node-integration-tests/suites/tracing/spans/test.ts

@@ -1,36 +1,48 @@
-import nock from 'nock';
+import { createRunner } from '../../../utils/runner';
+import { createTestServer } from '../../../utils/server';
 
-import { TestEnv, assertSentryTransaction } from '../../../utils';
+test('should capture spans for outgoing http requests', done => {
+  expect.assertions(3);
 
-// TODO: Convert this test to the new test runner
-// eslint-disable-next-line jest/no-disabled-tests
-test.skip('should capture spans for outgoing http requests', async () => {
-  const match1 = nock('http://match-this-url.com').get('/api/v0').reply(200);
-  const match2 = nock('http://match-this-url.com').get('/api/v1').reply(200);
-
-  const env = await TestEnv.init(__dirname);
-  const envelope = await env.getEnvelopeRequest({ envelopeType: 'transaction' });
-
-  expect(match1.isDone()).toBe(true);
-  expect(match2.isDone()).toBe(true);
-
-  expect(envelope).toHaveLength(3);
-
-  assertSentryTransaction(envelope[2], {
-    transaction: 'test_transaction',
-    spans: [
-      {
-        description: 'GET http://match-this-url.com/api/v0',
-        op: 'http.client',
-        origin: 'auto.http.node.http',
-        status: 'ok',
-      },
-      {
-        description: 'GET http://match-this-url.com/api/v1',
-        op: 'http.client',
-        origin: 'auto.http.node.http',
-        status: 'ok',
+  createTestServer(done)
+    .get('/api/v0', () => {
+      // Just ensure we're called
+      expect(true).toBe(true);
+    })
+    .get(
+      '/api/v1',
+      () => {
+        // Just ensure we're called
+        expect(true).toBe(true);
       },
-    ],
-  });
+      404,
+    )
+    .start()
+    .then(SERVER_URL => {
+      createRunner(__dirname, 'scenario.ts')
+        .withEnv({ SERVER_URL })
+        .expect({
+          transaction: {
+            transaction: 'test_transaction',
+            spans: expect.arrayContaining([
+              expect.objectContaining({
+                description: expect.stringMatching(/GET .*\/api\/v0/),
+                op: 'http.client',
+                origin: 'auto.http.otel.http',
+                status: 'ok',
+              }),
+              expect.objectContaining({
+                description: expect.stringMatching(/GET .*\/api\/v1/),
+                op: 'http.client',
+                origin: 'auto.http.otel.http',
+                status: 'unknown_error',
+                data: expect.objectContaining({
+                  'http.response.status_code': 404,
+                }),
+              }),
+            ]),
+          },
+        })
+        .start(done);
+    });
 });

dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/scenario.ts

@@ -1,3 +1,4 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
 import * as Sentry from '@sentry/node';
 
 Sentry.init({
@@ -6,13 +7,14 @@ Sentry.init({
   tracesSampleRate: 1.0,
   tracePropagationTargets: [/\/v0/, 'v1'],
   integrations: [],
+  transport: loggingTransport,
 });
 
 import * as http from 'http';
 
 Sentry.startSpan({ name: 'test_span' }, () => {
-  http.get('http://match-this-url.com/api/v0');
-  http.get('http://match-this-url.com/api/v1');
-  http.get('http://dont-match-this-url.com/api/v2');
-  http.get('http://dont-match-this-url.com/api/v3');
+  http.get(`${process.env.SERVER_URL}/api/v0`);
+  http.get(`${process.env.SERVER_URL}/api/v1`);
+  http.get(`${process.env.SERVER_URL}/api/v2`);
+  http.get(`${process.env.SERVER_URL}/api/v3`);
 });

dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/test.ts

@@ -1,44 +1,35 @@
-import nock from 'nock';
-
-import { TestEnv, runScenario } from '../../../utils';
-
-// TODO: Convert this test to the new test runner
-// eslint-disable-next-line jest/no-disabled-tests
-test.skip('HttpIntegration should instrument correct requests when tracePropagationTargets option is provided', async () => {
-  const match1 = nock('http://match-this-url.com')
-    .get('/api/v0')
-    .matchHeader('baggage', val => typeof val === 'string')
-    .matchHeader('sentry-trace', val => typeof val === 'string')
-    .reply(200);
-
-  const match2 = nock('http://match-this-url.com')
-    .get('/api/v1')
-    .matchHeader('baggage', val => typeof val === 'string')
-    .matchHeader('sentry-trace', val => typeof val === 'string')
-    .reply(200);
-
-  const match3 = nock('http://dont-match-this-url.com')
-    .get('/api/v2')
-    .matchHeader('baggage', val => val === undefined)
-    .matchHeader('sentry-trace', val => val === undefined)
-    .reply(200);
-
-  const match4 = nock('http://dont-match-this-url.com')
-    .get('/api/v3')
-    .matchHeader('baggage', val => val === undefined)
-    .matchHeader('sentry-trace', val => val === undefined)
-    .reply(200);
-
-  const env = await TestEnv.init(__dirname);
-  await runScenario(env.url);
-
-  env.server.close();
-  nock.cleanAll();
-
-  await new Promise(resolve => env.server.close(resolve));
-
-  expect(match1.isDone()).toBe(true);
-  expect(match2.isDone()).toBe(true);
-  expect(match3.isDone()).toBe(true);
-  expect(match4.isDone()).toBe(true);
+import { createRunner } from '../../../utils/runner';
+import { createTestServer } from '../../../utils/server';
+
+test('HttpIntegration should instrument correct requests when tracePropagationTargets option is provided', done => {
+  expect.assertions(9);
+
+  createTestServer(done)
+    .get('/api/v0', headers => {
+      expect(typeof headers['baggage']).toBe('string');
+      expect(typeof headers['sentry-trace']).toBe('string');
+    })
+    .get('/api/v1', headers => {
+      expect(typeof headers['baggage']).toBe('string');
+      expect(typeof headers['sentry-trace']).toBe('string');
+    })
+    .get('/api/v2', headers => {
+      expect(headers['baggage']).toBeUndefined();
+      expect(headers['sentry-trace']).toBeUndefined();
+    })
+    .get('/api/v3', headers => {
+      expect(headers['baggage']).toBeUndefined();
+      expect(headers['sentry-trace']).toBeUndefined();
+    })
+    .start()
+    .then(SERVER_URL => {
+      createRunner(__dirname, 'scenario.ts')
+        .withEnv({ SERVER_URL })
+        .expect({
+          transaction: {
+            // we're not too concerned with the actual transaction here since this is tested elsewhere
+          },
+        })
+        .start(done);
+    });
 });

dev-packages/node-integration-tests/utils/runner.ts

@@ -126,6 +126,7 @@ export function createRunner(...paths: string[]) {
   const expectedEnvelopes: Expected[] = [];
   const flags: string[] = [];
   const ignored: EnvelopeItemType[] = [];
+  let withEnv: Record<string, string> = {};
   let withSentryServer = false;
   let dockerOptions: DockerOptions | undefined;
   let ensureNoErrorOutput = false;
@@ -144,6 +145,10 @@ export function createRunner(...paths: string[]) {
       expectError = true;
       return this;
     },
+    withEnv: function (env: Record<string, string>) {
+      withEnv = env;
+      return this;
+    },
     withFlags: function (...args: string[]) {
       flags.push(...args);
       return this;
@@ -263,8 +268,8 @@ export function createRunner(...paths: string[]) {
           }
 
           const env = mockServerPort
-            ? { ...process.env, SENTRY_DSN: `http://public@localhost:${mockServerPort}/1337` }
-            : process.env;
+            ? { ...process.env, ...withEnv, SENTRY_DSN: `http://public@localhost:${mockServerPort}/1337` }
+            : { ...process.env, ...withEnv };
 
           // eslint-disable-next-line no-console
           if (process.env.DEBUG) console.log('starting scenario', testPath, flags, env.SENTRY_DSN);

dev-packages/node-integration-tests/utils/server.ts

@@ -31,3 +31,40 @@ export function createBasicSentryServer(onEnvelope: (env: Envelope) => void): Pr
     });
   });
 }
+
+type HeaderAssertCallback = (headers: Record<string, string | string[] | undefined>) => void;
+
+/** Creates a test server that can be used to check headers */
+// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
+export function createTestServer(done: (error: unknown) => void) {
+  const gets: Array<[string, HeaderAssertCallback, number]> = [];
+
+  return {
+    get: function (path: string, callback: HeaderAssertCallback, result = 200) {
+      gets.push([path, callback, result]);
+      return this;
+    },
+    start: async (): Promise<string> => {
+      const app = express();
+
+      for (const [path, callback, result] of gets) {
+        app.get(path, (req, res) => {
+          try {
+            callback(req.headers);
+          } catch (e) {
+            done(e);
+          }
+
+          res.status(result).send();
+        });
+      }
+
+      return new Promise(resolve => {
+        const server = app.listen(0, () => {
+          const address = server.address() as AddressInfo;
+          resolve(`http://localhost:${address.port}`);
+        });
+      });
+    },
+  };
+}