Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Updating SLA-Breached evaluation periods #1398

Merged
merged 2 commits into from
Apr 22, 2024
Merged

fix: Updating SLA-Breached evaluation periods #1398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
64a8e2d
fix: Updating SLA-Breached evaluation periods
truggeriaws Apr 22, 2024
7af2366
chore: self mutation
invalid-email-address Apr 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/__tests__/__snapshots__/construct-hub.test.ts.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/__tests__/devapp/__snapshots__/snapshot.test.ts.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions src/package-sources/npmjs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import { S3StorageFactory } from '../s3/storage';
/**
* The periodicity at which the NpmJs follower will run. This MUST be a valid
* CloudWatch Metric grain, as this will also be the period of the CloudWatch
* alarm that montiors the health of the follower.
* alarm that monitors the health of the follower.
*/
const FOLLOWER_RUN_RATE = Duration.minutes(5);

Expand Down Expand Up @@ -552,7 +552,7 @@ export class NpmJs implements IPackageSource {
// Finally - the "not running" alarm depends on the schedule (it won't run until the schedule
// exists!), and the schedule depends on the failure alarm existing (we don't want it to run
// before we can know it is failing). This means the returned `IDependable` effectively ensures
// all alarms have been provisionned already! Isn't it nice!
// all alarms have been provisioned already! Isn't it nice!
notRunningAlarm.node.addDependency(schedule);
schedule.node.addDependency(failureAlarm);
}
Expand Down Expand Up @@ -581,7 +581,7 @@ export class NpmJs implements IPackageSource {
// able to register new canary package versions within the SLA. In such cases, there is
// nothing that can be done except for waiting until the replica has finally caught up. We
// hence suppress the alarm if the replica lag is getting within 3 evaluation periods of the
// visbility SLA.
// visibility SLA.
expression: `IF(FILL(mLag, REPEAT) < ${Math.max(
visibilitySla.toSeconds() - 3 * period.toSeconds(),
3 * period.toSeconds()
Expand All @@ -599,7 +599,7 @@ export class NpmJs implements IPackageSource {
`Runbook: ${RUNBOOK_URL}`,
].join('\n'),
comparisonOperator: ComparisonOperator.GREATER_THAN_THRESHOLD,
evaluationPeriods: 2,
evaluationPeriods: 4,
Copy link

Choose a reason for hiding this comment

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

In order to make a similar functional change, I would just have to update this parameter, correct? Everything else is just computer-updated or typo fixes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's correct. There's lots of snapshot tests in the test suite - this was my attempt to update them; although, I think those changes were actually unnecessary.

treatMissingData: TreatMissingData.NOT_BREACHING,
threshold: visibilitySla.toSeconds(),
});
Expand Down