Skip to content

Conversation

@Sheeproid
Copy link
Contributor

@Sheeproid Sheeproid commented Oct 23, 2025

No description provided.

Uses this config
toolsets:
  grafana/dashboards:
    enabled: true
    config:
      url: http://localhost:8118
      healthcheck: "healthz"
      api_key:  <Service account token>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 23, 2025

Walkthrough

Adds a Grafana dashboards toolset and config, a BaseGrafanaTool with a unified request pipeline, four dashboard tools (SearchDashboards, GetDashboardByUID, GetHomeDashboard, GetDashboardTags), a Jinja2 toolset template, documentation/navigation, a small docstring fix, and a Toolset helper to load LLM instruction files from disk.

Changes

Cohort / File(s) Change Summary
Grafana toolset & config
holmes/plugins/toolsets/grafana/toolset_grafana.py
Added GrafanaDashboardConfig (healthcheck default "api/health") and GrafanaToolset (internal name "grafana/dashboards") exposing dashboard tools.
Base request & tool base
holmes/plugins/toolsets/grafana/toolset_grafana.py
Added BaseGrafanaTool (abstract) providing _make_grafana_request to centralize URL composition, Authorization header, HTTP GET, and return StructuredToolResult.
Dashboard tools (new)
holmes/plugins/toolsets/grafana/toolset_grafana.py
Added SearchDashboards, GetDashboardByUID, GetHomeDashboard, GetDashboardTags; each implements _invoke (uses _make_grafana_request) and get_parameterized_one_liner.
Dashboard tool template
holmes/plugins/toolsets/grafana/toolset_grafana_dashboard.jinja2
Added Jinja2 template with guidance/rules for extracting and executing Prometheus/dashboard queries (instructional content only).
Docs navigation & pages
docs/data-sources/builtin-toolsets/.nav.yml, docs/data-sources/builtin-toolsets/grafanadashboards.md, docs/data-sources/builtin-toolsets/index.md
Added Grafana Dashboards documentation entry and page describing prerequisites, configuration, capabilities, examples, and extraction behavior.
Toolset helper
holmes/core/tools.py
Added Toolset._load_llm_instructions_from_file(file_dir: str, filename: str) which resolves a local path and delegates to existing loader via a file:// URL.
Docstring fix
holmes/plugins/toolsets/grafana/common.py
Minor wording correction in GrafanaConfig docstring ("it is assumed").

Sequence Diagram(s)

sequenceDiagram
    participant Agent as Agent/User
    participant Tool as GrafanaTool (Search/Get/etc)
    participant Base as BaseGrafanaTool._make_grafana_request
    participant Grafana as Grafana API

    Agent->>Tool: invoke(params, context)
    Note right of Tool #eef7ff: compose endpoint & query_params
    Tool->>Base: _make_grafana_request(endpoint, params, query_params)
    Base->>Grafana: HTTP GET (Authorization header, built URL)
    Grafana-->>Base: response / error
    Base-->>Tool: StructuredToolResult
    Tool-->>Agent: StructuredToolResult
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Points to review closely:

  • BaseGrafanaTool._make_grafana_request: URL composition, Authorization header handling, timeouts and error-to-StructuredToolResult mapping.
  • SearchDashboards: parameter assembly and query param encoding (query, tag, type, dashboardIds/UIDs, folderUIDs, starred, limit, page).
  • Endpoints used: /api/search, /api/dashboards/uid/{uid}, /api/dashboards/home, /api/dashboards/tags.
  • GrafanaToolset initialization and config_class wiring.
  • Toolset._load_llm_instructions_from_file: path resolution and correct delegation to the file:// loader.

Suggested reviewers

  • aantn
  • arikalon1

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description Check ❓ Inconclusive No pull request description was provided by the author. The evaluation criteria for this very lenient check require only that the description be "not completely off-topic" and related to the changeset in some way. However, since there is no description present at all, it is impossible to verify whether it is related to the changeset. This falls outside the clearly defined pass/fail criteria and cannot be conclusively assessed. Consider adding a brief pull request description that outlines the purpose and scope of these four new Grafana tools. Even a minimal description explaining why these tools were added and what problems they solve would help reviewers understand the changeset more quickly.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "Add 4 tools for getting grafana dashboards" accurately reflects the main objective of the changeset. The changes introduce exactly four new Grafana tools (SearchDashboards, GetDashboardByUID, GetHomeDashboard, and GetDashboardTags) within a new GrafanaToolset. The title is concise, specific, and clearly communicates the primary change without unnecessary details or vague terminology.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch grafana-dashboard-ts

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d95183e and 94c1746.

