-
Notifications
You must be signed in to change notification settings - Fork 7
[feat] Add synthetics plugin #153
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
base: master
Are you sure you want to change the base?
Conversation
98f973d
to
d0b4490
Compare
f66613e
to
7914418
Compare
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.
Pull Request Overview
This PR introduces a new Synthetics plugin that enables the local development server for build-time testing and integrates with datadog-ci for build status reporting.
- Added Synthetics plugin implementation across various bundler plugins.
- Updated exports and factory configuration to include SyntheticsTypes.
- Introduced plugin validation, routes for build status and server shutdown, and comprehensive test coverage.
Reviewed Changes
Copilot reviewed 16 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
packages/published/webpack-plugin/src/index.ts | Added SyntheticsTypes export in the plugin. |
packages/published/vite-plugin/src/index.ts | Added SyntheticsTypes export in the plugin. |
packages/published/rspack-plugin/src/index.ts | Added SyntheticsTypes export in the plugin. |
packages/published/rollup-plugin/src/index.ts | Added SyntheticsTypes export in the plugin. |
packages/published/esbuild-plugin/src/index.ts | Added SyntheticsTypes export in the plugin. |
packages/plugins/synthetics/src/{validate,types,index,constants}.ts | Introduced core implementation for the new Synthetics plugin. |
packages/plugins/synthetics/src/index.test.ts | Added tests validating server behavior and plugin initialization. |
packages/plugins/synthetics/README.md | Provided documentation for the new Synthetics plugin. |
packages/factory/src/index.ts | Updated factory exports and configuration to include the Synthetics plugin. |
packages/core/src/types.ts | Extended core types to support configuration for the Synthetics plugin. |
README.md | Updated root documentation to include Synthetics plugin guidelines. |
Files not reviewed (4)
- .github/CODEOWNERS: Language not supported
- packages/factory/package.json: Language not supported
- packages/plugins/synthetics/package.json: Language not supported
- packages/plugins/synthetics/tsconfig.json: Language not supported
Comments suppressed due to low confidence (1)
packages/plugins/synthetics/src/index.ts:66
- [nitpick] Multiple server shutdown calls (server.close, server.closeAllConnections, server.closeIdleConnections) occur in immediate succession, which may lead to unexpected behavior. Consider consolidating these calls or adding checks to ensure that the server shutdown process is idempotent.
server.close();
.github/CODEOWNERS
Outdated
@@ -29,3 +29,6 @@ packages/plugins/analytics @yoannmoin | |||
|
|||
# Custom Hooks | |||
packages/plugins/custom-hooks @yoannmoinet | |||
|
|||
# Synthetics | |||
packages/plugins/synthetics @yoannmoinet @etnbrd |
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.
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.
Good catch.
I'll wait for his review, see if I should instead maybe use a team (cc @etnbrd)
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.
You should definitely use the @DataDog/synthetics-ct team, indeed.
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.
LGTM but i'm not versed in s8s
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.
LGTM, left a few questions, but nothing blocking.
Putting this on hold for now. We'll redesign a bit the feature to have the server part running on While testing with Instead, we'll have |
Stacked on
Related to
What and why?
Add a Synthetics plugin.
It will be used in collaboration with
datadog-ci
for a new "Build and test" workflow:datadog-ci synthetics build-and-test
(name TBD).datadog-ci
will run the build command of the project passingBUILD_PLUGINS_S8S_PORT
.build-plugins
will recogniseBUILD_PLUGINS_S8S_PORT
and will run a local dev server over theoutDir
of the build, with a small api/_datadog-ci_/build-status
fordatadog-ci
to follow the build progression.datadog-ci
opens a tunnel.datadog-ci
trigger a test batch using the local assets from the build through theresourceUrlSubstitutionRegexes
configuration.More details
The local server offers two routes (in addition to the file server over the out directory):
/_datadog-ci_/build-status
responding with:/_datadog-ci_/kill
kills the server.TODO
s8s
.