Skip to content

Commit 2f53df7

Browse files
Lms24Lms24
authored
fix(core): Don't return trace data in getTraceData and getTraceMetaTags 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`.
1 parent c009b0d commit 2f53df7

File tree

5 files changed

+139
-1
lines changed

5 files changed

+139
-1
lines changed

‎dev-packages/node-integration-tests/suites/tracing/meta-tags/server-sdk-disabled.js

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
const { loggingTransport } = require('@sentry-internal/node-integration-tests');
2+
const Sentry = require('@sentry/node');
3+
4+
Sentry.init({
5+
dsn: 'https://[email protected]/1337',
6+
tracesSampleRate: 1.0,
7+
transport: loggingTransport,
8+
enabled: false,
9+
});
10+
11+
// express must be required after Sentry is initialized
12+
const express = require('express');
13+
const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests');
14+
15+
const app = express();
16+
17+
app.get('/test', (_req, res) => {
18+
res.send({
19+
response: `
20+
<html>
21+
<head>
22+
${Sentry.getTraceMetaTags()}
23+
</head>
24+
<body>
25+
Hi :)
26+
</body>
27+
</html>
28+
`,
29+
});
30+
});
31+
32+
Sentry.setupExpressErrorHandler(app);
33+
34+
startExpressServerAndSendPortToRunner(app);

‎dev-packages/node-integration-tests/suites/tracing/meta-tags/server-tracesSampleRate-zero.js

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
const { loggingTransport } = require('@sentry-internal/node-integration-tests');
2+
const Sentry = require('@sentry/node');
3+
4+
Sentry.init({
5+
dsn: 'https://[email protected]/1337',
6+
tracesSampleRate: 0,
7+
transport: loggingTransport,
8+
});
9+
10+
// express must be required after Sentry is initialized
11+
const express = require('express');
12+
const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests');
13+
14+
const app = express();
15+
16+
app.get('/test', (_req, res) => {
17+
res.send({
18+
response: `
19+
<html>
20+
<head>
21+
${Sentry.getTraceMetaTags()}
22+
</head>
23+
<body>
24+
Hi :)
25+
</body>
26+
</html>
27+
`,
28+
});
29+
});
30+
31+
Sentry.setupExpressErrorHandler(app);
32+
33+
startExpressServerAndSendPortToRunner(app);

‎dev-packages/node-integration-tests/suites/tracing/meta-tags/test.ts

+50-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ describe('getTraceMetaTags', () => {
55
cleanupChildProcesses();
66
});
77

8-
test('injects sentry tracing <meta> tags', async () => {
8+
test('injects <meta> tags with trace from incoming headers', async () => {
99
const traceId = 'cd7ee7a6fe3ebe7ab9c3271559bc203c';
1010
const parentSpanId = '100ff0980e7a4ead';
1111

@@ -22,4 +22,53 @@ describe('getTraceMetaTags', () => {
2222
expect(html).toMatch(/<meta name="sentry-trace" content="cd7ee7a6fe3ebe7ab9c3271559bc203c-[a-z0-9]{16}-1"\/>/);
2323
expect(html).toContain('<meta name="baggage" content="sentry-environment=production"/>');
2424
});
25+
26+
test('injects <meta> tags with new trace if no incoming headers', async () => {
27+
const runner = createRunner(__dirname, 'server.js').start();
28+
29+
const response = await runner.makeRequest('get', '/test');
30+
31+
// @ts-ignore - response is defined, types just don't reflect it
32+
const html = response?.response as unknown as string;
33+
34+
const traceId = html.match(/<meta name="sentry-trace" content="([a-z0-9]{32})-[a-z0-9]{16}-1"\/>/)?.[1];
35+
expect(traceId).not.toBeUndefined();
36+
37+
expect(html).toContain('<meta name="baggage"');
38+
expect(html).toContain(`sentry-trace_id=${traceId}`);
39+
});
40+
41+
test('injects <meta> tags with negative sampling decision if tracesSampleRate is 0', async () => {
42+
const runner = createRunner(__dirname, 'server-tracesSampleRate-zero.js').start();
43+
44+
const response = await runner.makeRequest('get', '/test');
45+
46+
// @ts-ignore - response is defined, types just don't reflect it
47+
const html = response?.response as unknown as string;
48+
49+
const traceId = html.match(/<meta name="sentry-trace" content="([a-z0-9]{32})-[a-z0-9]{16}-0"\/>/)?.[1];
50+
expect(traceId).not.toBeUndefined();
51+
52+
expect(html).toContain('<meta name="baggage"');
53+
expect(html).toContain(`sentry-trace_id=${traceId}`);
54+
expect(html).toContain('sentry-sampled=false');
55+
});
56+
57+
test("doesn't inject sentry tracing <meta> tags if SDK is disabled", async () => {
58+
const traceId = 'cd7ee7a6fe3ebe7ab9c3271559bc203c';
59+
const parentSpanId = '100ff0980e7a4ead';
60+
61+
const runner = createRunner(__dirname, 'server-sdk-disabled.js').start();
62+
63+
const response = await runner.makeRequest('get', '/test', {
64+
'sentry-trace': `${traceId}-${parentSpanId}-1`,
65+
baggage: 'sentry-environment=production',
66+
});
67+
68+
// @ts-ignore - response is defined, types just don't reflect it
69+
const html = response?.response as unknown as string;
70+
71+
expect(html).not.toContain('"sentry-trace"');
72+
expect(html).not.toContain('"baggage"');
73+
});
2574
});

‎packages/core/src/utils/traceData.ts

+5
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
import { getAsyncContextStrategy } from '../asyncContext';
99
import { getMainCarrier } from '../carrier';
1010
import { getClient, getCurrentScope } from '../currentScopes';
11+
import { isEnabled } from '../exports';
1112
import { getDynamicSamplingContextFromClient, getDynamicSamplingContextFromSpan } from '../tracing';
1213
import { getActiveSpan, getRootSpan, spanToTraceHeader } from './spanUtils';
1314

@@ -23,6 +24,10 @@ import { getActiveSpan, getRootSpan, spanToTraceHeader } from './spanUtils';
2324
* or meta tag name.
2425
*/
2526
export function getTraceData(): SerializedTraceData {
27+
if (!isEnabled()) {
28+
return {};
29+
}
30+
2631
const carrier = getMainCarrier();
2732
const acs = getAsyncContextStrategy(carrier);
2833
if (acs.getTraceData) {

‎packages/core/test/lib/utils/traceData.test.ts

+17
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { SentrySpan, getTraceData } from '../../../src/';
22
import * as SentryCoreCurrentScopes from '../../../src/currentScopes';
3+
import * as SentryCoreExports from '../../../src/exports';
34
import * as SentryCoreTracing from '../../../src/tracing';
45
import * as SentryCoreSpanUtils from '../../../src/utils/spanUtils';
56

@@ -22,6 +23,14 @@ const mockedScope = {
2223
} as any;
2324

2425
describe('getTraceData', () => {
26+
beforeEach(() => {
27+
jest.spyOn(SentryCoreExports, 'isEnabled').mockReturnValue(true);
28+
});
29+
30+
afterEach(() => {
31+
jest.clearAllMocks();
32+
});
33+
2534
it('returns the tracing data from the span, if a span is available', () => {
2635
{
2736
jest.spyOn(SentryCoreTracing, 'getDynamicSamplingContextFromSpan').mockReturnValueOnce({
@@ -139,6 +148,14 @@ describe('getTraceData', () => {
139148

140149
expect(traceData).toEqual({});
141150
});
151+
152+
it('returns an empty object if the SDK is disabled', () => {
153+
jest.spyOn(SentryCoreExports, 'isEnabled').mockReturnValueOnce(false);
154+
155+
const traceData = getTraceData();
156+
157+
expect(traceData).toEqual({});
158+
});
142159
});
143160

144161
describe('isValidBaggageString', () => {

0 commit comments

Comments
 (0)