📒 Files selected for processing (1)
  • holmes/plugins/toolsets/grafana/toolset_grafana.py (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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:

  • holmes/plugins/toolsets/grafana/toolset_grafana.py
holmes/plugins/toolsets/**/*

📄 CodeRabbit inference engine (CLAUDE.md)

Toolsets must be located as holmes/plugins/toolsets/{name}.yaml or holmes/plugins/toolsets/{name}/

Files:

  • holmes/plugins/toolsets/grafana/toolset_grafana.py
🧬 Code graph analysis (1)
holmes/plugins/toolsets/grafana/toolset_grafana.py (4)
holmes/plugins/toolsets/grafana/common.py (3)
  • get_base_url (52-56)
  • GrafanaConfig (9-22)
  • build_headers (25-36)
holmes/plugins/toolsets/utils.py (1)
  • toolset_name_for_one_liner (232-236)
holmes/plugins/toolsets/grafana/base_grafana_toolset.py (1)
  • BaseGrafanaToolset (11-54)
holmes/core/tools.py (6)
  • _load_llm_instructions_from_file (770-778)
  • Tool (173-365)
  • StructuredToolResult (79-103)
  • StructuredToolResultStatus (52-76)
  • ToolParameter (155-161)
  • ToolInvokeContext (164-170)
🪛 Ruff (0.14.1)
holmes/plugins/toolsets/grafana/toolset_grafana.py

154-154: Unused method argument: context

(ARG002)


191-191: Unused method argument: params

(ARG002)


210-210: Unused method argument: context

(ARG002)


227-227: Unused method argument: context

(ARG002)


230-230: Unused method argument: params

(ARG002)


243-243: Unused method argument: context

(ARG002)


246-246: Unused method argument: params

(ARG002)

⏰ 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: Pre-commit checks
  • GitHub Check: llm_evals
  • GitHub Check: build
🔇 Additional comments (2)
holmes/plugins/toolsets/grafana/toolset_grafana.py (2)

99-193: Implementation looks correct.

The parameter handling logic properly builds query parameters, including splitting comma-separated lists for dashboardIds, dashboardUIDs, and folderUIDs. The starred boolean conversion to string is appropriate for query parameters.

Note: The unused context and params parameters flagged by static analysis are part of the Tool interface and can be safely ignored.


195-247: LGTM - Clean tool implementations.

All three tool classes (GetDashboardByUID, GetHomeDashboard, GetDashboardTags) correctly implement the Tool interface and use the unified _make_grafana_request method. The get_parameterized_one_liner implementations provide good user-facing descriptions.

Note: Unused parameter warnings from static analysis are expected for interface methods.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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)
holmes/plugins/toolsets/grafana/toolset_grafana.py (5)

107-141: Simplify query param building; accept both arrays and CSV; pass booleans and clamp limit.

Support both array inputs (preferred) and legacy comma-separated strings. Keep booleans as bool and clamp limit to API cap.

-        query_params = {}
-        if params.get("query"):
-            query_params["query"] = params["query"]
-        if params.get("tag"):
-            query_params["tag"] = params["tag"]
-        if params.get("type"):
-            query_params["type"] = params["type"]
-        if params.get("dashboardIds"):
-            # Check if dashboardIds also needs to be passed as multiple params
-            dashboard_ids = params["dashboardIds"].split(",")
-            query_params["dashboardIds"] = [
-                dashboard_id.strip()
-                for dashboard_id in dashboard_ids
-                if dashboard_id.strip()
-            ]
-        if params.get("dashboardUIDs"):
-            # Handle dashboardUIDs as a list - split comma-separated values
-            dashboard_uids = params["dashboardUIDs"].split(",")
-            query_params["dashboardUIDs"] = [
-                uid.strip() for uid in dashboard_uids if uid.strip()
-            ]
-        if params.get("folderUIDs"):
-            # Check if folderUIDs also needs to be passed as multiple params
-            folder_uids = params["folderUIDs"].split(",")
-            query_params["folderUIDs"] = [
-                uid.strip() for uid in folder_uids if uid.strip()
-            ]
-        if params.get("starred") is not None:
-            query_params["starred"] = str(params["starred"]).lower()
-        if params.get("limit"):
-            query_params["limit"] = params["limit"]
-        if params.get("page"):
-            query_params["page"] = params["page"]
+        query_params: Dict = {}
+        for key in ("query", "tag", "type"):
+            if params.get(key) is not None:
+                query_params[key] = params[key]
+
+        def to_list(v):
+            if v is None:
+                return None
+            if isinstance(v, list):
+                return [x for x in v if str(x).strip()]
+            if isinstance(v, str):
+                return [x.strip() for x in v.split(",") if x.strip()]
+            return None
+
+        for key in ("dashboardIds", "dashboardUIDs", "folderUIDs"):
+            seq = to_list(params.get(key))
+            if seq:
+                query_params[key] = seq
+
+        if params.get("starred") is not None:
+            query_params["starred"] = bool(params["starred"])
+
+        if params.get("limit") is not None:
+            try:
+                limit = int(params["limit"])
+                query_params["limit"] = max(1, min(5000, limit))
+            except (TypeError, ValueError):
+                pass
+        if params.get("page") is not None:
+            try:
+                query_params["page"] = int(params["page"])
+            except (TypeError, ValueError):
+                pass

