Add log link to alarms - #2943
Conversation
38879d1 to
4baf3fb
Compare
|
4baf3fb to
281a44a
Compare
281a44a to
01a2de8
Compare
|
I think it might be better if this is moved to a separate class, with a properties object requiring space, app, stack, stage. Not sure. |
There was a problem hiding this comment.
Pull request overview
This PR enhances CloudWatch alarm descriptions by optionally appending a deep link to the relevant ELK (logs.gutools.co.uk) view, making it faster to jump from an alarm to the corresponding logs during incident response.
Changes:
- Adds
logLink/logSpaceoptional props to alarm configuration to support attaching a logs deep link. - Introduces a
logsLink(...)helper to generate a pre-filtered logs.gutools.co.uk discover URL. - Updates
GuLambdaErrorPercentageAlarmto append the generated/provided log link to the alarm description.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/constructs/cloudwatch/lambda-alarms.ts | Appends an optional logs deep link to Lambda error-percentage alarm descriptions. |
| src/constructs/cloudwatch/alarm.ts | Adds alarm props for log linking and introduces a helper to generate logs.gutools.co.uk discover links. |
Suppressed comments (2)
src/constructs/cloudwatch/alarm.ts:38
logsLinkinterpolatesspace/app/stack/stagedirectly into the URL/RISON payload without encoding. If any value contains reserved characters (e.g. spaces,/,#,&), the generated link can become malformed (or change meaning). Encoding the inserted values makes the link generation robust.
"_a=(columns:!(stack,stage,message,app),filters:!(",
`('$state':(store:appState),meta:(alias:!n,disabled:!f,key:stack.keyword,negate:!f,params:(query:${stack}),type:phrase),query:(match_phrase:(stack.keyword:${stack}))),`,
`('$state':(store:appState),meta:(alias:!n,disabled:!f,key:app.keyword,negate:!f,params:(query:${app}),type:phrase),query:(match_phrase:(app.keyword:${app}))),`,
src/constructs/cloudwatch/lambda-alarms.ts:39
- This introduces new behavior (appending a log deep-link to the alarm description via
logLink/logSpace), but the existinglambda-alarms.test.tscases only cover the default/custom description without any link. Adding assertions forlogLinkandlogSpacewould prevent regressions in the generatedAlarmDescription.
const logLinkValue: string =
props.logLink ??
(props.logSpace && scope.app ? logsLink(props.logSpace, scope.app, scope.stack, scope.stage) : "");
const logLink: string = logLinkValue ? `\n${logLinkValue}` : "";
const alarmDescription = props.alarmDescription ?? defaultDescription;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9289912 to
70a2921
Compare
7cbf419 to
7bf6396
Compare
| } | ||
|
|
||
| export class GuAlarmCta { | ||
| ctaLinks: string[]; |
There was a problem hiding this comment.
Makes more sense if it creates zero or more links.
7bf6396 to
32217c3
Compare
What does this change?
This adds a useful 3am feature to an alarm: a direct deep link to the relevant logging.
It either takes a complete link, or (and this should be sufficient for most purposes) the ELK space in which to search for app/stack/stage.
How to test
How can we measure success?
Have we considered potential risks?
Checklist
Footnotes
Consider whether this is something that will mean changes to projects that have already been migrated, or to the CDK CLI tool. If changes are required, consider adding a checklist here and/or linking to related PRs. ↩
If you are adding a new construct or pattern, has new documentation been added? If you are amending defaults or changing behaviour, are the existing docs still valid? ↩