Skip to content

Commit

Permalink
feat(core): Deprecate APIs around RequestSessions
Browse files Browse the repository at this point in the history
  • Loading branch information
lforst committed Dec 4, 2024
1 parent 0b349eb commit 3442ff7
Show file tree
Hide file tree
Showing 17 changed files with 114 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import express from 'express';

const app = express();

// eslint-disable-next-line deprecation/deprecation
const flusher = (Sentry.getClient() as Sentry.NodeClient)['_sessionFlusher'] as SessionFlusher;

// Flush after 2 seconds (to avoid waiting for the default 60s)
Expand Down
7 changes: 7 additions & 0 deletions docs/migration/draft-v9-migration-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@
- Deprecated `addTracingHeadersToFetchRequest` method - this was only meant for internal use and is not needed anymore.
- Deprecated `generatePropagationContext()` in favor of using `generateTraceId()` directly.
- Deprecated `spanId` field on `propagationContext` - this field will be removed in v9, and should neither be read or set anymore.
- Deprecated `RequestSession` type. No replacements.
- Deprecated `RequestSessionStatus` type. No replacements.
- Deprecated `SessionFlusherLike` type. No replacements.
- Deprecated `SessionFlusher`. No replacements.

## `@sentry/nestjs`

Expand All @@ -69,6 +73,9 @@
- **The `@sentry/types` package has been deprecated. Import everything from `@sentry/core` instead.**

- Deprecated `Request` in favor of `RequestEventData`.
- Deprecated `RequestSession`. No replacements.
- Deprecated `RequestSessionStatus`. No replacements.
- Deprecated `SessionFlusherLike`. No replacements.

