Skip to content

feat(telemetry): @ngaf/telemetry library — opt-out Node, opt-in browser#319

Merged
blove merged 16 commits into
mainfrom
worktree-gtm+ngaf-telemetry
May 14, 2026
Merged

feat(telemetry): @ngaf/telemetry library — opt-out Node, opt-in browser#319
blove merged 16 commits into
mainfrom
worktree-gtm+ngaf-telemetry

Conversation

@blove
Copy link
Copy Markdown
Contributor

@blove blove commented May 14, 2026

Summary

Implements Spec 1B (analytics-foundation sub-spec B): the @ngaf/telemetry library with three subpath exports honoring the public trust contract at libs/telemetry/README.md.

  • New Nx library at libs/telemetry/ named telemetry
  • Three subpath exports (when published):
    • @ngaf/telemetry — shared types, env detection, SHA-256 hashing, per-process anon-id, sample-rate math
    • @ngaf/telemetry/node — posthog-node client, postinstall script, adapter helpers (captureRuntimeInstanceCreated, captureStream{Started,Ended,Errored}, disableTelemetry)
    • @ngaf/telemetry/browser — provideNgafTelemetry() returning EnvironmentProviders, NgafTelemetryService with lazy posthog-js import
  • Hybrid build: @nx/js:tsc (shared + node) + @nx/angular:package (browser)
  • Vitest test infra (corrected mid-execution from initial Jest scaffold; the rest of the monorepo uses Vitest)
  • 45 unit tests including the permanent browser silence test that pins the trust contract
  • telemetry added to the publishable group in nx.json (joins the fixed-version cadence)
  • Postinstall script wired with opt-out notice (CI-suppressed) and || true so it never breaks npm install

Spec: docs/superpowers/specs/gtm/2026-05-15-analytics-foundation-1b-ngaf-telemetry-design.md
Plan: docs/superpowers/plans/gtm/2026-05-15-analytics-foundation-1b-ngaf-telemetry.md

Trust contract enforcement

  • Browser silence: @ngaf/telemetry/browser never imports posthog-js at module load. Service uses lazy dynamic import gated on enabled:true AND posthogKey present. The permanent browser-silence.spec.ts asserts this stays true forever.
  • Node opt-out: five paths — DO_NOT_TRACK, NGAF_TELEMETRY_DISABLED, five CI-auto-detect env vars, disableTelemetry() programmatic kill switch.
  • Hashing: all sensitive identifiers (apiKey etc.) are SHA-256-hashed before becoming event properties.
  • Anonymous id: per-process UUID, regenerated each boot, no persistence.
  • Silent fail: every public surface wrapped so npm install never breaks.
  • No vendor key shipped: the public PostHog Project API Key in client.ts is safe (proxy re-keys server-side; Personal API Keys never appear).

