-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
chore: onboarding fixes and added new datasource #7236
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.
❌ Changes requested. Reviewed everything up to 388364b in 3 minutes and 38 seconds
More details
- Looked at
1426
lines of code in2
files - Skipped
7
files when reviewing. - Skipped posting
6
drafted comments based on config settings.
1. frontend/src/container/OnboardingV2Container/AddDataSource/AddDataSource.tsx:381
- Draft comment:
Added new cases for 'infra-monitoring-hosts' and 'infra-monitoring-k8s'. Ensure that ROUTES.INFRASTRUCTURE_MONITORING_HOSTS and ROUTES.INFRASTRUCTURE_MONITORING_KUBERNETES are defined and tested. Consider centralizing your route constants if similar logic repeats. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =50%
<= threshold50%
The comment suggests ensuring that certain routes are defined and tested, which violates the rule against asking the PR author to ensure that changes are tested. However, it also provides a suggestion to centralize route constants, which is a valid code suggestion. The comment is partially useful.
2. frontend/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json:19
- Draft comment:
Documentation URLs have been switched from production to localhost. Make sure these local URLs are not merged into production accidentally. Consider using environment variables or a config file to manage documentation URLs. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
This appears to be a valid concern - changing URLs to localhost could cause issues in production. However, the comment is speculative ("Make sure these...") and asks for verification rather than pointing out a clear issue. It also suggests using environment variables which goes beyond just identifying the problem.
The URLs might be intentionally changed for local development or testing. Without more context about the purpose of these changes, I can't be certain this is actually an issue.
Even if this is a potential issue, the comment violates the rules by asking for verification ("Make sure...") and making speculative suggestions rather than pointing out a clear problem that needs fixing.
The comment should be deleted because it asks for verification and makes speculative suggestions rather than identifying a clear, actionable issue.
3. frontend/src/container/OnboardingV2Container/AddDataSource/AddDataSource.tsx:448
- Draft comment:
Avoid using inline styles (e.g. on the Header component). Extract these styles to a CSS class to improve maintainability and theming. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. frontend/src/container/OnboardingV2Container/AddDataSource/AddDataSource.tsx:704
- Draft comment:
The iframe uses a referrerPolicy of 'unsafe-url' which could expose sensitive referrer information. Consider using a more secure referrer policy like 'no-referrer' if possible. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. frontend/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json:108
- Draft comment:
Many URLs in the JSON config are repeated and hardcoded. Consider abstracting the base docs URL into a single configuration variable to reduce duplication and ease future updates. - Reason this comment was not posted:
Comment was on unchanged code.
6. frontend/src/container/OnboardingV2Container/AddDataSource/AddDataSource.tsx:460
- Draft comment:
Possible typographical error: The class name 'perlian-bg' appears on line 460. If this was intended to be 'periscope-bg' (to match other similar class names in the file), it should be corrected. Please verify the correct naming. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_ePjtXdTulKVPTgmS
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.
...end/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json
Outdated
Show resolved
Hide resolved
...end/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json
Outdated
Show resolved
Hide resolved
...end/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json
Outdated
Show resolved
Hide resolved
...end/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json
Outdated
Show resolved
Hide resolved
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.
👍 Looks good to me! Incremental review on 781eef6 in 1 minute and 30 seconds
More details
- Looked at
1357
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
6
drafted comments based on config settings.
1. frontend/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json:16
- Draft comment:
All URLs have been updated from local development routes to the production signoz.io endpoints. The changes are consistent and follow the expected pattern. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. frontend/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json:1172
- Draft comment:
Duplicate key 'metrics' found in Cloud Functions options. Consider using a unique key (e.g., 'customMetrics') for the third option. - Reason this comment was not posted:
Comment was on unchanged code.
3. frontend/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json:880
- Draft comment:
Inconsistent dataSource naming: 'nodejs-winston-logs/' has an unnecessary trailing slash. Remove it for consistency. - Reason this comment was not posted:
Comment was on unchanged code.
4. frontend/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json:912
- Draft comment:
Typo in label: 'Kuberenetes Monitoring' should be 'Kubernetes Monitoring'. - Reason this comment was not posted:
Comment was on unchanged code.
5. frontend/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json:912
- Draft comment:
Typo found: 'Kuberenetes Monitoring' should be corrected to 'Kubernetes Monitoring'. - Reason this comment was not posted:
Comment was on unchanged code.
6. frontend/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json:880
- Draft comment:
Minor issue: The dataSource value 'nodejs-winston-logs/' has a trailing slash. Consider removing the trailing slash for consistency with other entries. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_IUylSOyp5LQ8cmFU
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and 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.
👍 Looks good to me! Incremental review on e9c1c49 in 1 minute and 0 seconds
More details
- Looked at
22
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
6
drafted comments based on config settings.
1. frontend/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json:877
- Draft comment:
Removed trailing slash from the dataSource value for Winston Logs. Ensure this change is consistent with other datasource keys. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the author to ensure consistency with other datasource keys, which is a form of asking for confirmation or verification. This violates the rule against asking the PR author to confirm or ensure consistency.
2. frontend/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json:912
- Draft comment:
Corrected typo in the label: 'Kuberenetes' to 'Kubernetes'. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, as it only states that a typo was corrected. It doesn't provide any actionable feedback or suggestions for improvement.
3. frontend/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json:877
- Draft comment:
Removed trailing slash from 'nodejs-winston-logs'; this ensures consistent dataSource naming. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, explaining a change that was made without suggesting any action or asking for clarification. It doesn't provide any actionable feedback or raise any concerns about the code.
4. frontend/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json:910
- Draft comment:
Corrected typo: 'Kuberenetes Monitoring' is now 'Kubernetes Monitoring'. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative as it only points out a typo correction. It does not provide any actionable feedback or suggestions for improvement.
5. frontend/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json:877
- Draft comment:
Typographical issue: The dataSource value for Winston Logs had an unnecessary trailing slash ('nodejs-winston-logs/'). It has been corrected to 'nodejs-winston-logs' for consistency. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, pointing out a typographical correction that has already been made. It does not provide a suggestion or request for action, nor does it relate to any of the specific rules provided.
6. frontend/src/container/OnboardingV2Container/onboarding-configs/onboarding-config-with-links.json:910
- Draft comment:
Typographical issue: The label 'Kuberenetes Monitoring' was misspelled. It has been corrected to 'Kubernetes Monitoring'. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative as it only points out a typographical correction that has already been made. It does not provide any actionable feedback or suggestions for improvement.
Workflow ID: wflow_KWKsoyZd16jE0ihB
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and 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.
Skipped PR review on 5007f98 because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.
Summary
Related Issues / PR's
SigNoz/signoz-web#1223
Screenshots
NA
Affected Areas and Manually Tested Areas
NA
Important
Enhance onboarding with new data sources and improved navigation for infrastructure monitoring.
infra-monitoring-hosts
andinfra-monitoring-k8s
inOnboardingAddDataSource.tsx
to navigate to respective routes.opentelemetry-cpp
,aws-lambda-nodejs-logs
,python-logs-auto-instrumentation
, and others inonboarding-config-with-links.json
.onboarding-config-with-links.json
to include new data sources with their respective labels, tags, modules, and documentation links.id
fields to existing data sources for better identification.onboarding-config-with-links.json
.This description was created by
for e9c1c49. It will automatically update as commits are pushed.