feat(node): Implement Sentry-specific http instrumentation

getsentry · Sep 24, 2024 · a3294d1 · a3294d1
1 parent 40ebfad
commit a3294d1
Show file tree

Hide file tree

Showing 13 changed files with 691 additions and 425 deletions.
diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/package.json b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/package.json
@@ -11,10 +11,11 @@
     "test:assert": "pnpm test"
   },
   "dependencies": {
-    "@opentelemetry/sdk-trace-node": "1.25.1",
-    "@opentelemetry/exporter-trace-otlp-http": "0.52.1",
-    "@opentelemetry/instrumentation-undici": "0.4.0",
-    "@opentelemetry/instrumentation": "0.52.1",
+    "@opentelemetry/sdk-trace-node": "1.26.0",
+    "@opentelemetry/exporter-trace-otlp-http": "0.53.0",
+    "@opentelemetry/instrumentation-undici": "0.6.0",
+    "@opentelemetry/instrumentation-http": "0.53.0",
+    "@opentelemetry/instrumentation": "0.53.0",
     "@sentry/core": "latest || *",
     "@sentry/node": "latest || *",
     "@sentry/opentelemetry": "latest || *",

diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/app.ts b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/app.ts
@@ -7,6 +7,8 @@ import express from 'express';
 const app = express();
 const port = 3030;
 
+Sentry.setTag('root-level-tag', 'yes');
+
 app.get('/test-success', function (req, res) {
   res.send({ version: 'v1' });
 });
@@ -23,8 +25,6 @@ app.get('/test-transaction', function (req, res) {
 
     await fetch('http://localhost:3030/test-success');
 
-    await Sentry.flush();
-
     res.send({});
   });
 });
@@ -38,7 +38,10 @@ app.get('/test-error', async function (req, res) {
 });
 
 app.get('/test-exception/:id', function (req, _res) {
-  throw new Error(`This is an exception with id ${req.params.id}`);
+  const id = req.params.id;
+  Sentry.setTag(`param-${id}`, id);
+
+  throw new Error(`This is an exception with id ${id}`);
 });
 
 Sentry.setupExpressErrorHandler(app);

diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts
@@ -1,11 +1,12 @@
+const { HttpInstrumentation } = require('@opentelemetry/instrumentation-http');
 const { NodeTracerProvider, BatchSpanProcessor } = require('@opentelemetry/sdk-trace-node');
 const { OTLPTraceExporter } = require('@opentelemetry/exporter-trace-otlp-http');
 const Sentry = require('@sentry/node');
-const { SentrySpanProcessor, SentryPropagator } = require('@sentry/opentelemetry');
+const { SentryPropagator } = require('@sentry/opentelemetry');
 const { UndiciInstrumentation } = require('@opentelemetry/instrumentation-undici');
 const { registerInstrumentations } = require('@opentelemetry/instrumentation');
 
-const sentryClient = Sentry.init({
+Sentry.init({
   environment: 'qa', // dynamic sampling bias to keep transactions
   dsn:
     process.env.E2E_TEST_DSN ||
@@ -15,6 +16,8 @@ const sentryClient = Sentry.init({
   tunnel: `http://localhost:3031/`, // proxy server
   // Tracing is completely disabled
 
+  integrations: [Sentry.httpIntegration({ spans: true })],
+
   // Custom OTEL setup
   skipOpenTelemetrySetup: true,
 });
@@ -37,5 +40,5 @@ provider.register({
 });
 
 registerInstrumentations({
-  instrumentations: [new UndiciInstrumentation()],
+  instrumentations: [new UndiciInstrumentation(), new HttpInstrumentation()],
 });
diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/errors.test.ts
@@ -28,3 +28,24 @@ test('Sends correct error event', async ({ baseURL }) => {
     span_id: expect.any(String),
   });
 });
+
+test('Isolates requests correctly', async ({ baseURL }) => {
+  const errorEventPromise1 = waitForError('node-otel-without-tracing', event => {
+    return !event.type && event.exception?.values?.[0]?.value === 'This is an exception with id 555-a';
+  });
+  const errorEventPromise2 = waitForError('node-otel-without-tracing', event => {
+    return !event.type && event.exception?.values?.[0]?.value === 'This is an exception with id 555-b';
+  });
+
+  fetch(`${baseURL}/test-exception/555-a`);
+  fetch(`${baseURL}/test-exception/555-b`);
+
+  const errorEvent1 = await errorEventPromise1;
+  const errorEvent2 = await errorEventPromise2;
+
+  expect(errorEvent1.transaction).toEqual('GET /test-exception/555-a');
+  expect(errorEvent1.tags).toEqual({ 'root-level-tag': 'yes', 'param-555-a': '555-a' });
+
+  expect(errorEvent2.transaction).toEqual('GET /test-exception/555-b');
+  expect(errorEvent2.tags).toEqual({ 'root-level-tag': 'yes', 'param-555-b': '555-b' });
+});
diff --git a/...packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts b/...packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts
@@ -22,7 +22,7 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => {
     );
   });
 
