Add custom query parameter support to FetchDatadogTracesList for enhanced search flexibility #883
Conversation
|
|
WalkthroughAdds an optional Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Tool as Tool (FetchDatadogTracesList / FetchDatadogSpansByFilter)
participant DD as Datadog Spans Search API
User->>Tool: Invoke with [query?, service?, operation?, resource?, min_duration?, tags?]
rect rgb(220,235,255)
note over Tool: Build search parts
Tool->>Tool: parts = [query?] + [filters...]
Tool->>Tool: if tags (dict) -> convert to @key:value terms and append
Tool->>Tool: final_query = join(parts) or "*"
end
Tool->>DD: POST /spans/search {query: final_query, time/window}
DD-->>Tool: Results
rect rgb(235,255,235)
alt Traces list
Tool->>Tool: format_traces_list(results)
else Spans filter
Tool->>Tool: format_spans_search(results)
Note right of Tool: format_spans_search renders attrs["custom"]["error"] and truncated stack when present
end
end
Tool-->>User: Formatted response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
holmes/plugins/toolsets/**/*📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧬 Code graph analysis (1)holmes/plugins/toolsets/datadog/toolset_datadog_traces.py (2)
⏰ 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)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
holmes/plugins/toolsets/datadog/toolset_datadog_traces.py (4)
208-210: Guard against empty/whitespace-only query in one-liner.Using only a key-presence check can produce “Fetch Traces ()” when query is empty. Prefer a truthy/trimmed check.
Apply this diff:
- if "query" in params: - return f"{toolset_name_for_one_liner(self.toolset.name)}: Fetch Traces ({params['query']})" + q = str(params.get("query") or "").strip() + if q: + return f"{toolset_name_for_one_liner(self.toolset.name)}: Fetch Traces ({q})"
211-221: Include resource in the one-liner filters for better signal.When only resource is provided, the one-liner still shows “all”. Add resource to the filter summary.
Apply this diff:
filters = [] if "service" in params: filters.append(f"service={params['service']}") if "operation" in params: filters.append(f"operation={params['operation']}") + if "resource" in params: + filters.append(f"resource={params['resource']}") if "min_duration" in params: filters.append(f"duration>{params['min_duration']}")
249-252: Trim and validate the base query before appending.Prevents accidental leading/trailing spaces and ignores empty strings.
Apply this diff:
- if params.get("query"): - query_parts.append(params["query"]) + q = params.get("query") + if isinstance(q, str) and q.strip(): + query_parts.append(q.strip())
246-279: DRY the query construction across traces and spans.Both tools build nearly identical query parts (service, operation, resource, tags, min_duration). A shared helper (e.g., _build_spans_query(params)) will reduce drift and ease future changes.
Example (outside this range, for illustration):
def _quote_if_needed(value: str) -> str: v = str(value) if any(c.isspace() for c in v): v = v.replace('"', r'\"') return f'"{v}"' return v def _build_spans_query(params: dict[str, Any]) -> str: parts: list[str] = [] q = str(params.get("query") or "").strip() if q: parts.append(q) if params.get("service"): parts.append(f"service:{_quote_if_needed(params['service'])}") if params.get("operation"): parts.append(f"operation_name:{_quote_if_needed(params['operation'])}") if params.get("resource"): parts.append(f"resource_name:{_quote_if_needed(params['resource'])}") if params.get("tags") and isinstance(params["tags"], dict): for k, v in params["tags"].items(): parts.append(f"@{k}:{_quote_if_needed(v)}") if params.get("min_duration"): # existing duration parsing logic... parts.append(f"@duration:>{duration_ns}") return " ".join(parts) if parts else "*"Then call this helper from both _invoke implementations.
Also applies to: 596-621
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
holmes/plugins/toolsets/datadog/toolset_datadog_traces.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py: ALWAYS place Python imports at the top of the file, not inside functions or methods
Type hints are required (project is type-checked with mypy)
Use Ruff for formatting and linting (configured in pyproject.toml)
Files:
holmes/plugins/toolsets/datadog/toolset_datadog_traces.py
🧬 Code Graph Analysis (1)
holmes/plugins/toolsets/datadog/toolset_datadog_traces.py (2)
holmes/core/tools.py (1)
ToolParameter(121-126)holmes/plugins/toolsets/utils.py (1)
toolset_name_for_one_liner(144-148)
🔇 Additional comments (2)
holmes/plugins/toolsets/datadog/toolset_datadog_traces.py (2)
162-166: Nice addition: optional raw Datadog query for traces.This keeps backward compatibility while enabling advanced searches. Matches the spans tool behavior and reads well.
254-276: Quote filter values containing spaces or special charactersUser-provided service/operation/resource values may include spaces (e.g. “web app”) or other characters that Datadog requires to be quoted. Without quotes, queries like
service:web appwill be parsed asservice:webANDapp.• Wrap each value in quotes when building the filter, e.g.:
- if params.get("service"): - query_parts.append(f"service:{params['service']}") + if params.get("service"): + svc = params["service"] + # Quote if it contains spaces or special chars + if re.search(r'[\s:"]', svc): + svc = f'"{svc}"' + query_parts.append(f"service:{svc}")• Apply the same quoting logic to
operation_nameandresource_name.• Consider extracting this into a small helper (DRY) for reuse across filters and other tools.
Note: a quick code-search didn’t find any obvious callers passing space-containing values, but absence of matches isn’t proof. Please verify that no clients supply unquoted, space-containing values for these parameters.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
holmes/plugins/toolsets/datadog/datadog_traces_formatter.py (2)
285-314: Optional: Make the stack truncation limit configurable for callersHardcoding 5 lines is okay, but allowing configuration can help during investigations. Two low-impact options:
- Module constant (no API changes):
# near the top of the module MAX_STACK_LINES = 5Then replace the slice with:
stack_lines = lines[:MAX_STACK_LINES] if len(lines) > MAX_STACK_LINES: output_lines.append(" │ ... (truncated)")
- Or add a keyword argument
max_stack_lines: int = 5toformat_spans_search(verify call sites before adopting).
285-314: Add focused tests for the new error block (happy path + edge cases)Recommend unit tests covering:
- Full error object with id/file/message/type/stack (stack > 5 lines → shows 5 + “… (truncated)”).
- Only some fields present (e.g., message only).
- Non-dict
customorerrorgracefully ignored.stackas list vs string vs empty.I can draft pytest cases if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
holmes/plugins/toolsets/datadog/datadog_traces_formatter.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: ALWAYS place Python imports at the top of the file, not inside functions or methods
Type hints are required (project is type-checked with mypy)
Use Ruff for formatting and linting (configured in pyproject.toml)
Files:
holmes/plugins/toolsets/datadog/datadog_traces_formatter.py
🪛 Ruff (0.12.2)
holmes/plugins/toolsets/datadog/datadog_traces_formatter.py
289-289: f-string without any placeholders
Remove extraneous f prefix
(F541)
313-313: f-string without any placeholders
Remove extraneous f prefix
(F541)
|
Hi @skysbsb, sorry on the delay in replying! We're actively working on improving HolmesGPT on trace data, so this is super timely. Would you be open to adding an eval that tests the functionality you added? As an example, here is one of the evals we use to test Tempo traces. The goal of the eval is to make sure HolmesGPT doesn't just have the technical ability to query the data, but it also calls the tool consistently when appropriate. Even if not, we still would like to get this merged and reviewed, we'll probably add an eval ourselves to test it first. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
holmes/plugins/toolsets/datadog/toolset_datadog_traces.py (3)
207-209: Avoid blank/overlong one-liners whenqueryis empty; use truthiness+truncatePrevents rendering
(... )for empty strings and keeps previews concise.- if "query" in params: - return f"{toolset_name_for_one_liner(self.toolset.name)}: Fetch Traces ({params['query']})" + q = params.get("query") + if q and str(q).strip(): + q_str = str(q).strip() + preview = q_str[:120] + ("…" if len(q_str) > 120 else "") + return f"{toolset_name_for_one_liner(self.toolset.name)}: Fetch Traces ({preview})"
250-253: Trim and validate custom query before appendingProtects against whitespace-only inputs.
- if params.get("query"): - query_parts.append(params["query"]) + q = params.get("query") + if q: + q = str(q).strip() + if q: + query_parts.append(q)
265-277: Hardenmin_durationparsing (support ns/us, validation, friendly error)Prevents unexpected crashes on bad input and broadens unit support.
- duration_str = params["min_duration"].lower() - if duration_str.endswith("ms"): - duration_ns = int(float(duration_str[:-2]) * 1_000_000) - elif duration_str.endswith("s"): - duration_ns = int(float(duration_str[:-1]) * 1_000_000_000) - elif duration_str.endswith("m"): - duration_ns = int(float(duration_str[:-1]) * 60 * 1_000_000_000) - else: - # Assume milliseconds if no unit - duration_ns = int(float(duration_str) * 1_000_000) + duration_str = str(params["min_duration"]).strip().lower() + units = {"ns": 1, "us": 1_000, "ms": 1_000_000, "s": 1_000_000_000, "m": 60 * 1_000_000_000} + try: + for suffix, multiplier in units.items(): + if duration_str.endswith(suffix): + value = float(duration_str[: -len(suffix)]) + break + else: + # Assume milliseconds if no unit + value = float(duration_str) + multiplier = units["ms"] + if value < 0: + return StructuredToolResult( + status=ToolResultStatus.ERROR, + error="min_duration must be >= 0", + params=params, + ) + duration_ns = int(value * multiplier) + except ValueError: + return StructuredToolResult( + status=ToolResultStatus.ERROR, + error=f"Invalid min_duration: {params['min_duration']}", + params=params, + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
holmes/plugins/toolsets/datadog/toolset_datadog_traces.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Always place Python imports at the top of the file, not inside functions or methods
Type hints are required (mypy is configured in pyproject.toml)
Files:
holmes/plugins/toolsets/datadog/toolset_datadog_traces.py
holmes/plugins/toolsets/**
📄 CodeRabbit inference engine (CLAUDE.md)
Toolsets must live under holmes/plugins/toolsets as either {name}.yaml or a {name}/ directory
Files:
holmes/plugins/toolsets/datadog/toolset_datadog_traces.py
🧬 Code graph analysis (1)
holmes/plugins/toolsets/datadog/toolset_datadog_traces.py (2)
holmes/core/tools.py (1)
ToolParameter(126-131)holmes/plugins/toolsets/utils.py (1)
toolset_name_for_one_liner(144-148)
🔇 Additional comments (1)
holmes/plugins/toolsets/datadog/toolset_datadog_traces.py (1)
161-165: Good addition: optionalqueryparameter for tracesThis aligns the traces tool with spans and preserves backward compatibility. No concerns.
4 participants
Add custom query parameter support to FetchDatadogTracesList for enhanced search flexibility
Description
This change adds support for custom Datadog queries in the FetchDatadogTracesList tool, bringing it in line with the existing FetchDatadogSpansByFilter functionality.
Benefits
Changes implemented
This enhancement significantly improves the usability of the Datadog traces toolset by enabling more sophisticated and precise trace searches.