Add 1DS telemetry for power-pages with default-on opt-out - #127
Closed
amitjoshi438 wants to merge 55 commits into
Closed
Add 1DS telemetry for power-pages with default-on opt-out #127amitjoshi438 wants to merge 55 commits into
amitjoshi438 wants to merge 55 commits into
Conversation
Design for a shared telemetry library at shared/telemetry/ consumed by the power-pages plugin first, with interactive first-run consent, strict-allowlist payloads, and fail-closed emission via the existing PreToolUse/PostToolUse:Skill hook surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a shared, zero-dependency Node telemetry library (shared/telemetry/) and wires it into the power-pages plugin to emit 1DS Common Schema events for skill/script lifecycle, with default-on opt-out controls and fire-and-forget dispatch via a detached child process.
Changes:
- Introduces
shared/telemetry/library (dispatcher/spawn/events/consent/correlation/session/local-log) plus comprehensivenode:testcoverage and async-to-plugin.jscopier. - Wires
power-pageshooks (PreToolUse,PostToolUse,UserPromptSubmit) and adds atelemetry-runnerwrapper to instrument key scripts/validators. - Adds user-facing opt-out documentation in shared + plugin references, and repo/plugin guidance to avoid hand-editing synced telemetry copies.
Reviewed changes
Copilot reviewed 74 out of 75 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/telemetry/tests/with-telemetry.test.js | Tests script wrapper emits started/completed and rethrows errors. |
| shared/telemetry/tests/sync-to-plugin.test.js | Tests sync script copies expected telemetry assets into a plugin. |
| shared/telemetry/tests/session.test.js | Tests per-process session id behavior. |
| shared/telemetry/tests/scrubber.test.js | Tests scrubber no-op behavior and safety. |
| shared/telemetry/tests/prompt-detector.test.js | Tests slash-command detection logic. |
| shared/telemetry/tests/local-log.test.js | Tests local JSONL logging + rotation behavior. |
| shared/telemetry/tests/events.test.js | Tests allowlist enforcement and event envelope shape. |
| shared/telemetry/tests/emit-spawn.test.js | Tests detached spawn fire-and-forget behavior. |
| shared/telemetry/tests/emit-from-prompt.test.js | Tests prompt-based skill_started emission wiring. |
| shared/telemetry/tests/emit-dispatcher.test.js | Tests dispatcher consent gating and network/probe behaviors. |
| shared/telemetry/tests/correlation.test.js | Tests correlation file write/read/clear behavior. |
| shared/telemetry/tests/consent.test.js | Tests consent default-on + opt-out semantics and CLIs. |
| shared/telemetry/sync-to-plugin.js | Copies shared telemetry library + docs into a plugin. |
| shared/telemetry/references/telemetry-consent-reference.md | Documents what is sent and opt-out mechanisms. |
| shared/telemetry/lib/with-telemetry.js | Script wrapper emitting script_started/completed. |
| shared/telemetry/lib/session.js | Per-process UUID session id helper. |
| shared/telemetry/lib/scrubber.js | Placeholder scrubber seam (currently no-op). |
| shared/telemetry/lib/record-consent.js | CLI to persistently record consent on/off. |
| shared/telemetry/lib/prompt-detector.js | Detects /plugin:skill at start of prompt (tracked-only). |
| shared/telemetry/lib/local-log.js | Local JSONL dev logging + rotation. |
| shared/telemetry/lib/events.js | Event builders with strict allowlist + duration clamping. |
| shared/telemetry/lib/emit-spawn.js | Spawns detached dispatcher with minimal env. |
| shared/telemetry/lib/emit-from-prompt.js | Orchestrates slash-command skill_started emission. |
| shared/telemetry/lib/emit-dispatcher.js | Detached child: consent gate + local log or HTTPS POST. |
| shared/telemetry/lib/correlation.js | Temp-file correlation join for skill start/completion. |
| shared/telemetry/lib/consent.js | Default-on consent read/write with env kill switch. |
| shared/telemetry/lib/check-consent.js | CLI to print ENABLED/DISABLED. |
| shared/telemetry/ikey.json | Placeholder iKey + collector URL configuration. |
| shared/telemetry/README.md | Shared library overview and usage/sync instructions. |
| plugins/power-pages/skills/setup-datamodel/scripts/validate-datamodel.js | Adds telemetry wrapper around validator (currently double-invokes). |
| plugins/power-pages/skills/setup-auth/scripts/validate-auth.js | Adds telemetry wrapper around validator (currently double-invokes). |
| plugins/power-pages/skills/integrate-webapi/scripts/validate-webapi-integration.js | Adds telemetry wrapper around validator (currently double-invokes). |
| plugins/power-pages/skills/create-webroles/scripts/validate-webroles.js | Instruments validator via telemetry runner. |
| plugins/power-pages/skills/create-site/scripts/validate-site.js | Instruments validator via telemetry runner. |
| plugins/power-pages/skills/audit-permissions/scripts/validate-audit.js | Instruments validator via telemetry runner. |
| plugins/power-pages/skills/add-server-logic/scripts/validate-serverlogic.js | Instruments validator via telemetry runner (currently double-invokes). |
| plugins/power-pages/skills/add-seo/scripts/validate-seo.js | Instruments validator via telemetry runner. |
| plugins/power-pages/skills/add-cloud-flow/scripts/validate-cloudflow.js | Instruments validator via telemetry runner. |
| plugins/power-pages/skills/activate-site/scripts/validate-activation.js | Instruments validator via telemetry runner. |
| plugins/power-pages/scripts/verify-dataverse-access.js | Wraps script execution with telemetry runner; exports main. |
| plugins/power-pages/scripts/tests/telemetry-runner.test.js | Tests telemetry runner awaits/returns and rethrows errors. |
| plugins/power-pages/scripts/tests/telemetry-hook-pretool.test.js | Hook integration tests for pretool telemetry hook. |
| plugins/power-pages/scripts/tests/telemetry-hook-posttool.test.js | Hook integration tests for posttool behavior with consent disabled. |
| plugins/power-pages/scripts/tests/run-user-prompt-telemetry.test.js | Hook integration tests for prompt-submit telemetry hook. |
| plugins/power-pages/scripts/render-audit-report.js | Instruments script via telemetry runner; exports main. |
| plugins/power-pages/scripts/lib/telemetry/lib/with-telemetry.js | Synced telemetry library copy used at runtime by plugin. |
| plugins/power-pages/scripts/lib/telemetry/lib/session.js | Synced telemetry library copy used at runtime by plugin. |
| plugins/power-pages/scripts/lib/telemetry/lib/scrubber.js | Synced telemetry library copy used at runtime by plugin. |
| plugins/power-pages/scripts/lib/telemetry/lib/record-consent.js | Synced telemetry library copy used at runtime by plugin. |
| plugins/power-pages/scripts/lib/telemetry/lib/prompt-detector.js | Synced telemetry library copy used at runtime by plugin. |
| plugins/power-pages/scripts/lib/telemetry/lib/local-log.js | Synced telemetry library copy used at runtime by plugin. |
| plugins/power-pages/scripts/lib/telemetry/lib/events.js | Synced telemetry library copy used at runtime by plugin. |
| plugins/power-pages/scripts/lib/telemetry/lib/emit-spawn.js | Synced telemetry library copy used at runtime by plugin. |
| plugins/power-pages/scripts/lib/telemetry/lib/emit-from-prompt.js | Synced telemetry library copy used at runtime by plugin. |
| plugins/power-pages/scripts/lib/telemetry/lib/emit-dispatcher.js | Synced telemetry library copy used at runtime by plugin. |
| plugins/power-pages/scripts/lib/telemetry/lib/correlation.js | Synced telemetry library copy used at runtime by plugin. |
| plugins/power-pages/scripts/lib/telemetry/lib/consent.js | Synced telemetry library copy used at runtime by plugin. |
| plugins/power-pages/scripts/lib/telemetry/lib/check-consent.js | Synced telemetry library copy used at runtime by plugin. |
| plugins/power-pages/scripts/lib/telemetry/ikey.json | Synced iKey config shipped with plugin. |
| plugins/power-pages/scripts/lib/telemetry-runner.js | Power Pages wrapper that loads synced telemetry and instruments scripts. |
| plugins/power-pages/scripts/clear-site-cache.js | Instruments script via telemetry runner; exports main. |
| plugins/power-pages/scripts/check-activation-status.js | Instruments script via telemetry runner; exports main. |
| plugins/power-pages/references/telemetry-consent-reference.md | Plugin-shipped telemetry privacy/opt-out reference. |
| plugins/power-pages/hooks/run-user-prompt-telemetry.js | New hook to emit skill_started on slash-command prompts. |
| plugins/power-pages/hooks/run-skill-pretool-telemetry.js | PreToolUse hook emitting skill_started and writing correlation. |
| plugins/power-pages/hooks/run-skill-posttool-validation.js | PostToolUse hook runs validator + emits skill_completed. |
| plugins/power-pages/hooks/hooks.json | Registers telemetry hooks (PreToolUse + UserPromptSubmit) in plugin. |
| plugins/power-pages/AGENTS.md | Documents telemetry conventions and sync workflow for contributors. |
| docs/superpowers/specs/2026-04-27-1ds-telemetry-team-presentation.md | Design review/presentation doc for the telemetry approach. |
| docs/superpowers/specs/2026-04-23-slash-command-telemetry-design.md | Spec for slash-command skill_started telemetry. |
| docs/superpowers/specs/2026-04-20-1ds-telemetry-design.md | Full telemetry infrastructure design spec (updated posture). |
| docs/superpowers/plans/2026-04-23-slash-command-telemetry.md | Implementation plan for slash-command telemetry. |
| README.md | Adds repo-level telemetry section referencing shared docs. |
| AGENTS.md | Adds repo-level note about shared telemetry and sync workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
19
to
22
| function output(obj) { | ||
| process.stdout.write(JSON.stringify(obj)); | ||
| process.exit(obj.success ? 0 : 1); | ||
| } |
There was a problem hiding this comment.
Because main() calls output(), which calls process.exit(...), the runInstrumented(..., main) wrapper will never reach its script_completed emission for most code paths (the process exits before withTelemetry can run its finally). To ensure script_completed is emitted, consider refactoring output() to return a result/exitCode (or throw on failure) and let the require.main block handle process.exit after runInstrumented resolves.
Comment on lines
21
to
24
| function output(obj) { | ||
| process.stdout.write(JSON.stringify(obj)); | ||
| process.exit(0); | ||
| } |
There was a problem hiding this comment.
main() exits the process via output(), so the runInstrumented('check-activation-status', main) wrapper will usually not emit script_completed (the process terminates before withTelemetry can run its completion logic). If you want reliable completion telemetry, avoid calling process.exit inside main(); return/throw instead and exit in the require.main block after runInstrumented finishes.
Comment on lines
+69
to
72
| runInstrumented('validate-setup-auth', main).catch((err) => { | ||
| process.stderr.write(String((err && err.stack) || err) + '\n'); | ||
| process.exit(1); | ||
| }); |
There was a problem hiding this comment.
This file invokes runInstrumented('validate-setup-auth', main) at top-level and also again under if (require.main === module). When run as a script it will execute twice. Keep a single entrypoint (typically only under require.main) so validation/telemetry only runs once per invocation.
Comment on lines
35
to
+60
| try { | ||
| const input = JSON.parse(inputData); | ||
| const skillName = getTrackedSkillFromToolInput(input.tool_input); | ||
| skillName = getTrackedSkillFromToolInput(input.tool_input); | ||
| if (!skillName) { | ||
| debug('[power-pages hook] No tracked skill detected — skipping validation\n'); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| const validatorScript = getValidatorScript(skillName); | ||
| if (!validatorScript) { | ||
| debug(`[power-pages hook] Skill "${skillName}" has no validator — skipping\n`); | ||
| process.exit(0); | ||
| if (validatorScript) { | ||
| validatorRan = true; | ||
| const validatorPath = path.join(__dirname, '..', validatorScript); | ||
| const result = spawnSync(process.execPath, [validatorPath], { | ||
| input: inputData, | ||
| encoding: 'utf8', | ||
| cwd: input.cwd || process.cwd(), | ||
| }); | ||
| if (result.stdout) process.stdout.write(result.stdout); | ||
| if (result.stderr) process.stderr.write(result.stderr); | ||
| validatorStatus = result.status ?? 0; | ||
| debug(`[power-pages hook] Validator exited with code ${validatorStatus}\n`); | ||
| } | ||
| } catch (err) { | ||
| process.stderr.write(`[power-pages hook] Unexpected error: ${err.message}\n`); | ||
| validatorStatus = 0; | ||
| } |
There was a problem hiding this comment.
On JSON parse errors (e.g. empty/malformed stdin), this hook falls into the catch block, writes an error to stderr, and then continues into the telemetry block with skillName still null. That can emit an invalid skill_completed payload (skill_name: null) and may clear ppskills-corr-unknown.json. Consider treating malformed stdin as a non-error: exit 0 silently (or gate logging behind DEBUG) and return early so telemetry emission only runs when a valid tracked skillName was extracted.
|
|
||
| ## Telemetry | ||
|
|
||
| Plugins that ship 1DS telemetry (currently: `power-pages`) gather anonymous usage signals with explicit user consent. See `shared/telemetry/README.md` for what is sent and how to opt out. |
There was a problem hiding this comment.
This repo-level README says telemetry is collected with "explicit user consent", but the telemetry posture described elsewhere in this PR is default-on with opt-out (no first-run prompt). Please adjust this wording to match the implemented behavior (e.g., "enabled by default with documented opt-out").
Suggested change
| Plugins that ship 1DS telemetry (currently: `power-pages`) gather anonymous usage signals with explicit user consent. See `shared/telemetry/README.md` for what is sent and how to opt out. | |
| Plugins that ship 1DS telemetry (currently: `power-pages`) gather anonymous usage signals enabled by default, with a documented opt-out. See `shared/telemetry/README.md` for what is sent and how to opt out. |
Comment on lines
+70
to
+75
| // Hook is fire-and-forget via a detached child. Wait briefly for the | ||
| // dispatcher to write its probe. | ||
| const deadline = Date.now() + 5_000; | ||
| while (!fs.existsSync(probePath) && Date.now() < deadline) { | ||
| // busy-wait tight enough for CI; no sleep helper available cross-platform | ||
| } |
There was a problem hiding this comment.
This test uses tight busy-wait loops while polling for the probe file, which can burn CPU and make CI timing-sensitive. Consider making the tests async and using a small setTimeout/poll delay between checks. Also, rewriting the checked-in scripts/lib/telemetry/ikey.json is risky if node --test plugins/power-pages/scripts/tests/ runs files concurrently; prefer a test seam (e.g., override telemetry dir/iKey via env) or a temp copy so tests don’t mutate shared repo state.
Comment on lines
+19
to
+24
| if (!args.output || !args.data) { | ||
| console.error( | ||
| 'Usage: node render-audit-report.js --output <path> --data <json-file>' | ||
| ); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
On the missing-args path, main() calls process.exit(1), which will bypass runInstrumented/withTelemetry's script_completed emission. If completion events are important, prefer throwing an error (or returning a failure code) and let the require.main wrapper handle exiting after runInstrumented completes.
Comment on lines
+54
to
57
| runInstrumented('validate-setup-datamodel', main).catch((err) => { | ||
| process.stderr.write(String((err && err.stack) || err) + '\n'); | ||
| process.exit(1); | ||
| }); |
There was a problem hiding this comment.
This validator script currently runs runInstrumented('validate-setup-datamodel', main) twice: once at module top-level and again inside the if (require.main === module) guard. When executed as a CLI, this will run the validation twice (potentially duplicating output / making extra network calls). Remove the top-level invocation and keep only the require.main entrypoint (or remove the guard and keep only one call), so main runs exactly once.
Comment on lines
+62
to
65
| runInstrumented('validate-integrate-webapi', main).catch((err) => { | ||
| process.stderr.write(String((err && err.stack) || err) + '\n'); | ||
| process.exit(1); | ||
| }); |
There was a problem hiding this comment.
runInstrumented('validate-integrate-webapi', main) is called twice (top-level and again inside if (require.main === module)). This will run the validator twice when executed as a CLI. Remove one of the invocations (preferably the top-level one) so main runs exactly once.
Comment on lines
+217
to
220
| runInstrumented('validate-add-server-logic', main).catch((err) => { | ||
| process.stderr.write(String((err && err.stack) || err) + '\n'); | ||
| process.exit(1); | ||
| }); |
There was a problem hiding this comment.
This script currently calls runInstrumented('validate-add-server-logic', main) at top-level and again inside if (require.main === module), which means the validator will run twice when executed as a CLI. Keep only a single invocation (typically the require.main one) to avoid duplicate work and duplicate telemetry events.
7-milestone TDD plan derived from the design spec. Builds the shared library at shared/telemetry/ with per-file node:test coverage, syncs into power-pages, wires the PreToolUse/PostToolUse hooks, instruments high-value scripts via withTelemetry, and ends with a marketplace-install E2E smoke test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Revision after reviewing agency-microsoft/playground/claude-telemetry: - Drop @microsoft/1ds-core-js and 1ds-post-js. Use Node's built-in https module directly. No package.json, no node_modules, no npm install step anywhere in the telemetry tree. - Hooks use a detached-child dispatcher pattern: the hook parses stdin, calls emit-spawn.fireAndForget, and exits in ~50 ms. A detached dispatcher child does the HTTPS POST independently. - withTelemetry uses the same dispatcher — script emits are also fire-and-forget. - Payload shape stays Common Schema 4.0 (verified in the POC via acc:N). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Match the 2026-04-22 spec revision. Targeted rewrites only — most tasks unchanged. Affected: 0.x (Node banner), 1.1 (scaffold drops package.json), 1.7 (replaces client.js with emit-dispatcher.js), 1.7b (new emit-spawn.js task), 1.8 (with-telemetry uses emit-spawn), 2.1 (sync drops package.json copy), 2.3 (drops npm install), 3.1 / 3.2 (hooks use fireAndForget), 5.1 (runner simpler without client), 6.1 (AGENTS.md drops install step), 6.3 (README.md drops install step), 7.2 (E2E drops npm install step). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Global uncaughtException/unhandledRejection/stdin-error handlers
ensure the dispatcher never leaves a stack trace or nonzero exit in
the parent's process tree.
- Wrap new URL(COLLECTOR_URL) in try/catch to handle garbage URLs.
- Restore baseType assertion in happy-path test (plan specifies it).
- Add HTTPS-refused test to cover the req.on('error') branch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Use process.execPath instead of 'node' so the detached child always runs on the same interpreter as the parent, regardless of PATH. - Replace ...process.env spread with an explicit allowlist so the dispatcher child never inherits unrelated secrets (AZURE tokens, GitHub tokens, etc.). - Minor test cleanups. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…racked skills Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tion Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wrap all 10 per-skill validator scripts with runInstrumented to emit script_started and script_completed telemetry events. Each validator now calls runInstrumented at the module level, wrapping the runValidation call to track execution through 1DS telemetry. Validators wrapped: - validate-activate-site - validate-add-seo - validate-audit-permissions - validate-create-site - validate-create-webroles - validate-add-cloud-flow - validate-add-server-logic - validate-integrate-webapi - validate-setup-auth - validate-setup-datamodel Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
In dev/pre-provisioned state (iKey = PLACEHOLDER_REPLACE_BEFORE_SHIPPING or collector URL empty), the dispatcher now appends the event to ~/.power-platform-skills/events.jsonl instead of dropping it. Consent still gates the write — no capture of any kind when the user has disabled telemetry. Rotates to events.<yyyymmddhhmmss>.old at 10 MB. Once a real iKey ships, the dispatcher takes the POST path and the local log is never touched. The log is a one-way developer-inspection artifact, not a retry queue. - shared/telemetry/lib/local-log.js: new module, 5 tests - shared/telemetry/lib/emit-dispatcher.js: branch on keyMissing, reordered consent gate to happen before stdin read so it governs local-log writes too, 2 new tests - Re-synced into plugins/power-pages/scripts/lib/telemetry/ - Spec §1 non-goals and §2 layout updated Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Design for emitting skill_started when tracked skills are invoked via slash commands, closing the gap where PreToolUse:Skill never fires for slash-invoked skills. Explicitly scopes out skill_completed for this path and documents the six failure modes of Stop-based completion.
… path Add a 'Known Issue' section to the slash-command telemetry spec explaining that skill_completed's duration_ms and outcome fields do not measure workflow runtime or outcome — they fire when SKILL.md finishes loading, before any phases execute. Not fixed by this spec; flagged so downstream readers interpret the data correctly and so a future spec can address it.
Concrete, TDD-sequenced plan implementing the slash-command telemetry spec. Three milestones: shared library additions, power-pages hook wiring, sync+verification. Each step has exact file paths, code, and test commands.
… env vars emit-from-prompt.js now propagates POWER_PLATFORM_SKILLS_CONFIG_DIR and POWER_PLATFORM_SKILLS_FAKE_HTTPS from the hook's process.env into the detached dispatcher child's spawn opts. Without this, tests using FAKE_HTTPS probes or a custom config dir were silently ignored because emit-spawn.js only passes an explicit allowlist of env vars to the child. Matches the pattern already used by run-skill-pretool-telemetry. Discovered by the run-user-prompt-telemetry integration test.
…Data
The previous envelope wrapped every event in:
baseType: "Ms.WebClient.TraceEvent"
baseData: { eventName, eventType, severity, eventInfo: <stringified info> }
The real 1DS tenant rejected every POST with HTTP 400 and
"All events rejected due to parsing failure. InvalidEventFormat:all" —
the tenant's schema does not recognise the Ms.WebClient.TraceEvent
extension or the nested shape. Because the dispatcher is fire-and-forget
with stdio ignored and req.on("error") exitSilently, every POST silently
returned 400 and zero events ever landed in Kusto.
Empirical finding (6-variation probe against the live collector): the
envelope { ver, name, time, iKey, data: {...flat allowlisted fields} }
with a trailing newline for application/x-json-stream returns HTTP 200
and acc:1.
Changes:
- events.js: builders now return { name: "<event_type>", data: {...picked} }.
The event-type name (skill_started, skill_completed, script_started,
script_completed) rides on the envelope's top-level `name` field
instead of being wrapped inside baseData.eventName. The `data` object
carries the existing allowlisted fields flat — no more eventName /
eventType / severity / eventInfo wrapper.
- emit-dispatcher.js: buildEnvelope now produces
{ ver: "4.0", name, time, iKey, data: event.data }. Body is
newline-terminated to match the x-json-stream content type.
- All shared and plugin tests updated to the new shape (77 shared tests
and 4 plugin integration tests pass).
- No changes to hooks, consent, sync-to-plugin, or local-log semantics.
Verified end-to-end: real POST of the exact hook-produced envelope
returns HTTP 200 / acc:1 on every event type.
The 2026-04-20 design spec is now stale on §3 and §6.2 (event shape and
envelope). Follow-up PR should either update the spec to match this
reality or add an Ms.WebClient.TraceEvent schema declaration to the
tenant and restore the old shape.
…Kusto routing The prior commit d507e5c set envelope.name to the per-event identifier ("skill_started", "skill_completed", etc.), which caused events to miss the PagesPowerPlatformExtEvent Kusto table entirely. The 1DS OneCollector routes events to a Kusto table by matching envelope.name to the table's registered extension name — only events with name = "PagesPowerPlatformExtEvent" land in that table. Reshape the builders so: - envelope.name is the constant "PagesPowerPlatformExtEvent" (routing key). - data.EventName carries the per-event identifier (Kusto column EventName). - data.EventType = "Trace", data.Severity = "Info" match table columns. - data.EventInfo is an object (not a stringified JSON) holding the allowlisted payload fields — matches existing dynamic-typed column. The envelope shape stays at 5 top-level fields {ver, name, time, iKey, data} with trailing newline, same as d507e5c. baseType / baseData remain removed (those were what 1DS rejected originally). Verified end-to-end: - 78/78 shared telemetry tests pass. - 4/4 plugin integration tests pass. - Direct POST with this shape returns HTTP 200 / acc:1. - Hook-produced envelope (captured via FAKE_HTTPS) matches the verified shape byte-for-byte. - Kusto query "PagesPowerPlatformExtEvent | where EventName contains 'skill'" will now match emitted events.
… shape
The Kusto ingestion mapping for PagesPowerPlatformExtEvent populates
its PascalCase columns (EventName, EventType, Severity, EventInfo)
from camelCase data.* fields — the convention used by the 1ds-core-js
SDK. Events from the power-platform VSCode extension land correctly
because that extension uses the SDK; ours did not because a prior
refactor switched the keys to PascalCase directly and left eventInfo
as a nested object.
Reshape the builders to match the SDK convention exactly:
- data.eventName (camelCase)
- data.eventType (camelCase)
- data.severity (camelCase)
- data.eventInfo = JSON.stringify({...}) (string, not object)
envelope.name stays "PagesPowerPlatformExtEvent" to route to the
correct Kusto table. No baseType / baseData — this endpoint rejects
the legacy SDK wrapper when posted manually (returns 400
InvalidEventFormat:all). All payload fields (plugin_name, skill_name,
correlation_id, outcome, duration_ms, error_class, etc.) are carried
inside the stringified eventInfo per the same SDK convention.
Verified:
- 77/77 shared telemetry tests pass.
- 4/4 plugin integration tests pass.
- POST to the live collector returns HTTP 200 / acc:1.
Kusto query
PagesPowerPlatformExtEvent | where EventName contains "skill"
should now match emitted events after the usual 1DS ingestion lag.
…tation
Events with envelope.name = "PagesPowerPlatformExtEvent" were silently
dropped after OneCollector returned acc:1 because the tenant's
EventStreamingAnnotation binds events by (iKey, envelope.name) tuple,
and our tuple wasn't registered.
The tenant-side annotation discovered for our iKey is:
<EventStreamingAnnotation name="^PowerPlatformExtensionEvent$">
CollectorEventMappingList: "ffdb4c99...:VscodeEvent"
The second half of the colon-separated entry is the envelope.name the
pipeline expects. Setting envelope.name = "VscodeEvent" makes events
flow into the PowerPlatformExtensionEvent Kusto stream.
Key conceptual fix (documented in the module comment): envelope.name
is an opaque routing token, not the Kusto table name. OneCollector's
acc:1 ack is wire-layer only — it says nothing about whether an
annotation matches the event or whether it will be ingested.
Tests: 77/77 shared + 4/4 integration pass. Real hook invocation
produces the verified envelope shape.
Non-blocking follow-ups surfaced by the post-merge code review: - run-skill-pretool-telemetry.js: null-coalesce ikey/collector_url reads so a missing/null ikey.json field yields empty strings instead of the literal "undefined" reaching the dispatcher env. Forward POWER_PLATFORM_SKILLS_FAKE_HTTPS via fireAndForget opts so the test-seam works for this hook (matches the new user-prompt hook). - run-skill-posttool-validation.js: same two fixes. - docs/superpowers/specs/2026-04-20-1ds-telemetry-design.md §6.2: replace the stale baseType/baseData envelope with the shape that actually lands in Kusto (ver/name/time/iKey/data + trailing newline). Add the routing note explaining that envelope.name is a tenant-side registered token (iKey + name tuple must appear in the annotation's CollectorEventMappingList or events are silently dropped after acc:1). Document the camelCase → PascalCase FieldNameMapping convention and why eventInfo must be stringified. All 77 shared + 4 integration tests pass. No change to production envelope shape; this is documentation + hook-parity hardening only.
Anonymous, allowlist-only telemetry now ships enabled by default. The interactive first-run prompt is removed; users opt out via either POWER_PLATFORM_SKILLS_TELEMETRY=0 (env kill switch) or record-consent.js --answer no (persistent file at ~/.power-platform-skills/telemetry.json, honored across schema versions). Code: - consent.js: missing/malformed file -> enabled; explicit opt-out preserved across schema versions; drop prompt_version - check-consent.js: binary ENABLED/DISABLED output (no NEEDS_PROMPT) - emit-dispatcher.js: env kill switch checked before consent module loads, so an opted-out user is honored even if the module fails SKILL.md cleanup: removed the Phase-1 telemetry consent one-liner from every tracked skill (12 files). The dispatcher is now the single policy enforcement point. Docs: rewrote telemetry-consent-reference.md as an opt-out reference; updated README, AGENTS.md, the long design spec (added 2026-04-27 revision note + rewrote section 4), and the team-presentation doc (rewrote decision card 5.4). All 87 telemetry tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Walks other plugin owners through the shared telemetry library: five baked-in decisions and their tradeoffs, what is shared vs what each plugin would configure, a step-by-step adoption checklist, local + Kusto verification, and likely concerns. Frames the current cluster as a testing-only setup, with per-plugin clusters as the planned default. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the PLACEHOLDER_REPLACE_BEFORE_SHIPPING value with the provisioned iKey for tenant ffdb4c99... and point the dispatcher at the Aria INT OneCollector endpoint. With this in place the dispatcher takes the HTTPS POST path instead of writing to the local JSONL fallback. The iKey is a write-only identifier, safe to commit in plaintext per the design spec (Microsoft OSS precedent: VS Code, dotnet SDK, Azure CLI). Both shared/telemetry/ikey.json and the synced plugin copy are updated together.
amitjoshi438
force-pushed
the
users/amitjosh/1ds-telemetry
branch
from
May 11, 2026 07:07
ad48dc3 to
acc1746
Compare
Priyanshu Agrawal (priyanshu92)
approved these changes
Jun 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
power-platform-skillsmarketplace, wired intopower-pagesas the first consumer.shared/telemetry/(zero npm deps, Node built-ins only); synced into each adopting plugin viasync-to-plugin.js.POWER_PLATFORM_SKILLS_TELEMETRY=0or persistentrecord-consent.js --answer no). No interactive prompt.EventStreamingAnnotation(envelope.name = "VscodeEvent").emit-dispatcher.jschild; parent hooks return in ~50 ms regardless of collector latency.