fix(telemetry): disable telemetry in tests for local runs. - #2253
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Changes requested
Nice, focused change — moving AGENTCORE_TELEMETRY_DISABLED into src/testing/setup.ts correctly covers all local bun test runs (setup.ts is preloaded via bunfig.toml and client.tsx:118 reads that env var), and the workflow-level env becomes redundant for the bun test step.
One issue
.github/workflows/verify.yml (removed top-level env block) — removing the workflow-level env also removes it from the Smoke test binary step (${{ matrix.binary }} --help, line 63-64). That step runs the compiled binary outside of bun test, so it no longer picks up the preload. Per src/index.ts:105, every invocation emits a cli.command_run metric in the finally block, so this PR will start shipping one telemetry event per platform per CI run from ephemeral runners.
Options:
- Restore the workflow-level
env: AGENTCORE_TELEMETRY_DISABLED: "1"(simplest — mirrors prior behavior). - Set the env var only on the smoke-test step.
- Set
AGENTCORE_TELEMETRY_DISABLED=1in the CodeBuild runner environment so it applies to any binary invocation in CI.
Everything else looks good to merge once the smoke-test path is covered.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2253 +/- ##
=========================================
Coverage 97.04% 97.04%
=========================================
Files 566 566
Lines 39409 39409
=========================================
Hits 38244 38244
Misses 1165 1165 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
51a9890 to
d5b136d
Compare
|
Claude Security Review: no high-confidence findings. (run) |
|
Addressed harness comment by re-applying change so that smoketest does not emit telemetry. |
Problem
We are seeing test data show up in our metrics:
This is likely coming from https://github.com/aws/agentcore-cli/blob/refactor/src/telemetry/client.test.tsx#L109, on local runs of the tests where telemetry is not disabled.
Solution
testing/setup.tsthe setsAGENTCORE_TELEMETRY_DISABLEDto 1.AGENTCORE_TELEMETRY_DISABLEDfrom the CI workflow since its not baked into the tests.Verification
Setup a basic local collector:
Run the collector via
COLLECTOR_OUT=collector-out.txt COLLECTOR_PORT=4318 bun collector.ts &update the default endpoint used in the tests at
agentcore-cli/src/globalConfig/config.tsx
Line 10 in 0caec7f
Run the tests with and without the env var injection to verify that it limits what is emitted.