ROB-1741: datadog rds analysis toolset - #653
Conversation
…estamp for datadog API
WalkthroughThis change introduces a new Datadog RDS toolset for performance analysis of AWS RDS instances. It adds toolset registration, tool implementations for generating performance reports and identifying worst-performing instances, an instruction template, and comprehensive integration and live tests. No existing logic is modified. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Toolset (DatadogRDSToolset)
participant Tool (GenerateRDSPerformanceReport)
participant DatadogAPI
User->>Toolset: Selects GenerateRDSPerformanceReport tool
Toolset->>Tool: Invokes with parameters (instance, time range)
Tool->>DatadogAPI: Queries metrics for RDS instance
DatadogAPI-->>Tool: Returns metrics data
Tool->>Tool: Aggregates, analyzes, and formats report
Tool->>Toolset: Returns structured report
Toolset->>User: Presents performance report
sequenceDiagram
participant User
participant Toolset (DatadogRDSToolset)
participant Tool (GetTopWorstPerformingRDSInstances)
participant DatadogAPI
User->>Toolset: Selects GetTopWorstPerformingRDSInstances tool
Toolset->>Tool: Invokes with parameters (sort, time range)
Tool->>DatadogAPI: Lists all RDS instances
DatadogAPI-->>Tool: Returns instance list
loop For each instance (up to 50)
Tool->>DatadogAPI: Queries metrics for instance
DatadogAPI-->>Tool: Returns metrics
Tool->>Tool: Scores and analyzes issues
end
Tool->>Tool: Sorts and formats summary report
Tool->>Toolset: Returns summary report
Toolset->>User: Presents worst-performing instances
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ 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). (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…db_analysis_toolset
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
holmes/plugins/toolsets/datadog/toolset_datadog_rds.py (2)
374-374: Consider making the instance limit configurableThe hard-coded limit of 50 instances might be insufficient for large environments. Consider making this configurable.
- for instance_id in instances[:50]: # Limit to 50 instances to avoid timeout + max_instances = min(50, params.get("max_instances_to_analyze", 50)) + for instance_id in instances[:max_instances]: # Limit instances to avoid timeoutAlso add a parameter definition in the tool's
__init__method:"max_instances_to_analyze": ToolParameter( description="Maximum number of instances to analyze (default: 50, max: 50)", type="number", required=False, ),
93-98: Consider consolidating config None checksMultiple tools check for None config. Consider adding a property or method to handle this consistently.
Add a helper method to the base class:
def _ensure_config(self, params: Any) -> Optional[StructuredToolResult]: """Check if config is available, return error result if not.""" if not self.toolset.dd_config: return StructuredToolResult( status=ToolResultStatus.ERROR, error=TOOLSET_CONFIG_MISSING_ERROR, params=params, ) return NoneThen use it in tools:
- if not self.toolset.dd_config: - return StructuredToolResult( - status=ToolResultStatus.ERROR, - error=TOOLSET_CONFIG_MISSING_ERROR, - params=params, - ) + config_error = self._ensure_config(params) + if config_error: + return config_error
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
holmes/plugins/toolsets/__init__.py(2 hunks)holmes/plugins/toolsets/datadog/datadog_rds_instructions.jinja2(1 hunks)holmes/plugins/toolsets/datadog/toolset_datadog_rds.py(1 hunks)tests/plugins/toolsets/datadog/rds/test_datadog_rds_integration.py(1 hunks)tests/plugins/toolsets/datadog/rds/test_datadog_rds_live.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nherment
PR: robusta-dev/holmesgpt#436
File: tests/llm/fixtures/test_ask_holmes/42_dns_issues_result_new_tools/toolsets.yaml:6-20
Timestamp: 2025-06-05T06:14:11.571Z
Learning: nherment prefers to keep test fixtures simple rather than adding complex security restrictions, even when potential security issues are identified.
holmes/plugins/toolsets/datadog/toolset_datadog_rds.py (1)
Learnt from: nherment
PR: robusta-dev/holmesgpt#535
File: holmes/plugins/toolsets/bash/bash_toolset.py:207-209
Timestamp: 2025-06-24T05:51:04.543Z
Learning: The init_config method in toolsets should be idempotent - safely callable multiple times without errors. self.config should maintain consistent typing (not alternate between dict and config object types) throughout the object lifecycle.
🧬 Code Graph Analysis (1)
holmes/plugins/toolsets/__init__.py (1)
holmes/plugins/toolsets/datadog/toolset_datadog_rds.py (1)
DatadogRDSToolset(581-657)
🪛 GitHub Actions: Build and test HolmesGPT
holmes/plugins/toolsets/datadog/datadog_rds_instructions.jinja2
[error] 79-79: Pre-commit hook 'end-of-file-fixer' modified file to fix missing newline at end of file.
holmes/plugins/toolsets/datadog/toolset_datadog_rds.py
[error] 137-564: Mypy type checking errors: Unsupported target for indexed assignment, attribute access on None or object types, and incompatible argument types found in multiple lines.
🪛 Ruff (0.12.2)
tests/plugins/toolsets/datadog/rds/test_datadog_rds_live.py
14-14: datetime.datetime imported but unused
Remove unused import
(F401)
14-14: datetime.timezone imported but unused
Remove unused import
(F401)
14-14: datetime.timedelta imported but unused
Remove unused import
(F401)
tests/plugins/toolsets/datadog/rds/test_datadog_rds_integration.py
9-9: unittest.mock.Mock imported but unused
Remove unused import: unittest.mock.Mock
(F401)
⏰ 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: llm_evals
🔇 Additional comments (1)
holmes/plugins/toolsets/__init__.py (1)
24-26: LGTM!The import and registration of
DatadogRDSToolsetfollows the established pattern for other Datadog toolsets.Also applies to: 83-83
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
holmes/plugins/toolsets/datadog/datadog_rds_instructions.jinja2 (1)
45-82: Add missing newline at end of fileThe content provides excellent guidance on scenarios, result interpretation, and workflows. However, the file still lacks a trailing newline character, which was flagged in a previous review.
Apply this diff to add the missing newline:
-Always consider the time range - recent data (last hour) for current issues, longer ranges (last 24 hours) for trends. +Always consider the time range - recent data (last hour) for current issues, longer ranges (last 24 hours) for trends. +
🧹 Nitpick comments (1)
holmes/plugins/toolsets/datadog/datadog_rds_instructions.jinja2 (1)
36-44: Consider reviewing the memory threshold.The performance thresholds are well-defined and reasonable. However, the 100MB freeable memory warning threshold might be too aggressive for larger RDS instances (e.g., instances with 64GB+ RAM). Consider making this threshold relative to total memory or instance class.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
holmes/plugins/toolsets/datadog/datadog_rds_instructions.jinja2(1 hunks)holmes/plugins/toolsets/datadog/toolset_datadog_rds.py(1 hunks)tests/plugins/toolsets/datadog/rds/test_datadog_rds_integration.py(1 hunks)tests/plugins/toolsets/datadog/rds/test_datadog_rds_live.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/plugins/toolsets/datadog/rds/test_datadog_rds_live.py
- tests/plugins/toolsets/datadog/rds/test_datadog_rds_integration.py
- holmes/plugins/toolsets/datadog/toolset_datadog_rds.py
🔇 Additional comments (2)
holmes/plugins/toolsets/datadog/datadog_rds_instructions.jinja2 (2)
1-4: LGTM!Clear and informative introduction that sets proper expectations for the Datadog RDS analysis tools.
5-18: LGTM!Well-structured tool descriptions that clearly explain functionality and expected outputs for both RDS analysis tools.
2 participants
No description provided.