-  await fetch(`${baseURL}/test-transaction`);
+  fetch(`${baseURL}/test-transaction`);
 
   const otelData = await otelPromise;
 
@@ -63,102 +63,21 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => {
       startTimeUnixNano: expect.any(String),
       endTimeUnixNano: expect.any(String),
       attributes: [
-        {
-          key: 'http.url',
-          value: {
-            stringValue: 'http://localhost:3030/test-transaction',
-          },
-        },
-        {
-          key: 'http.host',
-          value: {
-            stringValue: 'localhost:3030',
-          },
-        },
-        {
-          key: 'net.host.name',
-          value: {
-            stringValue: 'localhost',
-          },
-        },
-        {
-          key: 'http.method',
-          value: {
-            stringValue: 'GET',
-          },
-        },
-        {
-          key: 'http.scheme',
-          value: {
-            stringValue: 'http',
-          },
-        },
-        {
-          key: 'http.target',
-          value: {
-            stringValue: '/test-transaction',
-          },
-        },
-        {
-          key: 'http.user_agent',
-          value: {
-            stringValue: 'node',
-          },
-        },
-        {
-          key: 'http.flavor',
-          value: {
-            stringValue: '1.1',
-          },
-        },
-        {
-          key: 'net.transport',
-          value: {
-            stringValue: 'ip_tcp',
-          },
-        },
-        {
-          key: 'sentry.origin',
-          value: {
-            stringValue: 'auto.http.otel.http',
-          },
-        },
-        {
-          key: 'net.host.ip',
-          value: {
-            stringValue: expect.any(String),
-          },
-        },
-        {
-          key: 'net.host.port',
-          value: {
-            intValue: 3030,
-          },
-        },
-        {
-          key: 'net.peer.ip',
-          value: {
-            stringValue: expect.any(String),
-          },
-        },
-        {
-          key: 'net.peer.port',
-          value: {
-            intValue: expect.any(Number),
-          },
-        },
-        {
-          key: 'http.status_code',
-          value: {
-            intValue: 200,
-          },
-        },
-        {
-          key: 'http.status_text',
-          value: {
-            stringValue: 'OK',
-          },
-        },
+        { key: 'http.url', value: { stringValue: 'http://localhost:3030/test-transaction' } },
+        { key: 'http.host', value: { stringValue: 'localhost:3030' } },
+        { key: 'net.host.name', value: { stringValue: 'localhost' } },
+        { key: 'http.method', value: { stringValue: 'GET' } },
+        { key: 'http.scheme', value: { stringValue: 'http' } },
+        { key: 'http.target', value: { stringValue: '/test-transaction' } },
+        { key: 'http.user_agent', value: { stringValue: 'node' } },
+        { key: 'http.flavor', value: { stringValue: '1.1' } },
+        { key: 'net.transport', value: { stringValue: 'ip_tcp' } },
+        { key: 'net.host.ip', value: { stringValue: expect.any(String) } },
+        { key: 'net.host.port', value: { intValue: 3030 } },
+        { key: 'net.peer.ip', value: { stringValue: expect.any(String) } },
+        { key: 'net.peer.port', value: { intValue: expect.any(Number) } },
+        { key: 'http.status_code', value: { intValue: 200 } },
+        { key: 'http.status_text', value: { stringValue: 'OK' } },
       ],
       droppedAttributesCount: 0,
       events: [],

diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-breadcrumbs/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-breadcrumbs/scenario.ts
@@ -22,7 +22,7 @@ async function run(): Promise<void> {
   Sentry.addBreadcrumb({ message: 'manual breadcrumb' });
 
   await makeHttpRequest(`${process.env.SERVER_URL}/api/v0`);
-  await makeHttpRequest(`${process.env.SERVER_URL}/api/v1`);
+  await makeHttpGet(`${process.env.SERVER_URL}/api/v1`);
   await makeHttpRequest(`${process.env.SERVER_URL}/api/v2`);
   await makeHttpRequest(`${process.env.SERVER_URL}/api/v3`);
 
@@ -46,3 +46,16 @@ function makeHttpRequest(url: string): Promise<void> {
       .end();
   });
 }
