Skip to content

ref: Hoist RequestData integration to @sentry/core #9597

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export type { ClientClass } from './sdk';
export type { AsyncContextStrategy, Carrier, Layer, RunWithAsyncContextOptions } from './hub';
export type { OfflineStore, OfflineTransportOptions } from './transports/offline';
export type { ServerRuntimeClientOptions } from './server-runtime-client';
export type { RequestDataIntegrationOptions } from './integrations/requestdata';

export * from './tracing';
export { createEventEnvelope } from './envelope';
Expand Down Expand Up @@ -56,6 +57,7 @@ export { hasTracingEnabled } from './utils/hasTracingEnabled';
export { isSentryRequestUrl } from './utils/isSentryRequestUrl';
export { DEFAULT_ENVIRONMENT } from './constants';
export { ModuleMetadata } from './integrations/metadata';
export { RequestData } from './integrations/requestdata';
import * as Integrations from './integrations';

export { Integrations };
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
// TODO (v8 or v9): Whenever this becomes a default integration for `@sentry/browser`, move this to `@sentry/core`. For
// now, we leave it in `@sentry/integrations` so that it doesn't contribute bytes to our CDN bundles.

import type { Event, EventProcessor, Hub, Integration, PolymorphicRequest, Transaction } from '@sentry/types';
import { extractPathForTransaction } from '@sentry/utils';

import type { AddRequestDataToEventOptions, TransactionNamingScheme } from '../requestdata';
import { addRequestDataToEvent } from '../requestdata';
import type { AddRequestDataToEventOptions, TransactionNamingScheme } from '@sentry/utils';
import { addRequestDataToEvent, extractPathForTransaction } from '@sentry/utils';

