ROB-1740: datadog metrics toolset - #645
Conversation
…estamp for datadog API
WalkthroughThis update introduces a new Datadog integration for Holmes, splitting logs and metrics functionality into separate, robust toolsets. It removes the old monolithic Datadog plugin, implements new API clients with retry and rate limit handling, and adds extensive tests. Documentation for creating toolsets and using Datadog metrics tools is provided, and dependencies are updated to include Changes
Sequence Diagram(s)sequenceDiagram
participant Holmes
participant DatadogLogsToolset
participant DatadogAPI
participant Datadog
Holmes->>DatadogLogsToolset: fetch_pod_logs(params)
DatadogLogsToolset->>DatadogAPI: execute_datadog_http_request()
DatadogAPI->>Datadog: HTTP POST /logs-queries/list
Datadog-->>DatadogAPI: JSON logs response / 429 error
DatadogAPI-->>DatadogLogsToolset: logs data or error
DatadogLogsToolset-->>Holmes: formatted logs or error
sequenceDiagram
participant Holmes
participant DatadogMetricsToolset
participant DatadogAPI
participant Datadog
Holmes->>DatadogMetricsToolset: list_active_metrics/query_metrics/get_metric_metadata
DatadogMetricsToolset->>DatadogAPI: execute_datadog_http_request()
DatadogAPI->>Datadog: HTTP GET /metrics or /query or /metrics/{name}
Datadog-->>DatadogAPI: JSON metrics/metadata or 429 error
DatadogAPI-->>DatadogMetricsToolset: metrics data or error
DatadogMetricsToolset-->>Holmes: formatted metrics or error
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (5)
tests/plugins/toolsets/datadog/logs/test_fetch_pod_logs.py (1)
93-93: Fix unused loop variable.The loop variable
iis not used within the loop body. Use an underscore to indicate it's intentionally unused.- for i, api_call in enumerate(mock_post.call_args_list): + for _, api_call in enumerate(mock_post.call_args_list):.claude/commands/create-toolset.md (3)
21-21: Fix typo: "pydandic" → "pydantic"-A toolset should not depend on env vars (some existing toolsets depend on end vars but this is not a good practice to follow). -b. Implement a live healthcheck in a `prerequisite_check()` method. The health check should be contained in a dedicated method that is called by `prerequisite_check()`. `prerequisite_check()` should also make sure the config has the expected format. This is done by passing the user's config into a pydandic model: `MyToolsetConfigPydanticBaseModel(**config)`. +A toolset should not depend on env vars (some existing toolsets depend on env vars but this is not a good practice to follow). +b. Implement a live healthcheck in a `prerequisite_check()` method. The health check should be contained in a dedicated method that is called by `prerequisite_check()`. `prerequisite_check()` should also make sure the config has the expected format. This is done by passing the user's config into a pydantic model: `MyToolsetConfigPydanticBaseModel(**config)`.
63-63: Fix duplicate section numbering: "6. Tests" → "7. Tests"-# 6. Tests +# 7. Tests
66-69: Fix list indentation and typoThe list items should not be indented, and there's a typo on line 69.
1. Implement a live test. The test should: - - Depend on env variables (never put any credentials in the code). Ask if you don't have access to the correct env vars. - - Test the health check (through toolset.check_prerequisites()) - - Test that each tool returns data as expected (verify that the data looks right) -2. Implement integration tests. Because you havew actually verified that the data returned by the system is what you expect, you can now mock its behaviour and implement integration tests for each tool and for different scenarios. +- Depend on env variables (never put any credentials in the code). Ask if you don't have access to the correct env vars. +- Test the health check (through toolset.check_prerequisites()) +- Test that each tool returns data as expected (verify that the data looks right) +2. Implement integration tests. Because you have actually verified that the data returned by the system is what you expect, you can now mock its behaviour and implement integration tests for each tool and for different scenarios.tests/plugins/toolsets/datadog/metrics/test_datadog_metrics_live.py (1)
190-190: Remove unnecessary f-string prefix- print(f"\nMetadata query results:") + print("\nMetadata query results:")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
.claude/commands/create-toolset.md(1 hunks)holmes/plugins/prompts/_fetch_logs.jinja2(3 hunks)holmes/plugins/toolsets/__init__.py(2 hunks)holmes/plugins/toolsets/datadog.py(0 hunks)holmes/plugins/toolsets/datadog/datadog_api.py(1 hunks)holmes/plugins/toolsets/datadog/datadog_metrics_instructions.jinja2(1 hunks)holmes/plugins/toolsets/datadog/toolset_datadog_logs.py(1 hunks)holmes/plugins/toolsets/datadog/toolset_datadog_metrics.py(1 hunks)holmes/plugins/toolsets/logging_utils/logging_api.py(1 hunks)holmes/plugins/toolsets/utils.py(1 hunks)pyproject.toml(1 hunks)tests/plugins/toolsets/datadog/logs/test_check_prerequisites.py(1 hunks)tests/plugins/toolsets/datadog/logs/test_fetch_pod_logs.py(1 hunks)tests/plugins/toolsets/datadog/logs/test_utils.py(1 hunks)tests/plugins/toolsets/datadog/metrics/test_datadog_metrics.py(1 hunks)tests/plugins/toolsets/datadog/metrics/test_datadog_metrics_live.py(1 hunks)
💤 Files with no reviewable changes (1)
- holmes/plugins/toolsets/datadog.py
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.
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/logging_utils/logging_api.py (1)
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:100-102
Timestamp: 2025-05-15T05:14:06.519Z
Learning: The `fetch_logs` method in KubernetesLogsToolset is designed to apply the limit parameter after filtering and combining both current and previous logs, rather than using the API's tail_lines parameter, to ensure the limit applies to the final combined log set.
holmes/plugins/prompts/_fetch_logs.jinja2 (2)
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:100-102
Timestamp: 2025-05-15T05:14:06.519Z
Learning: The `fetch_logs` method in KubernetesLogsToolset is designed to apply the limit parameter after filtering and combining both current and previous logs, rather than using the API's tail_lines parameter, to ensure the limit applies to the final combined log set.
holmes/plugins/toolsets/__init__.py (1)
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.
tests/plugins/toolsets/datadog/logs/test_utils.py (2)
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:100-102
Timestamp: 2025-05-15T05:14:06.519Z
Learning: The `fetch_logs` method in KubernetesLogsToolset is designed to apply the limit parameter after filtering and combining both current and previous logs, rather than using the API's tail_lines parameter, to ensure the limit applies to the final combined log set.
tests/plugins/toolsets/datadog/metrics/test_datadog_metrics.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.
tests/plugins/toolsets/datadog/logs/test_fetch_pod_logs.py (2)
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:100-102
Timestamp: 2025-05-15T05:14:06.519Z
Learning: The `fetch_logs` method in KubernetesLogsToolset is designed to apply the limit parameter after filtering and combining both current and previous logs, rather than using the API's tail_lines parameter, to ensure the limit applies to the final combined log set.
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.
tests/plugins/toolsets/datadog/logs/test_check_prerequisites.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.
.claude/commands/create-toolset.md (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.
holmes/plugins/toolsets/datadog/toolset_datadog_logs.py (1)
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.
holmes/plugins/toolsets/datadog/toolset_datadog_metrics.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 (2)
holmes/plugins/toolsets/logging_utils/logging_api.py (1)
holmes/core/tools.py (1)
ToolParameter(120-123)
tests/plugins/toolsets/datadog/logs/test_utils.py (2)
holmes/plugins/toolsets/datadog/toolset_datadog_logs.py (3)
DatadogLogsConfig(43-51)calculate_page_size(54-63)format_logs(120-129)holmes/plugins/toolsets/logging_utils/logging_api.py (1)
FetchPodLogsParams(29-35)
🪛 GitHub Actions: Build and test HolmesGPT
holmes/plugins/toolsets/datadog/datadog_metrics_instructions.jinja2
[error] 23-23: pre-commit formatting error: trailing whitespace removed
holmes/plugins/toolsets/__init__.py
[error] 15-15: pre-commit formatting error: import statement formatting fixed
tests/plugins/toolsets/datadog/metrics/test_datadog_metrics.py
[error] 185-185: pre-commit formatting error: added trailing comma
.claude/commands/create-toolset.md
[error] 35-35: pre-commit formatting error: trailing whitespace removed
holmes/plugins/toolsets/datadog/toolset_datadog_logs.py
[error] 7-7: pre-commit formatting error: removed unused import 'AnyUrl' from pydantic
holmes/plugins/toolsets/datadog/datadog_api.py
[error] 145-145: mypy: Argument "payload" to "DataDogRequestError" has incompatible type "dict[Any, Any] | None"; expected "dict[Any, Any]" [arg-type]
tests/plugins/toolsets/datadog/metrics/test_datadog_metrics_live.py
[error] 27-27: pre-commit formatting error: removed trailing whitespace
[error] 36-36: pre-commit formatting error: fixed list comprehension formatting
[error] 81-81: pre-commit formatting error: fixed list comprehension formatting
[error] 127-127: pre-commit formatting error: removed trailing whitespace
[error] 236-236: pre-commit formatting error: fixed multi-line dict formatting
[error] 260-260: pre-commit formatting error: removed trailing whitespace
holmes/plugins/toolsets/datadog/toolset_datadog_metrics.py
[error] 480-480: mypy: Item "None" of "DatadogMetricsConfig | None" has no attribute "site_api_url" [union-attr]
[error] 481-481: mypy: Argument 1 to "get_headers" has incompatible type "DatadogMetricsConfig | None"; expected "DatadogBaseConfig" [arg-type]
[error] 487-487: mypy: Item "None" of "DatadogMetricsConfig | None" has no attribute "request_timeout" [union-attr]
[error] 40-40: pre-commit formatting error: removed unused import 'AnyUrl' from pydantic
[error] 78-78: pre-commit formatting error: fixed trailing commas and indentation
[error] 141-141: pre-commit formatting error: fixed line continuation and trailing commas
[error] 219-219: pre-commit formatting error: fixed line continuation and trailing commas
[error] 348-348: pre-commit formatting error: fixed list comprehension formatting
[error] 366-366: pre-commit formatting error: fixed indentation and blank lines
[error] 378-378: pre-commit formatting error: fixed blank lines
[error] 388-388: pre-commit formatting error: fixed blank lines
[error] 404-404: pre-commit formatting error: fixed blank lines
[error] 525-525: pre-commit formatting error: fixed multi-line os.path.join call formatting
🪛 Ruff (0.12.2)
tests/plugins/toolsets/datadog/logs/test_fetch_pod_logs.py
93-93: Loop control variable i not used within loop body
(B007)
holmes/plugins/toolsets/datadog/toolset_datadog_logs.py
10-10: pydantic.AnyUrl imported but unused
Remove unused import: pydantic.AnyUrl
(F401)
tests/plugins/toolsets/datadog/metrics/test_datadog_metrics_live.py
190-190: f-string without any placeholders
Remove extraneous f prefix
(F541)
holmes/plugins/toolsets/datadog/toolset_datadog_metrics.py
14-14: pydantic.AnyUrl imported but unused
Remove unused import
(F401)
14-14: pydantic.BaseModel imported but unused
Remove unused import
(F401)
🪛 LanguageTool
.claude/commands/create-toolset.md
[grammar] ~21-~21: Ensure spelling is correct
Context: ...one by passing the user's config into a pydandic model: `MyToolsetConfigPydanticBaseMode...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~69-~69: Ensure spelling is correct
Context: ...mplement integration tests. Because you havew actually verified that the data returne...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.17.2)
.claude/commands/create-toolset.md
66-66: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
67-67: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
68-68: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
⏰ 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). (2)
- GitHub Check: Pre-commit checks
- GitHub Check: llm_evals
🔇 Additional comments (28)
pyproject.toml (1)
60-60: LGTM! Appropriate dependency addition for retry logic.The
tenacitylibrary is well-suited for implementing retry logic with backoff strategies, which aligns with the new Datadog API client's rate limit handling requirements.holmes/plugins/toolsets/logging_utils/logging_api.py (2)
63-63: Improved parameter description clarity.Good change from "timestamp" to "datetime" - this better reflects that the parameter expects RFC3339 formatted datetime strings rather than Unix timestamps.
68-68: Consistent documentation improvement.This change maintains consistency with the start_time parameter description update, clearly indicating the expected RFC3339 datetime format.
holmes/plugins/prompts/_fetch_logs.jinja2 (3)
6-6: Proper integration of new datadog/logs toolset.The variable declaration follows the established pattern for other log toolsets, correctly filtering for the "datadog/logs" toolset name.
23-24: Consistent conditional logic for datadog logs.The conditional block properly checks for the datadog toolset existence and enabled status, then includes the default log prompt template consistent with other log toolsets.
39-39: Complete integration with recommended toolsets.Adding "datadog/logs" to the recommended toolsets list ensures users are aware of this option when configuring log access.
holmes/plugins/toolsets/__init__.py (2)
17-18: Good modularization of Datadog toolsets.The import path updates properly reflect the split from a monolithic datadog.py into separate specialized toolsets for logs and metrics, improving maintainability and separation of concerns.
73-73: Appropriate addition of metrics toolset.Adding
DatadogMetricsToolset()to the toolsets list completes the integration of the new Datadog metrics functionality.holmes/plugins/toolsets/utils.py (2)
32-33: Improved timezone handling for naive datetime objects.Good defensive programming - explicitly assigning UTC timezone to naive datetime objects ensures consistent timestamp conversion behavior and prevents potential timezone-related issues.
39-40: Consistent timezone handling across timestamp functions.The same timezone handling logic is appropriately applied to the millisecond timestamp function, maintaining consistency between
to_unixandto_unix_ms.tests/plugins/toolsets/datadog/logs/test_utils.py (3)
1-11: LGTM - Well-structured test setup.The imports and test structure are well-organized, following pytest best practices with parameterized tests for comprehensive coverage.
12-151: Comprehensive test coverage for calculate_page_size.The parameterized test cases provide excellent coverage of different scenarios including:
- Default limits vs custom limits
- Different page sizes
- Various existing log counts
- Edge cases (limit reached, no logs)
The test logic correctly validates the page size calculation.
153-251: Well-designed format_logs test cases.The test cases effectively validate both successful log formatting (extracting message attributes) and error handling (JSON serialization fallback for malformed logs). The use of realistic Datadog log structure makes the tests more meaningful.
tests/plugins/toolsets/datadog/metrics/test_datadog_metrics.py (5)
10-22: Well-structured test setup.The test class setup follows pytest best practices with proper configuration initialization and toolset setup.
23-78: Comprehensive list_active_metrics test coverage.The tests effectively cover both basic metric listing and filtered queries, with proper verification of API call parameters and response parsing.
80-129: Well-designed query_metrics test coverage.The tests properly cover both successful metric queries and no-data scenarios, with appropriate status code validation and response handling.
131-226: Comprehensive metadata retrieval test coverage.The tests effectively cover single and multiple metric metadata retrieval, including partial failure scenarios and proper error handling.
227-322: Thorough error handling and edge case coverage.The tests effectively cover missing configuration, rate limiting, and healthcheck scenarios with proper error message validation.
tests/plugins/toolsets/datadog/logs/test_fetch_pod_logs.py (4)
14-100: Excellent pagination test coverage.The test properly simulates pagination with cursors and validates the correct handling of multiple API calls and log ordering.
148-203: Well-designed storage tier fallback tests.The test effectively validates the fallback behavior across different storage tiers, ensuring proper API calls and response handling.
204-322: Comprehensive rate limiting test coverage.The tests effectively cover rate limiting scenarios with proper retry behavior, header handling, and error message validation.
323-360: Good coverage of remaining edge cases.The tests for missing configuration and search filtering provide solid coverage of important functionality with proper error handling validation.
tests/plugins/toolsets/datadog/logs/test_check_prerequisites.py (3)
13-227: Comprehensive configuration validation test coverage.The tests effectively cover all configuration validation scenarios including missing fields, invalid types, and edge cases with proper error message validation.
59-154: Thorough healthcheck test coverage.The tests effectively cover all healthcheck scenarios including success, error conditions, and exception handling with proper status validation.
155-237: Good integration and custom configuration coverage.The tests effectively validate custom configuration handling and integration with the prerequisite system, ensuring proper toolset initialization.
holmes/plugins/toolsets/datadog/datadog_api.py (1)
68-118: Well-implemented retry logic for rate limitingThe custom retry predicate and wait strategy properly handle Datadog's rate limiting, including parsing the
X-RateLimit-Resetheader and falling back to incremental delays when needed.holmes/plugins/toolsets/datadog/toolset_datadog_logs.py (1)
96-118: Good implementation following Kubernetes logging conventionsThe implementation correctly fetches logs in descending order for efficiency and then reverses them to present oldest logs first, matching kubectl behavior. This aligns with the established pattern in the Kubernetes logs toolset.
holmes/plugins/toolsets/datadog/toolset_datadog_metrics.py (1)
525-530: Correct implementation of LLM instructions loadingThe method properly follows the documented pattern for loading toolset-specific instructions from a Jinja2 template.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (8)
.claude/commands/create-toolset.md (1)
35-35: Remove trailing whitespacetests/plugins/toolsets/datadog/metrics/test_datadog_metrics_live.py (4)
27-29: Remove trailing whitespace and extra blank line
53-57: Fix list comprehension formatting
135-145: Fix variable shadowing issue
283-283: Fix incorrect parameter nameholmes/plugins/toolsets/datadog/datadog_api.py (1)
148-148: Fix type error: handle None params caseholmes/plugins/toolsets/datadog/toolset_datadog_logs.py (1)
10-10: Remove unused importholmes/plugins/toolsets/datadog/toolset_datadog_metrics.py (1)
154-154: Remove duplicate RetryError importsAlso applies to: 301-301
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.claude/commands/create-toolset.md(1 hunks)holmes/plugins/toolsets/__init__.py(2 hunks)holmes/plugins/toolsets/datadog/datadog_api.py(1 hunks)holmes/plugins/toolsets/datadog/datadog_metrics_instructions.jinja2(1 hunks)holmes/plugins/toolsets/datadog/toolset_datadog_logs.py(1 hunks)holmes/plugins/toolsets/datadog/toolset_datadog_metrics.py(1 hunks)tests/plugins/toolsets/datadog/metrics/test_datadog_metrics.py(1 hunks)tests/plugins/toolsets/datadog/metrics/test_datadog_metrics_live.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- holmes/plugins/toolsets/init.py
🚧 Files skipped from review as they are similar to previous changes (2)
- holmes/plugins/toolsets/datadog/datadog_metrics_instructions.jinja2
- tests/plugins/toolsets/datadog/metrics/test_datadog_metrics.py
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.
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_logs.py (1)
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.
holmes/plugins/toolsets/datadog/toolset_datadog_metrics.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.
.claude/commands/create-toolset.md (3)
Learnt from: nherment
PR: robusta-dev/holmesgpt#610
File: .github/workflows/llm-evaluation.yaml:39-42
Timestamp: 2025-07-08T08:45:41.069Z
Learning: When suggesting improvements to environment variable handling in robusta-dev/holmesgpt, check first if validation logic already exists rather than reimplementing it.
Learnt from: nherment
PR: robusta-dev/holmesgpt#436
File: tests/llm/fixtures/test_ask_holmes/42_dns_issues_steps_new_all_tools/dns_troubleshooting_instructions.md:59-60
Timestamp: 2025-06-05T06:16:37.361Z
Learning: In the holmesgpt project, 4-space indentation for nested lists in markdown files is the preferred style, not the 2-space indentation suggested by default markdownlint rules.
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.
🪛 markdownlint-cli2 (0.17.2)
.claude/commands/create-toolset.md
66-66: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
67-67: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
68-68: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
⏰ 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)
- GitHub Check: llm_evals
- GitHub Check: Pre-commit checks
- GitHub Check: Pre-commit checks
🔇 Additional comments (1)
holmes/plugins/toolsets/datadog/toolset_datadog_metrics.py (1)
14-14: Remove all unused imports from pydanticBoth
AnyUrlandBaseModelare unused and should be removed.-from tenacity import RetryErrorAlso remove the duplicate import of
RetryErrorsince it's already imported fromtenacityat line 14.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.claude/commands/create-toolset.md (1)
17-23: Fix spelling error in configuration guidanceThe technical guidance about configuration, health checks, and Pydantic validation is excellent. However, there's a spelling error that needs correction.
-The prerequisites_check should save the validated config in an attribute different than `toolset.config` to not conflicty with the existing attribute. +The prerequisites_check should save the validated config in an attribute different than `toolset.config` to not conflict with the existing attribute.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.claude/commands/create-toolset.md(1 hunks)holmes/plugins/toolsets/datadog/toolset_datadog_metrics.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.
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_metrics.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.
.claude/commands/create-toolset.md (3)
Learnt from: nherment
PR: robusta-dev/holmesgpt#610
File: .github/workflows/llm-evaluation.yaml:39-42
Timestamp: 2025-07-08T08:45:41.069Z
Learning: When suggesting improvements to environment variable handling in robusta-dev/holmesgpt, check first if validation logic already exists rather than reimplementing it.
Learnt from: nherment
PR: robusta-dev/holmesgpt#436
File: tests/llm/fixtures/test_ask_holmes/42_dns_issues_steps_new_all_tools/dns_troubleshooting_instructions.md:59-60
Timestamp: 2025-06-05T06:16:37.361Z
Learning: In the holmesgpt project, 4-space indentation for nested lists in markdown files is the preferred style, not the 2-space indentation suggested by default markdownlint rules.
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.
🪛 LanguageTool
.claude/commands/create-toolset.md
[style] ~21-~21: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...idated config in an attribute different than toolset.config to not conflicty with ...
(DIFFERENT_THAN)
[grammar] ~21-~21: Ensure spelling is correct
Context: ... different than toolset.config to not conflicty with the existing attribute. Whenever u...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~73-~73: Consider using a different verb for a more formal wording.
Context: ...ache && pre-commit run --all-files` and fix all issues related to the new code.
(FIX_RESOLVE)
🪛 markdownlint-cli2 (0.17.2)
.claude/commands/create-toolset.md
66-66: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
67-67: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
68-68: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
⏰ 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)
- GitHub Check: llm_evals
- GitHub Check: Pre-commit checks
- GitHub Check: Pre-commit checks
🔇 Additional comments (13)
holmes/plugins/toolsets/datadog/toolset_datadog_metrics.py (7)
1-30: Well-organized importsThe import structure is clean and follows good practices with proper organization of standard library, third-party, and local imports.
32-41: Configuration structure looks goodThe configuration classes are well-designed with appropriate inheritance from DatadogBaseConfig and proper typing for the toolset reference.
47-183: Excellent implementation of ListActiveMetrics toolThe tool is well-implemented with:
- Proper parameter validation and time processing
- Comprehensive error handling for rate limits (429) and permissions (403)
- Clean retry logic with proper exception handling
- User-friendly formatted output with sorted metrics
- Good use of structured tool results
185-323: Solid QueryMetrics implementationThe tool follows consistent patterns with excellent features:
- Required query parameter with proper validation
- Appropriate NO_DATA handling for empty results
- Well-structured JSON response format
- Consistent error handling patterns matching other tools
325-448: Well-designed QueryMetricsMetadata toolExcellent implementation that handles multiple metrics gracefully:
- Proper parsing of comma-separated metric names with filtering
- Individual API calls with comprehensive error aggregation
- Specific handling for 404 (metric not found) vs other errors
- Informative response structure with success/failure statistics
450-532: Comprehensive DatadogMetricsToolset implementationThe toolset class is well-implemented with:
- Proper initialization with all required tools and metadata
- Health check that validates API connectivity via
/api/v1/validate- Prerequisites method that validates configuration using Pydantic models
- Clear example configuration structure
- Proper instruction loading from Jinja2 templates
The toolset follows the established patterns and includes appropriate error handling.
470-495: Health check method correctly implementedThe health check method properly:
- Accepts the validated configuration as a parameter
- Uses Datadog's
/api/v1/validateendpoint for connectivity testing- Provides clear success/failure responses with appropriate logging
- Handles exceptions gracefully with informative error messages
.claude/commands/create-toolset.md (6)
1-6: Clear and informative introductionThe introduction effectively explains the purpose of toolsets for HolmesGPT and sets appropriate expectations for the procedural instructions that follow.
7-11: Good guidance on examining existing patternsThe advice to check existing toolsets and follow the BasePodLoggingToolset pattern for log-fetching toolsets is valuable and promotes consistency.
12-16: Clear organizational guidanceThe folder structure recommendations are well-thought-out and provide concrete examples that make it easy to understand where to place new toolsets.
24-31: Excellent methodology for tool definitionThe three-step approach (user intention, similar toolsets, documentation research) provides a comprehensive framework for determining required tools. The mention of subagents is appropriate for the AI workflow.
32-62: Comprehensive implementation guidanceExcellent advice covering:
- Incremental implementation approach
- Parameter design with sane defaults and RFC3339 date handling
- Clear distinction between BasePodLoggingToolset and other toolsets
- Practical code example for loading LLM instructions
The guidance promotes consistency and best practices.
63-74: Comprehensive testing and quality guidelinesExcellent coverage of testing practices including:
- Live testing with environment variables (never hardcoded credentials)
- Integration testing based on verified live test results
- Data correctness verification
- Proper linting workflow with pre-commit hooks
The emphasis on security (no hardcoded credentials) and verification is particularly valuable.
2 participants
Should be merged after #636