From 59af8c19735f7346cd31a9ebdcaa1a729c489e26 Mon Sep 17 00:00:00 2001 From: mrcaidev Date: Thu, 22 Oct 2026 17:22:11 +0800 Subject: [PATCH] fix(skill): counter unit changes from "time" to "1" --- .../2026-10-16-skill-observability.md | 15 ++++++++++++++- .../2026-10-16-skill-observability.zh.md | 15 ++++++++++++++- specs/skill-observability/README.md | 6 +++--- specs/skill-observability/README.zh.md | 6 +++--- tests/skills/test_skill_telemetry.py | 2 +- veadk/tracing/telemetry/portal_metrics.py | 4 ++-- 6 files changed, 37 insertions(+), 11 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 832f0e158..5102bd88b 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 @@ -22,7 +22,7 @@ Count package acquisition and successful instruction delivery with the exact sou - 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. +The current schema follows technical document revision 2419, read on 2026-10-21, plus the user-approved 2026-10-22 correction from `time` to dimensionless `1`: both counters use the `_count` suffix and unit `1`, 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 @@ -80,3 +80,16 @@ Tested scope is the uncommitted diff over VeADK `d55fe8fa` and ACTB `7354ed5`. S | 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. + +### Unit correction (2026-10-22) + +The user explicitly approved unit `1` for both Skill counters across VeADK and ACTB Python/Go, superseding the earlier `time` decision. This is a correction to FR-1/FR-2/FR-6 and CON-1/CON-2. Update the two SDK instruments, sandbox instruments, existing unit assertions and bilingual contracts; preserve names, labels, increments, spans and provider lifecycle. No new initialization work or dependencies are needed. Existing historical `time` data is not rewritten; consumers filtering units must accept the new value during rollout. + +Direct design review (review-spec unavailable): checked all three producers, existing real-provider/export tests, bilingual parity and compatibility boundaries. The user's current instruction approves this exact scope; no additional design decision is pending. Acceptance is unit `1` with unchanged counts/labels in the existing late-provider and collector tests. Verification on the unit-only diff over VeADK `5a2aea21`, 2026-10-22: + +- `pass`: the updated VeADK/Python-sandbox late-provider tests and Go acquisition test failed against the old `time` instruments, then passed after the correction. +- `pass`: `uv run --no-sync --extra dev pytest tests/skills tests/test_portal_metrics.py tests/test_tracing_content.py -q` — 123 tests; the existing local environment was reused without dependency changes. +- `pass`: `uv run --no-sync --extra dev pre-commit run --all-files` — Ruff lint/format, secret scanners and dependency policy. +- `pass`: ACTB Python observability/reconciler/playground/catalog/agent/local-OTLP regression — 67 tests and 9 subtests; `TMPDIR=/private/tmp go run ./cmd/skill-go test -race -run 'TestSkill|TestInstalledSkillHub|TestTelemetry' -count=1 .`; `make verify PYTHON=/Users/bytedance/actb-mono/.venv/bin/python` — 29 smoke tests. +- `pass`: fetched origin and rebased onto the configured tracking branch before edits; the branch was already current. The follow-up retains the user's one-fix-per-commit delivery instruction; no push or deployment is included. +- `not_run`: repository-wide unrelated tests, image builds and live cloud collection; this correction only changes the two Skill units and has no dependency, startup or other-instrument changes. Historical data and backend unit-filter migration remain outside this local change. 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 e98ba34ea..4121f769a 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 @@ -22,7 +22,7 @@ - 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` 前缀。 +当前数据结构以 2026-10-21 读取的技术文档 revision 2419 及用户于 2026-10-22 批准将单位从 `time` 修正为无量纲 `1` 的要求为准：两个 counter 使用 `_count` 后缀和单位 `1`，身份新增 `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` 前缀。 ## 设计与受影响消费者 @@ -80,3 +80,16 @@ T-1 至 T-6 已实现。真实 ADK 加载响应保留 Registry 提供的身份 | Studio/浏览器门禁 | not_applicable | 未改变 Studio 前后端契约。 | Sandbox 初次全模块收集需要测试模型 API 值；最终命令使用占位值、禁用外部 exporter，并设置 `PYTHONPATH=src` 供子进程导入。创建内存 SDK provider 的测试局部移除 `OTEL_SDK_DISABLED`。未使用真实凭据、真实模型请求或修改全局机器配置。用户同意在此记录这些环境/工具限制。临时基线副本在对比后删除。受影响检查验证了本地实现行为，全仓验收尚未全绿。 + +### 单位修正（2026-10-22） + +用户明确批准 VeADK 与 ACTB Python/Go 的两个 Skill counter 统一采用单位 `1`，取代此前的 `time` 决策。这是对 FR-1/FR-2/FR-6 和 CON-1/CON-2 的修正。更新 SDK 的两个 instrument、Sandbox instrument、既有单位断言及双语契约；保持名称、标签、计数增量、Span 和 provider 生命周期。无需新增初始化工作或依赖。既有 `time` 历史数据不会重写；按单位过滤的消费者需在发布期间接纳新值。 + +直接设计评审（review-spec 不可用）：核对三个生产端、既有真实 provider/导出测试、双语一致性与兼容边界。用户当前指令已批准这一准确范围，无额外待定设计。验收要求为既有延迟 provider 和 collector 测试中的单位变为 `1`，计数和标签保持不变。验证于 2026-10-22 执行，范围为 VeADK `5a2aea21` 之上的单位修正 diff： + +- `pass`：更新后的 VeADK/Python Sandbox 延迟 provider 测试和 Go 获取测试在旧 `time` instrument 上失败，修正后通过。 +- `pass`：`uv run --no-sync --extra dev pytest tests/skills tests/test_portal_metrics.py tests/test_tracing_content.py -q` — 123 项；复用已有本地环境，未改变依赖。 +- `pass`：`uv run --no-sync --extra dev pre-commit run --all-files` — Ruff lint/format、密钥扫描与依赖策略检查。 +- `pass`：ACTB Python 观测/安装器/playground/catalog/agent/本地 OTLP 回归 — 67 项及 9 个子测试；`TMPDIR=/private/tmp go run ./cmd/skill-go test -race -run 'TestSkill|TestInstalledSkillHub|TestTelemetry' -count=1 .`；`make verify PYTHON=/Users/bytedance/actb-mono/.venv/bin/python` — 29 项 smoke。 +- `pass`：编辑前 fetch origin 并 rebase 到已配置的跟踪分支，分支已是最新。本次延续用户每个 fix 独立 commit 的交付要求；不包含推送或部署。 +- `not_run`：全仓无关测试、镜像构建和真实云端采集；本次只修改两个 Skill 单位，不涉及依赖、启动或其他 instrument。历史数据和后端按单位过滤的迁移不属于本地变更范围。 diff --git a/specs/skill-observability/README.md b/specs/skill-observability/README.md index d81516db0..2f1771966 100644 --- a/specs/skill-observability/README.md +++ b/specs/skill-observability/README.md @@ -2,7 +2,7 @@ [中文版](README.zh.md) -Component: `skill-observability`; status: implemented (local verification; full regression limitations in PRD); revised 2026-10-21. +Component: `skill-observability`; status: implemented (local verification; full regression limitations in PRD); revised 2026-10-22. Change: [skill-observability](../../prd-spec/features/skill-observability/2026-10-16-skill-observability.md). ## Ownership and interfaces @@ -11,8 +11,8 @@ VeADK owns Runtime package acquisition and ADK instruction-load instrumentation ## Contracts -- 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-1: `gen_ai_skill_download_count` is an OTEL counter, unit `1`; 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 `1`; 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. diff --git a/specs/skill-observability/README.zh.md b/specs/skill-observability/README.zh.md index 7d4005217..88fb43c7a 100644 --- a/specs/skill-observability/README.zh.md +++ b/specs/skill-observability/README.zh.md @@ -2,7 +2,7 @@ [English](README.md) -组件：`skill-observability`；状态：implemented（本地验证；全量回归限制见 PRD）；修订：2026-10-21。 +组件：`skill-observability`；状态：implemented（本地验证；全量回归限制见 PRD）；修订：2026-10-22。 关联变更：[skill-observability](../../prd-spec/features/skill-observability/2026-10-16-skill-observability.zh.md)。 ## 责任与接口 @@ -11,8 +11,8 @@ VeADK 在 `veadk/skills/` 与 `veadk/tracing/telemetry/telemetry.py` 负责 Runt ## 契约 -- 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-1：`gen_ai_skill_download_count` 是 OTEL counter，单位 `1`；维度严格为 `skill_source`、`skill_id`、`skill_name`、`skill_version`、`skill_space_id`。每次获取结束加一，包含缓存恢复和失败。单纯发现或本地软链接创建不产生事件；结果仅记录在 Span。 +- CON-2：`gen_ai_skill_load_count` 是 OTEL counter，单位 `1`；维度严格为上述五个身份字段。每次成功返回指令加一，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，不能仅依赖当前期望配置。 diff --git a/tests/skills/test_skill_telemetry.py b/tests/skills/test_skill_telemetry.py index 71868e8f7..cfd7b06d3 100644 --- a/tests/skills/test_skill_telemetry.py +++ b/tests/skills/test_skill_telemetry.py @@ -337,7 +337,7 @@ def test_default_proxy_instruments_are_bounded_and_support_late_provider(): 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_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 {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_")} == {"1"} assert values["gen_ai_skill_load_count"] == [(labels, 3)], values assert values["gen_ai_skill_download_count"] == [(labels, 3)], values finally: diff --git a/veadk/tracing/telemetry/portal_metrics.py b/veadk/tracing/telemetry/portal_metrics.py index 32cc1757b..e7ee18c21 100644 --- a/veadk/tracing/telemetry/portal_metrics.py +++ b/veadk/tracing/telemetry/portal_metrics.py @@ -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_COUNT, unit="time" + Meters.GEN_AI_SKILL_DOWNLOAD_COUNT, unit="1" ) self.skill_load_counter = self.meter.create_counter( - Meters.GEN_AI_SKILL_LOAD_COUNT, unit="time" + Meters.GEN_AI_SKILL_LOAD_COUNT, unit="1" ) def record_skill_download(self, attributes: dict[str, str]) -> None: