Skip to content
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

Revert "feat(node): Allow to configure maxSpanWaitDuration" #12511

Merged
merged 1 commit into from
Jun 18, 2024
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
6 changes: 1 addition & 5 deletions packages/node/src/sdk/initOtel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,7 @@ export function setupOtel(client: NodeClient): BasicTracerProvider {
}),
forceFlushTimeoutMillis: 500,
});
provider.addSpanProcessor(
new SentrySpanProcessor({
timeout: client.getOptions().maxSpanWaitDuration,
}),
);
provider.addSpanProcessor(new SentrySpanProcessor());

// Initialize the provider
provider.register({
Expand Down
11 changes: 0 additions & 11 deletions packages/node/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,6 @@ 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: 0 additions & 59 deletions packages/node/test/integration/transactions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -633,63 +633,4 @@ 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: 2 additions & 5 deletions packages/opentelemetry/src/spanExporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,16 @@ 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(options?: { timeout?: number }) {
public constructor() {
this._finishedSpans = [];
this._timeout = options?.timeout || DEFAULT_TIMEOUT;
}

/** Export a single span. */
Expand Down Expand Up @@ -106,7 +103,7 @@ export class SentrySpanExporter {
*/
private _cleanupOldSpans(spans = this._finishedSpans): void {
this._finishedSpans = spans.filter(span => {
const shouldDrop = shouldCleanupSpan(span, this._timeout);
const shouldDrop = shouldCleanupSpan(span, 5 * 60);
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(options?: { timeout?: number }) {
public constructor() {
setIsSetup('SentrySpanProcessor');
this._exporter = new SentrySpanExporter(options);
this._exporter = new SentrySpanExporter();
}

/**
Expand Down
Loading