Skip to content

Commit

Permalink
fix(node): Ensure autoSessionTracking is enabled by default (#12790)
Browse files Browse the repository at this point in the history
I noticed that we were not actually enabling auto session tracking
correctly in node, because we did not check on the correct `options`
object 😬
  • Loading branch information
mydea authored Jul 9, 2024
1 parent 25db805 commit 9b5cf26
Show file tree
Hide file tree
Showing 45 changed files with 180 additions and 152 deletions.
1 change: 1 addition & 0 deletions dev-packages/node-integration-tests/suites/anr/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ conditionalTest({ min: 16 })('should report ANR when event loop blocked', () =>
test('With session', done => {
createRunner(__dirname, 'basic-session.js')
.withMockSentryServer()
.unignore('session')
.expect({
session: {
status: 'abnormal',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ test("warns if using ESM on Node.js versions that don't support `register()`", a
return;
}

const runner = createRunner(__dirname, 'server.mjs').ignore('session', 'sessions', 'event').start();
const runner = createRunner(__dirname, 'server.mjs').ignore('event').start();

await runner.makeRequest('get', '/test/success');

Expand All @@ -26,15 +26,15 @@ test('does not warn if using ESM on Node.js versions that support `register()`',
return;
}

const runner = createRunner(__dirname, 'server.mjs').ignore('session', 'sessions', 'event').start();
const runner = createRunner(__dirname, 'server.mjs').ignore('event').start();

await runner.makeRequest('get', '/test/success');

expect(runner.getLogs()).not.toContain(esmWarning);
});

test('does not warn if using CJS', async () => {
const runner = createRunner(__dirname, 'server.js').ignore('session', 'sessions', 'event').start();
const runner = createRunner(__dirname, 'server.js').ignore('event').start();

await runner.makeRequest('get', '/test/success');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ afterAll(() => {
*/
test('withScope scope is NOT applied to thrown error caught by global handler', done => {
const runner = createRunner(__dirname, 'server.ts')
.ignore('session', 'sessions')
.expect({
event: {
exception: {
Expand Down Expand Up @@ -53,7 +52,6 @@ test('withScope scope is NOT applied to thrown error caught by global handler',
*/
test('isolation scope is applied to thrown error caught by global handler', done => {
const runner = createRunner(__dirname, 'server.ts')
.ignore('session', 'sessions')
.expect({
event: {
exception: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ afterAll(() => {

test('should capture and send Express controller error with txn name if tracesSampleRate is 0', done => {
const runner = createRunner(__dirname, 'server.ts')
.ignore('session', 'sessions', 'transaction')
.ignore('transaction')
.expect({
event: {
exception: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ afterAll(() => {

test('should capture and send Express controller error if tracesSampleRate is not set.', done => {
const runner = createRunner(__dirname, 'server.ts')
.ignore('session', 'sessions', 'transaction')
.ignore('transaction')
.expect({
event: {
exception: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ afterAll(() => {

test('allows to call init multiple times', done => {
const runner = createRunner(__dirname, 'server.ts')
.ignore('session', 'sessions')
.expect({
event: {
exception: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ afterAll(() => {

test('should construct correct url with common infixes with multiple parameterized routers.', done => {
createRunner(__dirname, 'server.ts')
.ignore('transaction', 'session', 'sessions')
.ignore('transaction')
.expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/user/:userId' } })
.start(done)
.makeRequest('get', '/api/v1/user/3212');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ afterAll(() => {

test('should construct correct url with common infixes with multiple routers.', done => {
createRunner(__dirname, 'server.ts')
.ignore('transaction', 'session', 'sessions')
.ignore('transaction')
.expect({ event: { message: 'Custom Message', transaction: 'GET /api2/v1/test' } })
.start(done)
.makeRequest('get', '/api2/v1/test');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ afterAll(() => {

test('should construct correct urls with multiple parameterized routers (use order reversed).', done => {
createRunner(__dirname, 'server.ts')
.ignore('transaction', 'session', 'sessions')
.ignore('transaction')
.expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/user/:userId' } })
.start(done)
.makeRequest('get', '/api/v1/user/1234/');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ afterAll(() => {

test('should construct correct urls with multiple parameterized routers.', done => {
createRunner(__dirname, 'server.ts')
.ignore('transaction', 'session', 'sessions')
.ignore('transaction')
.expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/user/:userId' } })
.start(done)
.makeRequest('get', '/api/v1/user/1234/');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ afterAll(() => {

test('should construct correct url with multiple parameterized routers of the same length (use order reversed).', done => {
createRunner(__dirname, 'server.ts')
.ignore('transaction', 'session', 'sessions')
.ignore('transaction')
.expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/:userId' } })
.start(done)
.makeRequest('get', '/api/v1/1234/');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ afterAll(() => {

test('should construct correct url with multiple parameterized routers of the same length.', done => {
createRunner(__dirname, 'server.ts')
.ignore('transaction', 'session', 'sessions')
.ignore('transaction')
.expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/:userId' } })
.start(done)
.makeRequest('get', '/api/v1/1234/');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ afterAll(() => {

test('should construct correct urls with multiple routers.', done => {
createRunner(__dirname, 'server.ts')
.ignore('transaction', 'session', 'sessions')
.ignore('transaction')
.expect({ event: { message: 'Custom Message', transaction: 'GET /api/v1/test' } })
.start(done)
.makeRequest('get', '/api/v1/test');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ conditionalTest({ min: 16 })('complex-router', () => {
};

createRunner(__dirname, 'server.ts')
.ignore('event', 'session', 'sessions')
.ignore('event')
.expect({ transaction: EXPECTED_TRANSACTION as any })
.start(done)
.makeRequest('get', '/api/api/v1/sub-router/users/123/posts/456');
Expand All @@ -54,7 +54,7 @@ conditionalTest({ min: 16 })('complex-router', () => {
};

createRunner(__dirname, 'server.ts')
.ignore('event', 'session', 'sessions')
.ignore('event')
.expect({ transaction: EXPECTED_TRANSACTION as any })
.start(done)
.makeRequest('get', '/api/api/v1/sub-router/users/123/posts/456?param=1');
Expand All @@ -80,7 +80,7 @@ conditionalTest({ min: 16 })('complex-router', () => {
};

createRunner(__dirname, 'server.ts')
.ignore('event', 'session', 'sessions')
.ignore('event')
.expect({ transaction: EXPECTED_TRANSACTION as any })
.start(done)
.makeRequest('get', '/api/api/v1/sub-router/users/123/posts/456/?param=1');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ conditionalTest({ min: 16 })('middle-layer-parameterized', () => {
};

createRunner(__dirname, 'server.ts')
.ignore('event', 'session', 'sessions')
.ignore('event')
.expect({ transaction: EXPECTED_TRANSACTION as any })
.start(done)
.makeRequest('get', '/api/v1/users/123/posts/456');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ afterAll(() => {

test('correctly applies isolation scope to span', done => {
createRunner(__dirname, 'server.ts')
.ignore('session', 'sessions')
.expect({
transaction: {
transaction: 'GET /test/isolationScope',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ describe('express tracing', () => {
describe('CJS', () => {
test('should create and send transactions for Express routes and spans for middlewares.', done => {
createRunner(__dirname, 'server.js')
.ignore('session', 'sessions')
.expect({
transaction: {
contexts: {
Expand Down Expand Up @@ -51,7 +50,6 @@ describe('express tracing', () => {

test('should set a correct transaction name for routes specified in RegEx', done => {
createRunner(__dirname, 'server.js')
.ignore('session', 'sessions')
.expect({
transaction: {
transaction: 'GET /\\/test\\/regex/',
Expand Down Expand Up @@ -80,7 +78,6 @@ describe('express tracing', () => {
'should set a correct transaction name for routes consisting of arrays of routes for %p',
((segment: string, done: () => void) => {
createRunner(__dirname, 'server.js')
.ignore('session', 'sessions')
.expect({
transaction: {
transaction: 'GET /test/array1,/\\/test\\/array[2-9]/',
Expand Down Expand Up @@ -117,7 +114,6 @@ describe('express tracing', () => {
['arr/requiredPath/optionalPath/lastParam'],
])('should handle more complex regexes in route arrays correctly for %p', ((segment: string, done: () => void) => {
createRunner(__dirname, 'server.js')
.ignore('session', 'sessions')
.expect({
transaction: {
transaction: 'GET /test/arr/:id,/\\/test\\/arr[0-9]*\\/required(path)?(\\/optionalPath)?\\/(lastParam)?/',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ describe('express tracing experimental', () => {
describe('CJS', () => {
test('should apply the scope transactionName to error events', done => {
createRunner(__dirname, 'server.js')
.ignore('session', 'sessions', 'transaction')
.ignore('transaction')
.expect({
event: {
exception: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ afterAll(() => {

test('correctly applies isolation scope even without tracing', done => {
const runner = createRunner(__dirname, 'server.ts')
.ignore('session', 'sessions')
.expect({
event: {
transaction: 'GET /test/isolationScope/1',
Expand Down
1 change: 0 additions & 1 deletion dev-packages/node-integration-tests/suites/proxy/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ afterAll(() => {
test('proxies sentry requests', done => {
createRunner(__dirname, 'basic.js')
.withMockSentryServer()
.ignore('session')
.expect({
event: {
message: 'Hello, via proxy!',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ conditionalTest({ min: 18 })('LocalVariables integration', () => {

test('Should not include local variables by default', done => {
createRunner(__dirname, 'no-local-variables.js')
.ignore('session')
.expect({
event: event => {
for (const frame of event.exception?.values?.[0]?.stacktrace?.frames || []) {
Expand All @@ -53,40 +52,30 @@ conditionalTest({ min: 18 })('LocalVariables integration', () => {
});

test('Should include local variables when enabled', done => {
createRunner(__dirname, 'local-variables.js')
.ignore('session')
.expect({ event: EXPECTED_LOCAL_VARIABLES_EVENT })
.start(done);
createRunner(__dirname, 'local-variables.js').expect({ event: EXPECTED_LOCAL_VARIABLES_EVENT }).start(done);
});

test('Should include local variables when instrumenting via --require', done => {
const requirePath = path.resolve(__dirname, 'local-variables-instrument.js');

createRunner(__dirname, 'local-variables-no-sentry.js')
.withFlags(`--require=${requirePath}`)
.ignore('session')
.expect({ event: EXPECTED_LOCAL_VARIABLES_EVENT })
.start(done);
});

test('Should include local variables with ESM', done => {
createRunner(__dirname, 'local-variables-caught.mjs')
.ignore('session')
.expect({ event: EXPECTED_LOCAL_VARIABLES_EVENT })
.start(done);
createRunner(__dirname, 'local-variables-caught.mjs').expect({ event: EXPECTED_LOCAL_VARIABLES_EVENT }).start(done);
});

conditionalTest({ min: 19 })('Node v19+', () => {
test('Should not import inspector when not in use', done => {
createRunner(__dirname, 'deny-inspector.mjs').ensureNoErrorOutput().ignore('session').start(done);
createRunner(__dirname, 'deny-inspector.mjs').ensureNoErrorOutput().start(done);
});
});

test('Includes local variables for caught exceptions when enabled', done => {
createRunner(__dirname, 'local-variables-caught.js')
.ignore('session')
.expect({ event: EXPECTED_LOCAL_VARIABLES_EVENT })
.start(done);
createRunner(__dirname, 'local-variables-caught.js').expect({ event: EXPECTED_LOCAL_VARIABLES_EVENT }).start(done);
});

test('Should not leak memory', done => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ test('should aggregate successful and crashed sessions', async () => {
});

const runner = createRunner(__dirname, 'server.ts')
.ignore('transaction', 'event', 'session')
.ignore('transaction', 'event')
.unignore('sessions')
.expectError()
.expect({
sessions: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ test('should aggregate successful, crashed and erroneous sessions', async () =>
});

const runner = createRunner(__dirname, 'server.ts')
.ignore('transaction', 'event', 'session')
.ignore('transaction', 'event')
.unignore('sessions')
.expectError()
.expect({
sessions: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ test('should aggregate successful sessions', async () => {
});

const runner = createRunner(__dirname, 'server.ts')
.ignore('transaction', 'event', 'session')
.ignore('transaction', 'event')
.unignore('sessions')
.expectError()
.expect({
sessions: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('connect auto-instrumentation', () => {

test('CJS - should capture errors in `connect` middleware.', done => {
createRunner(__dirname, 'scenario.js')
.ignore('transaction', 'session', 'sessions')
.ignore('transaction')
.expectError()
.expect({ event: EXPECTED_EVENT })
.start(done)
Expand All @@ -56,7 +56,7 @@ describe('connect auto-instrumentation', () => {

test('CJS - should report errored transactions.', done => {
createRunner(__dirname, 'scenario.js')
.ignore('event', 'session', 'sessions')
.ignore('event')
.expect({ transaction: { transaction: 'GET /error' } })
.expectError()
.start(done)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { createRunner } from '../../../../utils/runner';

test('envelope header for error event during active unsampled span is correct', done => {
createRunner(__dirname, 'scenario.ts')
.ignore('session', 'sessions', 'transaction')
.ignore('transaction')
.expectHeader({
event: {
trace: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { createRunner } from '../../../../utils/runner';

test('envelope header for error event during active span is correct', done => {
createRunner(__dirname, 'scenario.ts')
.ignore('session', 'sessions', 'transaction')
.ignore('transaction')
.expectHeader({
event: {
trace: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { createRunner } from '../../../../utils/runner';

test('envelope header for error events is correct', done => {
createRunner(__dirname, 'scenario.ts')
.ignore('session', 'sessions')
.expectHeader({
event: {
trace: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { createRunner } from '../../../../utils/runner';

test('envelope header for transaction event of route correct', done => {
createRunner(__dirname, 'scenario.ts')
.ignore('session', 'sessions')
.expectHeader({
transaction: {
trace: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { createRunner } from '../../../../utils/runner';

test('envelope header for transaction event with source=url correct', done => {
createRunner(__dirname, 'scenario.ts')
.ignore('session', 'sessions')
.expectHeader({
transaction: {
trace: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { createRunner } from '../../../../utils/runner';

test('envelope header for transaction event is correct', done => {
createRunner(__dirname, 'scenario.ts')
.ignore('session', 'sessions')
.expectHeader({
transaction: {
trace: {
Expand Down
Loading

0 comments on commit 9b5cf26

Please sign in to comment.