From 1ff954bfdb4fa6fcc9fe874672ee8c5a6b3901fd Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 24 Sep 2024 10:03:43 +0200 Subject: [PATCH 1/4] fix(core): Don't emit trace data in `getTraceData` and `getTraceMetaTags` if SDK is disabled --- .../suites/tracing/meta-tags/test.ts | 51 ++++++++++++++++++- packages/core/src/utils/traceData.ts | 5 ++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/meta-tags/test.ts b/dev-packages/node-integration-tests/suites/tracing/meta-tags/test.ts index c42269dd8504..ab63b1c9cb35 100644 --- a/dev-packages/node-integration-tests/suites/tracing/meta-tags/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags/test.ts @@ -5,7 +5,7 @@ describe('getTraceMetaTags', () => { cleanupChildProcesses(); }); - test('injects sentry tracing tags', async () => { + test('injects tags with trace from incoming headers', async () => { const traceId = 'cd7ee7a6fe3ebe7ab9c3271559bc203c'; const parentSpanId = '100ff0980e7a4ead'; @@ -22,4 +22,53 @@ describe('getTraceMetaTags', () => { expect(html).toMatch(//); expect(html).toContain(''); }); + + test('injects tags with new trace if no incoming headers', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + + const response = await runner.makeRequest('get', '/test'); + + // @ts-ignore - response is defined, types just don't reflect it + const html = response?.response as unknown as string; + + const traceId = html.match(//)?.[1]; + expect(traceId).not.toBeUndefined(); + + expect(html).toContain(' tags with negative sampling decision if tracesSampleRate is 0', async () => { + const runner = createRunner(__dirname, 'server-tracesSampleRate-zero.js').start(); + + const response = await runner.makeRequest('get', '/test'); + + // @ts-ignore - response is defined, types just don't reflect it + const html = response?.response as unknown as string; + + const traceId = html.match(//)?.[1]; + expect(traceId).not.toBeUndefined(); + + expect(html).toContain(' tags if SDK is disabled", async () => { + const traceId = 'cd7ee7a6fe3ebe7ab9c3271559bc203c'; + const parentSpanId = '100ff0980e7a4ead'; + + const runner = createRunner(__dirname, 'server-sdk-disabled.js').start(); + + const response = await runner.makeRequest('get', '/test', { + 'sentry-trace': `${traceId}-${parentSpanId}-1`, + baggage: 'sentry-environment=production', + }); + + // @ts-ignore - response is defined, types just don't reflect it + const html = response?.response as unknown as string; + + expect(html).not.toContain('"sentry-trace"'); + expect(html).not.toContain('"baggage"'); + }); }); diff --git a/packages/core/src/utils/traceData.ts b/packages/core/src/utils/traceData.ts index 831e8187996e..c56c8f71ba1d 100644 --- a/packages/core/src/utils/traceData.ts +++ b/packages/core/src/utils/traceData.ts @@ -8,6 +8,7 @@ import { import { getAsyncContextStrategy } from '../asyncContext'; import { getMainCarrier } from '../carrier'; import { getClient, getCurrentScope } from '../currentScopes'; +import { isEnabled } from '../exports'; import { getDynamicSamplingContextFromClient, getDynamicSamplingContextFromSpan } from '../tracing'; import { getActiveSpan, getRootSpan, spanToTraceHeader } from './spanUtils'; @@ -23,6 +24,10 @@ import { getActiveSpan, getRootSpan, spanToTraceHeader } from './spanUtils'; * or meta tag name. */ export function getTraceData(): SerializedTraceData { + if (!isEnabled()) { + return {}; + } + const carrier = getMainCarrier(); const acs = getAsyncContextStrategy(carrier); if (acs.getTraceData) { From e94f0062c12518246e6345c7db12e4f9523daae7 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 24 Sep 2024 13:11:29 +0200 Subject: [PATCH 2/4] fix tests and add unit test --- packages/core/test/lib/utils/traceData.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/core/test/lib/utils/traceData.test.ts b/packages/core/test/lib/utils/traceData.test.ts index a6fb3c57814e..0ae5830de2ae 100644 --- a/packages/core/test/lib/utils/traceData.test.ts +++ b/packages/core/test/lib/utils/traceData.test.ts @@ -1,5 +1,6 @@ import { SentrySpan, getTraceData } from '../../../src/'; import * as SentryCoreCurrentScopes from '../../../src/currentScopes'; +import * as SentryCoreExports from '../../../src/exports'; import * as SentryCoreTracing from '../../../src/tracing'; import * as SentryCoreSpanUtils from '../../../src/utils/spanUtils'; @@ -22,6 +23,10 @@ const mockedScope = { } as any; describe('getTraceData', () => { + beforeEach(() => { + jest.clearAllMocks(); + jest.spyOn(SentryCoreExports, 'isEnabled').mockReturnValue(true); + }); it('returns the tracing data from the span, if a span is available', () => { { jest.spyOn(SentryCoreTracing, 'getDynamicSamplingContextFromSpan').mockReturnValueOnce({ @@ -139,6 +144,14 @@ describe('getTraceData', () => { expect(traceData).toEqual({}); }); + + it('returns an empty object if the SDK is disabled', () => { + jest.spyOn(SentryCoreExports, 'isEnabled').mockReturnValueOnce(false); + + const traceData = getTraceData(); + + expect(traceData).toEqual({}); + }); }); describe('isValidBaggageString', () => { From ca986f6f7460d71deeb1cce9d4d2126a843dc284 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 24 Sep 2024 18:06:14 +0200 Subject: [PATCH 3/4] shot in the dark but maybe this fixes the tests? --- packages/core/test/lib/utils/traceData.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/core/test/lib/utils/traceData.test.ts b/packages/core/test/lib/utils/traceData.test.ts index 0ae5830de2ae..aa6d2497dd54 100644 --- a/packages/core/test/lib/utils/traceData.test.ts +++ b/packages/core/test/lib/utils/traceData.test.ts @@ -24,9 +24,13 @@ const mockedScope = { describe('getTraceData', () => { beforeEach(() => { - jest.clearAllMocks(); jest.spyOn(SentryCoreExports, 'isEnabled').mockReturnValue(true); }); + + afterEach(() => { + jest.clearAllMocks(); + }); + it('returns the tracing data from the span, if a span is available', () => { { jest.spyOn(SentryCoreTracing, 'getDynamicSamplingContextFromSpan').mockReturnValueOnce({ From 6746f66557013deeb800a8f285cb3d6a71eddcaf Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 26 Sep 2024 16:44:03 +0200 Subject: [PATCH 4/4] sometimes i question my sanity... --- .../tracing/meta-tags/server-sdk-disabled.js | 34 +++++++++++++++++++ .../meta-tags/server-tracesSampleRate-zero.js | 33 ++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 dev-packages/node-integration-tests/suites/tracing/meta-tags/server-sdk-disabled.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/meta-tags/server-tracesSampleRate-zero.js diff --git a/dev-packages/node-integration-tests/suites/tracing/meta-tags/server-sdk-disabled.js b/dev-packages/node-integration-tests/suites/tracing/meta-tags/server-sdk-disabled.js new file mode 100644 index 000000000000..3ad43cae7647 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags/server-sdk-disabled.js @@ -0,0 +1,34 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 1.0, + transport: loggingTransport, + enabled: false, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.get('/test', (_req, res) => { + res.send({ + response: ` + + + ${Sentry.getTraceMetaTags()} + + + Hi :) + + + `, + }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/meta-tags/server-tracesSampleRate-zero.js b/dev-packages/node-integration-tests/suites/tracing/meta-tags/server-tracesSampleRate-zero.js new file mode 100644 index 000000000000..31db69722c3a --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags/server-tracesSampleRate-zero.js @@ -0,0 +1,33 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 0, + transport: loggingTransport, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.get('/test', (_req, res) => { + res.send({ + response: ` + + + ${Sentry.getTraceMetaTags()} + + + Hi :) + + + `, + }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app);