feat(actions): HPA/VPA right-sizing audit with Datadog percentiles and Slack review card

server/chat/backend/agent/access/mode_access_controller.py

@@ -42,6 +42,15 @@ class ModeAccessController:
         "mcp_get_pull_request",
     }
 
+    # Non-prefixed tools that write and are therefore denied in Ask mode.
+    # One definition, consulted by both filter_tools and is_tool_allowed -- the
+    # two checks must agree, and duplicated literals are how they drift.
+    WRITE_TOOLS_BLOCKED_IN_ASK_MODE = frozenset({
+        "iac_tool",
+        "github_commit",
+        "send_hpa_vpa_recommendation",
+    })
+
     _POLICY = ReadOnlyPolicy(
         safe_tool_names=(
             "web_search",
@@ -85,7 +94,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 cls.WRITE_TOOLS_BLOCKED_IN_ASK_MODE:
                 LOGGER.info("ModeAccessController dropped tool %s for read-only mode", name)
                 continue
 
@@ -134,7 +143,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 cls.WRITE_TOOLS_BLOCKED_IN_ASK_MODE
 
 
 __all__ = ["ModeAccessController"]

server/chat/backend/agent/skills/integrations/datadog/SKILL.md

@@ -20,32 +20,111 @@ metadata:
 # Datadog Integration
 
 ## Overview
+
 Datadog integration for querying observability data during Root Cause Analysis. Datadog is a REMOTE service. Use ONLY the `query_datadog` API tool. All data is accessed via a single unified tool with `resource_type` parameter.
 
 ## 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
+
 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.
+
+### 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.
+
+A row may also carry `malformed_response: true` (with `malformed_series` at the top level).
+That means Datadog described a series but returned no values for it. It is a broken response,
+**not** an idle workload -- re-run rather than concluding anything from it.
+
+### Units -- read the `unit` field before any arithmetic
+
+Every row carries `unit`. Container CPU and memory are reported in different units, and
+mixing them up produces a wrong number that still looks plausible:
+
+- `kubernetes.cpu.usage.total` is a gauge in **nanocores**. Divide by `1e9` for cores, then
+  x1000 for millicores, before comparing against a `500m`-style request. Comparing raw
+  nanocores against millicores is a ~1,000,000x error.
+- `kubernetes.memory.usage` is a gauge in **bytes**. No scaling is needed; convert to Mi/Gi
+  for display only.
+- In a **ratio**, the numerator and denominator must be in the same unit. A CPU
+  usage/limits ratio built from nanocore usage needs the `/1e9`; a memory bytes/bytes ratio
+  does not. A ratio whose two sides disagree on units is silently meaningless.
+
+Ratio and scaling expressions work in a single `query` -- both of these are accepted:
+
+```text
+sum:kubernetes.cpu.usage.total{...} by {kube_deployment} / 1e9
+(sum:kubernetes.cpu.usage.total{...} by {kube_deployment} / 1e9) / sum:kubernetes.cpu.limits{...} by {kube_deployment}
+```
+
+Also keep **requests** and **limits** distinct: saturation monitors usually measure usage
+against *limits*, while right-sizing changes *requests*. Requests drive scheduling and cost;
+limits drive throttling and OOM-kills. Say which field any number refers to.
+
+### 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`
 - Filter by status: `status:error`
 - HTTP status codes: `@http.status_code:5*`
 - Filter by host: `host:X`
 - Filter by environment: `env:production`
 
 ### 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')`
 
@@ -70,8 +149,14 @@ Datadog integration for querying observability data during Root Cause Analysis.
 `query_datadog(resource_type='incidents')`
 
 ## 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.

server/chat/backend/agent/tools/cloud_tools.py

@@ -1035,12 +1035,16 @@ def get_cloud_tools():
     rca_flag = getattr(state_context, 'trigger_rca_requested', False) if state_context else False
     is_background = getattr(state_context, 'is_background', False) if state_context else False
     is_postmortem_action = getattr(state_context, 'is_postmortem_action', False) if state_context else False
+    is_hpa_vpa_action = getattr(state_context, 'is_hpa_vpa_action', False) if state_context else False
     is_pr_review = getattr(state_context, 'is_pr_review', False) if state_context else False
     is_rca_context = _is_background_rca(state_context, is_background)
     _action_id = getattr(state_context, 'trigger_action_id', None) if state_context else None
     _incident_id = getattr(state_context, 'incident_id', None) if state_context else None
     capture_tag = "capture" if tool_capture else "nocapture"
-    cache_key = f"{user_id}:{capture_tag}:{mode_suffix}:background={is_background}:rca={rca_flag}:postmortem={is_postmortem_action}:is_rca_ctx={is_rca_context}:pr_review={is_pr_review}:action_id={_action_id}:incident={_incident_id}"
+    # hpa_vpa is part of the key for the same reason postmortem is: it changes
+    # which tools are returned, so sharing a cache entry across contexts would
+    # leak the card tool into an ordinary chat (or withhold it from the action).
+    cache_key = f"{user_id}:{capture_tag}:{mode_suffix}:background={is_background}:rca={rca_flag}:postmortem={is_postmortem_action}:hpa_vpa={is_hpa_vpa_action}:is_rca_ctx={is_rca_context}:pr_review={is_pr_review}:action_id={_action_id}:incident={_incident_id}"
 
     current_time = time.time()
     if (
@@ -2213,10 +2217,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,
         ))
@@ -2471,6 +2480,71 @@ 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 -- scoped to the built-in Right-Sizing
+    # Audit action, and additionally requiring Slack (to post the card) and GitHub
+    # (the PR the card links to).
+    #
+    # is_hpa_vpa_action is resolved in chat/background/task.py from the run's
+    # trigger_metadata action_id via a system_key lookup, mirroring
+    # is_postmortem_action/save_postmortem. It fails closed, so an ordinary chat
+    # -- which has no action_id -- never sees a tool that writes to a customer
+    # Slack channel. Excluded from PR review, which is read-only.
+    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_hpa_vpa_action:
+            # Not the right-sizing action: say so at debug level only. This is the
+            # overwhelmingly common case (every chat, every other action), so a
+            # warning here would be noise that trains people to ignore the real one.
+            # The connectivity probes are deliberately NOT run in this branch: each
+            # one is a Vault + DB round trip, and paying for both on every chat turn
+            # and every other action just to log a debug line is pure overhead.
+            logging.debug(
+                "HPA/VPA right-sizing card tools withheld for user %s: not the "
+                "Right-Sizing Audit action", user_id,
+            )
+        elif is_pr_review:
+            # Read-only surface: nothing to register and nothing to warn about.
+            logging.debug(
+                "HPA/VPA right-sizing card tools withheld for user %s: PR review is "
+                "read-only", user_id,
+            )
+        else:
+            _slack_ok = _safe_connected(_is_slack_connected, "Slack")
+            _github_ok = _safe_connected(is_github_connected, "GitHub")
+            if _slack_ok and _github_ok:
+                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}")
+            else:
+                # Say WHY the tools are absent. Silence here is indistinguishable from
+                # "the agent chose not to notify": the right-sizing prompt treats a
+                # missing card tool as an acceptable outcome and still opens the PR, so
+                # a dropped tool surfaces to the user as a PR that never reached Slack
+                # with nothing in the logs to explain it. _safe_connected also returns
+                # False on an exception (Vault blip, token refresh failure), which looks
+                # identical to "not connected" -- hence logging the pair, not a guess.
+                _missing = [n for n, ok in (("Slack", _slack_ok), ("GitHub", _github_ok)) if not ok]
+                logging.warning(
+                    "HPA/VPA right-sizing card tools NOT registered for user %s: %s "
+                    "not connected (or its connectivity check failed). The right-sizing "
+                    "action will still open PRs but cannot post the review card.",
+                    user_id, " and ".join(_missing),
+                )
+    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