Skip to content

Commit 83bfcbd

Browse files
fix(metrics): count static markExecutionAsFailed terminal failures and omit unknown durations
1 parent 8673e5c commit 83bfcbd

2 files changed

Lines changed: 53 additions & 4 deletions

File tree

apps/sim/lib/logs/execution/logging-session.test.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,7 @@ describe('completeWithError cancelled-status guard', () => {
627627
describe('LoggingSession.markExecutionAsFailed workflowId scoping', () => {
628628
beforeEach(() => {
629629
vi.clearAllMocks()
630+
dbMocks.selectLimit.mockResolvedValue([{ status: 'running', trigger: 'api' }])
630631
dbMocks.updateWhere.mockResolvedValue(undefined)
631632
})
632633

@@ -674,7 +675,7 @@ describe('LoggingSession workflow metrics', () => {
674675
loops: {},
675676
parallels: {},
676677
})
677-
dbMocks.selectLimit.mockResolvedValue([{ status: 'running' }])
678+
dbMocks.selectLimit.mockResolvedValue([{ status: 'running', trigger: 'api' }])
678679
dbMocks.execute.mockResolvedValue(undefined)
679680
})
680681

@@ -745,6 +746,8 @@ describe('LoggingSession workflow metrics', () => {
745746
it('does not double-emit when markAsFailed runs after a completed session', async () => {
746747
const session = new LoggingSession('wf-1', 'exec-1', 'api', 'req-1')
747748
await session.complete({ totalDurationMs: 500 })
749+
750+
dbMocks.selectLimit.mockResolvedValue([{ status: 'completed', trigger: 'api' }])
748751
await session.markAsFailed('timeout')
749752

750753
expect(recordExecutionCompletedMock).toHaveBeenCalledTimes(1)
@@ -769,6 +772,22 @@ describe('LoggingSession workflow metrics', () => {
769772
)
770773
})
771774

775+
it('static markExecutionAsFailed emits failed only for non-terminal rows', async () => {
776+
dbMocks.selectLimit.mockResolvedValue([{ status: 'running', trigger: 'webhook' }])
777+
await LoggingSession.markExecutionAsFailed('exec-1', 'crash', undefined, 'wf-1')
778+
779+
expect(recordExecutionCompletedMock).toHaveBeenCalledTimes(1)
780+
expect(recordExecutionCompletedMock).toHaveBeenCalledWith({
781+
trigger: 'webhook',
782+
status: 'failed',
783+
})
784+
785+
recordExecutionCompletedMock.mockClear()
786+
dbMocks.selectLimit.mockResolvedValue([{ status: 'failed', trigger: 'webhook' }])
787+
await LoggingSession.markExecutionAsFailed('exec-1', 'crash again', undefined, 'wf-1')
788+
expect(recordExecutionCompletedMock).not.toHaveBeenCalled()
789+
})
790+
772791
it('skips the completion metric when the run was already cancelled elsewhere', async () => {
773792
dbMocks.selectLimit.mockResolvedValue([{ status: 'cancelled' }])
774793
const session = new LoggingSession('wf-1', 'exec-1', 'api', 'req-1')

apps/sim/lib/logs/execution/logging-session.ts

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,10 @@ export class LoggingSession {
519519
})
520520

521521
this.completed = true
522-
this.emitExecutionCompletedMetric('failed', Math.max(1, durationMs))
522+
this.emitExecutionCompletedMetric(
523+
'failed',
524+
typeof totalDurationMs === 'number' ? Math.max(1, totalDurationMs) : undefined
525+
)
523526

524527
try {
525528
const { PlatformEvents, createOTelSpansForWorkflowExecution } = await import(
@@ -613,7 +616,10 @@ export class LoggingSession {
613616
})
614617

615618
this.completed = true
616-
this.emitExecutionCompletedMetric('cancelled', Math.max(1, durationMs))
619+
this.emitExecutionCompletedMetric(
620+
'cancelled',
621+
typeof totalDurationMs === 'number' ? Math.max(1, totalDurationMs) : undefined
622+
)
617623

618624
try {
619625
const { PlatformEvents, createOTelSpansForWorkflowExecution } = await import(
@@ -992,7 +998,10 @@ export class LoggingSession {
992998
this.requestId,
993999
this.workflowId
9941000
)
995-
this.emitExecutionCompletedMetric('failed')
1001+
// The static helper emits the failure metric when the row transitions from
1002+
// a non-terminal status; mark this session as emitted either way so a later
1003+
// in-process completion attempt can't add a second point.
1004+
this.completionMetricEmitted = true
9961005
}
9971006

9981007
static async markExecutionAsFailed(
@@ -1003,6 +1012,21 @@ export class LoggingSession {
10031012
): Promise<void> {
10041013
try {
10051014
const message = errorMessage || 'Run failed'
1015+
const current = await db
1016+
.select({
1017+
status: workflowExecutionLogs.status,
1018+
trigger: workflowExecutionLogs.trigger,
1019+
})
1020+
.from(workflowExecutionLogs)
1021+
.where(
1022+
and(
1023+
eq(workflowExecutionLogs.executionId, executionId),
1024+
eq(workflowExecutionLogs.workflowId, workflowId)
1025+
)
1026+
)
1027+
.limit(1)
1028+
.then((rows) => rows[0])
1029+
10061030
await db
10071031
.update(workflowExecutionLogs)
10081032
.set({
@@ -1029,6 +1053,12 @@ export class LoggingSession {
10291053
)
10301054
)
10311055

1056+
// Only a transition from a non-terminal status is a new terminal outcome;
1057+
// rows already completed/failed/cancelled emitted their point elsewhere.
1058+
if (current && (current.status === 'running' || current.status === 'pending')) {
1059+
workflowMetrics.recordExecutionCompleted({ trigger: current.trigger, status: 'failed' })
1060+
}
1061+
10321062
logger.info(`[${requestId || 'unknown'}] Marked execution ${executionId} as failed`)
10331063
} catch (error) {
10341064
logger.error(`Failed to mark execution ${executionId} as failed:`, {

0 commit comments

Comments
 (0)