This is an automated email from the ASF dual-hosted git repository. wu-sheng pushed a commit to branch refactor/ai-profiling-oap-owned-rules in repository https://gitbox.apache.org/repos/asf/skywalking-horizon-ui.git
commit 902a1bc7a51f2f8799cabbb8c49fa5a968e22598 Author: Wu Sheng <[email protected]> AuthorDate: Wed Jul 29 08:29:13 2026 +0800 refactor(ai): let OAP decide what profiling is allowed, not our config The ask -> approval workflow is ours; the profiling rules are OAP's, and they differ per version, agent and storage backend. We had mirrored several of those rules on our side and turned them into hard refusals, so operators were blocked from tasks their backend would have accepted -- with a message that blamed the deployment for what was really Horizon configuration. Checked against OAP's create paths, none of these are things OAP validates: - ProfileTaskMutationService.checkDataSuccess (trace), AsyncProfilerMutation- Service / PprofMutationService.checkArgumentError and EBPFProfilingMutation- Service.checkCreateRequest take no layer argument and apply no layer gate -- but propose_profiling refused any type the layer template's `components` list omitted. That list is Horizon-side config; it is now a caveat. - Neither async nor pprof creation consults the instance's runtime language. The JVM-only / Go-only match still drives which instances we TARGET, but no longer withholds the card. - createEBPFProfilingFixedTimeTask never runs queryPrepareCreateEBPFProfiling- TaskData; `couldProfiling: false` is what OAP's own create FORM shows, and a rover restart or metadata lag flips it on a deployment that profiles fine. - The network create counts processes with getProcessCount(instanceId) and NO time window, so our 30-minute probe was strictly stricter. On a miss we now target an instance and let OAP answer -- "The instance doesn't have processes." is accurate and actionable where our guess was neither. Each of those now rides back to the model as a caveat it must relay before the user approves, so the doubt is still surfaced -- the decision just moves to the operator and the backend. Also corrects rules we had wrong outright: - pprof duration is MINUTES capped at 15 (PprofMutationService:125), not the 600 "seconds" we bounded it by -- 40x too loose, failing after approval. - eBPF has a 60s minimum (FIXED_TIME_MIN_DURATION) we did not enforce, and no maximum; ours is a route guard and is now labelled as such. - OAP fixes a network task at 10 minutes (NETWORK_PROFILING_DURATION) and takes no duration field, so any proposed window was fiction; say so on the card. - async has no server-side duration cap, so the 10-minute clamp (and the comment claiming OAP enforced it) is gone. - Over-cap instance lists are rejected with a reason instead of silently sliced to 32, which left the card advertising a fleet the task never covered. - OFF_CPU eBPF results were aggregated by COUNT, which OAP documents as scheduler switch count; DURATION is time blocked. Off-CPU is chosen to find blocking, so this inverted the ranking. The reply now names each profiler's real unit -- async/pprof "self" is a sample count, not milliseconds. Validated against a local e2e OAP: the pprof-on-Java, eBPF-without-rover and network-on-GENERAL paths now emit a card with the caveat relayed, and approving the network card surfaces OAP's own rejection. --- CHANGELOG.md | 5 +- apps/bff/src/ai/resources/tools/triggers.yaml | 16 ++-- apps/bff/src/ai/skill/triggers/tools.test.ts | 82 +++++++++------- apps/bff/src/ai/skill/triggers/tools.ts | 133 ++++++++++++++++---------- apps/bff/src/http/query/async-profile.ts | 43 ++++++--- apps/bff/src/http/query/ebpf.ts | 10 +- apps/bff/src/logic/oap/profiling.ts | 11 ++- docs/operate/ai-assistant.md | 4 +- 8 files changed, 193 insertions(+), 111 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72cb035..19f44e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,10 @@ The version line is shared by every package in the monorepo (apps + shared packa - **Read-only, and it inherits your permissions.** The assistant can list layers/services, read active **alarms** (the "what's unhealthy" signal), browse the per-layer **metric catalog** (the curated MQE for each metric), drill a service down to its instances/endpoints, and render those metrics — every data tool checks the same read verb you already hold, so the assistant never sees more than you can, and it never changes configuration, rules, or dashboards. - **It renders the real feature views inline — same components, focused on the service.** Rather than a link you open, the assistant mounts the *actual* product views inside the chat, read-only and scoped to the service: ask for **topology** and it embeds the real service map focused one hop (hex nodes, edges, RPM/latency, zoom controls); ask how a service maps across layers and it shows the real **Smartscape hierarchy** fan; ask for **traces** and it embeds the Traces view — the trace l [...] - **Read live Kubernetes pod logs, right in the chat.** For a k8s workload the assistant pulls a pod container's on-demand logs (the error stack) and shows the fetched lines inline as a read-only result — the same on-demand-log path as the Pod Logs tab, so nothing is stored and it inherits your `logs:read` permission. It's a result, not a console: no tail or refresh controls (operate a live tail in the Pod Logs tab); when a content filter was applied the block shows it, so an empty resul [...] -- **It can propose profiling — the right kind for the target — and only you start it.** When metrics and traces can't localise a cause, the assistant presents a **decision card** explaining what it found, why profiling would help, and what it expects to reveal; nothing runs until you **approve it in the popout**, and only if you hold the profiling permission. It picks the profiling flavour that fits the target — **trace** sampling, **async-profiler** for a JVM service, **pprof** for a Go [...] +- **It can propose profiling — the right kind for the target — and only you start it.** When metrics and traces can't localise a cause, the assistant presents a **decision card** explaining what it found, why profiling would help, and what it expects to reveal; nothing runs until you **approve it in the popout**, and only if you hold the profiling permission. It picks the profiling flavour that fits the target — **trace** sampling, **async-profiler** for a JVM service, **pprof** for a Go [...] +- **Your backend decides what profiling is allowed — the assistant no longer refuses on its own guesses.** Previously the assistant could decline to even show a profiling card: because the layer's template didn't list that profiling type, because the instances reported a runtime the profiler doesn't match, because no process had advertised eBPF support recently, or because no process had reported in the last 30 minutes — and it phrased those as facts about your deployment ("the GENERAL l [...] +- **Off-CPU eBPF profiles are ranked by time blocked, not by how often a thread yielded.** An OFF_CPU profile is what you reach for when the question is "what is this service waiting on", but its frames were ranked by scheduler switch *count* — so a method that yields thousands of times for microseconds outranked the one that blocks for a second, inverting the answer. Off-CPU results now aggregate by duration. Relatedly, the assistant now names each profiler's real unit when it reads a f [...] +- **pprof collection windows are validated in the unit OAP actually uses.** The pprof task duration is measured in **minutes** and capped at 15 by OAP, but Horizon was bounding it as if it were 600 seconds — so an over-long request sailed through and came back as an opaque backend error after you approved it. It is now bounded at OAP's real limit. Likewise, an async or pprof task targeting more instances than a single task allows is now **rejected with a clear reason** instead of quietly [...] - **Guided root-cause analysis.** Ask "what's the root cause?" and the assistant follows built-in investigation playbooks — a master method (locate the root service → calling chain → error stack; walk the dependency topology upstream and fix a sick upstream first; a remote / Virtual_* dependency exposes only its client-side edge metric) plus latency, error-rate/SLA, saturation, middleware, **Kubernetes-workload**, and service-mesh specializations. It can also follow the **cross-layer hie [...] - **Bring your own LLM — vendor-neutral, and off by default.** Enable it with the new `ai:` config block (`HORIZON_AI_*`). The default transport is **OpenAI-compatible** (any OpenAI-shaped endpoint — a hosted model, a local model, or an AI gateway; set model + base URL + API key); **Amazon Bedrock** is also supported (`provider: bedrock`). The API key is a secret, env-only, redacted from logs and excluded from the audit trail. The launcher shows for every signed-in user so the AI-powered [...] - **Both prompts are yours.** The assistant's system prompt (`ai.systemPrompt`) and the starter example chips shown in an empty chat (`ai.starters`) ship with sensible defaults and can be replaced entirely in `horizon.yaml`. diff --git a/apps/bff/src/ai/resources/tools/triggers.yaml b/apps/bff/src/ai/resources/tools/triggers.yaml index 06b7658..32568f0 100644 --- a/apps/bff/src/ai/resources/tools/triggers.yaml +++ b/apps/bff/src/ai/resources/tools/triggers.yaml @@ -22,7 +22,7 @@ propose_profiling: description: >- - PROPOSE a profiling task (does NOT start it — the user approves a decision card first). Use only when metrics + traces cannot localise the cause and a profile would confirm a specific hypothesis (a hot method, lock contention, a slow call, a saturated process). FIRST pick the profilingType by READING, not assuming: kb_layer_capabilities(layer).components lists which profiling this layer supports (traceProfiling / asyncProfiling / pprofProfiling / ebpfProfiling / networkProfiling) — p [...] + PROPOSE a profiling task (does NOT start it — the user approves a decision card first). Use only when metrics + traces cannot localise the cause and a profile would confirm a specific hypothesis (a hot method, lock contention, a slow call, a saturated process). FIRST pick the profilingType by READING, not assuming: kb_layer_capabilities(layer).components lists which profiling this layer is SET UP for (traceProfiling / asyncProfiling / pprofProfiling / ebpfProfiling / networkProfiling [...] params: layer: >- OAP layer key, e.g. GENERAL @@ -31,15 +31,15 @@ propose_profiling: service: >- service NAME profilingType: >- - the profiling flavor to propose — trace (in-process sampling), async (JVM async-profiler), pprof (Go), ebpf (kernel on/off-CPU), or network (process conversation graph). MUST be one the layer supports (kb_layer_capabilities.components); async needs a JVM service, pprof needs a Go service (check instance language). + the profiling flavor to propose — trace (in-process sampling; OAP decodes Java and Go agents), async (JVM async-profiler), pprof (Go), ebpf (kernel on/off-CPU, language-agnostic, needs Rover), or network (process conversation graph, needs Rover). Prefer one the layer declares (kb_layer_capabilities.components), and match it to the instance language. durationMinutes: >- - collection window in minutes, 1–15 (converted to each type's unit on start) + collection window in minutes. What OAP does with it differs per type: trace accepts 1–15; pprof accepts 1–15 and only for CPU/BLOCK/MUTEX (point-in-time events ignore it); eBPF converts to seconds and requires at least 60s (1 minute); async converts to seconds and OAP sets no upper bound; network IGNORES it entirely and always runs a fixed 10 minutes. endpoint: >- - trace only — endpoint (API) name to profile, when known + trace only — endpoint (API) name to profile. OAP requires it and only checks it is non-empty, so a name that does not exist creates a task that collects nothing AND blocks any other trace task on that service until its window elapses — use an endpoint you have actually seen in this investigation. event: >- - async/pprof only — the event to profile (default CPU; async also ALLOC/LOCK/WALL, pprof also HEAP/BLOCK/MUTEX) + async/pprof only — the event to profile (default CPU). async: CPU, ALLOC, LOCK, WALL, CTIMER, ITIMER (CTIMER/ITIMER work where perf_events is unavailable, e.g. hardened containers). pprof: CPU, HEAP, BLOCK, MUTEX, GOROUTINE, ALLOCS, THREADCREATE (HEAP/GOROUTINE/ALLOCS/THREADCREATE are point-in-time snapshots, not windows). targetType: >- - ebpf only — ON_CPU (default) or OFF_CPU + ebpf only — ON_CPU (default) ranks by on-CPU dump count; OFF_CPU ranks by time spent blocked, and is the one to pick for lock or I/O contention cause: >- the analyzed cause SO FAR — what the investigation found rationale: >- @@ -49,7 +49,7 @@ propose_profiling: analyze_profiling: description: >- - Read a COMPLETED profiling task and render its result inline as a CAPTURED block. Call this in a LATER turn, after a task the user approved has finished collecting (or when the user asks you to analyze an existing profile). It fetches the most recent task of the given type for the service and analyses it. trace/async/pprof/ebpf render the flame graph (a trace profile also shows the profiled trace's span waterfall beside the flame), and the reply lists the hottest frames so you can na [...] + Read a COMPLETED profiling task and render its result inline as a CAPTURED block. Call this in a LATER turn, after a task the user approved has finished collecting (or when the user asks you to analyze an existing profile). Pass the taskId of the task that was approved whenever you have it — without one the tool picks a task itself, and that may not be the one the user is asking about. trace/async/pprof/ebpf render the flame graph (a trace profile also shows the profiled trace's span [...] params: layer: >- OAP layer key, e.g. GENERAL @@ -58,4 +58,4 @@ analyze_profiling: profilingType: >- which profiling result to read — trace (in-process sampling; flame plus the profiled trace's span waterfall), pprof (Go), async (Java async-profiler), ebpf (on/off-CPU), or network (captures the process-conversation graph and renders it as a frozen graph block, not a flame; text-only when no process reports) taskId: >- - a specific task id to analyze; omit to use the most recent task of this type + the task id to analyze — always pass the id returned when the user approved a card. Omit ONLY when the user asks about an existing profile you have no id for; the tool then picks one of the service's tasks of this type, which need not be the newest. diff --git a/apps/bff/src/ai/skill/triggers/tools.test.ts b/apps/bff/src/ai/skill/triggers/tools.test.ts index b0dfcc3..185f015 100644 --- a/apps/bff/src/ai/skill/triggers/tools.test.ts +++ b/apps/bff/src/ai/skill/triggers/tools.test.ts @@ -132,19 +132,24 @@ describe('propose_profiling', () => { expect(emitProposal.mock.calls[0][0]).toMatchObject({ profilingType: 'async' }); }); - it('clamps async duration to the 10-minute (600s) server cap', async () => { + // OAP's only async duration rule is `duration <= 0` — there is no 600s cap to + // honour, so a proposed window must reach the card unchanged. + it('passes the async duration through — OAP sets no upper bound', async () => { const { ctx, emitProposal } = mockCtx(true); const [propose] = triggerTools(ctx); await propose.invoke({ ...base, profilingType: 'async', durationMinutes: 15 }); - expect(emitProposal.mock.calls[0][0].durationMinutes).toBe(10); + expect(emitProposal.mock.calls[0][0].durationMinutes).toBe(15); }); - it('refuses pprof (Go) when the instances report a JVM language', async () => { + // The runtime mismatch is real, but it is OURS: OAP's pprof create validates + // serviceId/events/duration/dumpPeriod only. So warn and still show the card. + it('warns but still proposes pprof when the instances report a JVM language', async () => { const { ctx, emitProposal } = mockCtx(true); const [propose] = triggerTools(ctx); const out = await propose.invoke({ ...base, profilingType: 'pprof' }); - expect(emitProposal).not.toHaveBeenCalled(); - expect(String(out)).toMatch(/Go-only|match the profiler/i); + expect(emitProposal).toHaveBeenCalledTimes(1); + expect(String(out)).toMatch(/Go-only/i); + expect(String(out)).toMatch(/collect nothing/i); }); // A mixed-language fleet must not be sent a profiler its runtime can't run — @@ -194,28 +199,33 @@ describe('propose_profiling', () => { expect(emitProposal.mock.calls[0][0]).toMatchObject({ profilingType: 'network', instanceIds: ['i-1'] }); }); - // ...but it IS the right gate for eBPF CPU profiling, which is what OAP's own - // create form checks. - it('refuses eBPF profiling when no process advertises eBPF support', async () => { + // queryPrepareCreateEBPFProfilingTaskData is what OAP's own create FORM checks + // before enabling its button — createEBPFProfilingFixedTimeTask never runs it. + // So a false is a readiness hint to relay, not grounds to withhold the card. + it('warns but still proposes eBPF when no process advertises eBPF support', async () => { (serviceCanEbpfProfile as unknown as Mock).mockResolvedValueOnce({ could: false }); const { ctx, emitProposal } = mockCtx(true); const [propose] = triggerTools(ctx); const out = await propose.invoke({ ...base, profilingType: 'ebpf' }); - expect(emitProposal).not.toHaveBeenCalled(); - expect(String(out)).toMatch(/eBPF-profiling support|Rover/i); + expect(emitProposal).toHaveBeenCalledTimes(1); + expect(String(out)).toMatch(/Rover/i); + expect(String(out)).toMatch(/does not consult it|may still run/i); }); - // A complete scan that finds nothing IS conclusive for network profiling. - it('reports network profiling unavailable only when the scan was complete and clean', async () => { + // OAP's network create counts processes with NO time window, so a miss in our + // rolling probe is weaker evidence than OAP's own gate — target an instance + // and let `getProcessCount` give the real answer. + it('still proposes network profiling when the process probe finds nothing', async () => { (findInstanceWithProcesses as unknown as Mock).mockResolvedValueOnce({ instance: null, checked: 2, total: 2, failed: 0, }); const { ctx, emitProposal } = mockCtx(true); const [propose] = triggerTools(ctx); const out = await propose.invoke({ ...base, profilingType: 'network' }); - expect(emitProposal).not.toHaveBeenCalled(); - expect(String(out)).toMatch(/looks unavailable for/i); - expect(String(out)).not.toMatch(/not conclusive/i); + expect(emitProposal).toHaveBeenCalledTimes(1); + expect(emitProposal.mock.calls[0][0]).toMatchObject({ instanceIds: ['i-1'] }); + expect(String(out)).toMatch(/without a time window/i); + expect(String(out)).toMatch(/Do not claim that before approval/i); }); // A failed lookup is NOT evidence of a missing Rover agent — reporting it as @@ -231,54 +241,58 @@ describe('propose_profiling', () => { const { ctx, emitProposal } = mockCtx(true); const [propose] = triggerTools(ctx); const out = await propose.invoke({ ...base, profilingType: 'network' }); - expect(emitProposal).not.toHaveBeenCalled(); - expect(String(out)).toMatch(/could not|failed/i); - expect(String(out)).not.toMatch(/needs a Rover eBPF agent/i); + expect(emitProposal).toHaveBeenCalledTimes(1); + expect(String(out)).toMatch(/every process lookup failed/i); + expect(String(out)).toMatch(/ECONNREFUSED/); }); - // A capped probe must say what it actually checked, and a failed lookup is not - // evidence of absence. - it('reports the truncation and any failed lookups rather than a bare negative', async () => { + // A capped probe must say what it actually checked — the caveat carries the + // scope so the model cannot present a partial scan as a whole-fleet negative. + it('reports the truncation and any failed lookups in the caveat', async () => { (findInstanceWithProcesses as unknown as Mock).mockResolvedValueOnce({ instance: null, checked: 60, total: 90, failed: 3, }); const { ctx, emitProposal } = mockCtx(true); const [propose] = triggerTools(ctx); const out = await propose.invoke({ ...base, profilingType: 'network' }); - expect(emitProposal).not.toHaveBeenCalled(); - expect(String(out)).toMatch(/only checked 60 of its 90 instances/i); + expect(emitProposal).toHaveBeenCalledTimes(1); + expect(String(out)).toMatch(/only 60 of 90 instances were checked/i); expect(String(out)).toMatch(/3 lookup\(s\) failed/i); - expect(String(out)).toMatch(/not conclusive/i); - expect(String(out)).not.toMatch(/looks unavailable for/i); }); - it('refuses when capabilities cannot be read (template store blocked)', async () => { + // An unreadable layer template says nothing about the BACKEND, so it cannot + // withhold the card either. + it('proposes with a caveat when capabilities cannot be read (template store blocked)', async () => { (layerCapabilities as unknown as Mock).mockResolvedValueOnce(null); (resolveEffectiveLayer as unknown as Mock).mockResolvedValueOnce({ template: null, blocked: true }); const { ctx, emitProposal } = mockCtx(true); const [propose] = triggerTools(ctx); const out = await propose.invoke({ ...base, profilingType: 'ebpf' }); - expect(emitProposal).not.toHaveBeenCalled(); - expect(String(out)).toMatch(/could not read|cannot confirm/i); + expect(emitProposal).toHaveBeenCalledTimes(1); + expect(String(out)).toMatch(/unreachable or disabled/i); + expect(String(out)).toMatch(/says nothing about whether OAP accepts/i); }); - // No template ≠ unsupported: still propose, but say support is unconfirmed. - it('proposes with an unconfirmed note when the layer ships no template', async () => { + it('proposes with a caveat when the layer ships no template', async () => { (layerCapabilities as unknown as Mock).mockResolvedValueOnce(null); const { ctx, emitProposal } = mockCtx(true); const [propose] = triggerTools(ctx); const out = await propose.invoke({ ...base, profilingType: 'ebpf' }); expect(emitProposal).toHaveBeenCalledTimes(1); - expect(String(out)).toMatch(/could NOT confirm/); + expect(String(out)).toMatch(/ships no layer template/i); }); - it('refuses a type the layer does not declare', async () => { + // The template's `components` list is Horizon config; OAP applies no layer + // gate to any profiling create. A missing flag must not veto the card. + it('proposes a type the layer does not declare, flagging it as Horizon-side config', async () => { (layerCapabilities as unknown as Mock).mockResolvedValueOnce({ components: ['traceProfiling'] }); const { ctx, emitProposal } = mockCtx(true); const [propose] = triggerTools(ctx); const out = await propose.invoke({ ...base, profilingType: 'ebpf' }); - expect(emitProposal).not.toHaveBeenCalled(); - expect(String(out)).toMatch(/does not support/i); + expect(emitProposal).toHaveBeenCalledTimes(1); + expect(String(out)).toMatch(/does not list ebpf profiling/i); + expect(String(out)).toMatch(/OAP applies no layer gate/i); + expect(String(out)).toMatch(/NOT that the deployment cannot do it/i); }); it('does NOT propose (or emit) without profile:enable', async () => { diff --git a/apps/bff/src/ai/skill/triggers/tools.ts b/apps/bff/src/ai/skill/triggers/tools.ts index fb4d9ca..d45f216 100644 --- a/apps/bff/src/ai/skill/triggers/tools.ts +++ b/apps/bff/src/ai/skill/triggers/tools.ts @@ -42,9 +42,10 @@ import { layerCapabilities } from '../../../logic/layers/capabilities.js'; import { resolveEffectiveLayer } from '../../../logic/layers/effective.js'; import { getServerOffsetMinutes } from '../../../util/window.js'; -// Which template `components` flag gates each proposable profiling type — the -// layer must declare it for the type to be offer-able. Read at runtime; never -// hardcode which layers support what. +// Which template `components` flag ADVERTISES each profiling type. This is +// Horizon-side template config, not an OAP capability: none of OAP's five +// profiling create paths takes a layer, so a type missing here is a hint that +// the layer wasn't set up for it — never proof the backend would refuse. const PROFILING_COMPONENT: Record<ProfilingProposalType, string> = { trace: 'traceProfiling', async: 'asyncProfiling', @@ -66,9 +67,6 @@ const GO_LANGUAGES = new Set(['go', 'golang']); const ASYNC_EVENTS = ['CPU', 'ALLOC', 'LOCK', 'WALL', 'CTIMER', 'ITIMER']; const PPROF_EVENTS = ['CPU', 'HEAP', 'BLOCK', 'MUTEX', 'GOROUTINE', 'ALLOCS', 'THREADCREATE']; -// async caps at 600s server-side; keep the proposed minutes honest so the card -// and the fired task agree (trace/pprof are minutes; eBPF's 30-min cap is looser). -const MAX_ASYNC_MINUTES = 10; // Top frames as text so the agent can reason about the hot path (the flame // itself is rendered for the user, not readable by the model). The ranking @@ -81,14 +79,19 @@ function summarizeProfile(a: ProfilingAnalysis): string { if (!all.length) return ''; const pct = (v: number, total: number): string => `${Math.round((v / total) * 100)}%`; if (a.profilingType === 'ebpf') { + // Mirrors the aggregate the eBPF read actually asked OAP for: OFF_CPU is + // aggregated by DURATION (time blocked), ON_CPU by COUNT (dump count). + // Naming the wrong one invites the model to call a switch count "time". + const offCpu = a.summary.events?.[0] === 'OFF_CPU'; const total = Math.max(...all.map((e) => e.count), 1); const top = [...all] .filter((e) => e.count > 0) .sort((x, y) => y.count - x.count) .slice(0, 8) .map((e) => `${e.codeSignature} (${pct(e.count, total)})`); + const basis = offCpu ? 'INCLUSIVE share of time spent OFF-CPU (blocked)' : 'INCLUSIVE share of on-CPU dump count'; return top.length - ? ` Heaviest frames by INCLUSIVE sample share (eBPF carries no self time, so entry/root frames rank highest — read the tree, not the order, for the hot leaf): ${top.join('; ')}.` + ? ` Heaviest frames by ${basis} (eBPF carries no self time, so entry/root frames rank highest — read the tree, not the order, for the hot leaf): ${top.join('; ')}.` : ''; } const totalSelf = all.reduce((n, e) => n + Math.max(e.durationChildExcluded, 0), 0); @@ -113,21 +116,27 @@ export function triggerTools(ctx: AiRequestContext): StructuredToolInterface[] { return 'You lack permission to start profiling (profile:enable). Do not propose it; explain what a profiling task would reveal instead.'; } const layerKey = layer.toUpperCase(); - // No descriptor means we could NOT read what the layer offers — never the - // same thing as "it offers this". Blocked (store down / layer disabled) is - // a refusal; a layer that simply ships no template still gets its card, - // flagged as unconfirmed, so a legitimate proposal isn't hard-failed. - let unconfirmed = ''; + // Readiness signals we can read are ADVICE, never a veto. OAP owns what a + // profiling create accepts, and its checkCreateRequest / checkArgumentError + // paths consult NONE of what we can see here — not the layer, not the + // instance's runtime language, not queryPrepareCreateEBPFProfilingTaskData. + // A Horizon-side "no" would block a task the backend would have taken, and + // would blame the deployment for our own config. So collect the doubt, show + // the card anyway, and let OAP be the one that refuses — its rejection is a + // fact about the deployment, ours is a guess. + const caveats: string[] = []; const cap = await layerCapabilities(ctx.uiTemplateClient, layerKey); if (cap) { const supported = supportedProfilingTypes(cap.components); if (!supported.includes(profilingType)) { - return `The ${layerKey} layer does not support ${profilingType} profiling (it supports: ${supported.join(', ') || 'none'}). Read kb_layer_capabilities and propose a supported type, or tell the user profiling is unavailable here.`; + caveats.push( + `${layerKey}'s layer template does not list ${profilingType} profiling (it lists: ${supported.join(', ') || 'none'}). That template is Horizon-side configuration — OAP applies no layer gate to profiling — so the task may well be accepted. Say the layer isn't set up for it, NOT that the deployment cannot do it.`, + ); } } else if ((await resolveEffectiveLayer(ctx.uiTemplateClient, layerKey)).blocked) { - return `I could not read ${layerKey}'s capabilities — its layer template is unreachable or disabled — so I cannot confirm ${layerKey} supports ${profilingType} profiling, and no card was shown. Read kb_layer_capabilities for ${layerKey} first; if that comes back empty too, tell the user profiling support cannot be confirmed at this deployment.`; + caveats.push(`${layerKey}'s layer template is unreachable or disabled, so I could not read what it advertises. This says nothing about whether OAP accepts the task.`); } else { - unconfirmed = ` NOTE: ${layerKey} has no layer template here, so I could NOT confirm it supports ${profilingType} profiling — say that when you tell the user, and expect the task creation to fail if this deployment does not actually support it.`; + caveats.push(`${layerKey} ships no layer template here, so I could not read what it advertises. This says nothing about whether OAP accepts the task.`); } // Trace profiling monitors ONE endpoint: OAP's ProfileTaskCreationRequest // takes `endpointName: String!` and rejects an empty one, so a card without @@ -155,57 +164,73 @@ export function triggerTools(ctx: AiRequestContext): StructuredToolInterface[] { // targets. The per-instance scan matches the real create check. const probeOffset = await getServerOffsetMinutes(ctx.config, ctx.fetch); const probe = await findInstanceWithProcesses(ctx.opts, insts, probeOffset); - if (!probe.instance) { - if (probe.error) { - return `Could not check whether ${service}'s instances report a process — every lookup failed (${probe.error}). No card was shown. Say the check could not be completed; do NOT tell the user network profiling is unavailable, that is not what this means.`; - } - // A bounded scan and any failed lookup both weaken the negative — say - // exactly what was and was not checked. - const caveats = [ - probe.checked < probe.total ? `I only checked ${probe.checked} of its ${probe.total} instances` : null, + if (probe.instance) { + instanceIds = [probe.instance.id]; + instanceLabel = probe.instance.name; + } else { + // Our probe is time-scoped; OAP's create check is NOT — it counts + // processes on the instance with no window at all. So a miss here + // (idle host, minute-bucket boundary, bounded scan, failed lookups) + // is weaker evidence than OAP's own gate. Target the first instance + // and let `getProcessCount` decide; its "The instance doesn't have + // processes." is the accurate answer, ours would be a guess. + instanceIds = [insts[0].id]; + instanceLabel = insts[0].name; + const scope = [ + probe.error ? `every process lookup failed (${probe.error})` : null, + probe.checked < probe.total ? `only ${probe.checked} of ${probe.total} instances were checked` : null, probe.failed > 0 ? `${probe.failed} lookup(s) failed` : null, ].filter(Boolean); - const conclusive = caveats.length === 0; - return `None of ${service}'s instances I checked reports a process in the last 30 minutes, and OAP rejects a network-profiling task on an instance with no processes — no card was shown.${caveats.length ? ` Note ${caveats.join('; ')}, so this is not conclusive — ask the user which instance to profile.` : ''}${conclusive ? ` Network profiling needs a Rover eBPF agent on the target host; tell the user it looks unavailable for ${service}.` : ''}`; + caveats.push( + `No process reported recently on the instances I could check${scope.length ? ` (${scope.join('; ')})` : ''}, so I targeted ${insts[0].name}. OAP checks this itself without a time window when the task is created — if it rejects with "The instance doesn't have processes", THAT is the real answer, and it points at a missing Rover eBPF agent. Do not claim that before approval.`, + ); } - instanceIds = [probe.instance.id]; - instanceLabel = probe.instance.name; } else { - // async-profiler is JVM-only, pprof is Go-only — target ONLY the - // instances that can run it, never the whole fleet. OAP reports - // "UNKNOWN" (never null) when it can't tell, so those stay in and a - // fleet with no language data still profiles. Uses the runtime - // language, not a per-layer assumption. + // async-profiler is JVM-only and pprof is Go-only as a matter of what + // the AGENT can collect — OAP itself applies no language check, so this + // steers TARGETING (profile the instances that can actually run it), + // it does not decide whether the task may exist. When the runtime rules + // every instance out, the reported language is the thing most likely to + // be wrong (a mislabelled or re-registered instance), so target the + // fleet and flag it rather than refusing a task OAP would accept. const wantGo = profilingType === 'pprof'; const runnable = wantGo ? GO_LANGUAGES : JVM_LANGUAGES; const targets = insts.filter((i) => { const l = (i.language ?? '').toLowerCase(); return !l || l === 'unknown' || runnable.has(l); }); - if (!targets.length) { + const matched = targets.length > 0; + const chosen = matched ? targets : insts; + if (!matched) { const langs = [...new Set(insts.map((i) => (i.language ?? '').toLowerCase()).filter(Boolean))]; - return `${service}'s instances report ${langs.join('/')}, but ${profilingType} profiling is ${wantGo ? 'Go' : 'JVM'}-only. Propose ${wantGo ? 'async (JVM) or trace' : 'pprof (Go) or trace'} instead — match the profiler to the runtime language.`; + caveats.push( + `${service}'s instances report ${langs.join('/')}, and ${profilingType} profiling is ${wantGo ? 'Go' : 'JVM'}-only, so the agent will most likely collect nothing. OAP accepts the task regardless — it applies no language check. Say this plainly and offer ${wantGo ? 'async (JVM) or trace' : 'pprof (Go) or trace'} instead; only go ahead if the user believes the reported runtime is wrong.`, + ); } - instanceIds = targets.map((i) => i.id); + instanceIds = chosen.map((i) => i.id); instanceLabel = - targets.length === 1 - ? targets[0].name - : targets.length === insts.length - ? `${targets.length} instances` - : `${targets.length} of ${insts.length} instances (${wantGo ? 'Go' : 'JVM'} runtime)`; + chosen.length === 1 + ? chosen[0].name + : chosen.length === insts.length + ? `${chosen.length} instances` + : `${chosen.length} of ${insts.length} instances (${wantGo ? 'Go' : 'JVM'} runtime)`; } } - // eBPF CPU profiling is the one type this query actually gates: OAP's own - // create form uses it, counting processes that advertise - // SUPPORT_EBPF_PROFILING. A false is conclusive here (unlike for network, - // whose create check is weaker), so refuse rather than emit a doomed card. + // What OAP's own create form shows before enabling its button — processes + // advertising SUPPORT_EBPF_PROFILING over a rolling 10 minutes. It is a + // READINESS hint, not the create gate: createEBPFProfilingFixedTimeTask + // validates only the service, that each submitted label exists, and the + // 60s minimum duration — it never runs this query. A rover restart or a + // metadata lag flips it to false on a deployment that profiles fine, so + // carry it as doubt instead of refusing. if (profilingType === 'ebpf') { const ready = await serviceCanEbpfProfile(ctx.opts, serviceId); if (ready.error) { - return `Could not check whether ${service} has an eBPF-profilable process — the lookup failed (${ready.error}). No card was shown. Say the check could not be completed rather than that eBPF profiling is unavailable.`; - } - if (!ready.could) { - return `${service} has no process advertising eBPF-profiling support in the last 10 minutes, so OAP would reject an eBPF task — no card was shown. eBPF profiling needs a Rover agent on the target host; tell the user it is unavailable for ${service}.`; + caveats.push(`I could not check whether ${service} has an eBPF-profilable process — the lookup failed (${ready.error}). Say the check could not be completed, not that eBPF profiling is unavailable.`); + } else if (!ready.could) { + caveats.push( + `No process on ${service} advertised eBPF-profiling support in the last 10 minutes, which is what OAP's own create form checks before enabling its button — but the create call itself does not consult it, so the task may still run. eBPF profiling needs a Rover agent on the target host; if it collects nothing, that is the likely reason.`, + ); } } // Normalise the event to one this profiler knows (default CPU) so the card @@ -216,14 +241,19 @@ export function triggerTools(ctx: AiRequestContext): StructuredToolInterface[] { const ev = (event ?? 'CPU').toUpperCase(); events = [known.includes(ev) ? ev : 'CPU']; } - const effMinutes = profilingType === 'async' ? Math.min(durationMinutes, MAX_ASYNC_MINUTES) : durationMinutes; + // OAP fixes a NETWORK task at 10 minutes (NETWORK_PROFILING_DURATION) and + // its create request carries no duration field at all, so any number the + // model proposed is fiction — say so rather than letting it narrate one. + if (profilingType === 'network') { + caveats.push('OAP runs every network-profiling task for a fixed 10 minutes and takes no duration argument, so the collection window you proposed does not apply. Tell the user 10 minutes.'); + } ctx.emitProposal({ kind: 'profiling', profilingType, layer: layerKey, serviceId, service, - durationMinutes: effMinutes, + durationMinutes, ...(endpoint ? { endpoint } : {}), ...(instanceIds ? { instanceIds, instanceLabel } : {}), ...(events ? { events } : {}), @@ -232,7 +262,8 @@ export function triggerTools(ctx: AiRequestContext): StructuredToolInterface[] { rationale, expectation, }); - return `Proposed a ${profilingType}-profiling task to the user as a decision card${instanceLabel ? ` (targets: ${instanceLabel})` : ''}. It is NOT running — the user must approve it. Do not analyze now; stop here, tell the user to approve it, and that once it has collected data you will call analyze_profiling to read the result.${unconfirmed}`; + const notes = caveats.length ? ` Tell the user these caveats BEFORE they approve: ${caveats.join(' ')}` : ''; + return `Proposed a ${profilingType}-profiling task to the user as a decision card${instanceLabel ? ` (targets: ${instanceLabel})` : ''}. It is NOT running — the user must approve it. Do not analyze now; stop here, tell the user to approve it, and that once it has collected data you will call analyze_profiling to read the result.${notes}`; }, { name: 'propose_profiling', diff --git a/apps/bff/src/http/query/async-profile.ts b/apps/bff/src/http/query/async-profile.ts index ae863af..ec70403 100644 --- a/apps/bff/src/http/query/async-profile.ts +++ b/apps/bff/src/http/query/async-profile.ts @@ -72,13 +72,16 @@ function clampTaskListLimit(raw: string | undefined): number { return Math.min(Math.floor(n), MAX_TASK_LIST_LIMIT); } -/** Per-task caps for async-profiler / pprof bodies. OAP itself only - * rejects `duration <= 0`, so without these a caller with `profile:enable` - * could submit an hours-long profile that pegs the target instance's - * CPU. Caps match the booster-ui defaults: 600s = 10 min duration; up - * to 32 target instances per task; up to 8 event types. */ +/** Per-task caps for async-profiler / pprof bodies. + * + * These two flavours take duration in DIFFERENT units, and only pprof's bound + * is OAP's: `PprofMutationService.checkArgumentError` rejects `duration > 15` + * with "duration cannot be greater than 15 minutes" (CPU/BLOCK/MUTEX only). + * Async is seconds and OAP bounds it only by `duration <= 0`, so the async cap + * below is OURS — a route-level guard, since any caller holding `profile:enable` + * could otherwise peg a fleet's CPU for hours. Never describe it as OAP's. */ const MAX_ASYNC_DURATION_SEC = 600; -const MAX_PPROF_DURATION_SEC = 600; +const MAX_PPROF_DURATION_MIN = 15; const MAX_PPROF_DUMP_PERIOD_SEC = 60; const MAX_TARGET_INSTANCES = 32; const MAX_EVENTS_PER_TASK = 8; @@ -90,11 +93,15 @@ function clampPositiveInt(v: unknown, max: number, fallback: number | null): num return Math.min(max, Math.round(v)); } -function clampInstanceIds(ids: unknown): string[] { +/** Target instances, or `null` when the caller asked for more than the guard + * allows. OAP caps nothing here, so the cap is ours — which is exactly why + * exceeding it must be an ERROR and not a silent slice: quietly profiling 32 + * of 40 instances leaves the approved decision card advertising a fleet the + * fired task never covered, with nothing on screen saying so. */ +function pickInstanceIds(ids: unknown): string[] | null { if (!Array.isArray(ids)) return []; - return ids - .filter((s): s is string => typeof s === 'string' && s.length > 0) - .slice(0, MAX_TARGET_INSTANCES); + const clean = ids.filter((s): s is string => typeof s === 'string' && s.length > 0); + return clean.length > MAX_TARGET_INSTANCES ? null : clean; } function clampEvents<E extends string>(events: unknown): E[] { @@ -284,11 +291,16 @@ export function registerAsyncProfileRoutes( payload.errorReason = `duration is required and must be 1..${MAX_ASYNC_DURATION_SEC} seconds`; return reply.send(payload); } + const instanceIds = pickInstanceIds(raw.serviceInstanceIds); + if (instanceIds === null) { + payload.errorReason = `too many target instances (max ${MAX_TARGET_INSTANCES} per task) — split the fleet across several tasks`; + return reply.send(payload); + } // Sanitised body — OAP gets exactly the fields it expects, all // bounded. Unknown keys are dropped. const sanitised: AsyncProfilingTaskCreationRequest = { serviceId: raw.serviceId, - serviceInstanceIds: clampInstanceIds(raw.serviceInstanceIds), + serviceInstanceIds: instanceIds, duration, events: clampEvents<AsyncProfilingEvent>(raw.events), ...(clampExecArgs(raw.execArgs) !== undefined ? { execArgs: clampExecArgs(raw.execArgs)! } : {}), @@ -386,12 +398,17 @@ export function registerAsyncProfileRoutes( // events (HEAP / GOROUTINE / ALLOCS / THREADCREATE) are point-in- // time and don't carry duration. Forward whatever the caller sent, // clamped when present so the same upper bound applies. + const pprofInstanceIds = pickInstanceIds(raw.serviceInstanceIds); + if (pprofInstanceIds === null) { + payload.errorReason = `too many target instances (max ${MAX_TARGET_INSTANCES} per task) — split the fleet across several tasks`; + return reply.send(payload); + } const sanitised: PprofTaskCreationRequest = { serviceId: raw.serviceId, - serviceInstanceIds: clampInstanceIds(raw.serviceInstanceIds), + serviceInstanceIds: pprofInstanceIds, events: typeof raw.events === 'string' ? raw.events : '', ...(raw.duration !== undefined - ? { duration: clampPositiveInt(raw.duration, MAX_PPROF_DURATION_SEC, null) ?? 0 } + ? { duration: clampPositiveInt(raw.duration, MAX_PPROF_DURATION_MIN, null) ?? 0 } : {}), ...(raw.dumpPeriod !== undefined ? { diff --git a/apps/bff/src/http/query/ebpf.ts b/apps/bff/src/http/query/ebpf.ts index 9bec899..da13929 100644 --- a/apps/bff/src/http/query/ebpf.ts +++ b/apps/bff/src/http/query/ebpf.ts @@ -238,6 +238,12 @@ function softErr<T extends { reachable: boolean; error?: string }>(p: T, e: unkn * KiB so a stray large value can't blow * OAP's serializer. */ +// OAP's own floor: `EBPFProfilingMutationService.FIXED_TIME_MIN_DURATION` = 60s, +// enforced as "the fixed time duration must be greater than or equals 60s". It +// sets NO maximum, so the ceiling below is ours — a route-level guard, not a +// backend rule. Rejecting below the floor here only turns OAP's own refusal into +// a clearer message; it never widens what OAP accepts. +const MIN_EBPF_DURATION_SEC = 60; const MAX_EBPF_DURATION_SEC = 30 * 60; const MAX_PROCESS_LABELS = 32; const MAX_LABEL_LEN = 128; @@ -429,8 +435,8 @@ export function registerEBPFRoutes(app: FastifyInstance, deps: EBPFRouteDeps): v return reply.send(payload); } const duration = clampPositiveInt(raw.duration, MAX_EBPF_DURATION_SEC, null); - if (duration === null) { - payload.errorReason = `duration is required and must be 1..${MAX_EBPF_DURATION_SEC} seconds`; + if (duration === null || duration < MIN_EBPF_DURATION_SEC) { + payload.errorReason = `duration is required and must be ${MIN_EBPF_DURATION_SEC}..${MAX_EBPF_DURATION_SEC} seconds`; return reply.send(payload); } const startTime = diff --git a/apps/bff/src/logic/oap/profiling.ts b/apps/bff/src/logic/oap/profiling.ts index 55fc13f..2141587 100644 --- a/apps/bff/src/logic/oap/profiling.ts +++ b/apps/bff/src/logic/oap/profiling.ts @@ -824,10 +824,19 @@ async function analyzeEbpf( if (!schedules.length) return base; // task exists but no schedules collected yet const scheduleIdList = schedules.map((s) => s.scheduleId); const timeRanges = schedules.map((s) => ({ start: s.startTime, end: s.endTime })); + // The aggregate type has to follow the TARGET, because OAP gives the same + // enum two different meanings: for OFF_CPU, COUNT is "the number of times the + // process is switched to off cpu by the scheduler" while DURATION is "the + // total time spent in off cpu". OFF_CPU is chosen precisely to find what + // BLOCKS a service, so counting switches ranks a frame that yields constantly + // for microseconds above the one that blocks once for a second — the exact + // inversion of the question being asked. ON_CPU has no DURATION meaning + // (COUNT is its dump count), so it stays on COUNT. + const aggregateType = task.targetType === 'OFF_CPU' ? 'DURATION' : 'COUNT'; const an = await graphqlPost<{ result: { tip: string | null; trees: Array<{ elements: EbpfStack[] }> } | null }>( opts, ANALYSIS_EBPF_RESULT, - { scheduleIdList, timeRanges, aggregateType: 'COUNT' }, + { scheduleIdList, timeRanges, aggregateType }, ); base.tip = an.result?.tip ?? null; base.trees = (an.result?.trees ?? []).map((t) => mapEbpfTree(t.elements)); diff --git a/docs/operate/ai-assistant.md b/docs/operate/ai-assistant.md index 6bb477a..8ecb4c7 100644 --- a/docs/operate/ai-assistant.md +++ b/docs/operate/ai-assistant.md @@ -11,13 +11,15 @@ The AI Assistant is an in-app chat that answers questions about your system in p - Show a service's **traces** inline — the trace list it read, and on a row click that trace's span **waterfall**, the same views as the Traces tab. It hands you the traces to read; it does not read span contents itself, so trace exploration stays your call. It follows the layer's trace configuration and supports **both** trace modes: on a **native** SkyWalking-tracing layer it reads traces by service; on a **Zipkin**-tracing layer (mesh / Kubernetes — Envoy ALS, rover) it first lists th [...] - Show a service's **logs** inline — the stored log stream with row → detail, the same view as the layer Logs tab; distinct from the Kubernetes live tail below (these are **stored** logs), gated by your `logs:read` permission. For a **browser** app it can likewise show the **Browser errors** list — the client-side JS error stream with a row → stack-trace detail, gated by your `browser-errors:read` permission. Both capture their rows, so the list and each row's detail open from what was read. - Read a Kubernetes pod's **on-demand logs** — pull a container's recent logs (the error stack) and show the fetched lines inline as a read-only result. This is the same on-demand-log path as the Pod Logs tab: logs are streamed live from the cluster and never stored, and it requires your `logs:read` permission. The block is a captured result, not a console — it doesn't refresh on its own; ask again to fetch a newer window, or open the **Pod Logs** tab to keep a live tail running. When a [...] -- Show a finished **profiling** result inline — for a code profile the **flame graph** the Profiling tab draws, with the hottest frames called out in the prose (by self time, or by share of samples for eBPF profiles, which carry no self time); for a **trace** profile, the profiled trace's span **waterfall** beside the flame; for **network** profiling, the **process-conversation graph** (the processes and the conversations between them). When there is nothing to show — no eBPF agent repor [...] +- Show a finished **profiling** result inline — for a code profile the **flame graph** the Profiling tab draws, with the hottest frames called out in the prose; for a **trace** profile, the profiled trace's span **waterfall** beside the flame; for **network** profiling, the **process-conversation graph** (the processes and the conversations between them). Frames are ranked in the unit that profiler reports, and the prose names it: trace profiles rank by **self time in milliseconds**, asy [...] - Run a **guided root-cause investigation** — it loads a matching playbook (a master **root-cause** method plus focused variants for latency, error-rate/SLA, saturation, middleware, Kubernetes-workload and service-mesh) and works root service → calling chain → error stack, walking the topology upstream and following the layer hierarchy down into backing infra/database layers. A middleware dependency (database / cache / MQ) is a topology leaf with nothing downstream, so the investigation [...] - Narrate an ordered answer: a sentence or two, then a numbered figure, then interpretation, then the next figure — referencing the figures in the prose. A single running **Figure N** counter numbers *every* inline block — figure, topology, hierarchy, deployment, instance map, API dependency, traces, Zipkin traces, logs, browser errors, pod logs, profiling flame, network process graph or profiling proposal — so the prose can point at any of them. - Every data block is a **captured snapshot**, not a live view. The assistant reads the data once, freezes it into the block, and the block stamps **captured <time>** beside a small **replay** badge. Figures, all five dependency maps (topology, hierarchy, deployment, instance map, API dependency), traces and Zipkin traces, logs, browser errors, pod logs, the profiling flame (with its trace waterfall) and the network process graph all replay from what was read — so reopening a conve [...] It is **read-only by default**: it observes and explains, and never changes configuration, rules, or dashboards. Every data action it takes checks the **same read permission you already hold**, so the assistant can never see more than you can. The one exception is **profiling**: when metrics and traces can't localise a cause, the assistant may *propose* a profiling task as a decision card (what it found, why profiling, what it expects) — nothing runs until **you approve it** in the popou [...] +**What the assistant will and won't decide for you.** Whether a profiling task is *allowed* is your OAP backend's call, not Horizon's. The assistant checks what it can see beforehand — whether the layer is set up for that profiling type, whether the instances report a runtime the profiler matches, whether any process is reporting eBPF support — but none of those are checks OAP itself performs when a task is created. So it treats them as **caveats, not vetoes**: it shows the card, tells y [...] + ## How it stays grounded The assistant does not free-associate over raw numbers. It answers by combining three sources, and it is the interplay between them — not any one alone — that lets it relate metrics, traces, logs and topology into one coherent picture instead of a pile of disconnected data:
