-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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.
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
feat(synthetics): Synthetics L2 Support #8824
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have pushed a few changes, mainly to the structure, and added a few comments inline.
A few general general comments:
- Add a short section in the end which lists all of the
Canary
Cfn resource properties with a short description, this will give the reader a place to comment on their implementation. - Under future work you should add features/capabilities that might be implemented in the future. For example the idea of having a set of blueprints scripts for the canary code. This is a good opportunity to get feedback on ideas you might have, even if they will not be implemented in the initial version.
- If there are any non trivial things you have encountered while researching the
Canary
resource add them under a "Note" section, readers might have ideas to how you can tackle them. Of the top pf my head - when deleting a canary it's resources, like the lambda function are not deleted.
@@ -0,0 +1,69 @@ | |||
import '@aws-cdk/assert/jest'; | |||
import { App, Stack } from '@aws-cdk/core'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a test that verifies a user can specify a custom runtime
@@ -0,0 +1,121 @@ | |||
import * as s3 from '@aws-cdk/aws-s3'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd actually move this entire file to canary.ts
- its rather simple and avoid the clunky test-prop
file name. We'll see what to do later if it grows too large.
//Wait for page to render. | ||
//Increase or decrease wait time based on endpoint being monitored. | ||
await page.waitFor(15000); | ||
await synthetics.takeScreenshot('loaded', 'loaded'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
await synthetics.takeScreenshot('loaded', 'loaded'); | |
await synthetics.takeScreenshot('loaded', 'loaded'); // This will take a snapshot that will be included in test output artifacts |
}); | ||
``` | ||
|
||
The following is an example of an `index.js` file which exports the `handler` function, note that the function **must** be called `handler`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following is an example of an `index.js` file which exports the `handler` function, note that the function **must** be called `handler`: | |
The following is an example of an `index.js` file which exports the `handler` function: |
exports.handler = async () => { | ||
return await pageLoadBlueprint(); | ||
}; | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here you can add a note section
> the function must be named `handler` blah blah
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few comments on the README, also I'd prefer we bring back the "Future work" sections which includes the a link to the blueprint issues. Lets ship it!
To illustrate how to use a canary, assume your application defines the following endpoint: | ||
|
||
```bash | ||
% curl "https://api.example.com/user/books/topbook/" | ||
The Hitchhikers Guide to the Galaxy | ||
|
||
``` | ||
|
||
The below code defines a canary that will hit the `books/topbook` endpoint every 5 minutes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example has been adapted to show how to take a screenshot. The use case is not an API anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to show off the power of the synthetics library, as you mentioned in a previous comment. The heartbeat blueprint works for APIs as well, and takes a screenshot of the rendered HTML (in this case, would capture a screenshot of a white page with The Hitchhikers Guide to the Galaxy
). I tested this on my own API endpoint to make sure.
While the screenshot is likely more powerful for websites, it can still be a valuable addition to APIs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🦄🎉🥳
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
This PR specifies an implementation for a CloudWatch Canary. It will be implemented in milestones specified below.
Closes #7687.
Currently, this PR implements up to Milestone 2.
Milestone 1: This implementation allows customers to specify a canary with these properties:
artifactBucket: IBucket
role: IRole
timeToLive: Duration
schedule: Schedule
startAfterCreation: boolean
successRetentionPeriod: Duration
failureRetentionPeriod: Duration
canaryName: string
And:
name
for users if not specifiedMilestone 2: This is the next step needed to allow customers to specify their own canary scripts:
test:Test
Test
class with static methodTest.custom()
Code
class with static methodCode.inline()
Code.fromAsset()
nodejs/node_modules
Code.fromBucket()
Milestone 3: These are features that make a more robust Canary API:
Test.apiEndpoint()
Test
memorySize
andtimeout
README Rendered version
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license