-
Notifications
You must be signed in to change notification settings - Fork 1.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
[DOCS-9725] Cloudcraft in Datadog #27718
base: master
Are you sure you want to change the base?
Conversation
Preview links (active after the
|
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.
My review feedback:
- The explanation of standalone Cloudcraft could be clearer. Consider rephrasing to make it more concise and direct.
- Group-bys and filters should be explained before presets, as presets reference group-by functionality.
- The image associated with the infrastructure diagram is incorrect—it should have the infrastructure layer selected.
- Need to clarify “Filters” vs. “Search and Highlight”
- Filter: Re-renders the diagram to show only infrastructure that matches the filter criteria.
- Search and Highlight: Highlights the search criteria in the diagram - does not create a new diagram but the elements that match the search criteria are highlighted and the rest are greyed.
- Security Overlay:
- By default, it displays Critical, High, and Medium severity issues.
- Instead of “can be configured by hovering over”, say “can be filtered.”
- Agent Overlay
- The red dot color will be changed to a greyish tone— confirm this before release.
@srividya-rajagopalan asked me to take a look, so here I'm. The documentation looks excelent, and Sri already pointed out a few issues, so I don't have much to add.
The minimal IAM policy from standalone Cloudcraft won't work for Cloudcraft in Datadog, so this portion should be removed.
That is true for standalone Cloudcraft, but not for Cloudcraft in Datadog. The connections are displayed in the diagram, though. I also think we may need to update the menu item for standalone Cloudcraft from "Cloudcraft" to "Standalone Cloudcraft", otherwise we have "Cloudcraft" twice in the menu, which may lead to confusion. We may also need a warning, note, or something like that on the documentation for both the standalone and in Datadog leading the user to the other one. Something like "If you're looking for documentation for standalone Cloudcraft, look here." |
Hi @srividya-rajagopalan and @jamesponddotco ! thank you both for your detailed review! I have addressed all of your comments except for the Agent overlay - Making a note here to review prior to release for Agent color and updated screenshot. Can you both please re-review when you get a chance? Thanks! |
@aliciascott Looks good to me, but I think @srividya-rajagopalan needs to approve it, since she was the requested reviewer. |
What does this PR do? What is the motivation?
Merge instructions
Merge readiness:
Merge queue is enabled in this repo. To have it automatically merged after it receives the required reviews, create the PR (from a branch that follows the
<yourname>/description
naming convention) and then add the following PR comment:Additional notes