diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ebb03dfe89ac..67d956b620be 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -895,6 +895,7 @@ jobs: 'node-express-cjs-preload', 'node-otel-sdk-node', 'node-otel-custom-sampler', + 'node-otel-without-tracing', 'ember-classic', 'ember-embroider', 'nextjs-app-dir', diff --git a/.gitignore b/.gitignore index 6d96b6c7678b..be853254d612 100644 --- a/.gitignore +++ b/.gitignore @@ -43,6 +43,7 @@ local.log .rpt2_cache lint-results.json +trace.zip # legacy tmp.js 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 index 1683d4166af9..afe666c2a8f1 100644 --- 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,10 @@ "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": "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/instrument.ts b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts index 8100d27af965..d887696b1e73 100644 --- 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 @@ -5,7 +5,7 @@ const { SentrySpanProcessor, 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 || diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts index abc55344327c..9c91a0ed9531 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts @@ -12,7 +12,9 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => { const scopeSpans = json.resourceSpans?.[0]?.scopeSpans; - const httpScope = scopeSpans?.find(scopeSpan => scopeSpan.scope.name === '@opentelemetry/instrumentation-http'); + const httpScope = scopeSpans?.find( + scopeSpan => scopeSpan.scope.name === '@opentelemetry_sentry-patched/instrumentation-http', + ); return ( httpScope && @@ -22,7 +24,7 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => { ); }); - await fetch(`${baseURL}/test-transaction`); + fetch(`${baseURL}/test-transaction`); const otelData = await otelPromise; @@ -38,7 +40,9 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => { // But our default node-fetch spans are not emitted expect(scopeSpans.length).toEqual(2); - const httpScopes = scopeSpans?.filter(scopeSpan => scopeSpan.scope.name === '@opentelemetry/instrumentation-http'); + const httpScopes = scopeSpans?.filter( + scopeSpan => scopeSpan.scope.name === '@opentelemetry_sentry-patched/instrumentation-http', + ); const undiciScopes = scopeSpans?.filter( scopeSpan => scopeSpan.scope.name === '@opentelemetry/instrumentation-undici', ); @@ -49,6 +53,38 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => { expect(undiciScopes.length).toBe(1); expect(undiciScopes[0].spans.length).toBe(1); + expect(undiciScopes[0].spans).toEqual([ + { + traceId: expect.any(String), + spanId: expect.any(String), + name: 'GET', + kind: 3, + startTimeUnixNano: expect.any(String), + endTimeUnixNano: expect.any(String), + attributes: expect.arrayContaining([ + { key: 'http.request.method', value: { stringValue: 'GET' } }, + { key: 'http.request.method_original', value: { stringValue: 'GET' } }, + { key: 'url.full', value: { stringValue: 'http://localhost:3030/test-success' } }, + { key: 'url.path', value: { stringValue: '/test-success' } }, + { key: 'url.query', value: { stringValue: '' } }, + { key: 'url.scheme', value: { stringValue: 'http' } }, + { key: 'server.address', value: { stringValue: 'localhost' } }, + { key: 'server.port', value: { intValue: 3030 } }, + { key: 'user_agent.original', value: { stringValue: 'node' } }, + { key: 'network.peer.address', value: { stringValue: expect.any(String) } }, + { key: 'network.peer.port', value: { intValue: 3030 } }, + { key: 'http.response.status_code', value: { intValue: 200 } }, + { key: 'http.response.header.content-length', value: { intValue: 16 } }, + ]), + droppedAttributesCount: 0, + events: [], + droppedEventsCount: 0, + status: { code: 0 }, + links: [], + droppedLinksCount: 0, + }, + ]); + // There may be another span from another request, we can ignore that const httpSpans = httpScopes[0].spans.filter(span => span.attributes.some(attr => attr.key === 'http.target' && attr.value?.stringValue === '/test-transaction'), @@ -62,104 +98,24 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => { kind: 2, 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', - }, - }, - ], + attributes: expect.arrayContaining([ + { 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' } }, + { key: 'sentry.origin', value: { stringValue: 'auto.http.otel.http' } }, + ]), droppedAttributesCount: 0, events: [], droppedEventsCount: 0, 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 index d7fb81d22e1f..3ae59e5ee6b7 100644 --- 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 { 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 { .end(); }); } + +function makeHttpGet(url: string): Promise { + return new Promise(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 index 8213ddf7034e..1eb618d97dcc 100644 --- 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 { 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 { .end(); }); } + +function makeHttpGet(url: string): Promise { + return new Promise(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 index d0b570625c2b..e65278c3efd5 100644 --- 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); diff --git a/packages/node/src/integrations/node-fetch.ts b/packages/node/src/integrations/node-fetch.ts index ee797b0587d7..60abee504758 100644 --- a/packages/node/src/integrations/node-fetch.ts +++ b/packages/node/src/integrations/node-fetch.ts @@ -1,7 +1,18 @@ +import { context, propagation, trace } from '@opentelemetry/api'; import type { UndiciRequest, UndiciResponse } from '@opentelemetry/instrumentation-undici'; import { UndiciInstrumentation } from '@opentelemetry/instrumentation-undici'; -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, addBreadcrumb, defineIntegration } from '@sentry/core'; -import { addOpenTelemetryInstrumentation } from '@sentry/opentelemetry'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + addBreadcrumb, + defineIntegration, + getCurrentScope, + hasTracingEnabled, +} from '@sentry/core'; +import { + addOpenTelemetryInstrumentation, + generateSpanContextForPropagationContext, + getPropagationContextFromSpan, +} from '@sentry/opentelemetry'; import type { IntegrationFn, SanitizedRequestData } from '@sentry/types'; import { getBreadcrumbLogLevelFromHttpStatusCode, getSanitizedUrlString, parseUrl } from '@sentry/utils'; @@ -32,7 +43,44 @@ const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => { const url = getAbsoluteUrl(request.origin, request.path); const shouldIgnore = _ignoreOutgoingRequests && url && _ignoreOutgoingRequests(url); - return !!shouldIgnore; + if (shouldIgnore) { + return true; + } + + // If tracing is disabled, we still want to propagate traces + // So we do that manually here, matching what the instrumentation does otherwise + if (!hasTracingEnabled()) { + const ctx = context.active(); + const addedHeaders: Record = {}; + + // We generate a virtual span context from the active one, + // Where we attach the URL to the trace state, so the propagator can pick it up + const activeSpan = trace.getSpan(ctx); + const propagationContext = activeSpan + ? getPropagationContextFromSpan(activeSpan) + : getCurrentScope().getPropagationContext(); + + const spanContext = generateSpanContextForPropagationContext(propagationContext); + // We know that in practice we'll _always_ haven a traceState here + spanContext.traceState = spanContext.traceState?.set('sentry.url', url); + const ctxWithUrlTraceState = trace.setSpanContext(ctx, spanContext); + + propagation.inject(ctxWithUrlTraceState, addedHeaders); + + const requestHeaders = request.headers; + if (Array.isArray(requestHeaders)) { + Object.entries(addedHeaders).forEach(headers => requestHeaders.push(...headers)); + } else { + request.headers += Object.entries(addedHeaders) + .map(([k, v]) => `${k}: ${v}\r\n`) + .join(''); + } + + // Prevent starting a span for this request + return true; + } + + return false; }, startSpanHook: () => { return {