+
+function makeHttpGet(url: string): Promise<void> {
+  return new Promise<void>(resolve => {
+    http.get(url, httpRes => {
+      httpRes.on('data', () => {
+        // we don't care about data
+      });
+      httpRes.on('end', () => {
+        resolve();
+      });
+    });
+  });
+}
diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing/scenario.ts
@@ -13,7 +13,7 @@ import * as http from 'http';
 
 async function run(): Promise<void> {
   await makeHttpRequest(`${process.env.SERVER_URL}/api/v0`);
-  await makeHttpRequest(`${process.env.SERVER_URL}/api/v1`);
+  await makeHttpGet(`${process.env.SERVER_URL}/api/v1`);
   await makeHttpRequest(`${process.env.SERVER_URL}/api/v2`);
   await makeHttpRequest(`${process.env.SERVER_URL}/api/v3`);
 
@@ -37,3 +37,16 @@ function makeHttpRequest(url: string): Promise<void> {
       .end();
   });
 }
+
+function makeHttpGet(url: string): Promise<void> {
+  return new Promise<void>(resolve => {
+    http.get(url, httpRes => {
+      httpRes.on('data', () => {
+        // we don't care about data
+      });
+      httpRes.on('end', () => {
+        resolve();
+      });
+    });
+  });
+}
diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing/test.ts
@@ -38,6 +38,56 @@ test('outgoing http requests are correctly instrumented with tracing disabled',
                 },
               ],
             },
+            breadcrumbs: [
+              {
+                message: 'manual breadcrumb',
+                timestamp: expect.any(Number),
+              },
+              {
+                category: 'http',
+                data: {
+                  'http.method': 'GET',
+                  url: `${SERVER_URL}/api/v0`,
+                  status_code: 404,
+                  ADDED_PATH: '/api/v0',
+                },
+                timestamp: expect.any(Number),
+                type: 'http',
+              },
+              {
+                category: 'http',
+                data: {
+                  'http.method': 'GET',
+                  url: `${SERVER_URL}/api/v1`,
+                  status_code: 404,
+                  ADDED_PATH: '/api/v1',
+                },
+                timestamp: expect.any(Number),
+                type: 'http',
+              },
+              {
+                category: 'http',
+                data: {
+                  'http.method': 'GET',
+                  url: `${SERVER_URL}/api/v2`,
+                  status_code: 404,
+                  ADDED_PATH: '/api/v2',
+                },
+                timestamp: expect.any(Number),
+                type: 'http',
+              },
+              {
+                category: 'http',
+                data: {
+                  'http.method': 'GET',
+                  url: `${SERVER_URL}/api/v3`,
+                  status_code: 404,
+                  ADDED_PATH: '/api/v3',
+                },
+                timestamp: expect.any(Number),
+                type: 'http',
+              },
+            ],
           },
         })
         .start(closeTestServer);