export type RequestDataIntegrationOptions = {
/**
Expand Down Expand Up @@ -59,7 +54,7 @@ export class RequestData implements Integration {
/**
* @inheritDoc
*/
public name: string = RequestData.id;
public name: string;

/**
* Function for adding request data to event. Defaults to `addRequestDataToEvent` from `@sentry/node` for now, but
Expand All @@ -74,6 +69,7 @@ export class RequestData implements Integration {
* @inheritDoc
*/
public constructor(options: RequestDataIntegrationOptions = {}) {
this.name = RequestData.id;
this._addRequestData = addRequestDataToEvent;
this._options = {
...DEFAULT_OPTIONS,
Expand Down
102 changes: 102 additions & 0 deletions packages/core/test/lib/integrations/requestdata.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import type { RequestDataIntegrationOptions } from '@sentry/core';
import { getCurrentHub, Hub, makeMain, RequestData } from '@sentry/core';
import type { Event, EventProcessor } from '@sentry/types';
import * as sentryUtils from '@sentry/utils';
import type { IncomingMessage } from 'http';

import { getDefaultTestClientOptions, TestClient } from '../../mocks/client';

const addRequestDataToEventSpy = jest.spyOn(sentryUtils, 'addRequestDataToEvent');
const requestDataEventProcessor = jest.fn();

const headers = { ears: 'furry', nose: 'wet', tongue: 'spotted', cookie: 'favorite=zukes' };
const method = 'wagging';
const protocol = 'mutualsniffing';
const hostname = 'the.dog.park';
const path = '/by/the/trees/';
const queryString = 'chase=me&please=thankyou';

function initWithRequestDataIntegrationOptions(integrationOptions: RequestDataIntegrationOptions): void {
const setMockEventProcessor = (eventProcessor: EventProcessor) =>
requestDataEventProcessor.mockImplementationOnce(eventProcessor);

const requestDataIntegration = new RequestData({
...integrationOptions,
});

const client = new TestClient(
getDefaultTestClientOptions({
dsn: 'https://[email protected]/12312012',
integrations: [requestDataIntegration],
}),
);
client.setupIntegrations = () => requestDataIntegration.setupOnce(setMockEventProcessor, getCurrentHub);
client.getIntegration = () => requestDataIntegration as any;

const hub = new Hub(client);

makeMain(hub);
}

describe('`RequestData` integration', () => {
let req: IncomingMessage, event: Event;

beforeEach(() => {
req = {
headers,
method,
protocol,
hostname,
originalUrl: `${path}?${queryString}`,
} as unknown as IncomingMessage;
event = { sdkProcessingMetadata: { request: req } };
});

afterEach(() => {
jest.clearAllMocks();
});

describe('option conversion', () => {
it('leaves `ip` and `user` at top level of `include`', () => {
initWithRequestDataIntegrationOptions({ include: { ip: false, user: true } });

requestDataEventProcessor(event);

const passedOptions = addRequestDataToEventSpy.mock.calls[0][2];

expect(passedOptions?.include).toEqual(expect.objectContaining({ ip: false, user: true }));
});

it('moves `transactionNamingScheme` to `transaction` include', () => {
initWithRequestDataIntegrationOptions({ transactionNamingScheme: 'path' });

requestDataEventProcessor(event);

const passedOptions = addRequestDataToEventSpy.mock.calls[0][2];

expect(passedOptions?.include).toEqual(expect.objectContaining({ transaction: 'path' }));
});

it('moves `true` request keys into `request` include, but omits `false` ones', async () => {
initWithRequestDataIntegrationOptions({ include: { data: true, cookies: false } });

requestDataEventProcessor(event);

const passedOptions = addRequestDataToEventSpy.mock.calls[0][2];

expect(passedOptions?.include?.request).toEqual(expect.arrayContaining(['data']));
expect(passedOptions?.include?.request).not.toEqual(expect.arrayContaining(['cookies']));
});

it('moves `true` user keys into `user` include, but omits `false` ones', async () => {
initWithRequestDataIntegrationOptions({ include: { user: { id: true, email: false } } });

requestDataEventProcessor(event);

const passedOptions = addRequestDataToEventSpy.mock.calls[0][2];

expect(passedOptions?.include?.user).toEqual(expect.arrayContaining(['id']));
expect(passedOptions?.include?.user).not.toEqual(expect.arrayContaining(['email']));
});
});
});
2 changes: 1 addition & 1 deletion packages/node/src/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
addRequestDataToTransaction,
dropUndefinedKeys,
extractPathForTransaction,
extractRequestData,
isString,
isThenable,
logger,
Expand All @@ -24,7 +25,6 @@ import {
import type * as http from 'http';

import type { NodeClient } from './client';
import { extractRequestData } from './requestdata';
// TODO (v8 / XXX) Remove this import
import type { ParseRequestOptions } from './requestDataDeprecated';
import { isAutoSessionTrackingEnabled } from './sdk';
Expand Down
5 changes: 2 additions & 3 deletions packages/node/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@ export type {
Transaction,
User,
} from '@sentry/types';
export type { AddRequestDataToEventOptions } from '@sentry/utils';
export type { AddRequestDataToEventOptions, TransactionNamingScheme } from '@sentry/utils';

export type { TransactionNamingScheme } from './requestdata';
export type { NodeOptions } from './types';

export {
Expand Down Expand Up @@ -72,7 +71,7 @@ export { autoDiscoverNodePerformanceMonitoringIntegrations } from './tracing';
export { NodeClient } from './client';
export { makeNodeTransport } from './transports';
export { defaultIntegrations, init, defaultStackParser, getSentryRelease } from './sdk';
export { addRequestDataToEvent, DEFAULT_USER_INCLUDES, extractRequestData } from './requestdata';
export { addRequestDataToEvent, DEFAULT_USER_INCLUDES, extractRequestData } from '@sentry/utils';
export { deepReadDirSync } from './utils';
export { getModuleFromFilename } from './module';
export { enableAnrDetection } from './anr';
Expand Down
2 changes: 1 addition & 1 deletion packages/node/src/integrations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ export { OnUnhandledRejection } from './onunhandledrejection';
export { Modules } from './modules';
export { ContextLines } from './contextlines';
export { Context } from './context';
export { RequestData } from './requestdata';
export { RequestData } from '@sentry/core';
export { LocalVariables } from './localvariables';
export { Undici } from './undici';
5 changes: 2 additions & 3 deletions packages/node/src/requestDataDeprecated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@
/* eslint-disable deprecation/deprecation */
/* eslint-disable @typescript-eslint/no-explicit-any */
import type { Event, ExtractedNodeRequestData, PolymorphicRequest } from '@sentry/types';

import type { AddRequestDataToEventOptions } from './requestdata';
import { addRequestDataToEvent, extractRequestData as _extractRequestData } from './requestdata';
import type { AddRequestDataToEventOptions } from '@sentry/utils';
import { addRequestDataToEvent, extractRequestData as _extractRequestData } from '@sentry/utils';

/**
* @deprecated `Handlers.ExpressRequest` is deprecated and will be removed in v8. Use `PolymorphicRequest` instead.
Expand Down
Loading