From d85ef098e3ae383c358d3aebe9319af00bb0155d Mon Sep 17 00:00:00 2001 From: caiyuwang Date: Fri, 16 Oct 2026 17:50:06 +0800 Subject: [PATCH 1/3] feat: add metrics and span attributes for skill download/load events --- .../2026-10-16-skill-observability.md | 80 ++++ .../2026-10-16-skill-observability.zh.md | 80 ++++ specs/skill-observability/README.md | 35 ++ specs/skill-observability/README.zh.md | 35 ++ tests/skills/test_skill_telemetry.py | 354 ++++++++++++++++++ veadk/skills/materializer.py | 15 + veadk/skills/registry.py | 8 +- veadk/skills/telemetry.py | 114 ++++++ veadk/tracing/telemetry/portal_metrics.py | 20 + veadk/tracing/telemetry/telemetry.py | 3 + 10 files changed, 743 insertions(+), 1 deletion(-) create mode 100644 prd-spec/features/skill-observability/2026-10-16-skill-observability.md create mode 100644 prd-spec/features/skill-observability/2026-10-16-skill-observability.zh.md create mode 100644 specs/skill-observability/README.md create mode 100644 specs/skill-observability/README.zh.md create mode 100644 tests/skills/test_skill_telemetry.py create mode 100644 veadk/skills/telemetry.py diff --git a/prd-spec/features/skill-observability/2026-10-16-skill-observability.md b/prd-spec/features/skill-observability/2026-10-16-skill-observability.md new file mode 100644 index 000000000..180e326cb --- /dev/null +++ b/prd-spec/features/skill-observability/2026-10-16-skill-observability.md @@ -0,0 +1,80 @@ +# Skill download and instruction-load observability + +[中文版](2026-10-16-skill-observability.zh.md) + +- Change ID: `skill-observability`; created/revised: 2026-10-16; status: implemented; affected checks passed, full regression has existing/environment failures. +- Contract: [skill-observability](../../../specs/skill-observability/README.md). +- Source: AgentKit Skills runtime observability design, revision 2102 (2026-10-16). +- Scope approval: the user approved the proposed Runtime/Python-tool scope and explicitly included `actb-mono/sandboxes/agentkit-skill`. Implementation is local; no commit, publication or cloud operations are authorized. + +## Evidence, goals and boundaries + +`VeSkillRegistry.get_skill` materializes packages but drops remote identity on conversion to ADK Skill. ADK 2.7.1 caches fetched skills per invocation, so registry calls cannot count instruction loads. `SkillsTool` has a latency histogram but no counters from the design. Sandbox Python pins VeADK 1.1.14 / ADK 1.34.0; Go has its own package store and tool wrapper. Both sandbox telemetry setups currently configure traces, not metric export. + +Count package acquisition and successful instruction delivery with the exact source identities and schema in the linked contract. Preserve SDK signatures, cache/install policy, existing metrics, tool results, cancellation and cleanup. Do not count discovery, resource reads, scripts, A2A forwarding, or task success as instruction loading. Do not implement issue reporting, dashboards, new dependencies or dependency upgrades. Changes to AIO protocols, daemon lifecycle and isolation are outside this change. + +## Requirements and scenarios + +- FR-1: A recognized remote package acquisition emits one `skill_download` observation on completion, including validated disk-cache restoration and failure. Invalid cache followed by download is one attempt, with `cache_hit=false`. +- FR-2: A successful instruction response emits one `skill_load`. Two successful `load_skill` calls in one invocation count twice even when ADK fetches once. Unknown skills, read failures and cancellation never count as successful loads. +- FR-3: Cloud identity survives conversion/cache. Sandbox Python distinguishes built-ins, SkillHub slugs, explicit AgentKit IDs and both space kinds. Go retains its existing four-source support. Unattributable local skills are not guessed to be built-ins. +- FR-4: Use existing process-global OTEL providers; sandbox installs a periodic OTLP metric exporter only when no provider exists. Trace and metric disable switches are independent. Export failures do not affect conversations; shutdown flush is bounded. +- FR-5: New telemetry never records archive URLs, credentials, instruction bodies, paths or raw exception messages. Existing content telemetry policy remains unchanged. +- FR-6: Python and Go use the same labels and source semantics for supported sources; prefetch produces downloads only. Existing installed packages/session links are retained. + +## Design and affected consumers + +Runtime: add a small `veadk/skills/telemetry.py` recorder, instrument the existing materializer, preserve cloud identity under reserved ADK frontmatter metadata, and extend the existing VeADK tool tracing callback for successful native ADK `LoadSkillTool` responses. No replacement toolset or new global cache. Runtime load telemetry follows the existing VeADK tracing-hook activation; metrics are recorded independently of span sampling once the hook is installed. + +Sandbox: implement a local recorder using already-installed OTEL packages, because the sandbox's released SDK must work independently of unpublished VeADK changes. Instrument `playground_skills/packages.py`, SkillHub reconciliation, and the existing `ObservedSkillsTool`. Persist the installed SkillHub slug alongside its package so a failed replacement cannot relabel an older built-in. The Go package store records downloads; catalog metadata carries identity into the existing tracked tool wrapper. Built-ins produce no download events. Configure metric export in existing telemetry setup and flush with existing shutdown handling. + +Counters count acquisition attempts after identity resolution. Metadata-list failures without a resolved skill ID cannot produce a per-skill download. Successful acquisition means a validated usable package, not just HTTP success. Python worker-thread downloads may finish after awaiting-task cancellation; count their actual acquisition result, never a cancelled load. Go context cancellation is an error outcome. Metrics contain only contract dimensions; `consumer`, `cache_hit` and errors are span attributes. Span duration covers acquisition, including validation/installation. Cache hit rates derived from spans are subject to trace sampling. + +## Tasks, tests and acceptance + +| Requirement | Task | Acceptance | Verification | +| --- | --- | --- | --- | +| FR-1, FR-3 | T-1: Runtime materializer, registry and recorder | AC-1: exact identity/counts for network, cache, invalid package and unknown source | `uv run --extra dev pytest tests/skills/` | +| FR-2, FR-5 | T-2: native ADK load hook and regression tests | AC-2: repeated cached loads count; failure/resource/script calls do not; no secrets | `uv run --extra dev pytest tests/test_portal_metrics.py tests/test_tracing_content.py tests/skills/` | +| FR-1, FR-2, FR-3, FR-6 | T-3: Sandbox Python packages, reconciliation and tool adapter | AC-3: all five sources, install failure/cache, preserved slug, no duplicate load | Sandbox Python targeted/full tests with isolated state | +| FR-1, FR-2, FR-3, FR-6 | T-4: Sandbox Go packages, catalog and tracked tool | AC-4: matching Go counts and cancellation/cleanup | `go run ./cmd/skill-go test ./...` in sandbox `go/` | +| FR-4, FR-5 | T-5: sandbox OTLP metrics and exporter tests | AC-5: local HTTP collector receives counters; switches/provider reuse/shutdown work | Python local collector tests; Go metric exporter tests | +| All | T-6: reconcile bilingual docs, examples and verification | AC-6: paired contracts and actual check evidence | SDK regression; sandbox `make verify`; `git diff --check` | + +SDK final regression: `uv run --extra dev pytest -n 2 -m "not codex_smoke and not piagent_smoke"` (two workers). No real cloud/provider tests. Before any later authorized commit, synchronize intended base and run repository pre-commit gates. Frontend/browser gates are not applicable: no Studio code or API changes. + +## Review, risks and delivery record + +Direct design review (review-spec unavailable), 2026-10-16: checked source ownership, cache/load separation, metric-label parity, API compatibility, error redaction, cancellation, provider ownership and bilingual equivalence. Resolved double counting by keeping Runtime forwarding uninstrumented and Sandbox counters in its adapter; resolved unpublished dependency coupling with independent sandbox instrumentation. The user confirmed sandbox business-code scope after source-access clarification. The user also explicitly approved keeping Go without `sp-*` support: this change adds observations only, with no SDK upgrade or source-support expansion. No AIO protocol claim is made. + +Risks: native ADK response metadata is a version-sensitive integration covered by real installed ADK tests; inventory joins are still required for never-used skills. New sandbox exporters add periodic background work, not synchronous network calls during tool execution. Existing span sampling limits trace-derived cache statistics. Tested bases: VeADK `d55fe8fa`; ACTB `7354ed5`. Execution date: 2026-10-16. + + +### Implementation review + +T-1 through T-6 are implemented. The actual ADK load response retains registry-owned identity across invocation-cache hits. Sandbox records installation independently of instruction delivery; Go reads the same installed SkillHub provenance receipt as Python. Archives cannot override the installer-written receipt. Source resolution failures do not invent labels. Existing public signatures, install/rollback rules and tool results remain unchanged. Go metric imports promote already-locked indirect modules to direct dependencies; versions and `go.sum` are unchanged. + +Direct implementation review checked cancellation, failed installations, cache reuse, failed provider recording, successful-instruction boundaries, source parity, endpoint configuration and provider ownership. Regression tests failed before implementation (missing recorder functions), and the implementation-switch provenance test reproduced a built-in misclassification before the fix. No blocking finding remains in the affected scope. No deployment/readiness claim is made. + +Startup tradeoff: SkillHub reconciliation is a short-lived process, so its observations must flush before exit. Only actual SkillHub configuration initializes this exporter; empty configuration and built-in disabling add no telemetry initialization. Startup exporters have 200 ms timeouts; regular Python metric export uses 3 s, and Go shutdown shares a 5 s context. A local slow-collector experiment measured 575 ms import/setup plus 533 ms shutdown with both signals timing out. These are local samples, not image startup guarantees; this overhead applies only to the Python startup installer, not each tool call. The local collector tests also verify metric delivery and provider reuse from that startup setup. + +### Verification record (2026-10-16) + +Tested scope is the uncommitted diff over VeADK `d55fe8fa` and ACTB `7354ed5`. SDK uses Python 3.12.15 / ADK 2.7.1; Sandbox uses its own Python environment / ADK 1.34.0 and the existing pinned Go SDK. + +| Check | Status | Actual result | +| --- | --- | --- | +| `uv run --extra dev pytest tests/skills tests/test_portal_metrics.py tests/test_tracing_content.py -q` | pass | 120 tests; actual ADK repeated loads, cache/error/cancellation, identity, redaction and fail-open recorder coverage. | +| `uv run --extra dev pytest -n 2 -m "not codex_smoke and not piagent_smoke"` | fail | 6971 passed, 16 failed, 59 skipped, 4 xfailed, 2 collection errors. Failures require unavailable optional `llama_index`, `openai_codex` and `anthropic` packages; unrelated dependency installation was not folded into this feature. Two workers were used. | +| Sandbox targeted Python: `TMPDIR=/private/tmp PYTHONPATH=src MODEL_AGENT_API_KEY=local-test-placeholder OTEL_TRACES_EXPORTER=none OTEL_METRICS_EXPORTER=none .venv/bin/python -m pytest tests/test_skill_observability.py tests/test_skill_reconciler.py tests/test_playground_skills.py tests/test_sandbox_agent.py tests/test_a2a_telemetry.py -q` | pass | 42 tests, 7 subtests. Collector tests launch clean child processes with metrics explicitly enabled. | +| Sandbox full Python: same environment, `.venv/bin/python -m pytest tests -q` | fail | 182 passed, 9 failed, 20 skipped, 13 subtests. All nine remaining cleanup/migration-contract failures reproduce against copied, tracked source from ACTB `7354ed5`; no new observability test fails. | +| `TMPDIR=/private/tmp go run ./cmd/skill-go test ./...` (Sandbox `go/`) | pass | All Go packages; canonical macOS temporary directory avoids pre-existing path-alias cleanup assertions. | +| `TMPDIR=/private/tmp go run ./cmd/skill-go test -race -run 'TestSkill|TestInstalledSkillHub' .` | pass | Affected Go acquisition/load/provenance and exporter tests, with race detection. | +| `make verify PYTHON=/Users/bytedance/actb-mono/sandboxes/agentkit-skill/.venv/bin/python` | pass | 28 root smoke tests, shell syntax and diff checks; does not establish container readiness. | +| Ruff 0.11.12 on changed Python files | pass | Used isolated `uvx --from ruff==0.11.12 ruff check `; the repository environment has no Ruff executable. | +| Bilingual pairs, schema identifiers, relative links and `git diff --check` | pass | PRD/spec pairs synchronized; Sandbox README and Unreleased changelog updated. | +| Real cloud/image readiness, deployed collector and grpc transport | not_run | No cloud/image operation authorized; local collector delivery uses HTTP/protobuf. | +| Repository pre-commit and remote synchronization | not_run | No commit requested; these remain mandatory before a later authorized commit. | +| Studio/browser gates | not_applicable | No frontend/backend Studio contract changed. | + +Initial Sandbox full collection required a dummy model API value; the final command uses a placeholder, disables external exporters and supplies `PYTHONPATH=src` for child processes. Tests that create an in-memory SDK provider explicitly remove `OTEL_SDK_DISABLED` locally. No real credentials, real model requests or machine-global configuration were used. The user approved recording these environment/tooling limitations here. Temporary baseline copies are removed after comparison. Affected checks establish local implementation behavior; full-repository acceptance is not green. diff --git a/prd-spec/features/skill-observability/2026-10-16-skill-observability.zh.md b/prd-spec/features/skill-observability/2026-10-16-skill-observability.zh.md new file mode 100644 index 000000000..3ebce39f0 --- /dev/null +++ b/prd-spec/features/skill-observability/2026-10-16-skill-observability.zh.md @@ -0,0 +1,80 @@ +# 技能下载与指令加载可观测 + +[English](2026-10-16-skill-observability.md) + +- 变更 ID：`skill-observability`；创建/修订：2026-10-16；状态：implemented；受影响检查通过，全量回归存在既有/环境失败。 +- 契约：[skill-observability](../../../specs/skill-observability/README.zh.md)。 +- 来源：AgentKit Skills 运行时可观测方案，revision 2102（2026-10-16）。 +- 范围批准：用户确认提出的 Runtime/Python 工具范围，并明确纳入 `actb-mono/sandboxes/agentkit-skill`。仅本地实现；未授权提交、发布或云端操作。 + +## 证据、目标与边界 + +`VeSkillRegistry.get_skill` 获取技能包，但转换成 ADK Skill 时丢失云端身份。ADK 2.7.1 按 invocation 缓存技能，因此 registry 调用不能计作指令加载。`SkillsTool` 有耗时 histogram，但没有方案要求的 counter。Sandbox Python 固定使用 VeADK 1.1.14 / ADK 1.34.0；Go 有独立的包存储和工具适配层。两种 Sandbox 遥测初始化目前都配置 trace，没有配置 metric 导出。 + +按照关联契约的准确来源身份和数据结构统计技能包获取与成功指令交付。保留 SDK 签名、缓存/安装策略、既有指标、工具结果、取消和清理行为。发现、资源读取、脚本、A2A 转发和任务成功均不计作指令加载。不实现问题主动汇报、看板、新增依赖或依赖升级。AIO 协议、守护进程生命周期和隔离机制不在本次变更范围。 + +## 需求与场景 + +- FR-1：已识别来源的远端技能包获取结束时记录一次 `skill_download`，覆盖验证成功的磁盘缓存恢复和失败。无效缓存后重新下载属于一次获取，`cache_hit=false`。 +- FR-2：成功返回指令时记录一次 `skill_load`。同一 invocation 两次成功 `load_skill` 计两次，即使 ADK 只获取一次。未知技能、读取失败和取消不计成功加载。 +- FR-3：云端身份在转换/缓存后保留。Sandbox Python 区分内置技能、SkillHub slug、显式 AgentKit ID 和两类空间；Go 保留既有四类来源支持。来源不明的本地技能不猜测成内置技能。 +- FR-4：使用既有进程级 OTEL provider；Sandbox 仅在未配置 provider 时安装周期 OTLP metric exporter。trace 与 metric 禁用开关独立。导出失败不影响对话；退出 flush 有时间上限。 +- FR-5：新增遥测不记录包 URL、凭据、指令正文、路径或原始异常消息。既有内容遥测策略保持不变。 +- FR-6：Python 与 Go 对已支持的来源使用相同标签和来源语义；预取只产生下载数据。保留既有安装包和会话链接。 + +## 设计与受影响消费者 + +Runtime：增加小型 `veadk/skills/telemetry.py` 记录模块，在既有 materializer 采集；将云端身份保存在 ADK frontmatter 的保留 metadata 中；在既有 VeADK 工具 tracing 回调中处理原生 ADK `LoadSkillTool` 的成功响应。不替换 toolset、不增加全局缓存。Runtime 加载采集沿用既有 VeADK tracing hook 的启用方式；hook 启用后，计数不依赖 Span 采样。 + +Sandbox：使用已经安装的 OTEL 包实现本地记录模块，确保已发布 SDK 无需依赖尚未发布的 VeADK 改动。在 `playground_skills/packages.py`、SkillHub 安装和既有 `ObservedSkillsTool` 采集。随安装包保存 SkillHub slug，避免替换失败时把旧内置技能标记成新的 SkillHub 技能。Go 包存储负责下载记录；目录元数据把身份传递给既有 tracked tool。内置技能不产生下载事件。在现有遥测初始化中配置 metric 导出，并复用退出 flush。 + +counter 从身份解析后的技能包获取开始计数。尚无技能 ID 的元数据列表失败无法产生按技能归属的下载。获取成功表示技能包通过验证且可用，不只是 HTTP 成功。Python 工作线程可能在等待任务取消后完成下载；按实际获取结果计数，但不计取消的加载。Go context 取消计入错误结果。metric 仅含契约规定的维度；`consumer`、`cache_hit` 和错误放在 Span。Span 耗时覆盖获取、验证和安装。通过 Span 计算的缓存命中率受 trace 采样影响。 + +## 任务、测试与验收 + +| 需求 | 任务 | 验收 | 验证 | +| --- | --- | --- | --- | +| FR-1, FR-3 | T-1：Runtime materializer、registry 与记录模块 | AC-1：网络、缓存、无效包和未知来源的身份/次数准确 | `uv run --extra dev pytest tests/skills/` | +| FR-2, FR-5 | T-2：原生 ADK 加载 hook 与回归测试 | AC-2：缓存后的重复加载计数；失败/资源/脚本不计；不泄露敏感信息 | `uv run --extra dev pytest tests/test_portal_metrics.py tests/test_tracing_content.py tests/skills/` | +| FR-1, FR-2, FR-3, FR-6 | T-3：Sandbox Python 包、安装与工具适配 | AC-3：五种来源、安装失败/缓存、slug 保留、不重复计数 | 使用隔离状态运行 Sandbox Python 定向/全模块测试 | +| FR-1, FR-2, FR-3, FR-6 | T-4：Sandbox Go 包、目录与 tracked tool | AC-4：Go 次数一致，取消/清理正确 | Sandbox `go/` 中运行 `go run ./cmd/skill-go test ./...` | +| FR-4, FR-5 | T-5：Sandbox OTLP metric 与导出测试 | AC-5：本地 HTTP collector 收到 counter；开关/provider 复用/退出有效 | Python 本地 collector 测试；Go metric exporter 测试 | +| 全部 | T-6：同步双语文档、示例与验证 | AC-6：契约成对，验证有实际证据 | SDK 回归；Sandbox `make verify`；`git diff --check` | + +SDK 最终回归：`uv run --extra dev pytest -n 2 -m "not codex_smoke and not piagent_smoke"`（两个 worker）。不运行真实云端/provider 测试。后续若获提交授权，须先同步目标基线并执行仓库 pre-commit 门禁。前端/浏览器门禁不适用：未改变 Studio 代码或 API。 + +## 评审、风险与交付记录 + +2026-10-16 直接设计评审（review-spec 不可用）：检查来源归属、缓存/加载区分、标签一致、API 兼容、错误脱敏、取消、provider 所有权与双语等价。Runtime 转发不采集、Sandbox 在适配层计数，消除重复计数；Sandbox 独立埋点，消除对未发布依赖的耦合。用户在源码访问澄清后确认 Sandbox 业务代码范围。用户还明确批准保留 Go 不支持 `sp-*` 的限制：本次只增加观测，不升级 SDK、不扩展来源支持。不对 AIO 协议作结论。 + +风险：原生 ADK 响应 metadata 是版本相关接入点，须用实际安装的 ADK 测试；“从未使用”仍需关联完整技能清单。新增 Sandbox exporter 增加周期后台工作，不在工具执行时同步请求网络。既有 Span 采样限制基于 trace 的缓存统计。测试基线：VeADK `d55fe8fa`；ACTB `7354ed5`。执行日期：2026-10-16。 + + +### 实现评审 + +T-1 至 T-6 已实现。真实 ADK 加载响应保留 Registry 提供的身份，invocation 缓存命中后仍保留。Sandbox 分别记录安装和指令交付；Go 与 Python 读取同一份已安装 SkillHub 来源记录。技能包不能覆盖安装器写入的来源记录。无法解析来源时不猜测标签。既有公开签名、安装/回滚规则和工具结果不变。Go 仅把已经锁定的间接 metric 模块提升为直接依赖，版本和 `go.sum` 不变。 + +直接实现评审检查了取消、安装失败、缓存复用、provider 记录失败、成功指令边界、来源一致性、endpoint 配置及 provider 所有权。回归测试在实现前因缺少记录函数失败；实现切换来源测试在修复前复现了误标为内置技能的问题。受影响范围无遗留阻断发现。不声明已部署或容器已就绪。 + +启动权衡：SkillHub 同步是短生命周期进程，观测必须在退出前尽力 flush。只有实际配置 SkillHub 才初始化 exporter；空配置和仅禁用内置技能不增加遥测初始化。启动 exporter 超时为 200 ms；常规 Python metric 导出为 3 s，Go 退出共用 5 s context。本地慢 collector 实验中，两种信号均超时时，导入/初始化为 575 ms、退出为 533 ms。这只是本地样本，不代表镜像启动时间保证；开销只发生在 Python 启动安装器，不发生在每次工具调用。本地 collector 测试也验证该启动初始化的 metric 发送和 provider 复用。 + +### 验证记录（2026-10-16） + +测试范围为 VeADK `d55fe8fa` 与 ACTB `7354ed5` 之上的未提交 diff。SDK 使用 Python 3.12.15 / ADK 2.7.1；Sandbox 使用自身 Python 环境 / ADK 1.34.0 及既有固定 Go SDK。 + +| 检查 | 状态 | 实际结果 | +| --- | --- | --- | +| `uv run --extra dev pytest tests/skills tests/test_portal_metrics.py tests/test_tracing_content.py -q` | pass | 120 项；覆盖真实 ADK 重复加载、缓存/错误/取消、身份、脱敏和记录失败不影响结果。 | +| `uv run --extra dev pytest -n 2 -m "not codex_smoke and not piagent_smoke"` | fail | 6971 passed、16 failed、59 skipped、4 xfailed、2 项收集错误。失败需要当前未安装的可选 `llama_index`、`openai_codex` 和 `anthropic` 包；本功能未纳入无关依赖安装。使用两个 worker。 | +| Sandbox 定向 Python：`TMPDIR=/private/tmp PYTHONPATH=src MODEL_AGENT_API_KEY=local-test-placeholder OTEL_TRACES_EXPORTER=none OTEL_METRICS_EXPORTER=none .venv/bin/python -m pytest tests/test_skill_observability.py tests/test_skill_reconciler.py tests/test_playground_skills.py tests/test_sandbox_agent.py tests/test_a2a_telemetry.py -q` | pass | 42 项、7 个 subtest。collector 测试在干净子进程中显式开启 metrics。 | +| Sandbox 全模块 Python：同上环境，`.venv/bin/python -m pytest tests -q` | fail | 182 passed、9 failed、20 skipped、13 个 subtest。余下九项 cleanup/迁移契约失败均在复制的 ACTB `7354ed5` 已跟踪源码上复现；新增观测测试无失败。 | +| `TMPDIR=/private/tmp go run ./cmd/skill-go test ./...`（Sandbox `go/`） | pass | 所有 Go package 通过；使用 macOS 规范临时目录，避开既有路径别名 cleanup 断言。 | +| `TMPDIR=/private/tmp go run ./cmd/skill-go test -race -run 'TestSkill|TestInstalledSkillHub' .` | pass | 受影响 Go 获取/加载/来源及 exporter 测试，开启 race 检查。 | +| `make verify PYTHON=/Users/bytedance/actb-mono/sandboxes/agentkit-skill/.venv/bin/python` | pass | 28 项根目录 smoke、shell 语法和 diff 检查；不代表容器就绪。 | +| 变更 Python 文件的 Ruff 0.11.12 检查 | pass | 使用隔离的 `uvx --from ruff==0.11.12 ruff check `；项目环境没有 Ruff 可执行文件。 | +| 双语配对、schema 标识符、相对链接及 `git diff --check` | pass | PRD/spec 成对同步；更新 Sandbox README 和 Unreleased 变更记录。 | +| 真实云端/镜像就绪、线上 collector 和 grpc 传输 | not_run | 未授权云端/镜像操作；本地 collector 发送采用 HTTP/protobuf。 | +| 仓库 pre-commit 与远端同步 | not_run | 未请求提交；后续获提交授权时仍必须执行。 | +| Studio/浏览器门禁 | not_applicable | 未改变 Studio 前后端契约。 | + +Sandbox 初次全模块收集需要测试模型 API 值；最终命令使用占位值、禁用外部 exporter，并设置 `PYTHONPATH=src` 供子进程导入。创建内存 SDK provider 的测试局部移除 `OTEL_SDK_DISABLED`。未使用真实凭据、真实模型请求或修改全局机器配置。用户同意在此记录这些环境/工具限制。临时基线副本在对比后删除。受影响检查验证了本地实现行为，全仓验收尚未全绿。 diff --git a/specs/skill-observability/README.md b/specs/skill-observability/README.md new file mode 100644 index 000000000..23de7890f --- /dev/null +++ b/specs/skill-observability/README.md @@ -0,0 +1,35 @@ +# Skill observability + +[中文版](README.zh.md) + +Component: `skill-observability`; status: implemented (local verification; full regression limitations in PRD); revised 2026-10-16. +Change: [skill-observability](../../prd-spec/features/skill-observability/2026-10-16-skill-observability.md). + +## Ownership and interfaces + +VeADK owns Runtime package acquisition and ADK instruction-load instrumentation in `veadk/skills/` and `veadk/tracing/telemetry/telemetry.py`. `actb-mono/sandboxes/agentkit-skill` owns Python/Go Sandbox installation, tool adaptation and exporter lifecycle. The maintained wire schema below is shared; the sandbox is independently released and does not import an unpublished SDK recorder. Existing tools/signatures/results and metrics remain compatible. Discovery, resource/script execution, issue reporting and business success are not owned here. + +## Contracts + +- CON-1: `skill_download` is an OTEL counter, unit `count`; dimensions are exactly `skill_source`, `skill_id`, `skill_version`, `skill_space_id`, `outcome`. `outcome` is `success` or `error` (including cancellation). One acquisition completion, including cache restore, adds one. No event for mere discovery or local symlink creation. +- CON-2: `skill_load` is an OTEL counter, unit `count`; dimensions are exactly the four identity fields above. One successful instruction response adds one. ADK invocation-cache hits still count. Errors, cancellation, forwarding and resource/script access do not. +- CON-3: Download Span `skill_download` has `skill.source`, `skill.id`, `skill.version`, `skill.space_id`, `outcome`, `cache_hit`, `consumer`. Failures add `error.type` and a safe static `error.description`, with ERROR status. Successful load annotates the existing tool Span with the four dotted identity fields and `consumer`. Consumer is `runtime` or `skill_sandbox`. Span duration is total acquisition time. Never add consumer/cache/error text to metric labels. +- CON-4: Identity follows the table. Missing version/space is the empty string, never a synthesized cache hash. Remote skills without a trustworthy ID and unclassified local skills are excluded. Registry-owned metadata overrides archive metadata for telemetry identity. SkillHub installation records the slug with the installed package, not just the current desired configuration. + +| skill_source | skill_id | skill_version | skill_space_id | +| --- | --- | --- | --- | +| builtin_skill | name | empty | empty | +| skillhub_skill | slug | empty | empty | +| skillhub_skill_space | s-* | sv-* when supplied | sp-* | +| agentkit_skill | s-* | vN | empty | +| agentkit_skill_space | s-* | vN when supplied or present in the legacy package path | ss-* | + +Python supports all five rows. The pinned Sandbox Go SDK still rejects `sp-*`; the user approved retaining that limitation. Local SkillHub receipts are shared across Python/Go selection. + +- CON-5: Record through the global OTEL provider, independently of Span sampling. Runtime load instrumentation is enabled with existing VeADK tracing hooks. No exporter is installed by the SDK recorder. Sandbox periodic OTLP metrics honor `OTEL_SDK_DISABLED`, `OTEL_METRICS_EXPORTER=none`, standard metric/base endpoint configuration and an existing provider. Trace disabling does not disable metrics. Default local collector uses OTLP/HTTP at port 4318; shutdown is bounded and failures are best effort. Metrics use `OTEL_EXPORTER_OTLP_METRICS_PROTOCOL` (or the base protocol), supporting HTTP/protobuf and grpc; interval defaults to 60000 ms. The short-lived SkillHub installer uses 200 ms exporter timeouts and flushes before exit; empty configuration and built-in disabling skip that initialization. Startup cost is recorded in the PRD. +- CON-6: Acquisition owns its observation until completion. Concurrent invocations do not share mutable observation state. Python `to_thread` cancellation does not stop an already-running acquisition: its actual result may arrive later, but no instruction-load success is recorded for the cancelled waiter. No new persistent state except the installed SkillHub provenance receipt. Existing locks, cache validation, retention and cleanup remain authoritative. +- CON-7: New telemetry excludes raw exceptions, URLs, tokens, file paths, instructions and user content. Export/recording failure must not change tool results or suppress application errors. No new credentials/configuration are required. Missing provider gives normal OTEL no-op behavior. + +## Verification and evolution + +CON-1/4/6 map to materializer/cache and sandbox package tests. CON-2/3 map to actual ADK tool and sandbox wrapper tests, including repeated loads, bad arguments, errors and cancellation. CON-5/7 map to in-memory OTEL and local collector tests for metrics, spans, redaction, switches, provider reuse and failures. Commands and actual evidence are maintained in the linked PRD. Adding dimensions or changing count semantics requires revisiting that design and both languages. Implementation status does not claim deployment or full-repository acceptance. diff --git a/specs/skill-observability/README.zh.md b/specs/skill-observability/README.zh.md new file mode 100644 index 000000000..fa3800f30 --- /dev/null +++ b/specs/skill-observability/README.zh.md @@ -0,0 +1,35 @@ +# 技能可观测 + +[English](README.md) + +组件：`skill-observability`；状态：implemented（本地验证；全量回归限制见 PRD）；修订：2026-10-16。 +关联变更：[skill-observability](../../prd-spec/features/skill-observability/2026-10-16-skill-observability.zh.md)。 + +## 责任与接口 + +VeADK 在 `veadk/skills/` 与 `veadk/tracing/telemetry/telemetry.py` 负责 Runtime 技能包获取及 ADK 指令加载埋点。`actb-mono/sandboxes/agentkit-skill` 负责 Python/Go Sandbox 安装、工具适配及 exporter 生命周期。共享下述上报结构；Sandbox 独立发布，不导入尚未发布的 SDK 记录模块。既有工具/签名/结果和指标保持兼容。技能发现、资源/脚本执行、主动问题汇报和业务成功不属于本组件。 + +## 契约 + +- CON-1：`skill_download` 是 OTEL counter，单位 `count`；维度严格为 `skill_source`、`skill_id`、`skill_version`、`skill_space_id`、`outcome`。`outcome` 为 `success` 或 `error`（含取消）。每次获取结束加一，包含缓存恢复。单纯发现或本地软链接创建不产生事件。 +- CON-2：`skill_load` 是 OTEL counter，单位 `count`；维度严格为上述四个身份字段。每次成功返回指令加一，ADK invocation 缓存命中仍计数。失败、取消、转发、资源/脚本访问不计数。 +- CON-3：下载 Span `skill_download` 包含 `skill.source`、`skill.id`、`skill.version`、`skill.space_id`、`outcome`、`cache_hit`、`consumer`。失败增加 `error.type` 和安全的静态 `error.description`，状态为 ERROR。成功加载在既有工具 Span 上添加四个点分身份字段和 `consumer`。consumer 为 `runtime` 或 `skill_sandbox`。Span 耗时为总获取时间。consumer/cache/错误文本不放入 metric 标签。 +- CON-4：身份遵循下表。缺失版本/空间用空字符串，不使用生成的缓存 hash。缺少可信 ID 的远端技能和无法分类的本地技能不采集。遥测身份以 Registry 元数据为准，覆盖技能包同名 metadata。SkillHub 安装时随实际安装包记录 slug，不能仅依赖当前期望配置。 + +| skill_source | skill_id | skill_version | skill_space_id | +| --- | --- | --- | --- | +| builtin_skill | name | 空 | 空 | +| skillhub_skill | slug | 空 | 空 | +| skillhub_skill_space | s-* | 接口提供时为 sv-* | sp-* | +| agentkit_skill | s-* | vN | 空 | +| agentkit_skill_space | s-* | 接口提供或旧包路径中存在的 vN | ss-* | + +Python 支持以上五类来源。Sandbox 固定 Go SDK 仍拒绝 `sp-*`，用户已批准保留这一限制。本地 SkillHub 来源记录供 Python/Go 共同读取。 + +- CON-5：通过全局 OTEL provider 记录，不依赖 Span 采样。Runtime 加载采集随既有 VeADK tracing hook 启用。SDK 记录模块不安装 exporter。Sandbox 周期 OTLP metric 遵循 `OTEL_SDK_DISABLED`、`OTEL_METRICS_EXPORTER=none`、标准 metric/base endpoint 配置，并复用已有 provider。关闭 trace 不关闭 metric。默认本地 collector 使用 4318 端口的 OTLP/HTTP；退出有时间上限，导出失败不阻断业务。Metrics 使用 `OTEL_EXPORTER_OTLP_METRICS_PROTOCOL`（或基础 protocol），支持 HTTP/protobuf 和 grpc；默认间隔为 60000 ms。短生命周期 SkillHub 安装器使用 200 ms exporter 超时，退出前尽力 flush；空配置和仅禁用内置技能跳过初始化。启动开销记录在 PRD。 +- CON-6：每次获取独立持有观测状态直到结束，并发 invocation 不共享可变观测状态。Python `to_thread` 等待取消不会停止已经运行的获取：实际获取结果可能稍后产生，但取消的等待者不计指令加载成功。除已安装 SkillHub 的来源记录外，不新增持久化状态。既有锁、缓存验证、保留和清理规则仍然有效。 +- CON-7：新增遥测不含原始异常、URL、token、文件路径、指令和用户内容。记录/导出失败不得改变工具结果或吞掉业务异常。不要求新凭据或配置。没有 provider 时采用 OTEL 正常空操作行为。 + +## 验证与演进 + +CON-1/4/6 对应 materializer/cache 和 Sandbox package 测试。CON-2/3 对应真实 ADK 工具与 Sandbox wrapper 测试，包含重复加载、参数错误、异常和取消。CON-5/7 对应内存 OTEL 和本地 collector 测试，覆盖 metric、Span、脱敏、开关、provider 复用及故障。具体命令和实际证据在关联 PRD 中维护。增加维度或改变计数语义须重新评审设计并同步双语。implemented 状态不表示已部署或全仓验收通过。 diff --git a/tests/skills/test_skill_telemetry.py b/tests/skills/test_skill_telemetry.py new file mode 100644 index 000000000..93080721e --- /dev/null +++ b/tests/skills/test_skill_telemetry.py @@ -0,0 +1,354 @@ +import asyncio +import zipfile +from types import SimpleNamespace + +import pytest +from google.adk.events import Event +from google.adk.tools.skill_toolset import LoadSkillTool, SkillToolset +from google.genai import types +from opentelemetry import metrics, trace +from opentelemetry.sdk.metrics import MeterProvider +from opentelemetry.sdk.metrics.export import InMemoryMetricReader +from opentelemetry.sdk.trace import TracerProvider +from opentelemetry.sdk.trace.export import SimpleSpanProcessor +from opentelemetry.sdk.trace.export.in_memory_span_exporter import InMemorySpanExporter + +from veadk.skills import materializer, registry +from veadk.skills import telemetry as skill_telemetry +from veadk.skills.skill import Skill +from veadk.tracing.telemetry import portal_metrics, telemetry + + +@pytest.fixture +def observation(monkeypatch): + reader = InMemoryMetricReader() + provider = MeterProvider(metric_readers=[reader]) + exporter = InMemorySpanExporter() + spans = TracerProvider() + spans.add_span_processor(SimpleSpanProcessor(exporter)) + monkeypatch.setattr(metrics, "get_meter", provider.get_meter) + monkeypatch.setattr( + portal_metrics, "portal_metric_recorder", portal_metrics.PortalMetricRecorder() + ) + monkeypatch.setattr(trace, "get_tracer", spans.get_tracer) + yield reader, exporter + provider.shutdown() + spans.shutdown() + + +def points(reader, name): + data = reader.get_metrics_data() + return [ + (dict(point.attributes), point.value) + for resource in (data.resource_metrics if data else []) + for scope in resource.scope_metrics + for metric in scope.metrics + if metric.name == name + for point in metric.data.data_points + ] + + +@pytest.fixture +def remote(monkeypatch): + skill = Skill( + name="demo", + description="Demo", + path="demo", + id="s-demo", + skill_space_id="sp-one", + source_type="skillhub", + version_id="sv-one", + ) + calls = [] + + def download(skill, path): + calls.append(skill.id) + with zipfile.ZipFile(path, "w") as archive: + archive.writestr( + "SKILL.md", "---\nname: demo\ndescription: Demo\n---\nInstructions" + ) + return True + + monkeypatch.setattr(materializer, "download_skillhub_skill", download) + monkeypatch.setattr(registry, "load_skills_from_cloud", lambda _: [skill]) + return skill, calls + + +def test_download_cache_and_corruption(observation, remote, tmp_path): + skill, calls = remote + path = materializer.materialize_remote_skill(skill, cache_dir=tmp_path) + materializer.materialize_remote_skill(skill, cache_dir=tmp_path) + (path / "SKILL.md").write_text("invalid") + materializer.materialize_remote_skill(skill, cache_dir=tmp_path) + assert calls == ["s-demo", "s-demo"] + labels = { + "skill_source": "skillhub_skill_space", + "skill_id": "s-demo", + "skill_version": "sv-one", + "skill_space_id": "sp-one", + "outcome": "success", + } + assert points(observation[0], "skill_download") == [(labels, 3)] + spans = observation[1].get_finished_spans() + assert [s.attributes["cache_hit"] for s in spans] == [False, True, False] + assert all( + s.name == "skill_download" and s.attributes["consumer"] == "runtime" + for s in spans + ) + + +def test_download_error_does_not_export_exception_text( + observation, remote, tmp_path, monkeypatch +): + skill, _ = remote + secret = "https://example.test/package?signature=DO_NOT_EXPORT" + + def fail(*args, **kwargs): + raise RuntimeError(secret) + + monkeypatch.setattr(materializer, "download_skillhub_skill", fail) + with pytest.raises(RuntimeError, match="DO_NOT_EXPORT"): + materializer.materialize_remote_skill(skill, cache_dir=tmp_path) + assert points(observation[0], "skill_download")[0][0]["outcome"] == "error" + span = observation[1].get_finished_spans()[0] + assert span.status.is_ok is False + assert span.attributes["error.type"] == "RuntimeError" + assert "DO_NOT_EXPORT" not in str( + (span.attributes, span.events, span.status.description) + ) + + +@pytest.mark.asyncio +async def test_real_adk_cached_loads_keep_identity_and_count_each_response( + observation, remote, tmp_path, monkeypatch +): + cloud = registry.VeSkillRegistry(skill_source_id="sp-one", cache_dir=tmp_path) + toolset = SkillToolset(registry=cloud) + tool = LoadSkillTool(toolset) + context = SimpleNamespace(invocation_id="turn", agent_name="agent", state={}) + monkeypatch.setattr(telemetry, "_upload_tool_call_metrics", lambda *a: None) + for _ in range(2): + result = await tool.run_async(args={"skill_name": "demo"}, tool_context=context) + event = Event( + content=types.Content( + parts=[ + types.Part( + function_response=types.FunctionResponse( + name="load_skill", response=result + ) + ) + ] + ) + ) + with trace.get_tracer("test").start_as_current_span("execute_tool load_skill"): + telemetry.trace_tool_call(tool, {"skill_name": "demo"}, event) + assert remote[1] == ["s-demo"] + assert points(observation[0], "skill_load") == [ + ( + { + "skill_source": "skillhub_skill_space", + "skill_id": "s-demo", + "skill_version": "sv-one", + "skill_space_id": "sp-one", + }, + 2, + ) + ] + assert points(observation[0], "skill_download")[0][1] == 1 + loads = [ + s + for s in observation[1].get_finished_spans() + if s.name.startswith("execute_tool") + ] + assert all(s.attributes["skill.id"] == "s-demo" for s in loads) + await toolset.close() + + +@pytest.mark.parametrize( + "response", [{"error": "failed"}, {"skill_name": "demo"}, None] +) +def test_failed_or_incomplete_load_never_counts( + observation, response, remote, tmp_path +): + event = Event( + content=types.Content( + parts=[ + types.Part( + function_response=types.FunctionResponse( + name="load_skill", response=response or {} + ) + ) + ] + ) + ) + toolset = SkillToolset( + registry=registry.VeSkillRegistry(skill_source_id="sp-one", cache_dir=tmp_path) + ) + skill_telemetry.record_adk_load(LoadSkillTool(toolset), event) + assert not points(observation[0], "skill_load") + + +def test_unknown_local_identity_and_legacy_version(): + assert ( + skill_telemetry.skill_identity( + Skill(name="demo", description="Demo", path="/tmp/demo") + ) + is None + ) + identity = skill_telemetry.skill_identity( + Skill( + name="demo", + description="Demo", + path="skills/s-demo/v12/demo.zip", + id="s-demo", + skill_space_id="ss-one", + ) + ) + assert identity == { + "skill_source": "agentkit_skill_space", + "skill_id": "s-demo", + "skill_version": "v12", + "skill_space_id": "ss-one", + } + + +@pytest.mark.asyncio +async def test_cancelled_registry_wait_does_not_load( + observation, remote, tmp_path, monkeypatch +): + async def cancelled(*args, **kwargs): + raise asyncio.CancelledError + + monkeypatch.setattr(registry.asyncio, "to_thread", cancelled) + toolset = SkillToolset( + registry=registry.VeSkillRegistry(skill_source_id="sp-one", cache_dir=tmp_path) + ) + with pytest.raises(asyncio.CancelledError): + await LoadSkillTool(toolset).run_async( + args={"skill_name": "demo"}, + tool_context=SimpleNamespace(invocation_id="turn"), + ) + assert not points(observation[0], "skill_load") + await toolset.close() + + +def test_recording_failure_preserves_download_result(remote, tmp_path, monkeypatch): + def broken(*args, **kwargs): + raise RuntimeError("provider unavailable") + + monkeypatch.setattr( + portal_metrics.portal_metric_recorder.skill_download_counter, "add", broken + ) + monkeypatch.setattr(trace, "get_tracer", broken) + assert ( + materializer.materialize_remote_skill(remote[0], cache_dir=tmp_path) + / "SKILL.md" + ).is_file() + + +def test_non_load_tool_cannot_count_forged_response(observation, remote): + labels = skill_telemetry.skill_identity(remote[0]) + result = { + "instructions": "data", + "frontmatter": {"metadata": {"veadk_skill_identity": labels}}, + } + event = Event( + content=types.Content( + parts=[ + types.Part( + function_response=types.FunctionResponse( + name="load_skill_resource", response=result + ) + ) + ] + ) + ) + skill_telemetry.record_adk_load(SimpleNamespace(name="load_skill_resource"), event) + assert not points(observation[0], "skill_load") + + +def test_default_proxy_instruments_are_bounded_and_support_late_provider(): + import os + import subprocess + import sys + import textwrap + from pathlib import Path + + script = textwrap.dedent(""" + from opentelemetry import metrics + from opentelemetry.sdk.metrics import MeterProvider + from opentelemetry.sdk.metrics.export import InMemoryMetricReader + from veadk.tracing.telemetry.portal_metrics import portal_metric_recorder as recorder + + proxy = metrics.get_meter_provider() + assert type(proxy).__name__ == "_ProxyMeterProvider" + assert recorder.skill_load_counter is not None + assert recorder.skill_download_counter is not None + assert recorder.skill_load_counter in recorder.meter._instruments + assert recorder.skill_download_counter in recorder.meter._instruments + initial_meters = tuple(proxy._meters) + initial_instruments = tuple(tuple(meter._instruments) for meter in initial_meters) + labels = {"skill_source": "agentkit_skill_space", "skill_id": "s-test", "skill_version": "v1", "skill_space_id": "ss-test"} + for _ in range(1000): + recorder.record_skill_load(labels) + recorder.record_skill_download({**labels, "outcome": "success"}) + assert tuple(proxy._meters) == initial_meters + assert tuple(tuple(meter._instruments) for meter in proxy._meters) == initial_instruments + reader = InMemoryMetricReader() + provider = MeterProvider(metric_readers=[reader]) + try: + metrics.set_meter_provider(provider) + for _ in range(3): + recorder.record_skill_load(labels) + recorder.record_skill_download({**labels, "outcome": "success"}) + data = reader.get_metrics_data() + values = {m.name: [(dict(p.attributes), p.value) for p in m.data.data_points] + for r in data.resource_metrics for s in r.scope_metrics for m in s.metrics} + assert {s.scope.name for r in data.resource_metrics for s in r.scope_metrics for m in s.metrics if m.name in {"skill_load", "skill_download"}} == {"veadk_portal"} + assert values["skill_load"] == [(labels, 3)], values + assert values["skill_download"] == [({**labels, "outcome": "success"}, 3)], values + finally: + provider.shutdown() + """) + environment = { + key: value for key, value in os.environ.items() if not key.startswith("OTEL_") + } + environment["PYTHONPATH"] = str(Path(__file__).resolve().parents[2]) + result = subprocess.run( + [sys.executable, "-c", script], + env=environment, + check=False, + capture_output=True, + text=True, + timeout=30, + ) + assert result.returncode == 0, result.stdout + result.stderr + + +@pytest.mark.parametrize("metric_name", ["skill_load", "skill_download"]) +def test_recording_failure_does_not_escape(monkeypatch, metric_name): + from unittest.mock import Mock + + broken_add = Mock(side_effect=RuntimeError("recording unavailable")) + recorder = portal_metrics.portal_metric_recorder + counter = ( + recorder.skill_download_counter + if metric_name == "skill_download" + else recorder.skill_load_counter + ) + record = ( + recorder.record_skill_download + if metric_name == "skill_download" + else recorder.record_skill_load + ) + monkeypatch.setattr(counter, "add", broken_add) + labels = { + "skill_source": "builtin_skill", + "skill_id": "demo", + "skill_version": "", + "skill_space_id": "", + } + if metric_name == "skill_download": + labels["outcome"] = "success" + record(labels) + broken_add.assert_called_once_with(1, labels) diff --git a/veadk/skills/materializer.py b/veadk/skills/materializer.py index 5c9e0c61f..9e296cad5 100644 --- a/veadk/skills/materializer.py +++ b/veadk/skills/materializer.py @@ -32,6 +32,7 @@ from veadk.skills.exceptions import SkillMaterializeError from veadk.skills.skill import Skill +from veadk.skills.telemetry import download, skill_identity from veadk.skills.utils import ( _get_agentkit_endpoint, _get_cloud_credentials, @@ -73,6 +74,19 @@ def materialize_remote_skill( region: str | None = None, ) -> Path: """Download one remote VeADK skill and return an ADK-loadable skill dir.""" + with download(skill_identity(skill)) as observation: + return _materialize_remote_skill( + skill, + cache_dir=cache_dir, + credentials=credentials, + region=region, + observation=observation, + ) + + +def _materialize_remote_skill( + skill, *, cache_dir, credentials, region, observation +) -> Path: base_cache_dir = _default_cache_dir() if cache_dir is None else Path(cache_dir) _ensure_cache_dir(base_cache_dir) @@ -98,6 +112,7 @@ def materialize_remote_skill( try: load_skill_from_dir(cached) logger.info(f"Using cached ADK skill '{skill.name}' from {cached}") + observation["cache_hit"] = True return cached except Exception as e: logger.warning( diff --git a/veadk/skills/registry.py b/veadk/skills/registry.py index 133aa23fe..ba60be78f 100644 --- a/veadk/skills/registry.py +++ b/veadk/skills/registry.py @@ -35,6 +35,7 @@ class SkillRegistry: # type: ignore[no-redef] from veadk.skills.materializer import materialize_remote_skill from veadk.skills.skill import Skill as VeADKSkill +from veadk.skills.telemetry import skill_identity from veadk.skills.utils import load_skills_from_cloud @@ -78,7 +79,12 @@ async def get_skill(self, *, name: str) -> ADKSkill: skill, cache_dir=self.cache_dir, ) - return await asyncio.to_thread(load_compatible_adk_skill_from_dir, skill_dir) + loaded = await asyncio.to_thread(load_compatible_adk_skill_from_dir, skill_dir) + # Cloud identity is authoritative, including over archive-supplied metadata. + loaded.frontmatter.metadata.pop("veadk_skill_identity", None) + if identity := skill_identity(skill): + loaded.frontmatter.metadata["veadk_skill_identity"] = identity + return loaded def search_tool_description(self) -> str | None: return ( diff --git a/veadk/skills/telemetry.py b/veadk/skills/telemetry.py new file mode 100644 index 000000000..5ecfcd582 --- /dev/null +++ b/veadk/skills/telemetry.py @@ -0,0 +1,114 @@ +"""Package acquisition and instruction delivery, never business execution.""" + +from contextlib import contextmanager, suppress + +from opentelemetry import trace +from opentelemetry.trace import Status, StatusCode + +from veadk.tracing.telemetry import portal_metrics +from veadk.utils.adk_compat import get_event_function_responses + + +def skill_identity(skill) -> dict[str, str] | None: + space = skill.skill_space_id or "" + if not skill.id or not space.startswith(("sp-", "ss-")): + return None + version = skill.version_id or "" + if not version and space.startswith("ss-"): + parts = skill.path.split("/") + if len(parts) >= 3 and parts[0] == "skills": + version = parts[2] + return { + "skill_source": "skillhub_skill_space" + if space.startswith("sp-") + else "agentkit_skill_space", + "skill_id": skill.id, + "skill_version": version, + "skill_space_id": space, + } + + +def _span_attributes(identity, consumer): + return { + **{ + key.replace("skill_", "skill.", 1): value for key, value in identity.items() + }, + "consumer": consumer, + } + + +@contextmanager +def download(identity, *, consumer="runtime"): + observation = {"cache_hit": False} + if identity is None: + yield observation + return + span = trace.INVALID_SPAN + with suppress(Exception): + span = trace.get_tracer("veadk.skills").start_span( + "skill_download", attributes=_span_attributes(identity, consumer) + ) + outcome = "error" + try: + yield observation + outcome = "success" + except BaseException as error: + with suppress(Exception): + span.set_attributes( + { + "error.type": type(error).__name__, + "error.description": "Skill package acquisition failed", + } + ) + span.set_status( + Status(StatusCode.ERROR, "Skill package acquisition failed") + ) + raise + finally: + portal_metrics.portal_metric_recorder.record_skill_download( + {**identity, "outcome": outcome} + ) + with suppress(Exception): + span.set_attributes( + {"outcome": outcome, "cache_hit": observation["cache_hit"]} + ) + span.end() + + +def record_adk_load(tool, event): + """Observe the final ADK response, including invocation-cache hits.""" + from google.adk.tools.skill_toolset import LoadSkillTool + + from veadk.skills.registry import VeSkillRegistry + + with suppress(Exception): + if not isinstance(tool, LoadSkillTool) or not isinstance( + tool._toolset._registry, VeSkillRegistry + ): + return + for response in get_event_function_responses(event): + if response.name != tool.name: + continue + result = response.response + if result.get("error") or not isinstance(result.get("instructions"), str): + continue + identity = ( + result.get("frontmatter", {}) + .get("metadata", {}) + .get("veadk_skill_identity") + ) + if not isinstance(identity, dict) or set(identity) != { + "skill_source", + "skill_id", + "skill_version", + "skill_space_id", + }: + continue + if not identity["skill_id"] or not all( + isinstance(value, str) for value in identity.values() + ): + continue + portal_metrics.portal_metric_recorder.record_skill_load(identity) + trace.get_current_span().set_attributes( + _span_attributes(identity, "runtime") + ) diff --git a/veadk/tracing/telemetry/portal_metrics.py b/veadk/tracing/telemetry/portal_metrics.py index a7a2fa5ac..73b735740 100644 --- a/veadk/tracing/telemetry/portal_metrics.py +++ b/veadk/tracing/telemetry/portal_metrics.py @@ -15,6 +15,7 @@ """Always-on VeADK Portal metric instrumentation.""" import time +from contextlib import suppress from dataclasses import dataclass from typing import Any @@ -144,6 +145,8 @@ class Meters: APMPLUS_TOOL_TOKEN_USAGE = "apmplus_tool_token_usage" # skill invoke latency GEN_AI_SKILL_INVOKE_LATENCY = "gen_ai_skill_invoke_latency" + SKILL_DOWNLOAD = "skill_download" + SKILL_LOAD = "skill_load" class PortalMetricRecorder: @@ -234,6 +237,23 @@ def __init__(self, name: str = "veadk_portal") -> None: explicit_bucket_boundaries_advisory=_GEN_AI_CLIENT_OPERATION_DURATION_BUCKETS, ) + self.skill_download_counter = self.meter.create_counter( + Meters.SKILL_DOWNLOAD, unit="count" + ) + self.skill_load_counter = self.meter.create_counter( + Meters.SKILL_LOAD, unit="count" + ) + + def record_skill_download(self, attributes: dict[str, str]) -> None: + """Count skill download observations without affecting tool results.""" + with suppress(Exception): + self.skill_download_counter.add(1, attributes) + + def record_skill_load(self, attributes: dict[str, str]) -> None: + """Count skill load observations without affecting tool results.""" + with suppress(Exception): + self.skill_load_counter.add(1, attributes) + @property def provider(self) -> metrics_api.MeterProvider: return metrics_api.get_meter_provider() diff --git a/veadk/tracing/telemetry/telemetry.py b/veadk/tracing/telemetry/telemetry.py index 418fc469a..9f2992f2b 100644 --- a/veadk/tracing/telemetry/telemetry.py +++ b/veadk/tracing/telemetry/telemetry.py @@ -338,6 +338,9 @@ def trace_tool_call( args: Dictionary of arguments passed to the tool function function_response_event: Event containing the tool's execution response """ + from veadk.skills.telemetry import record_adk_load + + record_adk_load(tool, function_response_event) span = trace.get_current_span() set_common_attributes_on_tool_span(current_span=span) # type: ignore From 8b1bf400078677fb65ae29b3c5236e5c1f628cb1 Mon Sep 17 00:00:00 2001 From: caiyuwang Date: Tue, 20 Oct 2026 19:24:29 +0800 Subject: [PATCH 2/3] chore: prefix metric with `gen_ai_` --- .../2026-10-16-skill-observability.md | 4 +-- .../2026-10-16-skill-observability.zh.md | 4 +-- specs/skill-observability/README.md | 4 +-- specs/skill-observability/README.zh.md | 4 +-- tests/skills/test_skill_telemetry.py | 32 ++++++++++--------- veadk/tracing/telemetry/portal_metrics.py | 8 ++--- 6 files changed, 29 insertions(+), 27 deletions(-) diff --git a/prd-spec/features/skill-observability/2026-10-16-skill-observability.md b/prd-spec/features/skill-observability/2026-10-16-skill-observability.md index 180e326cb..fb3869940 100644 --- a/prd-spec/features/skill-observability/2026-10-16-skill-observability.md +++ b/prd-spec/features/skill-observability/2026-10-16-skill-observability.md @@ -15,8 +15,8 @@ Count package acquisition and successful instruction delivery with the exact sou ## Requirements and scenarios -- FR-1: A recognized remote package acquisition emits one `skill_download` observation on completion, including validated disk-cache restoration and failure. Invalid cache followed by download is one attempt, with `cache_hit=false`. -- FR-2: A successful instruction response emits one `skill_load`. Two successful `load_skill` calls in one invocation count twice even when ADK fetches once. Unknown skills, read failures and cancellation never count as successful loads. +- FR-1: A recognized remote package acquisition emits one `gen_ai_skill_download` observation on completion, including validated disk-cache restoration and failure. Invalid cache followed by download is one attempt, with `cache_hit=false`. +- FR-2: A successful instruction response emits one `gen_ai_skill_load`. Two successful `load_skill` calls in one invocation count twice even when ADK fetches once. Unknown skills, read failures and cancellation never count as successful loads. - FR-3: Cloud identity survives conversion/cache. Sandbox Python distinguishes built-ins, SkillHub slugs, explicit AgentKit IDs and both space kinds. Go retains its existing four-source support. Unattributable local skills are not guessed to be built-ins. - FR-4: Use existing process-global OTEL providers; sandbox installs a periodic OTLP metric exporter only when no provider exists. Trace and metric disable switches are independent. Export failures do not affect conversations; shutdown flush is bounded. - FR-5: New telemetry never records archive URLs, credentials, instruction bodies, paths or raw exception messages. Existing content telemetry policy remains unchanged. diff --git a/prd-spec/features/skill-observability/2026-10-16-skill-observability.zh.md b/prd-spec/features/skill-observability/2026-10-16-skill-observability.zh.md index 3ebce39f0..4af5b08dd 100644 --- a/prd-spec/features/skill-observability/2026-10-16-skill-observability.zh.md +++ b/prd-spec/features/skill-observability/2026-10-16-skill-observability.zh.md @@ -15,8 +15,8 @@ ## 需求与场景 -- FR-1：已识别来源的远端技能包获取结束时记录一次 `skill_download`，覆盖验证成功的磁盘缓存恢复和失败。无效缓存后重新下载属于一次获取，`cache_hit=false`。 -- FR-2：成功返回指令时记录一次 `skill_load`。同一 invocation 两次成功 `load_skill` 计两次，即使 ADK 只获取一次。未知技能、读取失败和取消不计成功加载。 +- FR-1：已识别来源的远端技能包获取结束时记录一次 `gen_ai_skill_download`，覆盖验证成功的磁盘缓存恢复和失败。无效缓存后重新下载属于一次获取，`cache_hit=false`。 +- FR-2：成功返回指令时记录一次 `gen_ai_skill_load`。同一 invocation 两次成功 `load_skill` 计两次，即使 ADK 只获取一次。未知技能、读取失败和取消不计成功加载。 - FR-3：云端身份在转换/缓存后保留。Sandbox Python 区分内置技能、SkillHub slug、显式 AgentKit ID 和两类空间；Go 保留既有四类来源支持。来源不明的本地技能不猜测成内置技能。 - FR-4：使用既有进程级 OTEL provider；Sandbox 仅在未配置 provider 时安装周期 OTLP metric exporter。trace 与 metric 禁用开关独立。导出失败不影响对话；退出 flush 有时间上限。 - FR-5：新增遥测不记录包 URL、凭据、指令正文、路径或原始异常消息。既有内容遥测策略保持不变。 diff --git a/specs/skill-observability/README.md b/specs/skill-observability/README.md index 23de7890f..d59151ec9 100644 --- a/specs/skill-observability/README.md +++ b/specs/skill-observability/README.md @@ -11,8 +11,8 @@ VeADK owns Runtime package acquisition and ADK instruction-load instrumentation ## Contracts -- CON-1: `skill_download` is an OTEL counter, unit `count`; dimensions are exactly `skill_source`, `skill_id`, `skill_version`, `skill_space_id`, `outcome`. `outcome` is `success` or `error` (including cancellation). One acquisition completion, including cache restore, adds one. No event for mere discovery or local symlink creation. -- CON-2: `skill_load` is an OTEL counter, unit `count`; dimensions are exactly the four identity fields above. One successful instruction response adds one. ADK invocation-cache hits still count. Errors, cancellation, forwarding and resource/script access do not. +- CON-1: `gen_ai_skill_download` is an OTEL counter, unit `count`; dimensions are exactly `skill_source`, `skill_id`, `skill_version`, `skill_space_id`, `outcome`. `outcome` is `success` or `error` (including cancellation). One acquisition completion, including cache restore, adds one. No event for mere discovery or local symlink creation. +- CON-2: `gen_ai_skill_load` is an OTEL counter, unit `count`; dimensions are exactly the four identity fields above. One successful instruction response adds one. ADK invocation-cache hits still count. Errors, cancellation, forwarding and resource/script access do not. - CON-3: Download Span `skill_download` has `skill.source`, `skill.id`, `skill.version`, `skill.space_id`, `outcome`, `cache_hit`, `consumer`. Failures add `error.type` and a safe static `error.description`, with ERROR status. Successful load annotates the existing tool Span with the four dotted identity fields and `consumer`. Consumer is `runtime` or `skill_sandbox`. Span duration is total acquisition time. Never add consumer/cache/error text to metric labels. - CON-4: Identity follows the table. Missing version/space is the empty string, never a synthesized cache hash. Remote skills without a trustworthy ID and unclassified local skills are excluded. Registry-owned metadata overrides archive metadata for telemetry identity. SkillHub installation records the slug with the installed package, not just the current desired configuration. diff --git a/specs/skill-observability/README.zh.md b/specs/skill-observability/README.zh.md index fa3800f30..988df3269 100644 --- a/specs/skill-observability/README.zh.md +++ b/specs/skill-observability/README.zh.md @@ -11,8 +11,8 @@ VeADK 在 `veadk/skills/` 与 `veadk/tracing/telemetry/telemetry.py` 负责 Runt ## 契约 -- CON-1：`skill_download` 是 OTEL counter，单位 `count`；维度严格为 `skill_source`、`skill_id`、`skill_version`、`skill_space_id`、`outcome`。`outcome` 为 `success` 或 `error`（含取消）。每次获取结束加一，包含缓存恢复。单纯发现或本地软链接创建不产生事件。 -- CON-2：`skill_load` 是 OTEL counter，单位 `count`；维度严格为上述四个身份字段。每次成功返回指令加一，ADK invocation 缓存命中仍计数。失败、取消、转发、资源/脚本访问不计数。 +- CON-1：`gen_ai_skill_download` 是 OTEL counter，单位 `count`；维度严格为 `skill_source`、`skill_id`、`skill_version`、`skill_space_id`、`outcome`。`outcome` 为 `success` 或 `error`（含取消）。每次获取结束加一，包含缓存恢复。单纯发现或本地软链接创建不产生事件。 +- CON-2：`gen_ai_skill_load` 是 OTEL counter，单位 `count`；维度严格为上述四个身份字段。每次成功返回指令加一，ADK invocation 缓存命中仍计数。失败、取消、转发、资源/脚本访问不计数。 - CON-3：下载 Span `skill_download` 包含 `skill.source`、`skill.id`、`skill.version`、`skill.space_id`、`outcome`、`cache_hit`、`consumer`。失败增加 `error.type` 和安全的静态 `error.description`，状态为 ERROR。成功加载在既有工具 Span 上添加四个点分身份字段和 `consumer`。consumer 为 `runtime` 或 `skill_sandbox`。Span 耗时为总获取时间。consumer/cache/错误文本不放入 metric 标签。 - CON-4：身份遵循下表。缺失版本/空间用空字符串，不使用生成的缓存 hash。缺少可信 ID 的远端技能和无法分类的本地技能不采集。遥测身份以 Registry 元数据为准，覆盖技能包同名 metadata。SkillHub 安装时随实际安装包记录 slug，不能仅依赖当前期望配置。 diff --git a/tests/skills/test_skill_telemetry.py b/tests/skills/test_skill_telemetry.py index 93080721e..3f881d3eb 100644 --- a/tests/skills/test_skill_telemetry.py +++ b/tests/skills/test_skill_telemetry.py @@ -88,7 +88,7 @@ def test_download_cache_and_corruption(observation, remote, tmp_path): "skill_space_id": "sp-one", "outcome": "success", } - assert points(observation[0], "skill_download") == [(labels, 3)] + assert points(observation[0], "gen_ai_skill_download") == [(labels, 3)] spans = observation[1].get_finished_spans() assert [s.attributes["cache_hit"] for s in spans] == [False, True, False] assert all( @@ -109,7 +109,7 @@ def fail(*args, **kwargs): monkeypatch.setattr(materializer, "download_skillhub_skill", fail) with pytest.raises(RuntimeError, match="DO_NOT_EXPORT"): materializer.materialize_remote_skill(skill, cache_dir=tmp_path) - assert points(observation[0], "skill_download")[0][0]["outcome"] == "error" + assert points(observation[0], "gen_ai_skill_download")[0][0]["outcome"] == "error" span = observation[1].get_finished_spans()[0] assert span.status.is_ok is False assert span.attributes["error.type"] == "RuntimeError" @@ -143,7 +143,7 @@ async def test_real_adk_cached_loads_keep_identity_and_count_each_response( with trace.get_tracer("test").start_as_current_span("execute_tool load_skill"): telemetry.trace_tool_call(tool, {"skill_name": "demo"}, event) assert remote[1] == ["s-demo"] - assert points(observation[0], "skill_load") == [ + assert points(observation[0], "gen_ai_skill_load") == [ ( { "skill_source": "skillhub_skill_space", @@ -154,7 +154,7 @@ async def test_real_adk_cached_loads_keep_identity_and_count_each_response( 2, ) ] - assert points(observation[0], "skill_download")[0][1] == 1 + assert points(observation[0], "gen_ai_skill_download")[0][1] == 1 loads = [ s for s in observation[1].get_finished_spans() @@ -185,7 +185,7 @@ def test_failed_or_incomplete_load_never_counts( registry=registry.VeSkillRegistry(skill_source_id="sp-one", cache_dir=tmp_path) ) skill_telemetry.record_adk_load(LoadSkillTool(toolset), event) - assert not points(observation[0], "skill_load") + assert not points(observation[0], "gen_ai_skill_load") def test_unknown_local_identity_and_legacy_version(): @@ -228,7 +228,7 @@ async def cancelled(*args, **kwargs): args={"skill_name": "demo"}, tool_context=SimpleNamespace(invocation_id="turn"), ) - assert not points(observation[0], "skill_load") + assert not points(observation[0], "gen_ai_skill_load") await toolset.close() @@ -237,7 +237,9 @@ def broken(*args, **kwargs): raise RuntimeError("provider unavailable") monkeypatch.setattr( - portal_metrics.portal_metric_recorder.skill_download_counter, "add", broken + portal_metrics.portal_metric_recorder.skill_download_counter, + "add", + broken, ) monkeypatch.setattr(trace, "get_tracer", broken) assert ( @@ -264,7 +266,7 @@ def test_non_load_tool_cannot_count_forged_response(observation, remote): ) ) skill_telemetry.record_adk_load(SimpleNamespace(name="load_skill_resource"), event) - assert not points(observation[0], "skill_load") + assert not points(observation[0], "gen_ai_skill_load") def test_default_proxy_instruments_are_bounded_and_support_late_provider(): @@ -304,9 +306,9 @@ def test_default_proxy_instruments_are_bounded_and_support_late_provider(): data = reader.get_metrics_data() values = {m.name: [(dict(p.attributes), p.value) for p in m.data.data_points] for r in data.resource_metrics for s in r.scope_metrics for m in s.metrics} - assert {s.scope.name for r in data.resource_metrics for s in r.scope_metrics for m in s.metrics if m.name in {"skill_load", "skill_download"}} == {"veadk_portal"} - assert values["skill_load"] == [(labels, 3)], values - assert values["skill_download"] == [({**labels, "outcome": "success"}, 3)], values + assert {s.scope.name for r in data.resource_metrics for s in r.scope_metrics for m in s.metrics if m.name in {"gen_ai_skill_load", "gen_ai_skill_download"}} == {"veadk_portal"} + assert values["gen_ai_skill_load"] == [(labels, 3)], values + assert values["gen_ai_skill_download"] == [({**labels, "outcome": "success"}, 3)], values finally: provider.shutdown() """) @@ -325,7 +327,7 @@ def test_default_proxy_instruments_are_bounded_and_support_late_provider(): assert result.returncode == 0, result.stdout + result.stderr -@pytest.mark.parametrize("metric_name", ["skill_load", "skill_download"]) +@pytest.mark.parametrize("metric_name", ["gen_ai_skill_load", "gen_ai_skill_download"]) def test_recording_failure_does_not_escape(monkeypatch, metric_name): from unittest.mock import Mock @@ -333,12 +335,12 @@ def test_recording_failure_does_not_escape(monkeypatch, metric_name): recorder = portal_metrics.portal_metric_recorder counter = ( recorder.skill_download_counter - if metric_name == "skill_download" + if metric_name == "gen_ai_skill_download" else recorder.skill_load_counter ) record = ( recorder.record_skill_download - if metric_name == "skill_download" + if metric_name == "gen_ai_skill_download" else recorder.record_skill_load ) monkeypatch.setattr(counter, "add", broken_add) @@ -348,7 +350,7 @@ def test_recording_failure_does_not_escape(monkeypatch, metric_name): "skill_version": "", "skill_space_id": "", } - if metric_name == "skill_download": + if metric_name == "gen_ai_skill_download": labels["outcome"] = "success" record(labels) broken_add.assert_called_once_with(1, labels) diff --git a/veadk/tracing/telemetry/portal_metrics.py b/veadk/tracing/telemetry/portal_metrics.py index 73b735740..fa1200a87 100644 --- a/veadk/tracing/telemetry/portal_metrics.py +++ b/veadk/tracing/telemetry/portal_metrics.py @@ -145,8 +145,8 @@ class Meters: APMPLUS_TOOL_TOKEN_USAGE = "apmplus_tool_token_usage" # skill invoke latency GEN_AI_SKILL_INVOKE_LATENCY = "gen_ai_skill_invoke_latency" - SKILL_DOWNLOAD = "skill_download" - SKILL_LOAD = "skill_load" + GEN_AI_SKILL_DOWNLOAD = "gen_ai_skill_download" + GEN_AI_SKILL_LOAD = "gen_ai_skill_load" class PortalMetricRecorder: @@ -238,10 +238,10 @@ def __init__(self, name: str = "veadk_portal") -> None: ) self.skill_download_counter = self.meter.create_counter( - Meters.SKILL_DOWNLOAD, unit="count" + Meters.GEN_AI_SKILL_DOWNLOAD, unit="count" ) self.skill_load_counter = self.meter.create_counter( - Meters.SKILL_LOAD, unit="count" + Meters.GEN_AI_SKILL_LOAD, unit="count" ) def record_skill_download(self, attributes: dict[str, str]) -> None: From 4558b12dd8bf4ecdb4c5d169431a96f950e6d1db Mon Sep 17 00:00:00 2001 From: caiyuwang Date: Wed, 21 Oct 2026 10:28:46 +0800 Subject: [PATCH 3/3] chore: add license headers --- tests/skills/test_skill_telemetry.py | 14 ++++++++++++++ veadk/skills/telemetry.py | 14 ++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/tests/skills/test_skill_telemetry.py b/tests/skills/test_skill_telemetry.py index 3f881d3eb..37d490f85 100644 --- a/tests/skills/test_skill_telemetry.py +++ b/tests/skills/test_skill_telemetry.py @@ -1,3 +1,17 @@ +# Copyright (c) 2025 Beijing Volcano Engine Technology Co., Ltd. and/or its affiliates. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + import asyncio import zipfile from types import SimpleNamespace diff --git a/veadk/skills/telemetry.py b/veadk/skills/telemetry.py index 5ecfcd582..81970f714 100644 --- a/veadk/skills/telemetry.py +++ b/veadk/skills/telemetry.py @@ -1,3 +1,17 @@ +# Copyright (c) 2025 Beijing Volcano Engine Technology Co., Ltd. and/or its affiliates. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + """Package acquisition and instruction delivery, never business execution.""" from contextlib import contextmanager, suppress