fix(termination-watcher): only emit SpotInterruptionWarning metric for actual spot interruption events

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<SpotTerminationDetail>,
-  context: Context,
-): Promise<void> {
+export async function interruptionWarning(event: TerminationWatcherEvent, context: Context): Promise<void> {
   setContext(context, 'lambda.ts');
   logger.logEventIfEnabled(event);
   logger.debug('Configuration of the lambda', { config });

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 } from './types';
 import { metricEvent } from './metric-event';
 import { deregisterRunner } from './deregister';
 
@@ -36,7 +36,7 @@ const config = {
   ghesApiUrl: '',
 };
 
-const event: SpotInterruptionWarning<SpotTerminationDetail> = {
+const spotEvent: SpotInterruptionWarning<SpotTerminationDetail> = {
   version: '0',
   id: '1',
   'detail-type': 'EC2 Spot Instance Interruption Warning',
@@ -51,8 +51,23 @@ const event: SpotInterruptionWarning<SpotTerminationDetail> = {
   },
 };
 
+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 an metric', 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_' },
@@ -101,4 +115,13 @@ 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]);
+
+    await handle(stateChangeEvent, config);
+
+    expect(metricEvent).toHaveBeenCalledWith(instance, stateChangeEvent, undefined, expect.anything());
+    expect(deregisterRunner).toHaveBeenCalledWith(instance, config);
+  });
 });

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<SpotTerminationDetail>, config: Config): Promise<void> {
+async function handle(event: TerminationWatcherEvent, config: Config): Promise<void> {
   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,14 +19,16 @@ async function handle(event: SpotInterruptionWarning<SpotTerminationDetail>, con
 
 async function createMetricForInstances(
   instances: Instance[],
-  event: SpotInterruptionWarning<SpotTerminationDetail>,
+  event: TerminationWatcherEvent,
   config: Config,
 ): Promise<void> {
   for (const instance of instances) {
     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(

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<SpotTerminationDetail> | InstanceStateChangeEvent;