Skip to content

Commit

Permalink
feat(v8/core): Add normalizedRequest to samplingContext (#14903)
Browse files Browse the repository at this point in the history
Backport of #14902
  • Loading branch information
s1gr1d authored Jan 7, 2025
1 parent 845b7aa commit 6fa3797
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -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',
release: '1.0',
transport: loggingTransport,
tracesSampler: samplingContext => {
// The sampling decision is based on whether the data in `normalizedRequest` is available --> this is what we want to test for
return (
samplingContext.normalizedRequest.url.includes('/test-normalized-request?query=123') &&
samplingContext.normalizedRequest.method &&
samplingContext.normalizedRequest.query_string === 'query=123' &&
!!samplingContext.normalizedRequest.headers
);
},
});

// express must be required after Sentry is initialized
const express = require('express');
const cors = require('cors');
const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests');

const app = express();

app.use(cors());

app.get('/test-normalized-request', (_req, res) => {
res.send('Success');
});

Sentry.setupExpressErrorHandler(app);

startExpressServerAndSendPortToRunner(app);
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ Sentry.init({
samplingContext.attributes['http.method'] === 'GET'
);
},
debug: true,
});

// express must be required after Sentry is initialized
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,23 @@ describe('express tracesSampler', () => {
});
});
});

describe('express tracesSampler includes normalizedRequest data', () => {
afterAll(() => {
cleanupChildProcesses();
});

describe('CJS', () => {
test('correctly samples & passes data to tracesSampler', done => {
const runner = createRunner(__dirname, 'scenario-normalizedRequest.js')
.expect({
transaction: {
transaction: 'GET /test-normalized-request',
},
})
.start(done);

runner.makeRequest('get', '/test-normalized-request?query=123');
});
});
});
18 changes: 14 additions & 4 deletions packages/core/src/tracing/sampling.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { Options, SamplingContext } from '../types-hoist';
import type { Options, RequestEventData, SamplingContext } from '../types-hoist';

import { getIsolationScope } from '../currentScopes';
import { DEBUG_BUILD } from '../debug-build';
import { logger } from '../utils-hoist/logger';
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
Expand All @@ -20,13 +21,22 @@ export function sampleSpan(
return [false];
}

// Casting this from unknown, as the type of `sdkProcessingMetadata` is only changed in v9 and `normalizedRequest` is set in SentryHttpInstrumentation
const normalizedRequest = getIsolationScope().getScopeData().sdkProcessingMetadata
.normalizedRequest as RequestEventData;

const enhancedSamplingContext = {
...samplingContext,
normalizedRequest: samplingContext.normalizedRequest || normalizedRequest,
};

// we would have bailed already if neither `tracesSampler` nor `tracesSampleRate` nor `enableTracing` were defined, so one of these should
// work; prefer the hook if so
let sampleRate;
if (typeof options.tracesSampler === 'function') {
sampleRate = options.tracesSampler(samplingContext);
} else if (samplingContext.parentSampled !== undefined) {
sampleRate = samplingContext.parentSampled;
sampleRate = options.tracesSampler(enhancedSamplingContext);
} else if (enhancedSamplingContext.parentSampled !== undefined) {
sampleRate = enhancedSamplingContext.parentSampled;
} else if (typeof options.tracesSampleRate !== 'undefined') {
sampleRate = options.tracesSampleRate;
} else {
Expand Down
9 changes: 8 additions & 1 deletion packages/core/src/types-hoist/samplingcontext.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { RequestEventData } from '../types-hoist/request';
import type { ExtractedNodeRequestData, WorkerLocation } from './misc';
import type { SpanAttributes } from './span';

Expand Down Expand Up @@ -35,10 +36,16 @@ export interface SamplingContext extends CustomSamplingContext {
location?: WorkerLocation;

/**
* Object representing the incoming request to a node server. Passed by default when using the TracingHandler.
* Object representing the incoming request to a node server.
* @deprecated This attribute is currently never defined and will be removed in v9. Use `normalizedRequest` instead
*/
request?: ExtractedNodeRequestData;

/**
* Object representing the incoming request to a node server in a normalized format.
*/
normalizedRequest?: RequestEventData;

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

Expand Down

0 comments on commit 6fa3797

Please sign in to comment.