107-107: Silence Ruff ARG002 by renaming unused params to “_”.

Keeps signatures intact while removing linter noise.

-def _invoke(self, params: dict, context: ToolInvokeContext) -> StructuredToolResult:
+def _invoke(self, params: dict, _: ToolInvokeContext) -> StructuredToolResult:
@@
-def get_parameterized_one_liner(self, params: Dict) -> str:
+def get_parameterized_one_liner(self, _: Dict) -> str:
@@
-def _invoke(self, params: dict, context: ToolInvokeContext) -> StructuredToolResult:
+def _invoke(self, params: dict, _: ToolInvokeContext) -> StructuredToolResult:
@@
-def _invoke(self, params: dict, context: ToolInvokeContext) -> StructuredToolResult:
+def _invoke(self, params: dict, _: ToolInvokeContext) -> StructuredToolResult:
@@
-def get_parameterized_one_liner(self, params: Dict) -> str:
+def get_parameterized_one_liner(self, _: Dict) -> str:
@@
-def _invoke(self, params: dict, context: ToolInvokeContext) -> StructuredToolResult:
+def _invoke(self, params: dict, _: ToolInvokeContext) -> StructuredToolResult:
@@
-def get_parameterized_one_liner(self, params: Dict) -> str:
+def get_parameterized_one_liner(self, _: Dict) -> str:

As per coding guidelines (Ruff).

Also applies to: 144-144, 163-163, 180-180, 183-183, 196-196, 199-199


12-12: Remove unnecessary “type: ignore” on requests import.

requests is typed; prefer clean import.

-import requests  # type: ignore
+import requests

As per typing/mypy hygiene.


206-215: Set docs_url to Dashboard HTTP API and replace icon with official Grafana asset.

The toolset provides dashboard operations (search, get by UID, home, tags). Set docs_url to https://grafana.com/docs/grafana/latest/developers/http_api/dashboard/ for primary dashboard API reference. Replace the third-party CDN icon with an official Grafana-hosted asset (e.g., https://grafana.com/media/docs/grafana-icon.png or equivalent from Grafana's official assets).


59-103: Use native array types and enum constraints for improved API alignment and UX.

The Grafana HTTP API expects arrays for dashboardUIDs and folderUIDs across v9–v11 and accepts repeated query parameters. Update parameter definitions to reflect this and add enum constraint for type:

         "type": ToolParameter(
             description="Filter by type: 'dash-folder' or 'dash-db'",
             type="string",
+            enum=["dash-folder", "dash-db"],
             required=False,
         ),
         "dashboardIds": ToolParameter(
-            description="List of dashboard IDs to filter (comma-separated)",
-            type="string",
+            description="Dashboard numeric IDs to filter",
+            type="array",
+            items=ToolParameter(type="integer"),
             required=False,
         ),
         "dashboardUIDs": ToolParameter(
-            description="List of dashboard UIDs to search for (comma-separated)",
-            type="string",
+            description="Dashboard UIDs to search for",
+            type="array",
+            items=ToolParameter(type="string"),
             required=False,
         ),
         "folderUIDs": ToolParameter(
-            description="List of folder UIDs to search within (comma-separated)",
-            type="string",
+            description="Folder UIDs to search within",
+            type="array",
+            items=ToolParameter(type="string"),
             required=False,
         ),

The ToolParameter class supports both enum and items attributes. Update the _invoke() method to handle actual array inputs directly instead of parsing comma-separated strings.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2fa86b9 and 4fb560b.

