From 5a2aea2148a95b9be3747676e2ed3eab318da144 Mon Sep 17 00:00:00 2001 From: mrcaidev Date: Wed, 21 Oct 2026 17:15:06 +0800 Subject: [PATCH] chore: adjust skill telemetry namings --- .../2026-10-16-skill-observability.md | 8 ++- .../2026-10-16-skill-observability.zh.md | 8 ++- specs/skill-observability/README.md | 26 +++---- specs/skill-observability/README.zh.md | 26 +++---- tests/skills/test_skill_telemetry.py | 68 ++++++++++++------- veadk/skills/telemetry.py | 22 +++--- veadk/tracing/telemetry/portal_metrics.py | 8 +-- 7 files changed, 98 insertions(+), 68 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 fb3869940..832f0e158 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,20 +15,22 @@ Count package acquisition and successful instruction delivery with the exact sou ## Requirements and scenarios -- 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-1: A recognized remote package acquisition emits one `gen_ai_skill_download_count` observation on completion, including validated disk-cache restoration and failure. Invalid cache followed by download is one attempt, with `skill.download.cache_hit=false`. +- FR-2: A successful instruction response emits one `gen_ai_skill_load_count`. 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. +The current schema follows technical document revision 2419, read on 2026-10-21, plus the subsequent user-requested unit change to `time`: both counters use the `_count` suffix and unit `time`, with `skill_name` added to identity. Outcome is only `skill.download.outcome`, no longer a metric label. Spans are named `download_skill {skill_name}` / `load_skill {skill_name}` and use `skill.consumer` and `skill.download.cache_hit`. Name provenance and empty-value rules are in the component contract. Preserve fixed Counter instruments; fields and record methods do not need a `gen_ai` prefix. + ## 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. +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; `skill.consumer`, `skill.download.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 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 4af5b08dd..e98ba34ea 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,20 +15,22 @@ ## 需求与场景 -- FR-1：已识别来源的远端技能包获取结束时记录一次 `gen_ai_skill_download`，覆盖验证成功的磁盘缓存恢复和失败。无效缓存后重新下载属于一次获取，`cache_hit=false`。 -- FR-2：成功返回指令时记录一次 `gen_ai_skill_load`。同一 invocation 两次成功 `load_skill` 计两次，即使 ADK 只获取一次。未知技能、读取失败和取消不计成功加载。 +- FR-1：已识别来源的远端技能包获取结束时记录一次 `gen_ai_skill_download_count`，覆盖验证成功的磁盘缓存恢复和失败。无效缓存后重新下载属于一次获取，`skill.download.cache_hit=false`。 +- FR-2：成功返回指令时记录一次 `gen_ai_skill_load_count`。同一 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 对已支持的来源使用相同标签和来源语义；预取只产生下载数据。保留既有安装包和会话链接。 +当前数据结构以 2026-10-21 读取的技术文档 revision 2419 及随后用户要求的单位 `time` 调整为准：两个 counter 使用 `_count` 后缀和单位 `time`，身份新增 `skill_name`；结果仅放在 `skill.download.outcome`，不再作为 metric 标签。Span 命名为 `download_skill {skill_name}` / `load_skill {skill_name}`，并使用 `skill.consumer` 和 `skill.download.cache_hit`。具体名称来源与空值规则见组件契约。保留固定 Counter 实现；字段和 record 方法名无需 `gen_ai` 前缀。 + ## 设计与受影响消费者 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 采样影响。 +counter 从身份解析后的技能包获取开始计数。尚无技能 ID 的元数据列表失败无法产生按技能归属的下载。获取成功表示技能包通过验证且可用，不只是 HTTP 成功。Python 工作线程可能在等待任务取消后完成下载；按实际获取结果计数，但不计取消的加载。Go context 取消计入错误结果。metric 仅含契约规定的维度；`skill.consumer`、`skill.download.cache_hit` 和错误放在 Span。Span 耗时覆盖获取、验证和安装。通过 Span 计算的缓存命中率受 trace 采样影响。 ## 任务、测试与验收 diff --git a/specs/skill-observability/README.md b/specs/skill-observability/README.md index d59151ec9..d81516db0 100644 --- a/specs/skill-observability/README.md +++ b/specs/skill-observability/README.md @@ -2,27 +2,29 @@ [中文版](README.zh.md) -Component: `skill-observability`; status: implemented (local verification; full regression limitations in PRD); revised 2026-10-16. +Component: `skill-observability`; status: implemented (local verification; full regression limitations in PRD); revised 2026-10-21. 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. +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 remain compatible. Skill metrics use the new names and unit below; consumers must update queries. Old metric names are not emitted in parallel. Discovery, resource/script execution, issue reporting and business success are not owned here. ## Contracts -- 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-1: `gen_ai_skill_download_count` is an OTEL counter, unit `time`; dimensions are exactly `skill_source`, `skill_id`, `skill_name`, `skill_version`, `skill_space_id`. One acquisition completion, including cache restore and failure, adds one. No event for mere discovery or local symlink creation; outcome is recorded only on the Span. +- CON-2: `gen_ai_skill_load_count` is an OTEL counter, unit `time`; dimensions are exactly the five 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 `download_skill {skill_name}` has `skill.source`, `skill.id`, `skill.name`, `skill.version`, `skill.space_id`, `skill.consumer`, `skill.download.outcome`, `skill.download.cache_hit`. Outcome is `success` or `error` (including cancellation); failures add `error.type` and a safe static `error.description`, with ERROR status. Successful loading renames the existing tool Span to `load_skill {skill_name}` and adds the five dotted identity fields and `skill.consumer`, without creating a duplicate Span. `skill.consumer` is `runtime` or `skill_sandbox`. Span duration covers the actual operation. Consumer/cache/outcome/error text never becomes a metric label. - 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-* | +`skill_name` uses the name resolved by the current source: metadata for Runtime/skill spaces, the loaded name for local skills, and the resolved installation name for SkillHub installation. Explicit TOS acquisition first propagates the version API `Name`; when absent, successful acquisition or cache restoration reads the validated package name. Failure before a name is available uses the empty string, never an ID, slug or path as a substitute. Reading a name for telemetry must not change the acquisition result. + +| skill_source | skill_id | skill_name | skill_version | skill_space_id | +| --- | --- | --- | --- | --- | +| builtin_skill | name | name | empty | empty | +| skillhub_skill | slug | resolved name or empty | empty | empty | +| skillhub_skill_space | s-* | resolved name or empty | sv-* when supplied | sp-* | +| agentkit_skill | s-* | resolved name or empty | vN | empty | +| agentkit_skill_space | s-* | resolved name or empty | 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. diff --git a/specs/skill-observability/README.zh.md b/specs/skill-observability/README.zh.md index 988df3269..7d4005217 100644 --- a/specs/skill-observability/README.zh.md +++ b/specs/skill-observability/README.zh.md @@ -2,27 +2,29 @@ [English](README.md) -组件：`skill-observability`；状态：implemented（本地验证；全量回归限制见 PRD）；修订：2026-10-16。 +组件：`skill-observability`；状态：implemented（本地验证；全量回归限制见 PRD）；修订：2026-10-21。 关联变更：[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 记录模块。既有工具/签名/结果和指标保持兼容。技能发现、资源/脚本执行、主动问题汇报和业务成功不属于本组件。 +VeADK 在 `veadk/skills/` 与 `veadk/tracing/telemetry/telemetry.py` 负责 Runtime 技能包获取及 ADK 指令加载埋点。`actb-mono/sandboxes/agentkit-skill` 负责 Python/Go Sandbox 安装、工具适配及 exporter 生命周期。共享下述上报结构；Sandbox 独立发布，不导入尚未发布的 SDK 记录模块。既有工具/签名/结果保持兼容。技能指标采用下述新名称和单位，查询方须同步切换；不重复发送旧名称。技能发现、资源/脚本执行、主动问题汇报和业务成功不属于本组件。 ## 契约 -- 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-1：`gen_ai_skill_download_count` 是 OTEL counter，单位 `time`；维度严格为 `skill_source`、`skill_id`、`skill_name`、`skill_version`、`skill_space_id`。每次获取结束加一，包含缓存恢复和失败。单纯发现或本地软链接创建不产生事件；结果仅记录在 Span。 +- CON-2：`gen_ai_skill_load_count` 是 OTEL counter，单位 `time`；维度严格为上述五个身份字段。每次成功返回指令加一，ADK invocation 缓存命中仍计数。失败、取消、转发、资源/脚本访问不计数。 +- CON-3：下载 Span `download_skill {skill_name}` 包含 `skill.source`、`skill.id`、`skill.name`、`skill.version`、`skill.space_id`、`skill.consumer`、`skill.download.outcome`、`skill.download.cache_hit`。结果为 `success` 或 `error`（含取消）；失败增加 `error.type` 和安全的静态 `error.description`，状态为 ERROR。成功加载将既有工具 Span 命名为 `load_skill {skill_name}`，并添加五个点分身份字段和 `skill.consumer`，不新增重复 Span。`skill.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-* | +`skill_name` 使用当前来源已解析的技能名称：Runtime/技能空间用元数据，本地技能用已加载名称，SkillHub 安装用已解析的安装名称。显式 TOS 获取优先传递版本接口的 `Name`；缺失时在成功获取或缓存恢复后读取已验证包的名称。获取失败且尚无名称时使用空字符串，不用 ID、slug 或路径冒充名称。名称读取仅用于观测，失败不得影响获取结果。 + +| skill_source | skill_id | skill_name | skill_version | skill_space_id | +| --- | --- | --- | --- | --- | +| builtin_skill | name | 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 共同读取。 diff --git a/tests/skills/test_skill_telemetry.py b/tests/skills/test_skill_telemetry.py index 37d490f85..71868e8f7 100644 --- a/tests/skills/test_skill_telemetry.py +++ b/tests/skills/test_skill_telemetry.py @@ -98,15 +98,21 @@ def test_download_cache_and_corruption(observation, remote, tmp_path): labels = { "skill_source": "skillhub_skill_space", "skill_id": "s-demo", + "skill_name": "demo", "skill_version": "sv-one", "skill_space_id": "sp-one", - "outcome": "success", } - assert points(observation[0], "gen_ai_skill_download") == [(labels, 3)] + assert points(observation[0], "gen_ai_skill_download_count") == [(labels, 3)] spans = observation[1].get_finished_spans() - assert [s.attributes["cache_hit"] for s in spans] == [False, True, False] + assert all(s.attributes["skill.name"] == "demo" for s in spans) + assert all(s.attributes["skill.download.outcome"] == "success" for s in spans) + assert [s.attributes["skill.download.cache_hit"] for s in spans] == [ + False, + True, + False, + ] assert all( - s.name == "skill_download" and s.attributes["consumer"] == "runtime" + s.name == "download_skill demo" and s.attributes["skill.consumer"] == "runtime" for s in spans ) @@ -123,8 +129,12 @@ 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], "gen_ai_skill_download")[0][0]["outcome"] == "error" + assert points(observation[0], "gen_ai_skill_download_count") == [ + (skill_telemetry.skill_identity(skill), 1) + ] span = observation[1].get_finished_spans()[0] + assert span.attributes["skill.download.outcome"] == "error" + assert span.name == "download_skill demo" assert span.status.is_ok is False assert span.attributes["error.type"] == "RuntimeError" assert "DO_NOT_EXPORT" not in str( @@ -157,24 +167,29 @@ 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], "gen_ai_skill_load") == [ + assert points(observation[0], "gen_ai_skill_load_count") == [ ( { "skill_source": "skillhub_skill_space", "skill_id": "s-demo", + "skill_name": "demo", "skill_version": "sv-one", "skill_space_id": "sp-one", }, 2, ) ] - assert points(observation[0], "gen_ai_skill_download")[0][1] == 1 + assert points(observation[0], "gen_ai_skill_download_count")[0][1] == 1 loads = [ - s - for s in observation[1].get_finished_spans() - if s.name.startswith("execute_tool") + s for s in observation[1].get_finished_spans() if s.name == "load_skill demo" ] - assert all(s.attributes["skill.id"] == "s-demo" for s in loads) + assert len(loads) == 2 + assert all( + s.attributes["skill.id"] == "s-demo" + and s.attributes["skill.name"] == "demo" + and s.attributes["skill.consumer"] == "runtime" + for s in loads + ) await toolset.close() @@ -199,7 +214,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], "gen_ai_skill_load") + assert not points(observation[0], "gen_ai_skill_load_count") def test_unknown_local_identity_and_legacy_version(): @@ -221,6 +236,7 @@ def test_unknown_local_identity_and_legacy_version(): assert identity == { "skill_source": "agentkit_skill_space", "skill_id": "s-demo", + "skill_name": "demo", "skill_version": "v12", "skill_space_id": "ss-one", } @@ -242,7 +258,7 @@ async def cancelled(*args, **kwargs): args={"skill_name": "demo"}, tool_context=SimpleNamespace(invocation_id="turn"), ) - assert not points(observation[0], "gen_ai_skill_load") + assert not points(observation[0], "gen_ai_skill_load_count") await toolset.close() @@ -280,7 +296,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], "gen_ai_skill_load") + assert not points(observation[0], "gen_ai_skill_load_count") def test_default_proxy_instruments_are_bounded_and_support_late_provider(): @@ -304,10 +320,10 @@ def test_default_proxy_instruments_are_bounded_and_support_late_provider(): 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"} + labels = {"skill_source": "agentkit_skill_space", "skill_id": "s-test", "skill_name": "demo", "skill_version": "v1", "skill_space_id": "ss-test"} for _ in range(1000): recorder.record_skill_load(labels) - recorder.record_skill_download({**labels, "outcome": "success"}) + recorder.record_skill_download(labels) assert tuple(proxy._meters) == initial_meters assert tuple(tuple(meter._instruments) for meter in proxy._meters) == initial_instruments reader = InMemoryMetricReader() @@ -316,13 +332,14 @@ def test_default_proxy_instruments_are_bounded_and_support_late_provider(): metrics.set_meter_provider(provider) for _ in range(3): recorder.record_skill_load(labels) - recorder.record_skill_download({**labels, "outcome": "success"}) + recorder.record_skill_download(labels) 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 {"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 + 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_count", "gen_ai_skill_download_count"}} == {"veadk_portal"} + assert {m.unit for r in data.resource_metrics for s in r.scope_metrics for m in s.metrics if m.name.startswith("gen_ai_skill_")} == {"time"} + assert values["gen_ai_skill_load_count"] == [(labels, 3)], values + assert values["gen_ai_skill_download_count"] == [(labels, 3)], values finally: provider.shutdown() """) @@ -341,7 +358,9 @@ def test_default_proxy_instruments_are_bounded_and_support_late_provider(): assert result.returncode == 0, result.stdout + result.stderr -@pytest.mark.parametrize("metric_name", ["gen_ai_skill_load", "gen_ai_skill_download"]) +@pytest.mark.parametrize( + "metric_name", ["gen_ai_skill_load_count", "gen_ai_skill_download_count"] +) def test_recording_failure_does_not_escape(monkeypatch, metric_name): from unittest.mock import Mock @@ -349,22 +368,21 @@ def test_recording_failure_does_not_escape(monkeypatch, metric_name): recorder = portal_metrics.portal_metric_recorder counter = ( recorder.skill_download_counter - if metric_name == "gen_ai_skill_download" + if metric_name == "gen_ai_skill_download_count" else recorder.skill_load_counter ) record = ( recorder.record_skill_download - if metric_name == "gen_ai_skill_download" + if metric_name == "gen_ai_skill_download_count" else recorder.record_skill_load ) monkeypatch.setattr(counter, "add", broken_add) labels = { "skill_source": "builtin_skill", "skill_id": "demo", + "skill_name": "demo", "skill_version": "", "skill_space_id": "", } - if metric_name == "gen_ai_skill_download": - labels["outcome"] = "success" record(labels) broken_add.assert_called_once_with(1, labels) diff --git a/veadk/skills/telemetry.py b/veadk/skills/telemetry.py index 81970f714..e79a00f9e 100644 --- a/veadk/skills/telemetry.py +++ b/veadk/skills/telemetry.py @@ -37,6 +37,7 @@ def skill_identity(skill) -> dict[str, str] | None: if space.startswith("sp-") else "agentkit_skill_space", "skill_id": skill.id, + "skill_name": skill.name, "skill_version": version, "skill_space_id": space, } @@ -47,7 +48,7 @@ def _span_attributes(identity, consumer): **{ key.replace("skill_", "skill.", 1): value for key, value in identity.items() }, - "consumer": consumer, + "skill.consumer": consumer, } @@ -60,7 +61,8 @@ def download(identity, *, consumer="runtime"): span = trace.INVALID_SPAN with suppress(Exception): span = trace.get_tracer("veadk.skills").start_span( - "skill_download", attributes=_span_attributes(identity, consumer) + f"download_skill {identity['skill_name']}", + attributes=_span_attributes(identity, consumer), ) outcome = "error" try: @@ -79,12 +81,13 @@ def download(identity, *, consumer="runtime"): ) raise finally: - portal_metrics.portal_metric_recorder.record_skill_download( - {**identity, "outcome": outcome} - ) + portal_metrics.portal_metric_recorder.record_skill_download(identity) with suppress(Exception): span.set_attributes( - {"outcome": outcome, "cache_hit": observation["cache_hit"]} + { + "skill.download.outcome": outcome, + "skill.download.cache_hit": observation["cache_hit"], + } ) span.end() @@ -114,6 +117,7 @@ def record_adk_load(tool, event): if not isinstance(identity, dict) or set(identity) != { "skill_source", "skill_id", + "skill_name", "skill_version", "skill_space_id", }: @@ -123,6 +127,6 @@ def record_adk_load(tool, event): ): continue portal_metrics.portal_metric_recorder.record_skill_load(identity) - trace.get_current_span().set_attributes( - _span_attributes(identity, "runtime") - ) + span = trace.get_current_span() + span.update_name(f"load_skill {identity['skill_name']}") + span.set_attributes(_span_attributes(identity, "runtime")) diff --git a/veadk/tracing/telemetry/portal_metrics.py b/veadk/tracing/telemetry/portal_metrics.py index fa1200a87..32cc1757b 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" - GEN_AI_SKILL_DOWNLOAD = "gen_ai_skill_download" - GEN_AI_SKILL_LOAD = "gen_ai_skill_load" + GEN_AI_SKILL_DOWNLOAD_COUNT = "gen_ai_skill_download_count" + GEN_AI_SKILL_LOAD_COUNT = "gen_ai_skill_load_count" class PortalMetricRecorder: @@ -238,10 +238,10 @@ def __init__(self, name: str = "veadk_portal") -> None: ) self.skill_download_counter = self.meter.create_counter( - Meters.GEN_AI_SKILL_DOWNLOAD, unit="count" + Meters.GEN_AI_SKILL_DOWNLOAD_COUNT, unit="time" ) self.skill_load_counter = self.meter.create_counter( - Meters.GEN_AI_SKILL_LOAD, unit="count" + Meters.GEN_AI_SKILL_LOAD_COUNT, unit="time" ) def record_skill_download(self, attributes: dict[str, str]) -> None: