Skip to content
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions server/chat/backend/agent/access/mode_access_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def filter_tools(cls, mode: Optional[str], tools: Sequence[StructuredTool]) -> L
LOGGER.info("ModeAccessController dropped tool %s due to read-only mode prefix match", name)
continue

if name in {"iac_tool", "github_commit"}:
if name in {"iac_tool", "github_commit", "send_hpa_vpa_recommendation"}:
LOGGER.info("ModeAccessController dropped tool %s for read-only mode", name)
continue

Expand Down Expand Up @@ -134,7 +134,7 @@ def is_tool_allowed(cls, mode: Optional[str], tool_name: str) -> bool:
if any(name.startswith(prefix) for prefix in cls._POLICY.blocked_tool_prefixes):
return False

return name not in {"iac_tool", "github_commit"}
return name not in {"iac_tool", "github_commit", "send_hpa_vpa_recommendation"}


__all__ = ["ModeAccessController"]
63 changes: 55 additions & 8 deletions server/chat/backend/agent/skills/integrations/datadog/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,56 @@ Datadog integration for querying observability data during Root Cause Analysis.
## Instructions

### Tool Usage
`query_datadog(resource_type=TYPE, query=QUERY, time_from=START, time_to=END, limit=N)`
`query_datadog(resource_type=TYPE, query=QUERY, time_from=START, time_to=END, limit=N, interval=MS)`

### Resource Types

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

markdownlint MD022: add a blank line after these headings.

### Resource Types, ### The interval Parameter, ### Percentiles, and ### Reconciliation vs Sizing are immediately followed by content.

Also applies to: 42-42, 49-49, 70-70

🧰 Tools
🪛 markdownlint-cli2 (0.23.1)

[warning] 30-30: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/chat/backend/agent/skills/integrations/datadog/SKILL.md` at line 30,
Add a blank line immediately after the headings Resource Types, The interval
Parameter, Percentiles, and Reconciliation vs Sizing in the Datadog skill
documentation, before their following content.

Source: Linters/SAST tools

1. `'logs'` -- Search log entries. query=Datadog log query syntax e.g. `"service:web status:error"`
2. `'metrics'` -- Query metric timeseries. query=metric query e.g. `"avg:system.cpu.user{*}"`
3. `'monitors'` -- List monitors with status. query=name filter (optional)
4. `'events'` -- Platform events. query=source filter (optional)
5. `'traces'` -- APM spans/traces. query=span query e.g. `"service:web @http.status_code:500"`
6. `'hosts'` -- Infrastructure hosts. query=host filter (optional)
7. `'incidents'` -- Datadog incidents. Lists active/recent incidents (requires Incident Management; may 403 if not enabled).
2. `'metrics'` -- Query metric timeseries (raw points). query=metric query e.g. `"avg:system.cpu.user{*}"`
3. `'metric_stats'` -- Percentile summary per series (p50/p95/p99/max/mean). Same metric query
syntax as `'metrics'`, but returns one compact row per series instead of raw points.
Use this for capacity and right-sizing questions over long windows.
4. `'monitors'` -- List monitors with status. query=name filter (optional)
5. `'events'` -- Platform events. query=source filter (optional)
6. `'traces'` -- APM spans/traces. query=span query e.g. `"service:web @http.status_code:500"`
7. `'hosts'` -- Infrastructure hosts. query=host filter (optional)
8. `'incidents'` -- Datadog incidents. Lists active/recent incidents (requires Incident Management; may 403 if not enabled).

### The `interval` Parameter
`interval` is the rollup granularity in **milliseconds**, and applies to `'metrics'` and
`'metric_stats'`. Omit it and a granularity is auto-picked that keeps each series under
~1000 points -- a 30-day window auto-picks `3600000` (1 hour, 720 points). Values are
clamped to `60000`..`14400000`. Datadog caps a series at 1500 points, so a long window
with a fine interval returns less than you asked for; prefer the auto-pick.
Comment on lines +42 to +47

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Auto-pick claim is wrong for 'metrics'.

_query_metrics deliberately does not auto-pick — omitting interval leaves Datadog's own default resolution (see datadog_tool.py Lines 151-156 and test_plain_metrics_does_not_auto_pick_an_interval). Only metric_stats auto-picks. As written, the agent will assume a ~1000-point rollup for plain metrics queries.

📝 Proposed doc fix
 `interval` is the rollup granularity in **milliseconds**, and applies to `'metrics'` and
-`'metric_stats'`. Omit it and a granularity is auto-picked that keeps each series under
-~1000 points -- a 30-day window auto-picks `3600000` (1 hour, 720 points). Values are
-clamped to `60000`..`14400000`. Datadog caps a series at 1500 points, so a long window
-with a fine interval returns less than you asked for; prefer the auto-pick.
+`'metric_stats'`. Values are clamped to `60000`..`14400000`.
+
+- `'metric_stats'`: omit it and a granularity is auto-picked that keeps each series under
+  ~1000 points -- a 30-day window auto-picks `3600000` (1 hour, 720 points).
+- `'metrics'`: omitting it leaves Datadog's own default resolution in place; no auto-pick.
+
+Datadog caps a series at 1500 points, so a long window with a fine interval returns less
+than you asked for; for `metric_stats` prefer the auto-pick.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
### The `interval` Parameter
`interval` is the rollup granularity in **milliseconds**, and applies to `'metrics'` and
`'metric_stats'`. Omit it and a granularity is auto-picked that keeps each series under
~1000 points -- a 30-day window auto-picks `3600000` (1 hour, 720 points). Values are
clamped to `60000`..`14400000`. Datadog caps a series at 1500 points, so a long window
with a fine interval returns less than you asked for; prefer the auto-pick.
### The `interval` Parameter
`interval` is the rollup granularity in **milliseconds**, and applies to `'metrics'` and
`'metric_stats'`. Values are clamped to `60000`..`14400000`.
- `'metric_stats'`: omit it and a granularity is auto-picked that keeps each series under
~1000 points -- a 30-day window auto-picks `3600000` (1 hour, 720 points).
- `'metrics'`: omitting it leaves Datadog's own default resolution in place; no auto-pick.
Datadog caps a series at 1500 points, so a long window with a fine interval returns less
than you asked for; for `metric_stats` prefer the auto-pick.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)

[warning] 42-42: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/chat/backend/agent/skills/integrations/datadog/SKILL.md` around lines
42 - 47, Correct the interval documentation in the “The interval Parameter”
section to state that automatic granularity selection applies only to
metric_stats. Document that omitting interval for metrics uses Datadog’s default
resolution, while retaining the existing millisecond units and clamping guidance
where applicable.


### Percentiles
**Datadog cannot compute a time-percentile.** Do not attempt any of these -- every one
is rejected or silently empty:
- `.rollup(percentile, 95, 3600)` and `.rollup(p95, 3600)` -- `400 Unrecognized rollup method`.
`.rollup()` accepts only `avg`, `sum`, `min`, `max`, `count`.
- `p95:my.metric{...}` -- returns `200` with **zero series** for gauges. The `pXX:` prefix
needs *distribution* metrics; `kubernetes.cpu.usage.total` and `kubernetes.memory.usage`
are gauges, so it can never apply to them.
- `formula: "p95(a)"` -- `400 function "p95()" does not exist`.
- `formula: "percentile(a, 95, 3600)"` -- `percentile()` exists but is a **space** aggregator:
arguments 2 and 3 are *group tags*, not a percentile value and window.
- scalar `aggregator: "percentile"` or `"p95"` -- `400`.

Use `resource_type='metric_stats'` instead. It fetches the rolled-up points and computes
percentiles server-side, using **nearest-rank** order statistics -- so every reported p95
is a value that actually occurred and a reviewer can find it in the Datadog UI.

Each row carries `points` and `nulls`. **Always check them.** Datadog emits nulls for gaps,
and a row with `p95: null` plus a `note` means *no data*, which is not the same as *low
usage* -- never treat an empty series as an idle workload.

### Reconciliation vs Sizing
These are two different questions and must not be conflated:
- **Reconciliation** -- "does our view match the team's?" Read the org's own monitor
definitions with `resource_type='monitors'` to get their real `query` strings and
`options.thresholds`, then reproduce that formula exactly. This tells you which
workloads run hot. It is *not* the basis of any recommended number.
- **Sizing** -- "what should this value be?" Use `resource_type='metric_stats'` for the
usage distribution over the window. Never derive a sizing number from a monitor threshold.

### Datadog Query Syntax
- Filter by service: `service:X`
Expand All @@ -46,6 +86,8 @@ Datadog integration for querying observability data during Root Cause Analysis.
### Examples
- Logs: `query_datadog(resource_type='logs', query='service:web status:error', time_from='-1h')`
- Metrics: `query_datadog(resource_type='metrics', query='avg:system.cpu.user{*}', time_from='-2h')`
- Metric stats (30-day p95 per deployment, one query for all of them):
`query_datadog(resource_type='metric_stats', query='sum:kubernetes.memory.usage{env:production} by {kube_deployment}', time_from='-30d')`
- Traces: `query_datadog(resource_type='traces', query='service:web @http.status_code:500', time_from='-1h')`
- Monitors: `query_datadog(resource_type='monitors', query='web')`

Expand All @@ -71,7 +113,12 @@ Datadog integration for querying observability data during Root Cause Analysis.

## Important Rules
- Datadog is a REMOTE service. Use ONLY the `query_datadog` API tool.
- The `resource_type` parameter is required and must be one of: logs, metrics, monitors, events, traces, hosts, incidents.
- The `resource_type` parameter is required and must be one of: logs, metrics, metric_stats, monitors, events, traces, hosts, incidents.
- Time parameters accept relative strings (`'-1h'`, `'-24h'`, `'-7d'`) or ISO 8601 timestamps.
- The `incidents` resource type requires Datadog Incident Management to be enabled; may return 403 if not.
- Results are truncated at the output size limit. Use more specific queries to narrow results.
- Never reason from a truncated result set. If a response carries `truncated`, `truncated_all`,
`series_truncated` or `series_dropped`, narrow the query and re-run before drawing a conclusion.
- Group with `by {tag}` rather than issuing one query per workload -- a single grouped query
returns every deployment at once. A 30-day multi-group query is close to the request timeout,
so query one environment at a time.
39 changes: 36 additions & 3 deletions server/chat/backend/agent/tools/cloud_tools.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like chats can now post a slack card. The posting of the slack card should be exclusive to the seeded templated action being ran

Original file line number Diff line number Diff line change
Expand Up @@ -2213,10 +2213,15 @@ def _pinned_trigger(action_id: str = "", _pid=pinned_id, _fn=final_func, **kw):
func=final_dd_func,
name="query_datadog",
description=(
"Query Datadog for logs, metrics, monitors, events, traces, hosts, or incidents. "
"Set resource_type to 'logs', 'metrics', 'monitors', 'events', 'traces', 'hosts', or 'incidents'. "
"Query Datadog for logs, metrics, metric_stats, monitors, events, traces, hosts, or incidents. "
"Set resource_type to 'logs', 'metrics', 'metric_stats', 'monitors', 'events', 'traces', "
"'hosts', or 'incidents'. Use 'metric_stats' for p50/p95/p99/max per series over long "
"windows (capacity and right-sizing questions) -- Datadog cannot compute a time-percentile "
"in a query, so 'metrics' plus a percentile rollup does not work. "
"Examples: query_datadog(resource_type='logs', query='service:web status:error', time_from='-1h') "
"or query_datadog(resource_type='metrics', query='avg:system.cpu.user{*}', time_from='-2h')"
"or query_datadog(resource_type='metrics', query='avg:system.cpu.user{*}', time_from='-2h') "
"or query_datadog(resource_type='metric_stats', "
"query='sum:kubernetes.memory.usage{env:production} by {kube_deployment}', time_from='-30d')"
),
args_schema=QueryDatadogArgs,
))
Expand Down Expand Up @@ -2471,6 +2476,34 @@ def _pinned_trigger(action_id: str = "", _pid=pinned_id, _fn=final_func, **kw):
except Exception as e:
logging.warning(f"Failed to add Notion tools: {e}")

# Add HPA/VPA right-sizing card tools -- needs Slack (to post the card) and
# GitHub (the PR the card links to). Gated on connectors only: State
# .trigger_action_id is never set for background action runs, so there is no
# way today to scope these to the right-sizing action specifically.
# Excluded from PR review, which is read-only, like every other write tool here.
try:
# Imported here rather than relying on the Slack block ~1000 lines above:
# that name is only bound if this function reached that try block without
# raising, so depending on it would fail with UnboundLocalError.
from .slack_tool import is_slack_connected as _is_slack_connected
from .hpa_vpa_card_tool import HPA_VPA_TOOL_SPECS
if (not is_pr_review
and _safe_connected(_is_slack_connected, "Slack")
and _safe_connected(is_github_connected, "GitHub")):
for _func, _name, _schema, _desc in HPA_VPA_TOOL_SPECS:
_ctx = with_user_context(_func)
_notif = with_completion_notification(_ctx)
_final = wrap_func_with_capture(_notif, _name) if tool_capture else _notif
tools.append(StructuredTool.from_function(
func=_final,
name=_name,
description=_desc,
args_schema=_schema,
))
logging.info(f"Added {len(HPA_VPA_TOOL_SPECS)} HPA/VPA right-sizing tools for user {user_id}")
except Exception as e:
logging.warning(f"Failed to add HPA/VPA right-sizing tools: {e}")

# Add Jira tools if enabled
try:
from utils.flags.feature_flags import is_jira_enabled
Expand Down
Loading
Loading