## `@sentry/nuxt`

Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export {
export { setAsyncContextStrategy } from './asyncContext';
export { getMainCarrier } from './carrier';
export { makeSession, closeSession, updateSession } from './session';
// eslint-disable-next-line deprecation/deprecation
export { SessionFlusher } from './sessionflusher';
export { Scope } from './scope';
export { notifyEventProcessors } from './eventProcessors';
Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ class ScopeClass implements ScopeInterface {
protected _session?: Session;

/** Request Mode Session Status */
// eslint-disable-next-line deprecation/deprecation
protected _requestSession?: RequestSession;

/** The client on this scope */
Expand Down Expand Up @@ -222,13 +223,15 @@ class ScopeClass implements ScopeInterface {
/**
* @inheritDoc
*/
// eslint-disable-next-line deprecation/deprecation
public getRequestSession(): RequestSession | undefined {
return this._requestSession;
}

/**
* @inheritDoc
*/
// eslint-disable-next-line deprecation/deprecation
public setRequestSession(requestSession?: RequestSession): this {
this._requestSession = requestSession;
return this;
Expand Down Expand Up @@ -350,7 +353,8 @@ class ScopeClass implements ScopeInterface {

const [scopeInstance, requestSession] =
scopeToMerge instanceof Scope
? [scopeToMerge.getScopeData(), scopeToMerge.getRequestSession()]
? // eslint-disable-next-line deprecation/deprecation
[scopeToMerge.getScopeData(), scopeToMerge.getRequestSession()]
: isPlainObject(scopeToMerge)
? [captureContext as ScopeContext, (captureContext as ScopeContext).requestSession]
: [];
Expand Down
30 changes: 22 additions & 8 deletions packages/core/src/server-runtime-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export interface ServerRuntimeClientOptions extends ClientOptions<BaseTransportO
export class ServerRuntimeClient<
O extends ClientOptions & ServerRuntimeClientOptions = ServerRuntimeClientOptions,
> extends BaseClient<O> {
// eslint-disable-next-line deprecation/deprecation
protected _sessionFlusher: SessionFlusher | undefined;

/**
Expand Down Expand Up @@ -80,10 +81,12 @@ export class ServerRuntimeClient<
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
public captureException(exception: any, hint?: EventHint, scope?: Scope): string {
// Check if the flag `autoSessionTracking` is enabled, and if `_sessionFlusher` exists because it is initialised only
// when the `requestHandler` middleware is used, and hence the expectation is to have SessionAggregates payload
// sent to the Server only when the `requestHandler` middleware is used
// Check if `_sessionFlusher` exists because it is initialized (defined) only when the `autoSessionTracking` is enabled.
// The expectation is that session aggregates are only sent when `autoSessionTracking` is enabled.
// TODO(v9): Our goal in the future is to not have the `autoSessionTracking` option and instead rely on integrations doing the creation and sending of sessions. We will not have a central kill-switch for sessions.
// TODO(v9): This should move into the httpIntegration.
if (this._options.autoSessionTracking && this._sessionFlusher) {
// eslint-disable-next-line deprecation/deprecation
const requestSession = getIsolationScope().getRequestSession();

// Necessary checks to ensure this is code block is executed only within a request
Expand All @@ -100,16 +103,18 @@ export class ServerRuntimeClient<
* @inheritDoc
*/
public captureEvent(event: Event, hint?: EventHint, scope?: Scope): string {
// Check if the flag `autoSessionTracking` is enabled, and if `_sessionFlusher` exists because it is initialised only
// when the `requestHandler` middleware is used, and hence the expectation is to have SessionAggregates payload
// sent to the Server only when the `requestHandler` middleware is used
// Check if `_sessionFlusher` exists because it is initialized only when the `autoSessionTracking` is enabled.
// The expectation is that session aggregates are only sent when `autoSessionTracking` is enabled.
// TODO(v9): Our goal in the future is to not have the `autoSessionTracking` option and instead rely on integrations doing the creation and sending of sessions. We will not have a central kill-switch for sessions.
// TODO(v9): This should move into the httpIntegration.
if (this._options.autoSessionTracking && this._sessionFlusher) {
const eventType = event.type || 'exception';
const isException =
eventType === 'exception' && event.exception && event.exception.values && event.exception.values.length > 0;

// If the event is of type Exception, then a request session should be captured
if (isException) {
// eslint-disable-next-line deprecation/deprecation
const requestSession = getIsolationScope().getRequestSession();

// Ensure that this is happening within the bounds of a request, and make sure not to override
Expand All @@ -134,12 +139,19 @@ export class ServerRuntimeClient<
return super.close(timeout);
}

/** Method that initialises an instance of SessionFlusher on Client */
/**
* Initializes an instance of SessionFlusher on the client which will aggregate and periodically flush session data.
*
* NOTICE: This method will implicitly create an interval that is periodically called.
* To clean up this resources, call `.close()` when you no longer intend to use the client.
* Not doing so will result in a memory leak.
*/
public initSessionFlusher(): void {
const { release, environment } = this._options;
if (!release) {
DEBUG_BUILD && logger.warn('Cannot initialise an instance of SessionFlusher if no release is provided!');
DEBUG_BUILD && logger.warn('Cannot initialize an instance of SessionFlusher if no release is provided!');
} else {
// eslint-disable-next-line deprecation/deprecation
this._sessionFlusher = new SessionFlusher(this, {
release,
environment,
Expand Down Expand Up @@ -214,6 +226,8 @@ export class ServerRuntimeClient<
/**
* Method responsible for capturing/ending a request session by calling `incrementSessionStatusCount` to increment
* appropriate session aggregates bucket
*
* @deprecated This method should not be used or extended. It's functionality will move into the `httpIntegration` and not be part of any public API.
*/
protected _captureRequestSession(): void {
if (!this._sessionFlusher) {
Expand Down
7 changes: 6 additions & 1 deletion packages/core/src/sessionflusher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ type ReleaseHealthAttributes = {
};

/**
* @inheritdoc
* @deprecated `SessionFlusher` is deprecated and will be removed in the next major version of the SDK.
*/
// TODO(v9): The goal for the SessionFlusher is to become a stupidly simple mechanism to aggregate "Sessions" (actually "RequestSessions"). It should probably live directly inside the Http integration/instrumentation.
// eslint-disable-next-line deprecation/deprecation
export class SessionFlusher implements SessionFlusherLike {
public readonly flushTimeout: number;
private _pendingAggregates: Map<number, AggregationCounts>;
Expand Down Expand Up @@ -80,12 +82,14 @@ export class SessionFlusher implements SessionFlusherLike {
return;
}
const isolationScope = getIsolationScope();
// eslint-disable-next-line deprecation/deprecation
const requestSession = isolationScope.getRequestSession();

if (requestSession && requestSession.status) {
this._incrementSessionStatusCount(requestSession.status, new Date());
// This is not entirely necessarily but is added as a safe guard to indicate the bounds of a request and so in
// case captureRequestSession is called more than once to prevent double count
// eslint-disable-next-line deprecation/deprecation
isolationScope.setRequestSession(undefined);
/* eslint-enable @typescript-eslint/no-unsafe-member-access */
}
Expand All @@ -95,6 +99,7 @@ export class SessionFlusher implements SessionFlusherLike {
* Increments status bucket in pendingAggregates buffer (internal state) corresponding to status of
* the session received
*/
// eslint-disable-next-line deprecation/deprecation
private _incrementSessionStatusCount(status: RequestSessionStatus, date: Date): number {
// Truncate minutes and seconds on Session Started attribute to have one minute bucket keys
const sessionStartedTrunc = new Date(date).setSeconds(0, 0);
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/types-hoist/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,11 @@ export type {
Session,
SessionContext,
SessionStatus,
// eslint-disable-next-line deprecation/deprecation
RequestSession,
// eslint-disable-next-line deprecation/deprecation
RequestSessionStatus,
// eslint-disable-next-line deprecation/deprecation
SessionFlusherLike,
SerializedSession,
} from './session';
Expand Down
7 changes: 7 additions & 0 deletions packages/core/src/types-hoist/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export interface ScopeContext {
contexts: Contexts;
tags: { [key: string]: Primitive };
fingerprint: string[];
// eslint-disable-next-line deprecation/deprecation
requestSession: RequestSession;
propagationContext: PropagationContext;
}
Expand Down Expand Up @@ -168,12 +169,18 @@ export interface Scope {

/**
* Returns the `RequestSession` if there is one
*
* @deprecated Use `getSession()` and `setSession()` instead of `getRequestSession()` and `setRequestSession()`;
*/
// eslint-disable-next-line deprecation/deprecation
getRequestSession(): RequestSession | undefined;

/**
* Sets the `RequestSession` on the scope
*
* @deprecated Use `getSession()` and `setSession()` instead of `getRequestSession()` and `setRequestSession()`;
*/
// eslint-disable-next-line deprecation/deprecation
setRequestSession(requestSession?: RequestSession): this;

/**
Expand Down
11 changes: 11 additions & 0 deletions packages/core/src/types-hoist/session.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import type { User } from './user';

/**
* @deprecated This type is deprecated and will be removed in the next major version of the SDK.
*/
export interface RequestSession {
// eslint-disable-next-line deprecation/deprecation
status?: RequestSessionStatus;
}

Expand Down Expand Up @@ -35,6 +39,10 @@ export interface Session {
export type SessionContext = Partial<Session>;

export type SessionStatus = 'ok' | 'exited' | 'crashed' | 'abnormal';

/**
* @deprecated This type is deprecated and will be removed in the next major version of the SDK.
*/
export type RequestSessionStatus = 'ok' | 'errored' | 'crashed';

/** JSDoc */
Expand All @@ -46,6 +54,9 @@ export interface SessionAggregates {
aggregates: Array<AggregationCounts>;
}

/**
* @deprecated This type is deprecated and will be removed in the next major version of the SDK.
*/
export interface SessionFlusherLike {
/**
* Increments the Session Status bucket in SessionAggregates Object corresponding to the status of the session
Expand Down
9 changes: 9 additions & 0 deletions packages/core/test/lib/scope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,10 @@ describe('Scope', () => {

test('_requestSession clone', () => {
const parentScope = new Scope();
// eslint-disable-next-line deprecation/deprecation
parentScope.setRequestSession({ status: 'errored' });
const scope = parentScope.clone();
// eslint-disable-next-line deprecation/deprecation
expect(parentScope.getRequestSession()).toEqual(scope.getRequestSession());
});

Expand All @@ -288,15 +290,19 @@ describe('Scope', () => {
// Test that ensures if the status value of `status` of `_requestSession` is changed in a child scope
// that it should also change in parent scope because we are copying the reference to the object
const parentScope = new Scope();
// eslint-disable-next-line deprecation/deprecation
parentScope.setRequestSession({ status: 'errored' });

const scope = parentScope.clone();
// eslint-disable-next-line deprecation/deprecation
const requestSession = scope.getRequestSession();
if (requestSession) {
requestSession.status = 'ok';
}

// eslint-disable-next-line deprecation/deprecation
expect(parentScope.getRequestSession()).toEqual({ status: 'ok' });
// eslint-disable-next-line deprecation/deprecation
expect(scope.getRequestSession()).toEqual({ status: 'ok' });
});

Expand All @@ -316,6 +322,7 @@ describe('Scope', () => {
scope.setUser({ id: '1' });
scope.setFingerprint(['abcd']);
scope.addBreadcrumb({ message: 'test' });
// eslint-disable-next-line deprecation/deprecation
scope.setRequestSession({ status: 'ok' });
expect(scope['_extra']).toEqual({ a: 2 });
scope.clear();
Expand Down Expand Up @@ -349,6 +356,7 @@ describe('Scope', () => {
scope.setUser({ id: '1337' });
scope.setLevel('info');
scope.setFingerprint(['foo']);
// eslint-disable-next-line deprecation/deprecation
scope.setRequestSession({ status: 'ok' });
});

Expand Down Expand Up @@ -453,6 +461,7 @@ describe('Scope', () => {
level: 'warning' as const,
tags: { bar: '3', baz: '4' },
user: { id: '42' },
// eslint-disable-next-line deprecation/deprecation
requestSession: { status: 'errored' as RequestSessionStatus },
propagationContext: {
traceId: '8949daf83f4a4a70bee4c1eb9ab242ed',
Expand Down
7 changes: 7 additions & 0 deletions packages/core/test/lib/sessionflusher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ describe('Session Flusher', () => {
});

test('test incrementSessionStatusCount updates the internal SessionFlusher state', () => {
// eslint-disable-next-line deprecation/deprecation
const flusher = new SessionFlusher(mockClient, { release: '1.0.0', environment: 'dev' });

const date = new Date('2021-04-08T12:18:23.043Z');
Expand All @@ -42,6 +43,7 @@ describe('Session Flusher', () => {
});

test('test undefined attributes are excluded, on incrementSessionStatusCount call', () => {
// eslint-disable-next-line deprecation/deprecation
const flusher = new SessionFlusher(mockClient, { release: '1.0.0' });

const date = new Date('2021-04-08T12:18:23.043Z');
Expand All @@ -55,6 +57,7 @@ describe('Session Flusher', () => {
});

test('flush is called every 60 seconds after initialisation of an instance of SessionFlusher', () => {
// eslint-disable-next-line deprecation/deprecation
const flusher = new SessionFlusher(mockClient, { release: '1.0.0', environment: 'dev' });
const flusherFlushFunc = jest.spyOn(flusher, 'flush');
jest.advanceTimersByTime(59000);
Expand All @@ -68,6 +71,7 @@ describe('Session Flusher', () => {
});

test('sendSessions is called on flush if sessions were captured', () => {
// eslint-disable-next-line deprecation/deprecation
const flusher = new SessionFlusher(mockClient, { release: '1.0.0', environment: 'dev' });
const flusherFlushFunc = jest.spyOn(flusher, 'flush');
const date = new Date('2021-04-08T12:18:23.043Z');
Expand All @@ -88,6 +92,7 @@ describe('Session Flusher', () => {
});

test('sendSessions is not called on flush if no sessions were captured', () => {
// eslint-disable-next-line deprecation/deprecation
const flusher = new SessionFlusher(mockClient, { release: '1.0.0', environment: 'dev' });
const flusherFlushFunc = jest.spyOn(flusher, 'flush');

Expand All @@ -98,12 +103,14 @@ describe('Session Flusher', () => {
});

test('calling close on SessionFlusher should disable SessionFlusher', () => {
// eslint-disable-next-line deprecation/deprecation
const flusher = new SessionFlusher(mockClient, { release: '1.0.x' });
flusher.close();
expect((flusher as any)._isEnabled).toEqual(false);
});

test('calling close on SessionFlusher will force call flush', () => {
// eslint-disable-next-line deprecation/deprecation
const flusher = new SessionFlusher(mockClient, { release: '1.0.x' });
const flusherFlushFunc = jest.spyOn(flusher, 'flush');
const date = new Date('2021-04-08T12:18:23.043Z');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns

const client = getClient<NodeClient>();
if (client && client.getOptions().autoSessionTracking) {
// eslint-disable-next-line deprecation/deprecation
isolationScope.setRequestSession({ status: 'ok' });
}

Expand Down
1 change: 1 addition & 0 deletions packages/node/src/integrations/tracing/express.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ export function expressErrorHandler(options?: ExpressHandlerOptions): ExpressMid
// running in SessionAggregates mode
const isSessionAggregatesMode = client['_sessionFlusher'] !== undefined;
if (isSessionAggregatesMode) {
// eslint-disable-next-line deprecation/deprecation
const requestSession = getIsolationScope().getRequestSession();
// If an error bubbles to the `errorHandler`, then this is an unhandled error, and should be reported as a
// Crashed session. The `_requestSession.status` is checked to ensure that this error is happening within
Expand Down
1 change: 1 addition & 0 deletions packages/node/src/sdk/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ function _init(

logger.log(`Running in ${isCjs() ? 'CommonJS' : 'ESM'} mode.`);

// TODO(V9): Remove this code since all of the logic should be in an integration
if (options.autoSessionTracking) {
startSessionTracking();
}
Expand Down
Loading

0 comments on commit 3442ff7

Please sign in to comment.