Skip to content

feat(tracing): Expose cancelIdleTimeout and add option to make it permanent #7236

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 13 commits into from
Mar 7, 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
46 changes: 35 additions & 11 deletions packages/core/src/tracing/idletransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ export class IdleTransaction extends Transaction {
// We should not use heartbeat if we finished a transaction
private _finished: boolean = false;

// Idle timeout was canceled and we should finish the transaction with the last span end.
private _idleTimeoutCanceledPermanently: boolean = false;

private readonly _beforeFinishCallbacks: BeforeFinishCallback[] = [];

/**
Expand Down Expand Up @@ -104,7 +107,7 @@ export class IdleTransaction extends Transaction {
_idleHub.configureScope(scope => scope.setSpan(this));
}

this._startIdleTimeout();
this._restartIdleTimeout();
setTimeout(() => {
if (!this._finished) {
this.setStatus('deadline_exceeded');
Expand Down Expand Up @@ -203,20 +206,37 @@ export class IdleTransaction extends Transaction {
}

/**
* Cancels the existing idletimeout, if there is one
* Cancels the existing idle timeout, if there is one.
* @param restartOnChildSpanChange Default is `true`.
* If set to false the transaction will end
* with the last child span.
*/
private _cancelIdleTimeout(): void {
public cancelIdleTimeout(
endTimestamp?: Parameters<IdleTransaction['finish']>[0],
{
restartOnChildSpanChange,
}: {
restartOnChildSpanChange?: boolean;
} = {
restartOnChildSpanChange: true,
},
): void {
if (this._idleTimeoutID) {
clearTimeout(this._idleTimeoutID);
this._idleTimeoutID = undefined;
this._idleTimeoutCanceledPermanently = restartOnChildSpanChange === false;

if (Object.keys(this.activities).length === 0 && this._idleTimeoutCanceledPermanently) {
this.finish(endTimestamp);
}
}
}

/**
* Creates an idletimeout
* Restarts idle timeout, if there is no running idle timeout it will start one.
*/
private _startIdleTimeout(endTimestamp?: Parameters<IdleTransaction['finish']>[0]): void {
this._cancelIdleTimeout();
private _restartIdleTimeout(endTimestamp?: Parameters<IdleTransaction['finish']>[0]): void {
this.cancelIdleTimeout();
this._idleTimeoutID = setTimeout(() => {
if (!this._finished && Object.keys(this.activities).length === 0) {
this.finish(endTimestamp);
Expand All @@ -229,7 +249,7 @@ export class IdleTransaction extends Transaction {
* @param spanId The span id that represents the activity
*/
private _pushActivity(spanId: string): void {
this._cancelIdleTimeout();
this.cancelIdleTimeout();
__DEBUG_BUILD__ && logger.log(`[Tracing] pushActivity: ${spanId}`);
this.activities[spanId] = true;
__DEBUG_BUILD__ && logger.log('[Tracing] new activities count', Object.keys(this.activities).length);
Expand All @@ -248,10 +268,14 @@ export class IdleTransaction extends Transaction {
}

if (Object.keys(this.activities).length === 0) {
// We need to add the timeout here to have the real endtimestamp of the transaction
// Remember timestampWithMs is in seconds, timeout is in ms
const endTimestamp = timestampWithMs() + this._idleTimeout / 1000;
this._startIdleTimeout(endTimestamp);
const endTimestamp = timestampWithMs();
if (this._idleTimeoutCanceledPermanently) {
this.finish(endTimestamp);
} else {
// We need to add the timeout here to have the real endtimestamp of the transaction
// Remember timestampWithMs is in seconds, timeout is in ms
this._restartIdleTimeout(endTimestamp + this._idleTimeout / 1000);
}
}
}

Expand Down
39 changes: 39 additions & 0 deletions packages/tracing/test/idletransaction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,45 @@ describe('IdleTransaction', () => {
});
});

describe('cancelIdleTimeout', () => {
it('permanent idle timeout cancel finishes transaction if there are no activities', () => {
const idleTimeout = 10;
const transaction = new IdleTransaction({ name: 'foo', startTimestamp: 1234 }, hub, idleTimeout);
transaction.initSpanRecorder(10);

const span = transaction.startChild({});
span.finish();

jest.advanceTimersByTime(2);

transaction.cancelIdleTimeout(undefined, { restartOnChildSpanChange: false });

expect(transaction.endTimestamp).toBeDefined();
});

it('default idle cancel timeout is restarted by child span change', () => {
const idleTimeout = 10;
const transaction = new IdleTransaction({ name: 'foo', startTimestamp: 1234 }, hub, idleTimeout);
transaction.initSpanRecorder(10);

const span = transaction.startChild({});
span.finish();

jest.advanceTimersByTime(2);

transaction.cancelIdleTimeout();

const span2 = transaction.startChild({});
span2.finish();

jest.advanceTimersByTime(8);
expect(transaction.endTimestamp).toBeUndefined();

jest.advanceTimersByTime(2);
expect(transaction.endTimestamp).toBeDefined();
});
});

describe('heartbeat', () => {
it('does not mark transaction as `DeadlineExceeded` if idle timeout has not been reached', () => {
// 20s to exceed 3 heartbeats
Expand Down