Repository navigation
(micro-fix)Add AWS CloudWatch MCP tool - #7304
codeWithkrish123 wants to merge 1 commit into
Conversation
Sorry, something went wrong.
Adds cloudwatch_tool with SigV4 signing (no boto3 dependency): - cloudwatch_list_metrics - cloudwatch_get_metric_statistics - cloudwatch_describe_alarms - cloudwatch_logs_start_query - cloudwatch_logs_get_query_results - cloudwatch_logs_describe_log_groups Registers CLOUDWATCH_CREDENTIALS in credentials/__init__.py and register_cloudwatch in tools/__init__.py (both verified + unverified). Credentials: AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_REGION. Closes aden-hive#7
PR Requirements WarningThis PR does not meet the contribution requirements. PR Author: @codeWithkrish123 To fix:
Exception: To bypass this requirement, you can:
Micro-fix requirements (must meet ALL):
Why is this required? See #472 for details. |
Sorry, something went wrong.
WalkthroughThis PR adds AWS CloudWatch integration to the tools framework: new credential specs for AWS access/secret keys, a new cloudwatch_tool package implementing hand-rolled SigV4 signing, and MCP tool registrations for CloudWatch Metrics, Alarms, and Logs Insights, wired into the existing registries. ChangesCloudWatch Integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant register_tools
participant _cw_post
participant _logs_post
participant CloudWatchAPI
Agent->>register_tools: invoke cloudwatch_list_metrics/describe_alarms
register_tools->>_cw_post: signed request with SigV4 headers
_cw_post->>CloudWatchAPI: POST monitoring.<region>.amazonaws.com
CloudWatchAPI-->>_cw_post: XML response
_cw_post-->>register_tools: parsed result or error dict
Agent->>register_tools: invoke cloudwatch_logs_start_query
register_tools->>_logs_post: signed request with SigV4 headers
_logs_post->>CloudWatchAPI: POST logs.<region>.amazonaws.com
CloudWatchAPI-->>_logs_post: JSON response
_logs_post-->>register_tools: parsed result or error dict
register_tools-->>Agent: return tool result
Possibly related issues
❌ Failed checks (1 warning)
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. ❤️ ShareComment @coderabbitai help to get the list of available commands. |
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)tools/src/aden_tools/tools/cloudwatch_tool/cloudwatch_tool.py (1)🤖 Prompt for all review comments with AI agents30-43: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
No support for AWS temporary/session credentials.
_get_config only reads AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY; there's no AWS_SESSION_TOKEN support (needed for STS temporary credentials, assumed roles, or federated logins), and the signing helper doesn't add an x-amz-security-token header. This will block usage in environments relying on short-lived credentials.
Suggested fixdef _get_config() -> tuple[str, str, str] | dict: """Return (access_key, secret_key, region) or an error dict.""" access_key = os.getenv("AWS_ACCESS_KEY_ID", "") secret_key = os.getenv("AWS_SECRET_ACCESS_KEY", "") region = os.getenv("AWS_REGION", "us-east-1") + session_token = os.getenv("AWS_SESSION_TOKEN", "") if not access_key or not secret_key: return { "error": "AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY are required", ... } - return access_key, secret_key, region + return access_key, secret_key, region, session_tokenThen thread session_token through _signed_headers to add x-amz-security-token as a signed header when present.
🤖 Prompt for AI AgentsVerify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/src/aden_tools/tools/cloudwatch_tool/cloudwatch_tool.py` around lines 30 - 43, _support temporary AWS session credentials by extending _get_config in cloudwatch_tool.py to read AWS_SESSION_TOKEN and return it alongside access_key, secret_key, and region; then update the signing path in _signed_headers to accept and include the session token as an x-amz-security-token signed header when present, while preserving the existing behavior for long-lived credentials._
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/src/aden_tools/tools/cloudwatch_tool/cloudwatch_tool.py`:
- Around line 190-234: The return shape in cloudwatch_list_metrics (and the
matching cloudwatch_get_metric_statistics flow) does not match the documented
structured output. Either parse the CloudWatch XML into the promised
metrics/datapoints lists before returning, or update the docstrings and callers
to explicitly expect the raw XML response; keep the fix aligned with _cw_post
and the tool wrappers that currently return {"region": ..., "response":
result["raw_xml"]}.
- Around line 69-71: The CloudWatch signing timestamp in cloudwatch_tool uses
datetime.datetime.utcnow(), which is deprecated for current Python runtimes.
Update the timestamp creation in the CloudWatch request-signing logic to use
datetime.datetime.now(datetime.timezone.utc), and keep the existing
amzdate/datestamp formatting behavior unchanged; if a naive datetime is needed
elsewhere, explicitly convert it after creating the UTC-aware value.
---
Nitpick comments:
In `@tools/src/aden_tools/tools/cloudwatch_tool/cloudwatch_tool.py`:
- Around line 30-43: _support temporary AWS session credentials by extending
_get_config in cloudwatch_tool.py to read AWS_SESSION_TOKEN and return it
alongside access_key, secret_key, and region; then update the signing path in
_signed_headers to accept and include the session token as an
x-amz-security-token signed header when present, while preserving the existing
behavior for long-lived credentials._
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info ⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 24d5a67a-1b91-4123-bf9e-5719d3e1a0b3
📥 CommitsReviewing files that changed from the base of the PR and between 3c390c9 and 8ac978f.
📒 Files selected for processing (5)- tools/src/aden_tools/credentials/__init__.py
- tools/src/aden_tools/credentials/cloudwatch.py
- tools/src/aden_tools/tools/__init__.py
- tools/src/aden_tools/tools/cloudwatch_tool/__init__.py
- tools/src/aden_tools/tools/cloudwatch_tool/cloudwatch_tool.py
Sorry, something went wrong.
| now = datetime.datetime.utcnow() | ||
| amzdate = now.strftime("%Y%m%dT%H%M%SZ") | ||
| datestamp = now.strftime("%Y%m%d") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain🌐 Web query:
Is datetime.datetime.utcnow() removed in Python 3.14?
💡 Result:
Yes, datetime.datetime.utcnow has been removed in Python 3.14 [1][2]. The method was deprecated in Python 3.12, where it began emitting a DeprecationWarning, and it was officially removed in the Python 3.14 release [1][3][4]. To replace datetime.datetime.utcnow, the recommended approach depends on whether you require a timezone-aware or a timezone-naive datetime object: 1. For a timezone-aware object (recommended): Use datetime.datetime.now(datetime.timezone.utc) [1][3]. 2. For a timezone-naive object (to maintain strict backward compatibility): Use datetime.datetime.now(datetime.timezone.utc).replace(tzinfo=None) [3][5]. Note that simply replacing utcnow with datetime.now(datetime.timezone.utc) will return an aware datetime object, which may cause errors if your existing code expects a naive datetime object or if you are performing arithmetic/comparisons with other naive objects [3][5]. Use the.replace(tzinfo=None) method if you must preserve the naive nature of the original object [3][5].
Citations:
- 1: https://docs.python.org/3/library/datetime.html
- 2: https://docs.python.org/3/whatsnew/3.14.html
- 3: GH-103857: Deprecate utcnow and utcfromtimestamp python/cpython#103858
- 4: python/cpython@0b7fd8f
- 5: utcnow deprecation note is misleading python/cpython#118542
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate project Python version declarations and the target file context.
git ls-files | rg '(^|/)(pyproject\.toml|setup\.py|setup\.cfg|requirements.*\.txt|tox\.ini|noxfile\.py|README.*|cloudwatch_tool\.py)$'
echo
echo "=== python version references ==="
rg -n --hidden --no-ignore-vcs -S 'python_version|requires-python|Programming Language :: Python|Python 3\.[0-9]+' \
pyproject.toml setup.py setup.cfg tox.ini noxfile.py README.md requirements*.txt tools/src/aden_tools/tools/cloudwatch_tool/cloudwatch_tool.py || true
echo
echo "=== target file excerpt ==="
cat -n tools/src/aden_tools/tools/cloudwatch_tool/cloudwatch_tool.py | sed -n '55,85p'Repository: aden-hive/hive
Length of output: 7605
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== root pyproject.toml ==="
cat -n pyproject.toml | sed -n '1,220p'
echo
echo "=== tools/pyproject.toml ==="
cat -n tools/pyproject.toml | sed -n '1,220p'
echo
echo "=== all Python version markers in repo ==="
rg -n --hidden --no-ignore-vcs -S 'requires-python|python_requires|python_version|Python 3\.[0-9]+\+|>= *3\.[0-9]+' pyproject.toml tools/pyproject.toml core/pyproject.toml README.md tools/README.mdRepository: aden-hive/hive
Length of output: 4655
Replace datetime.datetime.utcnow()
tools/pyproject.toml supports Python 3.11+, so this affects current runtimes: utcnow() is deprecated in 3.12 and removed in 3.14. Use datetime.datetime.now(datetime.timezone.utc) here (or .replace(tzinfo=None) if a naive timestamp is required).
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/src/aden_tools/tools/cloudwatch_tool/cloudwatch_tool.py` around lines 69 - 71, The CloudWatch signing timestamp in cloudwatch_tool uses datetime.datetime.utcnow(), which is deprecated for current Python runtimes. Update the timestamp creation in the CloudWatch request-signing logic to use datetime.datetime.now(datetime.timezone.utc), and keep the existing amzdate/datestamp formatting behavior unchanged; if a naive datetime is needed elsewhere, explicitly convert it after creating the UTC-aware value.
Sorry, something went wrong.
| @mcp.tool(name="cloudwatch_list_metrics") | ||
| def cloudwatch_list_metrics( | ||
| namespace: str = "", | ||
| metric_name: str = "", | ||
| dimensions: str = "", | ||
| ) -> dict[str, Any]: | ||
| """List available CloudWatch metrics. | ||
|
|
||
| Args: | ||
| namespace: Optional AWS namespace filter, e.g. ``AWS/EC2``. | ||
| metric_name: Optional metric name filter, e.g. ``CPUUtilization``. | ||
| dimensions: Optional comma-separated ``Name=Value`` dimension filters, | ||
| e.g. ``InstanceId=i-1234567890abcdef0``. | ||
|
|
||
| Returns: | ||
| Dict with ``metrics`` list (each entry has ``Namespace``, ``MetricName``, | ||
| ``Dimensions``) or ``error`` on failure. | ||
| """ | ||
| cfg = _get_config() | ||
| if isinstance(cfg, dict): | ||
| return cfg | ||
| access_key, secret_key, region = cfg | ||
|
|
||
| params: dict[str, str] = { | ||
| "Action": "ListMetrics", | ||
| "Version": "2010-08-01", | ||
| } | ||
| if namespace: | ||
| params["Namespace"] = namespace | ||
| if metric_name: | ||
| params["MetricName"] = metric_name | ||
| # Parse dimensions: "InstanceId=i-xxx,AutoScalingGroupName=asg-yyy" | ||
| if dimensions: | ||
| for idx, pair in enumerate(dimensions.split(","), start=1): | ||
| pair = pair.strip() | ||
| if "=" in pair: | ||
| name, value = pair.split("=", 1) | ||
| params[f"Dimensions.member.{idx}.Name"] = name.strip() | ||
| params[f"Dimensions.member.{idx}.Value"] = value.strip() | ||
|
|
||
| result = _cw_post(access_key, secret_key, region, params) | ||
| if "error" in result: | ||
| return result | ||
| # Return raw XML — agents can parse it; keep tool thin | ||
| return {"region": region, "response": result["raw_xml"]} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Docstrings promise structured metrics/datapoints output but implementation returns raw XML.
cloudwatch_list_metrics's docstring states the return value contains "metrics" list entries, and cloudwatch_get_metric_statistics's docstring promises a "datapoints" list, but both actually return {"region": ..., "response": result["raw_xml"]} (raw XML string) — there is no metrics/datapoints key at all. Any agent/consumer relying on the documented shape will fail to extract data.
Suggested directionEither parse the CloudWatch XML response into the documented structured keys before returning, or update the docstrings to accurately describe the raw XML response shape.
Also applies to: 236-297
🤖 Prompt for AI AgentsVerify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/src/aden_tools/tools/cloudwatch_tool/cloudwatch_tool.py` around lines
190 - 234, The return shape in cloudwatch_list_metrics (and the matching
cloudwatch_get_metric_statistics flow) does not match the documented structured
output. Either parse the CloudWatch XML into the promised metrics/datapoints
lists before returning, or update the docstrings and callers to explicitly
expect the raw XML response; keep the fix aligned with _cw_post and the tool
wrappers that currently return {"region": ..., "response": result["raw_xml"]}.
Sorry, something went wrong.
|
Hi, @fermano I'd like to work on this issue. Could someone assign it to me? |
Sorry, something went wrong.
|
Closing PR because the contribution requirements were not resolved within the 24-hour grace period. |
Sorry, something went wrong.
Description
Adds a full AWS CloudWatch MCP tool (cloudwatch_tool.py) to the aden_tools package, enabling Hive agents to interact with CloudWatch Metrics and CloudWatch Logs through 9 new MCP-registered tool functions. Authentication uses AWS SigV4 request signing with no external AWS SDK dependency.
Type of Change
Related Issues
Fixes #2805
Changes Made
Testing
Checklist
Summary by CodeRabbit