feat: aws integration #6954
Merged
feat: aws integration #6954
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to b8311bc in 1 minute and 51 seconds
More details
- Looked at
5093lines of code in60files - Skipped
6files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/hooks/integrations/aws/useAccountStatus.ts:18
- Draft comment:
Avoid using console.log for debugging in production code. Consider using a proper logging mechanism or remove it before deploying. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_TcDJP2aWNBJ44C8K
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
YounixM
changed the title
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 7698725 in 44 seconds
More details
- Looked at
12lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. frontend/src/container/AppLayout/index.tsx:433
- Draft comment:
Ensure thatisInfraMonitoring()correctly encompasses all cases previously covered byisInfraMonitoringHosts(). If not, this might lead to layout issues. - Reason this comment was not posted:
Comment did not seem useful.
2. frontend/src/container/AppLayout/index.tsx:433
- Draft comment:
Avoid using inline styles. Consider using external stylesheets, CSS classes, or styled components instead. This is also applicable to lines 434-439. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_6SUXk6gw9fPlorzM
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on fbb67e8 in 26 seconds
More details
- Looked at
40lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/pages/Integrations/IntegrationsList.tsx:45
- Draft comment:
The AWS integration is commented out, which is intentional as per the comment. Ensure to uncomment it once the backend changes are finalized. - Reason this comment was not posted:
Confidence changes required:20%
The PR introduces a new AWS integration feature but comments out its inclusion in the list. This is intentional as per the comment, so no changes are needed here. However, the PR does not address the high CPU usage issue mentioned in the linked issue.
Workflow ID: wflow_TIXbcoh5Y16eH8Bk
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
frontend/src/container/CloudIntegrationPage/ServicesSection/ServicesTabs.tsx
Outdated
Show resolved
Hide resolved
frontend/src/container/CloudIntegrationPage/ServicesSection/data.ts
Outdated
Show resolved
Hide resolved
SagarRajput-7
previously approved these changes
Jan 29, 2025
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 3a5351b in 26 seconds
More details
- Looked at
226lines of code in2files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_jnW4LU13vY2pqzZ1
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 929f8bb in 43 seconds
More details
- Looked at
46lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. frontend/src/container/CloudIntegrationPage/ServicesSection/ServicesTabs.tsx:80
- Draft comment:
Ensure thataccountIdis correctly retrieved from the URL query parameters. This change affects howaccountIdis passed toServicesFilterandServicesList. Double-check that these components handle theaccountIdcorrectly. - Reason this comment was not posted:
Comment did not seem useful.
2. frontend/src/container/CloudIntegrationPage/ServicesSection/ServicesTabs.tsx:75
- Draft comment:
Avoid using inline styles. Instead, use CSS classes or styled components for styling. Also, avoid using hardcoded color values. Use design tokens or predefined color constants instead. This applies to lines 67 and 71 as well. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_rbHmsgLFy91nDQ4r
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
ahmadshaheer
force-pushed
the
feat/aws-integration
branch
from
929f8bb to
5cefa40
Compare
amlannandy
previously approved these changes
Jan 29, 2025
frontend/src/container/CloudIntegrationPage/HeroSection/components/IntegrateNowFormSections.tsx
Outdated
Show resolved
Hide resolved
frontend/src/container/CloudIntegrationPage/HeroSection/components/IntegrateNowFormSections.tsx
Show resolved
Hide resolved
SagarRajput-7
previously approved these changes
Jan 29, 2025
ahmadshaheer
force-pushed
the
feat/aws-integration
branch
from
5cefa40 to
063039c
Compare
ahmadshaheer
dismissed stale reviews from amlannandy and SagarRajput-7
via
31f0bf7
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 31f0bf7 in 45 seconds
More details
- Looked at
19lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. frontend/src/types/api/integrations/types.ts:92
- Draft comment:
Avoid using 'any' type for 'config'. Consider defining a specific type or interface for better type safety. - Reason this comment was not posted:
Marked as duplicate.
2. frontend/src/types/api/integrations/types.ts:105
- Draft comment:
Avoid using 'any' type for 'data'. Consider defining a specific type or interface for better type safety. - Reason this comment was not posted:
Marked as duplicate.
3. frontend/src/types/api/integrations/types.ts:92
- Draft comment:
Avoid usinganytype forconfig. Consider defining a specific type or interface for better type safety. This is also applicable at line 105. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The use ofanydoes reduce type safety and makes the code more error-prone. However, looking at the context, these interfaces are for integration configuration and response data, which could vary significantly between different integrations. The eslint-disable comments suggest the developers are aware of the tradeoff and choseanydeliberately. Without seeing the actual integration implementations, we can't suggest a more specific type.
I might be too lenient on acceptinganytypes. Type safety is important and there might be a way to define union types or generic constraints that would be more type-safe while still being flexible.
While better typing would be ideal, suggesting it without knowing the full range of possible integration configs and response data could lead to overly restrictive or incorrect types. The explicit eslint-disable comments show this was a conscious decision.
Delete the comment. While the suggestion for better typing is generally good practice, we don't have enough context about the integration system to suggest specific types, and the current implementation explicitly acknowledges the use of any.
Workflow ID: wflow_pGv9Rk93NZnv12CP
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
ahmadshaheer
force-pushed
the
feat/aws-integration
branch
from
31f0bf7 to
c1780f0
Compare
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on c1780f0 in 1 minute and 1 seconds
More details
- Looked at
220lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/container/CloudIntegrationPage/HeroSection/components/IntegrateNowFormSections.tsx:34
- Draft comment:
Avoid using inline styles. Use external stylesheets, CSS classes, or styled components instead. This applies to thestyleattribute on line 34 and elsewhere in this file. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_tcbSXn2BVzvky1CQ
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 72a4f44 in 44 seconds
More details
- Looked at
64lines of code in4files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. frontend/src/container/CloudIntegrationPage/HeroSection/components/AccountSettingsModal.tsx:74
- Draft comment:
The changes in this PR are focused on styling and do not address the high CPU usage issue mentioned in the linked issue High CPU usage by ClickHouse #2112. The issue seems to be related to ClickHouse, which is not affected by these frontend changes. - Reason this comment was not posted:
Comment did not seem useful.
2. frontend/src/container/CloudIntegrationPage/HeroSection/components/AccountSettingsModal.tsx:71
- Draft comment:
Avoid using inline styles in React components. Use external stylesheets, CSS classes, or styled components instead. This is also applicable inIntegrateNowFormSections.tsx. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_41Qd1UwACE7mlaNv
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
amlannandy
previously approved these changes
Jan 30, 2025
* feat: add AWS integration in the integrations list and redirect to the new Cloud Integration page * feat: cloud integration details page header (i.e. breadcrumb and get help button) UI * feat: hero section UI * refactor: extract Header and HeroSection components from CloudIntegrationPage * feat: services tab bar and sidebar UI * feat: cloud integration details services UI * refactor: group and extract cloud integration components to files * fix: set default active service to the first service in the list if no service is specified * feat: add NEW flag for AWS integration in the integrations list page * chore: overall improvements * chore: move cloud integration pages to /container * fix: hero section background * AWS Integration: Account setup basic UI and functionality (#6806) * feat: implement basic cloud account management UI in HeroSection * AWS Integration: Integrate now modal (#6807) * feat: implement basic cloud account management UI in HeroSection * feat: start working on integrate now modal UI * feat: integrate now modal UI * feat: integrate now modal states and json server API integration * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * feat: display error state if last_heartbeat_ts_ms is null even after 5 minutes * chore: update import path for regions data in useRegionSelection hook * chore: move hero section components inside the HeroSection/components * feat: create a reusable modal component * refactor: make the cloud account setup modal readable / DRYer * AWS Integration: Account settings modal (#6808) * feat: implement basic cloud account management UI in HeroSection * feat: start working on integrate now modal UI * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * feat: integrate now modal UI * feat: integrate now modal states and json server API integration * feat: account settings * feat: service status UI * refactor: make account settings modal more readable and overall improvements * feat: Get data from json server api data in service sections (#6809) * feat: implement basic cloud account management UI in HeroSection * feat: start working on integrate now modal UI * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * refactor: make the cloud account setup modal readable / DRYer * feat: integrate now modal states and json server API integration * refactor: make account settings modal more readable and overall improvements * feat: integrate now modal states and json server API integration * feat: display error state if last_heartbeat_ts_ms is null even after 5 minutes * feat: get the services list and details from json server API response * feat: update account actions to set accountId in URL query on initial account load * feat: configure service modal (#6814) * feat: implement basic cloud account management UI in HeroSection * feat: start working on integrate now modal UI * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * refactor: make the cloud account setup modal readable / DRYer * feat: integrate now modal states and json server API integration * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * feat: integrate now modal states and json server API integration * feat: get accounts from json-server API, and redirect Add new account to the integrations modal * feat: display error state if last_heartbeat_ts_ms is null even after 5 minutes * feat: account settings * feat: service status UI * feat: get the services list and details from json server API response * feat: update account actions to set accountId in URL query on initial account load * feat: configure service modal UI * feat: configure service modal functionality and API changes * feat: replace loading indicators with Spinner component in ServiceDetails and ServicesList * fix: make the configure service modal work * Light mode support and overall improvements to AWS integration page (#6817) * refactor: make the cloud account setup modal readable / DRYer * feat: integrate now modal states and json server API integration * refactor: make account settings modal more readable and overall improvements * fix: integrate now modal button improvements * feat: aws integrations light mode * refactor: overall improvements * refactor: define react query keys in constant * feat: services filter * feat: render service overview as markdown * feat: integrate AWS integration page API (#6851) * feat: replace json-server APIs with actual APIs * fix: add null checks and fix the issues
ahmadshaheer
force-pushed
the
feat/aws-integration
branch
from
72a4f44 to
cf45010
Compare
SagarRajput-7
approved these changes
Jan 30, 2025
ahmadshaheer
approved these changes
Jan 30, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
AWS Integration
Fixes
https://github.com/SigNoz/engineering-pod/issues/2112
Screenshots
2025-01-14.19-36-59.mov
Important
Adds AWS integration with account and service management, including new UI components and hooks for AWS API interactions.
CloudIntegrationPagewith components for AWS account setup and service configuration.useAwsAccounts,useGetAccountServices,useServiceDetails, and other hooks for AWS API interactions.SignozModalfor AWS account and service configuration.RegionSelector,RegionForm, andSuccessViewcomponents for AWS setup.IntegrationDetailPageandIntegrationsListto support AWS integration.CloudIntegrationPage.This description was created by
for 72a4f44. It will automatically update as commits are pushed.