-
Notifications
You must be signed in to change notification settings - Fork 184
Aws mcp instructions #1084
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?
Aws mcp instructions #1084
Conversation
…ce latency and memory usage on the mcp server
WalkthroughThe PR increases CloudTrail query limits (--max-items from 50 to 100), adds container insights log gathering capability, introduces comprehensive memory optimization guidelines for AWS operations, and reorders imports in main.py and server.py to occur after security/certificate initialization blocks. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🧹 Nitpick comments (5)
helm/holmes/templates/mcp-servers/aws/_helpers.tpl (5)
35-40: Tighten CloudTrail example (UTC format and explicit end time)Add Z-suffixed ISO8601 and end-time to avoid timezone ambiguity and long scans.
- - Query CloudTrail for recent API calls: `aws cloudtrail lookup-events --start-time TIME --max-items 100` + - Query CloudTrail for recent API calls: `aws cloudtrail lookup-events --start-time TIME_Z --end-time NOW_Z --max-items 100` + - Example: `aws cloudtrail lookup-events --start-time $(date -u -d '2 hours ago' +%Y-%m-%dT%H:%M:%SZ) --end-time $(date -u +%Y-%m-%dT%H:%M:%SZ) --max-items 100`
46-50: Container Insights snippet: optional region/filter to reduce scan costConsider guiding users to set region and a basic filter to cut noise.
aws logs filter-log-events \ --log-group-name /aws/containerinsights/CLUSTER_NAME/application \ --start-time $(date -d '1 hour ago' +%s)000 \ - --max-items 500 + --max-items 500 \ + # optionally set AWS region explicitly + # --region $AWS_REGION \ + # optionally narrow results + # --filter-pattern "ERROR|Exception"
69-70: CloudTrail “last hour” example: use Z-suffixUse explicit Z to indicate UTC.
-aws cloudtrail lookup-events --start-time $(date -u -d '1 hour ago' +%Y-%m-%dT%H:%M:%S) --max-items 100 +aws cloudtrail lookup-events --start-time $(date -u -d '1 hour ago' +%Y-%m-%dT%H:%M:%SZ) --end-time $(date -u +%Y-%m-%dT%H:%M:%SZ) --max-items 100
79-80: Add time window to targeted CloudTrail searchPrevents scanning excessive history.
-aws cloudtrail lookup-events --lookup-attributes AttributeKey=EventName,AttributeValue=RevokeSecurityGroupIngress --max-items 20 +aws cloudtrail lookup-events \ + --lookup-attributes AttributeKey=EventName,AttributeValue=RevokeSecurityGroupIngress \ + --start-time $(date -u -d '4 hours ago' +%Y-%m-%dT%H:%M:%SZ) \ + --end-time $(date -u +%Y-%m-%dT%H:%M:%SZ) \ + --max-items 20
138-141: Remove duplicate headerThis repeats an earlier “Investigation Principles” header.
-## Investigation Principles -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
helm/holmes/templates/mcp-servers/aws/_helpers.tpl(5 hunks)holmes/main.py(1 hunks)server.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use Ruff for formatting and linting (configured in pyproject.toml) for all Python code
Type hints are required; code should pass mypy (configured in pyproject.toml)
ALWAYS place Python imports at the top of the file, not inside functions or methods
Files:
server.pyholmes/main.py
🧬 Code graph analysis (1)
server.py (1)
holmes/version.py (2)
get_version(48-130)is_official_release(40-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
holmes/main.py (1)
11-12: Import move under cert guard looks goodKeeps HTTPS clients from pre-initializing before custom cert install; E402 suppressed at file-level. No issues.
Please double-check no other imports were added above the guard in future patches.
server.py (2)
11-17: Good relocation of imports under cert guardThis aligns with the bootstrap pattern and prevents premature client init.
16-16: Verify consistency of import pattern with codebase conventionsThe package properly exports
get_versionandis_official_releasefromholmes/__init__.pyusing the correct re-export syntax. However, other files in the codebase (holmes/main.pyandholmes/utils/holmes_status.py) import the same functions with# type: ignore, whileserver.pydoes not. Choose one approach for consistency:
- Import from the defining module:
from holmes.version import get_version, is_official_release- Add
# type: ignoreto match existing codebase pattern:from holmes import get_version, is_official_release # type: ignore[attr-defined]helm/holmes/templates/mcp-servers/aws/_helpers.tpl (1)
221-229: Review comment is incorrect; original documentation is accurateThe original statement "CloudWatch Logs: Use --max-items and --starting-token" is correct for AWS CLI usage. The AWS CLI command
aws logs filter-log-eventsdoes NOT take a--next-tokenflag; the CLI exposes pagination via--starting-token(to resume),--page-sizeand--max-items.The reviewer's suggested fix introduces non-existent CLI parameters (
--limit,--next-token). While the CloudWatch Logs API FilterLogEvents usesnextTokenin the request and returnsnextTokenin the response, these are service-level API parameters, not CLI flags. The CLI correctly abstracts these via--starting-tokenand--max-items.If the intent is to document both API and CLI usage, clarify the distinction. Otherwise, the existing documentation is accurate and requires no changes.
Likely an incorrect or invalid review comment.
| ## ⚠️ MEMORY OPTIMIZATION GUIDELINES ⚠️ | ||
|
|
||
| **The AWS MCP server can experience memory pressure with very large queries. Follow these guidelines to balance data retrieval with stability:** | ||
|
|
||
| ### Query Limits by Service Type | ||
|
|
||
| | Service | Safe Limit | Max Limit | Notes | | ||
| |---------|-----------|-----------|--------| | ||
| | **CloudWatch Logs** | 500 items | 1000 items | ALWAYS use time constraints (1-hour window max initially) | | ||
| | **CloudTrail** | 200 items | 500 items | Heavy JSON payloads, use 2-hour windows initially | | ||
| | **EC2 Describe** | 500 items | 1000 items | Use filters when possible | | ||
| | **RDS/ELB** | 100 items | 200 items | Usually fewer resources | | ||
| | **S3 List** | 1000 items | 5000 items | Metadata only, not object contents | | ||
| | **Cost & Usage** | 30 days | 90 days | Use DAILY granularity, not HOURLY | | ||
|
|
||
| ### Critical Rules | ||
| 1. **NEVER download S3 object contents** (can be GBs) | ||
| 2. **ALWAYS use time constraints for logs** (CloudWatch, VPC Flow Logs) | ||
| 3. **ALWAYS use RECENT time constraints for logs** - Even with --max-items, AWS scans ALL data in the time range! | ||
| 4. **ALWAYS use small time windows. 1-2 hours. It's ok to do multiple queries. | ||
| 5. **START with recommended limits**, increase if needed | ||
| 6. **If query times out**, reduce time window FIRST (not just --max-items)3. **START with recommended limits**, increase if needed | ||
|
|
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.
Fix typos/duplication in MEMORY GUIDELINES
Clean up duplicated numbering and punctuation for clarity.
## ⚠️ MEMORY OPTIMIZATION GUIDELINES ⚠️
@@
-3. **ALWAYS use RECENT time constraints for logs** - Even with --max-items, AWS scans ALL data in the time range!
-4. **ALWAYS use small time windows. 1-2 hours. It's ok to do multiple queries.
-5. **START with recommended limits**, increase if needed
-6. **If query times out**, reduce time window FIRST (not just --max-items)3. **START with recommended limits**, increase if needed
+3. **ALWAYS use RECENT time constraints for logs** — even with --max-items, AWS scans ALL data in the time range.
+4. **ALWAYS use small time windows** (1–2 hours). It's OK to do multiple queries.
+5. **START with recommended limits**, increase if needed.
+6. **If a query times out**, reduce the time window first (not just --max-items).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## ⚠️ MEMORY OPTIMIZATION GUIDELINES ⚠️ | |
| **The AWS MCP server can experience memory pressure with very large queries. Follow these guidelines to balance data retrieval with stability:** | |
| ### Query Limits by Service Type | |
| | Service | Safe Limit | Max Limit | Notes | | |
| |---------|-----------|-----------|--------| | |
| | **CloudWatch Logs** | 500 items | 1000 items | ALWAYS use time constraints (1-hour window max initially) | | |
| | **CloudTrail** | 200 items | 500 items | Heavy JSON payloads, use 2-hour windows initially | | |
| | **EC2 Describe** | 500 items | 1000 items | Use filters when possible | | |
| | **RDS/ELB** | 100 items | 200 items | Usually fewer resources | | |
| | **S3 List** | 1000 items | 5000 items | Metadata only, not object contents | | |
| | **Cost & Usage** | 30 days | 90 days | Use DAILY granularity, not HOURLY | | |
| ### Critical Rules | |
| 1. **NEVER download S3 object contents** (can be GBs) | |
| 2. **ALWAYS use time constraints for logs** (CloudWatch, VPC Flow Logs) | |
| 3. **ALWAYS use RECENT time constraints for logs** - Even with --max-items, AWS scans ALL data in the time range! | |
| 4. **ALWAYS use small time windows. 1-2 hours. It's ok to do multiple queries. | |
| 5. **START with recommended limits**, increase if needed | |
| 6. **If query times out**, reduce time window FIRST (not just --max-items)3. **START with recommended limits**, increase if needed | |
| ## ⚠️ MEMORY OPTIMIZATION GUIDELINES ⚠️ | |
| **The AWS MCP server can experience memory pressure with very large queries. Follow these guidelines to balance data retrieval with stability:** | |
| ### Query Limits by Service Type | |
| | Service | Safe Limit | Max Limit | Notes | | |
| |---------|-----------|-----------|--------| | |
| | **CloudWatch Logs** | 500 items | 1000 items | ALWAYS use time constraints (1-hour window max initially) | | |
| | **CloudTrail** | 200 items | 500 items | Heavy JSON payloads, use 2-hour windows initially | | |
| | **EC2 Describe** | 500 items | 1000 items | Use filters when possible | | |
| | **RDS/ELB** | 100 items | 200 items | Usually fewer resources | | |
| | **S3 List** | 1000 items | 5000 items | Metadata only, not object contents | | |
| | **Cost & Usage** | 30 days | 90 days | Use DAILY granularity, not HOURLY | | |
| ### Critical Rules | |
| 1. **NEVER download S3 object contents** (can be GBs) | |
| 2. **ALWAYS use time constraints for logs** (CloudWatch, VPC Flow Logs) | |
| 3. **ALWAYS use RECENT time constraints for logs** — even with --max-items, AWS scans ALL data in the time range. | |
| 4. **ALWAYS use small time windows** (1–2 hours). It's OK to do multiple queries. | |
| 5. **START with recommended limits**, increase if needed. | |
| 6. **If a query times out**, reduce the time window first (not just --max-items). |
🤖 Prompt for AI Agents
In helm/holmes/templates/mcp-servers/aws/_helpers.tpl around lines 115-137, the
MEMORY OPTIMIZATION GUIDELINES contain duplicated entries, misnumbering and
punctuation errors; clean up by removing duplicated rules/lines (the repeated
"START with recommended limits" and the stray '3.'), renumber the Critical Rules
sequentially 1–6, fix punctuation (close the parenthesis on rule 4, add periods
where needed), and ensure each rule is unique, concise, and correctly ordered
with no duplicate content.
No description provided.