📒 Files selected for processing (1)
  • holmes/plugins/toolsets/grafana/toolset_grafana.py (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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:

  • holmes/plugins/toolsets/grafana/toolset_grafana.py
holmes/plugins/toolsets/**/*

📄 CodeRabbit inference engine (CLAUDE.md)

Toolsets must be located as holmes/plugins/toolsets/{name}.yaml or holmes/plugins/toolsets/{name}/

Files:

  • holmes/plugins/toolsets/grafana/toolset_grafana.py
🧬 Code graph analysis (1)
holmes/plugins/toolsets/grafana/toolset_grafana.py (3)
holmes/core/tools.py (5)
  • Tool (173-365)
  • StructuredToolResult (79-103)
  • StructuredToolResultStatus (52-76)
  • ToolParameter (155-161)
  • ToolInvokeContext (164-170)
holmes/plugins/toolsets/grafana/base_grafana_toolset.py (1)
  • BaseGrafanaToolset (11-54)
holmes/plugins/toolsets/utils.py (1)
  • toolset_name_for_one_liner (232-236)
🪛 Ruff (0.14.1)
holmes/plugins/toolsets/grafana/toolset_grafana.py

40-40: Probable use of requests call without timeout

(S113)


107-107: Unused method argument: context

(ARG002)


144-144: Unused method argument: params

(ARG002)


163-163: Unused method argument: context

(ARG002)


180-180: Unused method argument: context

(ARG002)


183-183: Unused method argument: params

(ARG002)


196-196: Unused method argument: context

(ARG002)


199-199: Unused method argument: params

(ARG002)

⏰ 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: build
🔇 Additional comments (1)
holmes/plugins/toolsets/grafana/toolset_grafana.py (1)

17-23: Good abstraction: shared HTTP plumbing in BaseGrafanaTool.

Centralizing request logic reduces duplication and standardizes results. LGTM.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
holmes/plugins/toolsets/grafana/toolset_grafana.py (1)

53-78: Critical: Still missing timeout, error handling, and has URL join issue.

This method has the same critical issues previously flagged:

  • Line 69: No timeout risks indefinite hangs on request threads.
  • Line 66: Using urljoin with endpoints that have leading "/" can drop subpaths from the base URL.
  • Lines 69-71: No exception handling for HTTP errors, connection failures, or JSON decode errors will cause the tool to crash.

As per the previous review, apply error handling with timeout, fix URL joining to preserve subpaths, and catch HTTP/connection/JSON exceptions. The suggested diff in the past review comment provides a complete solution.

🧹 Nitpick comments (1)
holmes/plugins/toolsets/grafana/toolset_grafana.py (1)

192-194: Consider URL-encoding the uid parameter.

While Grafana UIDs are typically alphanumeric, the uid parameter is directly interpolated into the URL path without encoding. If a UID contains special characters, this could cause issues.

Apply this diff to add defensive URL encoding:

+from urllib.parse import quote
 def _invoke(self, params: dict, context: ToolInvokeContext) -> StructuredToolResult:
     uid = params["uid"]
-    return self._make_grafana_request(f"/api/dashboards/uid/{uid}", params)
+    return self._make_grafana_request(f"/api/dashboards/uid/{quote(uid, safe='')}", params)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4fb560b and ce7e1c0.

📒 Files selected for processing (1)
  • holmes/plugins/toolsets/grafana/toolset_grafana.py (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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:

  • holmes/plugins/toolsets/grafana/toolset_grafana.py
holmes/plugins/toolsets/**/*

📄 CodeRabbit inference engine (CLAUDE.md)

Toolsets must be located as holmes/plugins/toolsets/{name}.yaml or holmes/plugins/toolsets/{name}/

Files:

  • holmes/plugins/toolsets/grafana/toolset_grafana.py
🧬 Code graph analysis (1)
holmes/plugins/toolsets/grafana/toolset_grafana.py (4)
holmes/plugins/toolsets/grafana/common.py (2)
  • get_base_url (52-56)
  • GrafanaConfig (9-22)
holmes/plugins/toolsets/utils.py (1)
  • toolset_name_for_one_liner (232-236)
holmes/plugins/toolsets/grafana/base_grafana_toolset.py (1)
  • BaseGrafanaToolset (11-54)
holmes/core/tools.py (5)
  • Tool (173-365)
  • StructuredToolResult (79-103)
  • StructuredToolResultStatus (52-76)
  • ToolParameter (155-161)
  • ToolInvokeContext (164-170)
🪛 Ruff (0.14.1)
holmes/plugins/toolsets/grafana/toolset_grafana.py

69-69: Probable use of requests call without timeout

(S113)


136-136: Unused method argument: context

(ARG002)


173-173: Unused method argument: params

(ARG002)


192-192: Unused method argument: context

(ARG002)


209-209: Unused method argument: context

(ARG002)


212-212: Unused method argument: params

(ARG002)


225-225: Unused method argument: context

(ARG002)


228-228: Unused method argument: params

(ARG002)

⏰ 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: build
🔇 Additional comments (4)
holmes/plugins/toolsets/grafana/toolset_grafana.py (4)

18-21: LGTM!

The configuration class correctly extends GrafanaConfig and provides a sensible default healthcheck endpoint for the dashboard toolset.


24-43: LGTM!

The toolset class is well-structured with proper config_class declaration, tool initialization, and a type-safe property for accessing the configuration.


81-174: LGTM!

The search implementation correctly builds query parameters from various optional filters, properly handles comma-separated list inputs, and follows the expected tool interface.

Note: The Ruff warnings about unused context and params parameters are false positives—these are required by the abstract Tool interface.


200-229: LGTM!

Both GetHomeDashboard and GetDashboardTags are clean, straightforward implementations that correctly use the base class's request helper.

Note: The Ruff warnings about unused context and params parameters are false positives—these are required by the abstract Tool interface.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
holmes/plugins/toolsets/grafana/toolset_grafana.py (1)

58-83: Critical: Past review issues remain unaddressed—missing timeout, error handling, and safe URL joining.

The verification confirms all three issues are still present in the code:

  1. No timeout: When endpoint starts with "/" urljoin replaces the entire base path. If base URL is http://host/grafana/ and endpoint is /api/search, the result becomes http://host/api/search instead of http://host/grafana/api/search, losing the configured subpath.

  2. No timeout on requests.get (Ruff S113): The request can hang indefinitely on slow/unresponsive Grafana instances, blocking the request thread.

  3. No error handling: HTTP errors, connection failures, and JSON decode errors will crash the tool instead of returning a structured error result.

Apply the robust implementation from the previous review:

-    def _make_grafana_request(
-        self, endpoint: str, params: dict, query_params: Optional[Dict] = None
-    ) -> StructuredToolResult:
-        """Make a GET request to Grafana API and return structured result.
-
-        Args:
-            endpoint: API endpoint path (e.g., "/api/search")
-            params: Original parameters passed to the tool
-            query_params: Optional query parameters for the request
-
-        Returns:
-            StructuredToolResult with the API response data
-        """
-        url = urljoin(get_base_url(self._toolset.grafana_config), endpoint)
-        headers = {"Authorization": f"Bearer {self._toolset.grafana_config.api_key}"}
-
-        response = requests.get(url, headers=headers, params=query_params)
-        response.raise_for_status()
-        data = response.json()
-
-        return StructuredToolResult(
-            status=StructuredToolResultStatus.SUCCESS,
-            data=data,
-            url=url,
-            params=params,
-        )
+    def _make_grafana_request(
+        self, endpoint: str, params: dict, query_params: Optional[Dict] = None
+    ) -> StructuredToolResult:
+        """Make a GET request to Grafana API and return structured result.
+
+        Args:
+            endpoint: API endpoint path (e.g., "/api/search")
+            params: Original parameters passed to the tool
+            query_params: Optional query parameters for the request
+
+        Returns:
+            StructuredToolResult with the API response data
+        """
+        from json import JSONDecodeError
+        from requests import exceptions as req_exc
+        
+        # Safe URL joining that preserves subpaths
+        base_url = get_base_url(self._toolset.grafana_config).rstrip("/") + "/"
+        url = urljoin(base_url, endpoint.lstrip("/"))
+        
+        headers = {
+            "Authorization": f"Bearer {self._toolset.grafana_config.api_key}",
+            "Accept": "application/json",
+        }
+        
+        # Connect timeout: 3.05s, Read timeout: 10s
+        timeout = (3.05, 10)
+        
+        try:
+            response = requests.get(
+                url,
+                headers=headers,
+                params=query_params,
+                timeout=timeout,
+            )
+            response.raise_for_status()
+            
+            try:
+                data = response.json()
+            except JSONDecodeError:
+                data = response.text
+            
+            return StructuredToolResult(
+                status=StructuredToolResultStatus.SUCCESS,
+                data=data,
+                url=url,
+                params=params,
+                return_code=response.status_code,
+            )
+        except req_exc.Timeout as e:
+            return StructuredToolResult(
+                status=StructuredToolResultStatus.ERROR,
+                error=f"Request to Grafana timed out: {e}",
+                url=url,
+                params=params,
+            )
+        except req_exc.HTTPError as e:
+            code = e.response.status_code if hasattr(e, "response") and e.response else None
+            message = e.response.text if hasattr(e, "response") and e.response else str(e)
+            return StructuredToolResult(
+                status=StructuredToolResultStatus.ERROR,
+                error=message,
+                return_code=code,
+                url=url,
+                params=params,
+            )
+        except req_exc.RequestException as e:
+            return StructuredToolResult(
+                status=StructuredToolResultStatus.ERROR,
+                error=str(e),
+                url=url,
+                params=params,
+            )
🧹 Nitpick comments (1)
holmes/plugins/toolsets/grafana/toolset_grafana.py (1)

141-177: Consider consolidating the repetitive query parameter building logic.

The query parameter construction follows a repetitive pattern across multiple parameters. While functional, consolidating this logic could improve maintainability.

Consider extracting a helper method:

def _build_query_params(self, params: dict) -> dict:
    """Build query parameters from tool parameters."""
    query_params = {}
    
    # Simple string parameters
    for key in ["query", "tag", "type", "limit", "page"]:
        if params.get(key):
            query_params[key] = params[key]
    
    # Comma-separated list parameters
    for key in ["dashboardIds", "dashboardUIDs", "folderUIDs"]:
        if params.get(key):
            values = params[key].split(",")
            query_params[key] = [v.strip() for v in values if v.strip()]
    
    # Boolean parameters
    if params.get("starred") is not None:
        query_params["starred"] = str(params["starred"]).lower()
    
    return query_params

def _invoke(self, params: dict, context: ToolInvokeContext) -> StructuredToolResult:
    query_params = self._build_query_params(params)
    return self._make_grafana_request("/api/search", params, query_params)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ce7e1c0 and 9a625e7.

📒 Files selected for processing (3)
  • holmes/core/tools.py (1 hunks)
  • holmes/plugins/toolsets/grafana/toolset_grafana.py (2 hunks)
  • holmes/plugins/toolsets/grafana/toolset_grafana_dashboard.jinja2 (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
holmes/plugins/toolsets/**/*

📄 CodeRabbit inference engine (CLAUDE.md)

Toolsets must be located as holmes/plugins/toolsets/{name}.yaml or holmes/plugins/toolsets/{name}/

Files:

  • holmes/plugins/toolsets/grafana/toolset_grafana_dashboard.jinja2
  • holmes/plugins/toolsets/grafana/toolset_grafana.py
**/*.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:

  • holmes/core/tools.py
  • holmes/plugins/toolsets/grafana/toolset_grafana.py
🧬 Code graph analysis (1)
holmes/plugins/toolsets/grafana/toolset_grafana.py (4)
holmes/plugins/toolsets/grafana/common.py (2)
  • get_base_url (52-56)
  • GrafanaConfig (9-22)
holmes/plugins/toolsets/utils.py (1)
  • toolset_name_for_one_liner (232-236)
holmes/plugins/toolsets/grafana/base_grafana_toolset.py (1)
  • BaseGrafanaToolset (11-54)
holmes/core/tools.py (9)
  • _load_llm_instructions_from_file (770-778)
  • Tool (173-365)
  • StructuredToolResult (79-103)
  • ToolParameter (155-161)
  • _invoke (352-361)
  • _invoke (414-446)
  • ToolInvokeContext (164-170)
  • get_parameterized_one_liner (364-365)
  • get_parameterized_one_liner (391-398)
🪛 Ruff (0.14.1)
holmes/plugins/toolsets/grafana/toolset_grafana.py

74-74: Probable use of requests call without timeout

(S113)


141-141: Unused method argument: context

(ARG002)


178-178: Unused method argument: params

(ARG002)


197-197: Unused method argument: context

(ARG002)


214-214: Unused method argument: context

(ARG002)


217-217: Unused method argument: params

(ARG002)


230-230: Unused method argument: context

(ARG002)


233-233: Unused method argument: params

(ARG002)

⏰ 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: Pre-commit checks
  • GitHub Check: llm_evals
  • GitHub Check: build
🔇 Additional comments (5)
holmes/core/tools.py (1)

770-778: LGTM! Clean helper method for file-based LLM instruction loading.

The implementation correctly constructs an absolute path and delegates to the existing _load_llm_instructions method using a file:// URL scheme. The method has clear documentation and proper type hints.

holmes/plugins/toolsets/grafana/toolset_grafana_dashboard.jinja2 (1)

1-26: LGTM! Clear and structured Prometheus query execution guidance.

The template provides well-organized instructions for running Prometheus queries from Grafana dashboards, including key rules, variable substitution guidelines, and a concrete example. The documentation will help ensure consistent query execution behavior.

holmes/plugins/toolsets/grafana/toolset_grafana.py (3)

19-23: LGTM! Appropriate config specialization for dashboard toolset.

The override of healthcheck to "api/health" is intentional and well-documented, differentiating the dashboard-specific configuration from the base Grafana configuration.


25-49: LGTM! Well-structured toolset initialization.

The toolset properly initializes the base class, loads LLM instructions from the template file using the new helper method, and provides typed access to the configuration through the grafana_config property.


182-234: LGTM! Clean and straightforward tool implementations.

The three dashboard tools (GetDashboardByUID, GetHomeDashboard, GetDashboardTags) are well-implemented with proper initialization, simple delegation to the shared request helper, and appropriate user-facing descriptions.

Note: The static analysis warnings about unused context and params parameters are false positives—these parameters are required by the abstract method signatures in the base Tool class.

@Sheeproid Sheeproid force-pushed the grafana-dashboard-ts branch from a9066ae to 738a81e Compare October 27, 2025 09:56
@Sheeproid Sheeproid marked this pull request as ready for review October 27, 2025 09:56
@Sheeproid Sheeproid enabled auto-merge (squash) October 27, 2025 09:56
@Sheeproid Sheeproid requested a review from aantn October 27, 2025 09:57
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 94241cb and d95183e.

📒 Files selected for processing (2)
  • docs/data-sources/builtin-toolsets/grafanadashboards.md (1 hunks)
  • holmes/plugins/toolsets/grafana/toolset_grafana.py (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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:

  • holmes/plugins/toolsets/grafana/toolset_grafana.py
holmes/plugins/toolsets/**/*

📄 CodeRabbit inference engine (CLAUDE.md)

Toolsets must be located as holmes/plugins/toolsets/{name}.yaml or holmes/plugins/toolsets/{name}/

Files:

  • holmes/plugins/toolsets/grafana/toolset_grafana.py
docs/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

In MkDocs content, always add a blank line between a header or bold text and a following list so lists render correctly

Files:

  • docs/data-sources/builtin-toolsets/grafanadashboards.md
🧬 Code graph analysis (1)
holmes/plugins/toolsets/grafana/toolset_grafana.py (4)
holmes/plugins/toolsets/grafana/common.py (3)
  • get_base_url (52-56)
  • GrafanaConfig (9-22)
  • build_headers (25-36)
holmes/plugins/toolsets/utils.py (1)
  • toolset_name_for_one_liner (232-236)
holmes/plugins/toolsets/grafana/base_grafana_toolset.py (1)
  • BaseGrafanaToolset (11-54)
holmes/core/tools.py (10)
  • _load_llm_instructions_from_file (770-778)
  • Tool (173-365)
  • StructuredToolResult (79-103)
  • StructuredToolResultStatus (52-76)
  • ToolParameter (155-161)
  • _invoke (352-361)
  • _invoke (414-446)
  • ToolInvokeContext (164-170)
  • get_parameterized_one_liner (364-365)
  • get_parameterized_one_liner (391-398)
🪛 LanguageTool
docs/data-sources/builtin-toolsets/grafanadashboards.md

[style] ~3-~3: Consider a different adjective to strengthen your wording.
Context: ...nd extraction of Prometheus queries for deeper analysis. ## Prerequisites A [Grafana...

(DEEP_PROFOUND)

🪛 markdownlint-cli2 (0.18.1)
docs/data-sources/builtin-toolsets/grafanadashboards.md

76-76: Code block style
Expected: indented; Actual: fenced

(MD046, code-block-style)


81-81: Code block style
Expected: indented; Actual: fenced

(MD046, code-block-style)


86-86: Code block style
Expected: indented; Actual: fenced

(MD046, code-block-style)

🪛 Ruff (0.14.1)
holmes/plugins/toolsets/grafana/toolset_grafana.py

81-81: Probable use of requests call without timeout

(S113)


148-148: Unused method argument: context

(ARG002)


185-185: Unused method argument: params

(ARG002)


204-204: Unused method argument: context

(ARG002)


221-221: Unused method argument: context

(ARG002)


224-224: Unused method argument: params

(ARG002)


237-237: Unused method argument: context

(ARG002)


240-240: Unused method argument: params

(ARG002)

⏰ 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: build
🔇 Additional comments (7)
holmes/plugins/toolsets/grafana/toolset_grafana.py (6)

23-26: LGTM!

The config class correctly extends GrafanaConfig and appropriately overrides the healthcheck endpoint to api/health for dashboard-specific health checks.


29-52: LGTM!

The toolset is well-structured:

  • Config class properly wired to GrafanaDashboardConfig
  • All four tools correctly instantiated
  • Proper use of the _load_llm_instructions_from_file helper for loading LLM instructions from the Jinja2 template
  • Type-safe access to config via the property

93-186: LGTM!

The SearchDashboards tool is well-implemented:

  • Comprehensive parameter definitions matching the Grafana API
  • Correct handling of comma-separated strings for list parameters (dashboardIds, dashboardUIDs, folderUIDs)
  • Proper boolean-to-string conversion for the API
  • Clean delegation to _make_grafana_request

Note: Ruff's warnings about unused context and params are false positives—these parameters are required by the abstract method signatures from the base Tool class.


189-209: LGTM!

Clean and focused implementation for retrieving a dashboard by UID. The endpoint construction is correct and the one-liner includes the UID for better traceability.


212-225: LGTM!

Straightforward implementation for retrieving the home dashboard. No parameters needed, clean delegation to the base request method.


228-241: LGTM!

Clean implementation for retrieving dashboard tags. Follows the same pattern as GetHomeDashboard with no parameters required.

docs/data-sources/builtin-toolsets/grafanadashboards.md (1)

1-88: LGTM!

Excellent documentation for the Grafana Dashboards integration:

  • Clear prerequisites and configuration instructions for both CLI and Helm
  • Accurate capabilities table matching the four implemented tools
  • Helpful examples demonstrating practical use cases
  • Proper formatting with blank lines before lists as per coding guidelines

Note: The static analysis hints about code block style (fenced vs. indented) and the word "deeper" are false positives—fenced code blocks are the modern standard and more readable.

@github-actions
Copy link
Contributor

Results of HolmesGPT evals

  • ask_holmes: 33/35 test cases were successful, 1 regressions, 1 setup failures
Test suite Test case Status
ask 01_how_many_pods
ask 02_what_is_wrong_with_pod
ask 04_related_k8s_events
ask 05_image_version
ask 09_crashpod
ask 10_image_pull_backoff
ask 110_k8s_events_image_pull
ask 11_init_containers
ask 13a_pending_node_selector_basic
ask 14_pending_resources
ask 15_failed_readiness_probe
ask 17_oom_kill
ask 19_detect_missing_app_details
ask 20_long_log_file_search
ask 24_misconfigured_pvc
ask 24a_misconfigured_pvc_basic
ask 28_permissions_error 🚧
ask 39_failed_toolset
ask 41_setup_argo
ask 42_dns_issues_steps_new_tools
ask 43_current_datetime_from_prompt
ask 45_fetch_deployment_logs_simple
ask 51_logs_summarize_errors
ask 53_logs_find_term
ask 54_not_truncated_when_getting_pods
ask 59_label_based_counting
ask 60_count_less_than
ask 61_exact_match_counting
ask 63_fetch_error_logs_no_errors
ask 79_configmap_mount_issue
ask 83_secret_not_found
ask 86_configmap_like_but_secret
ask 93_calling_datadog[0]
ask 93_calling_datadog[1]
ask 93_calling_datadog[2]

Legend

  • ✅ the test was successful
  • :minus: the test was skipped
  • ⚠️ the test failed but is known to be flaky or known to fail
  • 🚧 the test had a setup failure (not a code regression)
  • 🔧 the test failed due to mock data issues (not a code regression)
  • 🚫 the test was throttled by API rate limits/overload
  • ❌ the test failed and should be fixed before merging the PR

@Sheeproid Sheeproid merged commit 3d07e99 into master Oct 27, 2025
8 checks passed
@Sheeproid Sheeproid deleted the grafana-dashboard-ts branch October 27, 2025 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants