Skip to content

Commit 8dd8ac5

Browse files
authored
feat(core)!: Add normalizedRequest to samplingContext (#14902)
The sampling context didn't include the `request` object anymore. By adding the `normalizedRequest`, this data is added again.
1 parent d951476 commit 8dd8ac5

File tree

6 files changed

+70
-7
lines changed

6 files changed

+70
-7
lines changed
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+
release: '1.0',
7+
transport: loggingTransport,
8+
tracesSampler: samplingContext => {
9+
// The sampling decision is based on whether the data in `normalizedRequest` is available --> this is what we want to test for
10+
return (
11+
samplingContext.normalizedRequest.url.includes('/test-normalized-request?query=123') &&
12+
samplingContext.normalizedRequest.method &&
13+
samplingContext.normalizedRequest.query_string === 'query=123' &&
14+
!!samplingContext.normalizedRequest.headers
15+
);
16+
},
17+
});
18+
19+
// express must be required after Sentry is initialized
20+
const express = require('express');
21+
const cors = require('cors');
22+
const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests');
23+
24+
const app = express();
25+
26+
app.use(cors());
27+
28+
app.get('/test-normalized-request', (_req, res) => {
29+
res.send('Success');
30+
});
31+
32+
Sentry.setupExpressErrorHandler(app);
33+
34+
startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express/tracing/tracesSampler/server.js

-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ Sentry.init({
1515
samplingContext.attributes['http.method'] === 'GET'
1616
);
1717
},
18-
debug: true,
1918
});
2019

2120
// express must be required after Sentry is initialized

dev-packages/node-integration-tests/suites/express/tracing/tracesSampler/test.ts

+20
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,23 @@ describe('express tracesSampler', () => {
2222
});
2323
});
2424
});
25+
26+
describe('express tracesSampler includes normalizedRequest data', () => {
27+
afterAll(() => {
28+
cleanupChildProcesses();
29+
});
30+
31+
describe('CJS', () => {
32+
test('correctly samples & passes data to tracesSampler', done => {
33+
const runner = createRunner(__dirname, 'scenario-normalizedRequest.js')
34+
.expect({
35+
transaction: {
36+
transaction: 'GET /test-normalized-request',
37+
},
38+
})
39+
.start(done);
40+
41+
runner.makeRequest('get', '/test-normalized-request?query=123');
42+
});
43+
});
44+
});

docs/migration/v8-to-v9.md

+1
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ Since v9, the types have been merged into `@sentry/core`, which removed some of
220220
- The `TransactionNamingScheme` type has been removed. There is no replacement.
221221
- The `Request` type has been removed. Use `RequestEventData` type instead.
222222
- The `IntegrationClass` type is no longer exported - it was not used anymore. Instead, use `Integration` or `IntegrationFn`.
223+
- The `samplingContext.request` attribute in the `tracesSampler` has been removed. Use `samplingContext.normalizedRequest` instead. Note that the type of `normalizedRequest` differs from `request`.
223224

224225
# No Version Support Timeline
225226

packages/core/src/tracing/sampling.ts

+11-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { Options, SamplingContext } from '../types-hoist';
22

3+
import { getIsolationScope } from '../currentScopes';
34
import { DEBUG_BUILD } from '../debug-build';
45
import { logger } from '../utils-hoist/logger';
56
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
@@ -20,13 +21,20 @@ export function sampleSpan(
2021
return [false];
2122
}
2223

24+
const normalizedRequest = getIsolationScope().getScopeData().sdkProcessingMetadata.normalizedRequest;
25+
26+
const enhancedSamplingContext = {
27+
...samplingContext,
28+
normalizedRequest: samplingContext.normalizedRequest || normalizedRequest,
29+
};
30+
2331
// we would have bailed already if neither `tracesSampler` nor `tracesSampleRate` nor `enableTracing` were defined, so one of these should
2432
// work; prefer the hook if so
2533
let sampleRate;
2634
if (typeof options.tracesSampler === 'function') {
27-
sampleRate = options.tracesSampler(samplingContext);
28-
} else if (samplingContext.parentSampled !== undefined) {
29-
sampleRate = samplingContext.parentSampled;
35+
sampleRate = options.tracesSampler(enhancedSamplingContext);
36+
} else if (enhancedSamplingContext.parentSampled !== undefined) {
37+
sampleRate = enhancedSamplingContext.parentSampled;
3038
} else if (typeof options.tracesSampleRate !== 'undefined') {
3139
sampleRate = options.tracesSampleRate;
3240
} else {

packages/core/src/types-hoist/samplingcontext.ts

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import type { ExtractedNodeRequestData, WorkerLocation } from './misc';
1+
import type { RequestEventData } from '../types-hoist/request';
2+
import type { WorkerLocation } from './misc';
23
import type { SpanAttributes } from './span';
34

45
/**
@@ -35,9 +36,9 @@ export interface SamplingContext extends CustomSamplingContext {
3536
location?: WorkerLocation;
3637

3738
/**
38-
* Object representing the incoming request to a node server.
39+
* Object representing the incoming request to a node server in a normalized format.
3940
*/
40-
request?: ExtractedNodeRequestData;
41+
normalizedRequest?: RequestEventData;
4142

4243
/** The name of the span being sampled. */
4344
name: string;

0 commit comments

Comments
 (0)