Skip to content

Add log link to alarms - #2943

Open
aug24 wants to merge 1 commit into
mainfrom
add-log-link-to-alarms
Open

Add log link to alarms#2943
aug24 wants to merge 1 commit into
mainfrom
add-log-link-to-alarms

Conversation

@aug24

@aug24 aug24 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

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

  • I have listed any breaking changes, along with a migration path 1
  • I have updated the documentation as required for the described changes 2

Footnotes

  1. 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.

  2. 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?

@aug24
aug24 force-pushed the add-log-link-to-alarms branch from 38879d1 to 4baf3fb Compare August 3, 2026 08:11
@changeset-bot

changeset-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 32217c3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@aug24 aug24 added enhancement New feature or request feature Departmental tracking: work on a new feature labels Aug 3, 2026
@aug24
aug24 force-pushed the add-log-link-to-alarms branch from 4baf3fb to 281a44a Compare August 3, 2026 08:36
@aug24
aug24 requested a review from Copilot August 3, 2026 08:43
@aug24
aug24 force-pushed the add-log-link-to-alarms branch from 281a44a to 01a2de8 Compare August 3, 2026 08:46
@aug24

aug24 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 / logSpace optional 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 GuLambdaErrorPercentageAlarm to 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

  • logsLink interpolates space/app/stack/stage directly 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 existing lambda-alarms.test.ts cases only cover the default/custom description without any link. Adding assertions for logLink and logSpace would prevent regressions in the generated AlarmDescription.
    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.

Comment thread src/constructs/cloudwatch/lambda-alarms.ts Outdated
Comment thread src/constructs/cloudwatch/alarm.ts Outdated
@aug24
aug24 force-pushed the add-log-link-to-alarms branch 5 times, most recently from 9289912 to 70a2921 Compare August 7, 2026 08:32
@aug24
aug24 marked this pull request as ready for review August 7, 2026 08:32
@aug24
aug24 requested a review from a team as a code owner August 7, 2026 08:32
Comment thread src/constructs/cloudwatch/alarm.ts Outdated
Comment thread src/constructs/cloudwatch/lambda-alarms.ts Outdated
@aug24
aug24 force-pushed the add-log-link-to-alarms branch 2 times, most recently from 7cbf419 to 7bf6396 Compare August 7, 2026 10:32
}

export class GuAlarmCta {
ctaLinks: string[];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes more sense if it creates zero or more links.

@aug24
aug24 force-pushed the add-log-link-to-alarms branch from 7bf6396 to 32217c3 Compare August 7, 2026 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request feature Departmental tracking: work on a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants