Add lambda invocation failure alarm #244
Conversation
| }).createAlarm(this, "FailedInvocationAlarm", { | ||
| alarmDescription: `Invocation for '${this.lambdaFunctionName}' failed. ${props?.appendToAlarmDescription ?? ""}`, |
There was a problem hiding this comment.
Fjernet eksplisitt alarmName her. For å unngå collisions hvis man lager flere av samme construct or alarm.
|
|
||
| export interface LambdaAlarmsProps { | ||
| actions: cloudwatch.IAlarmAction[] | ||
| lambdaFunctionName: string |
There was a problem hiding this comment.
Endret til å ta inn bare en lambda name som string i stedet for lambda.Function. Unngår imports her og lettere å skrive test-kode.
There was a problem hiding this comment.
Har lurt litt på denne, og tror vi bør holde det til å sende inn selve funksjonen.
- Dersom vi senere introduserer andre alarmer/metrikker som ser på andre attributter i en lambda vil vi være nødt til å dytte ut en breaking change til alle konsumenter hvor vi endrer signaturen fra string (funksjonsnavn) til objekt (funksjon)
- Vi mister den eksplisitte CloudFormation-avhengigheten mellom metrikken og lambdaen. Det er ikke et problem i akkurat dette tilfellet, siden metrikken kan opprettes uavhengig, men det hadde vært kjekt å ha avhengigheten definert uansett
Så er ikke krise, men tror vi bør gjøre det for å ruste oss mot endringer
| import * as cloudwatch from "aws-cdk-lib/aws-cloudwatch" | ||
|
|
||
| export interface LambdaAlarmsProps { | ||
| actions: cloudwatch.IAlarmAction[] |
There was a problem hiding this comment.
Den er en array, for i DTP har vi sendt samme alarm til fler actions, både Slack og OpsGenie.
Kan alternativt vært en enkelt IAlarmAction og så instansiert LambdaAlarms to ganger og lagd 2 Alarms i DTP, spesielt nå som jeg fjernet eksplisitt alarmName.
There was a problem hiding this comment.
Tror det er enklest å holde det til en IAlarmAction for enkelhetens skyld, spesielt nå som det å opprette flere ikke er et problem etter fjerning av alarmName
krissrex
force-pushed
the
feat/kre/lambda-alarms
branch
from
e0e4da0
to
df6aa2c
Compare
| import * as cloudwatch from "aws-cdk-lib/aws-cloudwatch" | ||
|
|
||
| export interface LambdaAlarmsProps { | ||
| actions: cloudwatch.IAlarmAction[] |
There was a problem hiding this comment.
Tror det er enklest å holde det til en IAlarmAction for enkelhetens skyld, spesielt nå som det å opprette flere ikke er et problem etter fjerning av alarmName
|
|
||
| export interface LambdaAlarmsProps { | ||
| actions: cloudwatch.IAlarmAction[] | ||
| lambdaFunctionName: string |
There was a problem hiding this comment.
Har lurt litt på denne, og tror vi bør holde det til å sende inn selve funksjonen.
- Dersom vi senere introduserer andre alarmer/metrikker som ser på andre attributter i en lambda vil vi være nødt til å dytte ut en breaking change til alle konsumenter hvor vi endrer signaturen fra string (funksjonsnavn) til objekt (funksjon)
- Vi mister den eksplisitte CloudFormation-avhengigheten mellom metrikken og lambdaen. Det er ikke et problem i akkurat dette tilfellet, siden metrikken kan opprettes uavhengig, men det hadde vært kjekt å ha avhengigheten definert uansett
Så er ikke krise, men tror vi bør gjøre det for å ruste oss mot endringer
| }).createAlarm(this, "FailedInvocationAlarm", { | ||
| alarmDescription: `Invocation for '${this.lambdaFunctionName}' failed. ${props?.appendToAlarmDescription ?? ""}`, |
| metricName: "Errors", | ||
| namespace: "AWS/Lambda", | ||
| statistic: "Sum", | ||
| period: cdk.Duration.seconds(60), // Standard resolution metric has a minimum of 60s period |
| lambdaFunctionName: string | ||
| } | ||
|
|
||
| export class LambdaAlarms extends constructs.Construct { |
There was a problem hiding this comment.
Kan ha en docstring som oppsummerer klassens ansvar, gjerne med notis tilsvarende service-alarms om at ingen alarmer opprettes før man kaller de forskjellige metodene
Ports lambda-alarms from Team DTP.