Bugs caught + fixed during implementation

  1. Plan specified Jest test config; the repo uses Vitest. Migrated wiring (vite.config.mts + per-file // @vitest-environment jsdom pragma for browser specs + test-setup.ts for TestBed bootstrap).
  2. Plan-verbatim service.ts used decorator constructor params; esbuild (Vitest) rejects without the Angular compiler plugin. Refactored to functional inject() — equivalent semantics, modern Angular pattern, zoneless-compatible.
  3. ng-packagr enforces rootDir at entry-file level; browser couldn't cross-import from ../shared/. Inlined NgafBrowserEvent in service.ts.
  4. ng-packagr doesn't accept .js extensions on relative imports in the browser entry. Removed .js suffix from browser-entry imports (Node entry keeps them for native ESM).
  5. Source package.json was missing the exports map — restored per spec §4.3.
  6. posthog-node + posthog-js weren't installed at root — added as root devDeps via surgical npm install --save-dev --workspaces=false (zero @next/swc-linux deletions in lockfile, per the macOS lockfile rule).

Known follow-up (deferred to publish-prep PR)

The hybrid dist layout produces tsc Node output under dist/libs/telemetry/src/* and ng-packagr browser output under dist/libs/telemetry/browser/*. A small post-build merge script is needed before npm publish so the exports map paths resolve correctly. Not blocking Spec 1B's correctness or the next release cycle's preparation work. The 45 tests prove the library code is correct; the published-package shape is a packaging concern, not a library concern.

Test Plan

  • CI passes: lint, test, build (both targets)
  • Permanent browser silence test stays green
  • @ngaf/telemetry@ publishes alongside other libs on next release after the dist-merge follow-up
  • Postinstall opt-out notice fires correctly on a real npm install (manual after merge)

🤖 Generated with Claude Code

@vercel
Copy link
Copy Markdown

vercel Bot commented May 14, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cacheplane Ready Ready Preview, Comment May 14, 2026 8:53am

Request Review

blove and others added 15 commits May 14, 2026 01:06
Spec 1B of the GTM motion: single Nx library producing three published
subpath exports (@ngaf/telemetry, /node, /browser) honoring the trust
contract at libs/telemetry/README.md.

Architecture: hybrid @nx/js:tsc + @nx/angular:package build, lazy
posthog-js import gated on enabled:true, permanent browser silence
test pinning zero network calls when not opted in.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three subpath exports planned: ., ./node, ./browser. Build uses
@nx/js:tsc for shared+node entries and @nx/angular:package for the
browser (Angular DI) entry. Stub source files prove both builds
resolve before real code lands.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The repo standardized on Vitest (see libs/chat, libs/cockpit-registry).
The plan's Jest config wasn't installable: @nx/jest and ts-jest are not
in the lockfile, and jest.preset.js doesn't exist at repo root.

Replaced libs/telemetry/jest.config.ts + tsconfig.spec.json with
vite.config.mts matching the existing libs convention. Switched the
test target executor in project.json from @nx/jest:jest to
@nx/vitest:test.

Spec/plan files reference jest-style mocks (jest.mock, jest.fn);
follow-up Task 3 implementer rewrites them in vitest style (vi.mock,
vi.fn, vi.hoisted).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…, events

19 tests covering all five opt-out paths, SHA-256 determinism,
per-process anon-id, sample-rate clamping, and event-name types.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…able

Six tests: postinstall capture shape, opt-out paths, ingest URL
override, sample_weight stamping, awaits shutdown. Silent fail on
network errors. Public PostHog Project API Key is safe to ship in OSS
code (the ingest proxy re-keys server-side; never a Personal Key).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
4 tests cover capture, stdout notice formatting, CI suppression, and
silent failure when package.json can't be read. Wrapped so npm install
never fails on any path.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ecycle

5 tests. Raw apiKey is SHA-256 hashed before emit. Error objects emit
only their class name, never the message. All four helpers no-op
silently when capture throws — npm install path stays safe.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
NgafTelemetryService never imports posthog-js at module load. Dynamic
import only fires when capture() is called with enabled:true AND
posthogKey present. Five tests cover all opt-out paths.

Uses inject() instead of parameter decorators so vitest+esbuild can
transform without the Angular compiler plugin. Adds test-setup.ts to
initialize TestBed via BrowserTestingModule; per-file jsdom pragma on
browser specs keeps node specs in the node environment.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
4 tests verify config + service injection, sampleRate default,
posthogHost passthrough, and that enabled:true without posthogKey still
provides a service (which then no-ops at capture time).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three entry indices: src/index.ts, src/node/index.ts,
src/browser/public-api.ts. Permanent contract test (2 cases) asserts
posthog-js is never imported when provideNgafTelemetry is not called
or called with enabled:false. Test stays green permanently.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
telemetry joins the fixed-version publishable group; first published
version will be the next group bump (per user memory: never 0.1.0,
always patch). .env.example documents the four user-facing env vars.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds an Imports section documenting the three subpath entry points.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three small fixes uncovered by Task 12 build verification:

1. Source libs/telemetry/package.json was missing the exports map.
   Restored per spec §4.3 with subpath exports for ., ./node,
   ./browser, ./postinstall.
2. ng-packagr enforces rootDir at the entry-file level (src/browser/),
   so the browser entry cannot cross-import from ../shared/. Inlined
   NgafBrowserEvent in src/browser/service.ts.
3. ng-packagr resolves imports natively and doesn't accept .js
   extensions on relative imports — removed .js suffix from browser
   entry imports (matching libs/chat convention). Node entry retains
   .js for native ESM resolution.

All 45 tests still pass. `nx run telemetry:build` completes both
targets (build:node + build:browser) successfully.

Follow-up (deferred to publish-prep PR): the dist layout has tsc node
output under dist/libs/telemetry/src/* and ng-packagr browser output
under dist/libs/telemetry/browser/*. A post-build merge script is
needed before npm publish so the exports map paths resolve. Not
blocking Spec 1B's library correctness.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- adapter.spec.ts: removed unused sha256 import.
- postinstall.spec.ts: replaced empty arrow function with explicit
  undefined return (still a no-op, but satisfies @typescript-eslint/
  no-empty-function).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
After the rebase onto main, npm ci was failing on CI because the
lockfile didn't know about the new @ngaf/telemetry workspace package
or its posthog-* dependency versions.

Ran 'npm install --package-lock-only' to sync the lockfile without
touching node_modules. 50-line incremental update, zero
@next/swc-linux-* deletions (memory rule respected).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@blove blove merged commit a00eba3 into main May 14, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant