From 90b2299f40e73b4c6ef4797752ee8506e84b1ab1 Mon Sep 17 00:00:00 2001 From: Brend Smits Date: Wed, 5 Aug 2026 10:16:37 +0200 Subject: [PATCH 1/3] fix(termination-watcher): only emit SpotInterruptionWarning metric for actual spot events The instance-termination EventBridge rule added in #5055 targets the same interruptionWarning handler as the spot-notify rule. This caused the SpotInterruptionWarning metric to be emitted for all instance shutdowns (job completions, scale-down, manual termination), inflating it 100-350x. Gate metric emission on the event detail-type being 'EC2 Spot Instance Interruption Warning'. Runner deregistration still triggers for all event types as intended. Fixes #5244 Signed-off-by: Brend Smits --- .../src/termination-warning.test.ts | 16 +++++++++++++++- .../src/termination-warning.ts | 4 +++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lambdas/functions/termination-watcher/src/termination-warning.test.ts b/lambdas/functions/termination-watcher/src/termination-warning.test.ts index e9dc4a05af..4a97de379d 100644 --- a/lambdas/functions/termination-watcher/src/termination-warning.test.ts +++ b/lambdas/functions/termination-watcher/src/termination-warning.test.ts @@ -68,7 +68,7 @@ describe('handle termination warning', () => { vi.clearAllMocks(); }); - it('should log and create an metric', async () => { + it('should log and create a metric for spot interruption events', async () => { vi.mocked(getInstances).mockResolvedValue([instance]); await handle(event, config); @@ -101,4 +101,18 @@ describe('handle termination warning', () => { expect(metricEvent).not.toHaveBeenCalled(); expect(deregisterRunner).not.toHaveBeenCalled(); }); + + it('should not emit metric for instance state-change events but still deregister', async () => { + vi.mocked(getInstances).mockResolvedValue([instance]); + + const stateChangeEvent = { + ...event, + 'detail-type': 'EC2 Instance State-change Notification' as const, + }; + + await handle(stateChangeEvent as unknown as SpotInterruptionWarning, config); + + expect(metricEvent).toHaveBeenCalledWith(instance, stateChangeEvent, undefined, expect.anything()); + expect(deregisterRunner).toHaveBeenCalledWith(instance, config); + }); }); diff --git a/lambdas/functions/termination-watcher/src/termination-warning.ts b/lambdas/functions/termination-watcher/src/termination-warning.ts index 8e5330be25..e6b34ba067 100644 --- a/lambdas/functions/termination-watcher/src/termination-warning.ts +++ b/lambdas/functions/termination-watcher/src/termination-warning.ts @@ -26,7 +26,9 @@ async function createMetricForInstances( const matchFilter = tagFilter(instance, config.tagFilters); if (matchFilter) { - metricEvent(instance, event, config.createSpotWarningMetric ? 'SpotInterruptionWarning' : undefined, logger); + const isSpotInterruption = event['detail-type'] === 'EC2 Spot Instance Interruption Warning'; + const metricName = isSpotInterruption && config.createSpotWarningMetric ? 'SpotInterruptionWarning' : undefined; + metricEvent(instance, event, metricName, logger); await deregisterRunner(instance, config); } else { logger.debug( From cb3c690f7139945d038b036dfc9049fb1b6a479b Mon Sep 17 00:00:00 2001 From: Brend Smits Date: Wed, 5 Aug 2026 10:26:09 +0200 Subject: [PATCH 2/3] fix: widen handler type to TerminationWatcherEvent union Address review feedback: - Add InstanceStateChangeEvent and TerminationWatcherEvent union to types.d.ts - Update handle() and lambda handler to accept the union type so the detail-type check is no longer a compile-time constant - Replace unsafe cast in test with a properly-shaped InstanceStateChangeEvent fixture including state-change-specific detail fields Signed-off-by: Brend Smits --- .../termination-watcher/src/lambda.ts | 7 +-- .../src/termination-warning.test.ts | 45 +++++++++++-------- .../src/termination-warning.ts | 6 +-- .../termination-watcher/src/types.d.ts | 11 +++++ 4 files changed, 43 insertions(+), 26 deletions(-) diff --git a/lambdas/functions/termination-watcher/src/lambda.ts b/lambdas/functions/termination-watcher/src/lambda.ts index eda8e8d688..cb493d12a4 100644 --- a/lambdas/functions/termination-watcher/src/lambda.ts +++ b/lambdas/functions/termination-watcher/src/lambda.ts @@ -6,15 +6,12 @@ import { Context, SQSEvent } from 'aws-lambda'; import { handle as handleTerminationWarning } from './termination-warning'; import { handle as handleTermination } from './termination'; import { handleDeregisterRetry, DeregisterRetryMessage } from './deregister'; -import { BidEvictedDetail, BidEvictedEvent, SpotInterruptionWarning, SpotTerminationDetail } from './types'; +import { BidEvictedDetail, BidEvictedEvent, TerminationWatcherEvent } from './types'; import { Config } from './ConfigResolver'; const config = new Config(); -export async function interruptionWarning( - event: SpotInterruptionWarning, - context: Context, -): Promise { +export async function interruptionWarning(event: TerminationWatcherEvent, context: Context): Promise { setContext(context, 'lambda.ts'); logger.logEventIfEnabled(event); logger.debug('Configuration of the lambda', { config }); diff --git a/lambdas/functions/termination-watcher/src/termination-warning.test.ts b/lambdas/functions/termination-watcher/src/termination-warning.test.ts index 4a97de379d..5bcdd389c4 100644 --- a/lambdas/functions/termination-watcher/src/termination-warning.test.ts +++ b/lambdas/functions/termination-watcher/src/termination-warning.test.ts @@ -2,7 +2,7 @@ import { EC2Client, Instance } from '@aws-sdk/client-ec2'; import { mockClient } from 'aws-sdk-client-mock'; import 'aws-sdk-client-mock-jest'; import { handle } from './termination-warning'; -import { SpotInterruptionWarning, SpotTerminationDetail } from './types'; +import { SpotInterruptionWarning, SpotTerminationDetail, InstanceStateChangeEvent, InstanceStateChangeDetail } from './types'; import { metricEvent } from './metric-event'; import { deregisterRunner } from './deregister'; @@ -36,7 +36,7 @@ const config = { ghesApiUrl: '', }; -const event: SpotInterruptionWarning = { +const spotEvent: SpotInterruptionWarning = { version: '0', id: '1', 'detail-type': 'EC2 Spot Instance Interruption Warning', @@ -51,8 +51,23 @@ const event: SpotInterruptionWarning = { }, }; +const stateChangeEvent: InstanceStateChangeEvent = { + version: '0', + id: '2', + 'detail-type': 'EC2 Instance State-change Notification', + source: 'aws.ec2', + account: '123456789012', + time: '2015-11-11T21:30:00Z', + region: 'us-east-1', + resources: ['arn:aws:ec2:us-east-1b:instance/i-abcd1111'], + detail: { + 'instance-id': 'i-abcd1111', + state: 'shutting-down', + }, +}; + const instance: Instance = { - InstanceId: event.detail['instance-id'], + InstanceId: 'i-abcd1111', InstanceType: 't2.micro', Tags: [ { Key: 'Name', Value: 'test-instance' }, @@ -68,28 +83,27 @@ describe('handle termination warning', () => { vi.clearAllMocks(); }); - it('should log and create a metric for spot interruption events', async () => { + it('should emit metric for spot interruption events', async () => { vi.mocked(getInstances).mockResolvedValue([instance]); - await handle(event, config); + await handle(spotEvent, config); - expect(metricEvent).toHaveBeenCalled(); - expect(metricEvent).toHaveBeenCalledWith(instance, event, 'SpotInterruptionWarning', expect.anything()); + expect(metricEvent).toHaveBeenCalledWith(instance, spotEvent, 'SpotInterruptionWarning', expect.anything()); expect(deregisterRunner).toHaveBeenCalledWith(instance, config); }); - it('should log details and not create a metric', async () => { + it('should not emit metric when createSpotWarningMetric is false', async () => { vi.mocked(getInstances).mockResolvedValue([instance]); const noMetricConfig = { ...config, createSpotWarningMetric: false }; - await handle(event, noMetricConfig); - expect(metricEvent).toHaveBeenCalledWith(instance, event, undefined, expect.anything()); + await handle(spotEvent, noMetricConfig); + expect(metricEvent).toHaveBeenCalledWith(instance, spotEvent, undefined, expect.anything()); expect(deregisterRunner).toHaveBeenCalledWith(instance, noMetricConfig); }); - it('should not create a metric if filter not matched.', async () => { + it('should not emit metric or deregister if filter not matched', async () => { vi.mocked(getInstances).mockResolvedValue([instance]); - await handle(event, { + await handle(spotEvent, { createSpotWarningMetric: true, createSpotTerminationMetric: false, tagFilters: { 'ghr:environment': '_NO_MATCH_' }, @@ -105,12 +119,7 @@ describe('handle termination warning', () => { it('should not emit metric for instance state-change events but still deregister', async () => { vi.mocked(getInstances).mockResolvedValue([instance]); - const stateChangeEvent = { - ...event, - 'detail-type': 'EC2 Instance State-change Notification' as const, - }; - - await handle(stateChangeEvent as unknown as SpotInterruptionWarning, config); + await handle(stateChangeEvent, config); expect(metricEvent).toHaveBeenCalledWith(instance, stateChangeEvent, undefined, expect.anything()); expect(deregisterRunner).toHaveBeenCalledWith(instance, config); diff --git a/lambdas/functions/termination-watcher/src/termination-warning.ts b/lambdas/functions/termination-watcher/src/termination-warning.ts index e6b34ba067..b18d890a54 100644 --- a/lambdas/functions/termination-watcher/src/termination-warning.ts +++ b/lambdas/functions/termination-watcher/src/termination-warning.ts @@ -1,5 +1,5 @@ import { createChildLogger, getTracedAWSV3Client } from '@aws-github-runner/aws-powertools-util'; -import { SpotInterruptionWarning, SpotTerminationDetail } from './types'; +import { TerminationWatcherEvent } from './types'; import { EC2Client, Instance } from '@aws-sdk/client-ec2'; import { Config } from './ConfigResolver'; import { tagFilter, getInstances } from './ec2'; @@ -8,7 +8,7 @@ import { deregisterRunner } from './deregister'; const logger = createChildLogger('termination-warning'); -async function handle(event: SpotInterruptionWarning, config: Config): Promise { +async function handle(event: TerminationWatcherEvent, config: Config): Promise { logger.debug('Received spot notification warning:', { event }); const ec2 = getTracedAWSV3Client(new EC2Client({ region: process.env.AWS_REGION })); const instances = await getInstances(ec2, [event.detail['instance-id']]); @@ -19,7 +19,7 @@ async function handle(event: SpotInterruptionWarning, con async function createMetricForInstances( instances: Instance[], - event: SpotInterruptionWarning, + event: TerminationWatcherEvent, config: Config, ): Promise { for (const instance of instances) { diff --git a/lambdas/functions/termination-watcher/src/types.d.ts b/lambdas/functions/termination-watcher/src/types.d.ts index d242221142..2409c3c26c 100644 --- a/lambdas/functions/termination-watcher/src/types.d.ts +++ b/lambdas/functions/termination-watcher/src/types.d.ts @@ -42,3 +42,14 @@ interface UserIdentity { interface ServiceEventDetails { instanceIdSet: string[]; } + +export interface InstanceStateChangeDetail { + 'instance-id': string; + state: string; +} + +// eslint-disable-next-line @typescript-eslint/no-empty-object-type +export interface InstanceStateChangeEvent + extends EventBridgeEvent<'EC2 Instance State-change Notification', InstanceStateChangeDetail> {} + +export type TerminationWatcherEvent = SpotInterruptionWarning | InstanceStateChangeEvent; From 0025a031e9190f8dda13d65f1891c52a702cd386 Mon Sep 17 00:00:00 2001 From: Brend Smits Date: Wed, 5 Aug 2026 10:42:26 +0200 Subject: [PATCH 3/3] style: fix prettier formatting and remove unused import - Remove unused InstanceStateChangeDetail import - Collapse import to single line to satisfy prettier line-length rules Signed-off-by: Brend Smits --- .../termination-watcher/src/termination-warning.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lambdas/functions/termination-watcher/src/termination-warning.test.ts b/lambdas/functions/termination-watcher/src/termination-warning.test.ts index 5bcdd389c4..0e7792107b 100644 --- a/lambdas/functions/termination-watcher/src/termination-warning.test.ts +++ b/lambdas/functions/termination-watcher/src/termination-warning.test.ts @@ -2,7 +2,7 @@ import { EC2Client, Instance } from '@aws-sdk/client-ec2'; import { mockClient } from 'aws-sdk-client-mock'; import 'aws-sdk-client-mock-jest'; import { handle } from './termination-warning'; -import { SpotInterruptionWarning, SpotTerminationDetail, InstanceStateChangeEvent, InstanceStateChangeDetail } from './types'; +import { SpotInterruptionWarning, SpotTerminationDetail, InstanceStateChangeEvent } from './types'; import { metricEvent } from './metric-event'; import { deregisterRunner } from './deregister';