Skip to content

Commit

Permalink
feat(node): Allow to configure maxSpanWaitDuration (#12492)
Browse files Browse the repository at this point in the history
This adds a new config option for `@sentry/node`, `maxSpanWaitDuration`.

By configuring this, you can define how long the SDK waits for a
finished spans parent span to be finished before we discard this.

Today, this defaults to 5 min, so if a span is finished and its parent
span is not finished within 5 minutes, the child span will be discarded.
We do this to avoid memory leaks, as parent spans may be dropped/lost
and thus child spans would be kept in memory forever.

However, if you actually have very long running spans, they may be
incorrectly dropped due to this. So now, you can configure your SDK to
have a longer wait duration.

Fixes #12491
  • Loading branch information
mydea authored Jun 17, 2024
1 parent 1bb86db commit 1b6c22e
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 5 deletions.
6 changes: 5 additions & 1 deletion packages/node/src/sdk/initOtel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,11 @@ export function setupOtel(client: NodeClient): BasicTracerProvider {
}),
forceFlushTimeoutMillis: 500,
});
provider.addSpanProcessor(new SentrySpanProcessor());
provider.addSpanProcessor(
new SentrySpanProcessor({
timeout: client.getOptions().maxSpanWaitDuration,
}),
);

// Initialize the provider
provider.register({
Expand Down
11 changes: 11 additions & 0 deletions packages/node/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,17 @@ export interface BaseNodeOptions {
*/
skipOpenTelemetrySetup?: boolean;

/**
* The max. duration in seconds that the SDK will wait for parent spans to be finished before discarding a span.
* The SDK will automatically clean up spans that have no finished parent after this duration.
* This is necessary to prevent memory leaks in case of parent spans that are never finished or otherwise dropped/missing.
* However, if you have very long-running spans in your application, a shorter duration might cause spans to be discarded too early.
* In this case, you can increase this duration to a value that fits your expected data.
*
* Defaults to 300 seconds (5 minutes).
*/
maxSpanWaitDuration?: number;

/** Callback that is executed when a fatal global error occurs. */
onFatalError?(this: void, error: Error): void;
}
Expand Down
59 changes: 59 additions & 0 deletions packages/node/test/integration/transactions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -633,4 +633,63 @@ describe('Integration | Transactions', () => {
]),
);
});

it('allows to configure `maxSpanWaitDuration` to capture long running spans', async () => {
const transactions: TransactionEvent[] = [];
const beforeSendTransaction = jest.fn(event => {
transactions.push(event);
return null;
});

const now = Date.now();
jest.useFakeTimers();
jest.setSystemTime(now);

const logs: unknown[] = [];
jest.spyOn(logger, 'log').mockImplementation(msg => logs.push(msg));

mockSdkInit({
enableTracing: true,
beforeSendTransaction,
maxSpanWaitDuration: 100 * 60,
});

Sentry.startSpanManual({ name: 'test name' }, rootSpan => {
const subSpan = Sentry.startInactiveSpan({ name: 'inner span 1' });
subSpan.end();

Sentry.startSpanManual({ name: 'inner span 2' }, innerSpan => {
// Child span ends after 10 min
setTimeout(
() => {
innerSpan.end();
},
10 * 60 * 1_000,
);
});

// root span ends after 99 min
setTimeout(
() => {
rootSpan.end();
},
99 * 10 * 1_000,
);
});

// Now wait for 100 mins
jest.advanceTimersByTime(100 * 60 * 1_000);

expect(beforeSendTransaction).toHaveBeenCalledTimes(1);
expect(transactions).toHaveLength(1);
const transaction = transactions[0]!;

expect(transaction.transaction).toEqual('test name');
const spans = transaction.spans || [];

expect(spans).toHaveLength(2);

expect(spans[0]!.description).toEqual('inner span 1');
expect(spans[1]!.description).toEqual('inner span 2');
});
});
7 changes: 5 additions & 2 deletions packages/opentelemetry/src/spanExporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,19 @@ import { parseSpanDescription } from './utils/parseSpanDescription';
type SpanNodeCompleted = SpanNode & { span: ReadableSpan };

const MAX_SPAN_COUNT = 1000;
const DEFAULT_TIMEOUT = 300; // 5 min

/**
* A Sentry-specific exporter that converts OpenTelemetry Spans to Sentry Spans & Transactions.
*/
export class SentrySpanExporter {
private _flushTimeout: ReturnType<typeof setTimeout> | undefined;
private _finishedSpans: ReadableSpan[];
private _timeout: number;

public constructor() {
public constructor(options?: { timeout?: number }) {
this._finishedSpans = [];
this._timeout = options?.timeout || DEFAULT_TIMEOUT;
}

/** Export a single span. */
Expand Down Expand Up @@ -103,7 +106,7 @@ export class SentrySpanExporter {
*/
private _cleanupOldSpans(spans = this._finishedSpans): void {
this._finishedSpans = spans.filter(span => {
const shouldDrop = shouldCleanupSpan(span, 5 * 60);
const shouldDrop = shouldCleanupSpan(span, this._timeout);
DEBUG_BUILD &&
shouldDrop &&
logger.log(
Expand Down
4 changes: 2 additions & 2 deletions packages/opentelemetry/src/spanProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ function onSpanEnd(span: Span): void {
export class SentrySpanProcessor implements SpanProcessorInterface {
private _exporter: SentrySpanExporter;

public constructor() {
public constructor(options?: { timeout?: number }) {
setIsSetup('SentrySpanProcessor');
this._exporter = new SentrySpanExporter();
this._exporter = new SentrySpanExporter(options);
}

/**
Expand Down

0 comments on commit 1b6c22e

Please sign in to comment.