-
Notifications
You must be signed in to change notification settings - Fork 40
Add Capture & Replay dashboard with dynamic stage #1322
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
Add Capture & Replay dashboard with dynamic stage #1322
Conversation
Signed-off-by: Jugal Chauhan <[email protected]>
PR is still under 'work in progress' |
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.
Check my comments, I mentioned some ways to minimize the code here and keep a consistent pattern
deployment/cdk/opensearch-service-migration/lib/dashboards/capture-replay-dashboard.ts
Outdated
Show resolved
Hide resolved
...opensearch-service-migration/lib/dashboards/migrationassistant-capture-replay-dashboard.json
Outdated
Show resolved
Hide resolved
deployment/cdk/opensearch-service-migration/lib/migration-assistance-stack.ts
Outdated
Show resolved
Hide resolved
deployment/cdk/opensearch-service-migration/lib/migration-assistance-stack.ts
Outdated
Show resolved
Hide resolved
deployment/cdk/opensearch-service-migration/lib/dashboards/capture-replay-dashboard.ts
Outdated
Show resolved
Hide resolved
deployment/cdk/opensearch-service-migration/lib/dashboards/capture-replay-dashboard.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Jugal Chauhan <[email protected]>
…/jugal-chauhan/opensearch-migrations into feature/capture-replay-dashboard
Signed-off-by: Jugal Chauhan <[email protected]>
Signed-off-by: Jugal Chauhan <[email protected]>
…rd' into feature/capture-replay-dashboard
664a58f
to
daeaf92
Compare
daeaf92
to
2c5e785
Compare
2c5e785
to
22ee56b
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1322 +/- ##
============================================
+ Coverage 79.45% 79.91% +0.46%
+ Complexity 3224 2977 -247
============================================
Files 459 443 -16
Lines 16837 16217 -620
Branches 1109 1087 -22
============================================
- Hits 13377 12960 -417
+ Misses 2790 2617 -173
+ Partials 670 640 -30
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Jugal Chauhan <[email protected]>
22ee56b
to
7b50f93
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.
Thanks for simplifying this, added a couple more comments for things I noticed
...opensearch-service-migration/lib/components/migrationassistant-capture-replay-dashboard.json
Outdated
Show resolved
Hide resolved
...opensearch-service-migration/lib/components/migrationassistant-capture-replay-dashboard.json
Outdated
Show resolved
Hide resolved
deployment/cdk/opensearch-service-migration/lib/service-stacks/traffic-replayer-stack.ts
Outdated
Show resolved
Hide resolved
deployment/cdk/opensearch-service-migration/lib/service-stacks/traffic-replayer-stack.ts
Outdated
Show resolved
Hide resolved
...opensearch-service-migration/lib/components/migrationassistant-capture-replay-dashboard.json
Outdated
Show resolved
Hide resolved
Signed-off-by: Jugal Chauhan <[email protected]>
07068af
to
bf53f8e
Compare
Signed-off-by: Jugal Chauhan <[email protected]>
Signed-off-by: Jugal Chauhan <[email protected]>
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 see a couple checks are failing, one for CDK tests and one for Jenkins deployment. The fix I mentioned should help with Jenkins deployment but CDK tests will probably need to be updated
...opensearch-service-migration/lib/components/migrationassistant-capture-replay-dashboard.json
Show resolved
Hide resolved
deployment/cdk/opensearch-service-migration/lib/service-stacks/reindex-from-snapshot-stack.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Jugal Chauhan <[email protected]>
99a315d
to
5c766a3
Compare
5c766a3
to
91bb697
Compare
Signed-off-by: Jugal Chauhan <[email protected]>
91bb697
to
7fad58b
Compare
Signed-off-by: Jugal Chauhan <[email protected]>
94753ce
to
7f8b731
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.
LGTM, thanks for all the changes!
Description
This PR adds a new CaptureReplayDashboard class in capture-replay-dashboard.ts to encapsulate CloudWatch dashboard logic. It enables dynamic loading of dashboard configurations from migrationassistant-capture-replay-dashboard.json, allowing modifications without code changes. It implements stage parameter injection to filter metrics based on the deployment stage, defaulting to 'dev'. It introduces error handling to ensure a fallback to a default configuration if the JSON file is missing or invalid. Additionally, it updates migration-assistance-stack.ts to deploy the dashboard conditionally based on the trafficReplayerServiceEnabled flag in cdk.context.json, supporting more flexible deployment scenarios.
Issues Resolved
MIGRATIONS-2133
Testing
Work in Progress
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.