fix(core): Don't return trace data in getTraceData and `getTraceMet…

…aTags` if SDK is disabled (#13760) Ensures that we don't return trace data from the new `getTraceData` and `getTraceMetaTags` APIs if the SDK is disabled. Add more tests to ensure this is now properly covered, along with a test for behaviour with `tracesSampleRate: 0`.
getsentry · Sep 26, 2024 · 2f53df7 · 2f53df7
1 parent c009b0d
commit 2f53df7
Show file tree

Hide file tree

Showing 5 changed files with 139 additions and 1 deletion.
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
@@ -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: `
+    <html>
+      <head>
+        ${Sentry.getTraceMetaTags()}
+      </head>
+      <body>
+        Hi :)
+      </body>
+    </html>
+    `,
+  });
+});
+
+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
@@ -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: `
+    <html>
+      <head>
+        ${Sentry.getTraceMetaTags()}
+      </head>
+      <body>
+        Hi :)
+      </body>
+    </html>
+    `,
+  });
+});
+
+Sentry.setupExpressErrorHandler(app);
+
+startExpressServerAndSendPortToRunner(app);
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
@@ -5,7 +5,7 @@ describe('getTraceMetaTags', () => {
     cleanupChildProcesses();
   });
 
-  test('injects sentry tracing <meta> tags', async () => {
+  test('injects <meta> tags with trace from incoming headers', async () => {
     const traceId = 'cd7ee7a6fe3ebe7ab9c3271559bc203c';
     const parentSpanId = '100ff0980e7a4ead';
 
@@ -22,4 +22,53 @@ describe('getTraceMetaTags', () => {
     expect(html).toMatch(/<meta name="sentry-trace" content="cd7ee7a6fe3ebe7ab9c3271559bc203c-[a-z0-9]{16}-1"\/>/);
     expect(html).toContain('<meta name="baggage" content="sentry-environment=production"/>');
   });
+
+  test('injects <meta> 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(/<meta name="sentry-trace" content="([a-z0-9]{32})-[a-z0-9]{16}-1"\/>/)?.[1];
+    expect(traceId).not.toBeUndefined();
+
+    expect(html).toContain('<meta name="baggage"');
+    expect(html).toContain(`sentry-trace_id=${traceId}`);
+  });
+
+  test('injects <meta> 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(/<meta name="sentry-trace" content="([a-z0-9]{32})-[a-z0-9]{16}-0"\/>/)?.[1];
+    expect(traceId).not.toBeUndefined();
+
+    expect(html).toContain('<meta name="baggage"');
+    expect(html).toContain(`sentry-trace_id=${traceId}`);
+    expect(html).toContain('sentry-sampled=false');
+  });
+
+  test("doesn't inject sentry tracing <meta> 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
@@ -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) {

diff --git 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,14 @@ const mockedScope = {
 } as any;
 
 describe('getTraceData', () => {
+  beforeEach(() => {
+    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({
@@ -139,6 +148,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', () => {