Skip to content

adk-telemetry: show opt-out notice before the first emit (all paths) - #244

Open
amilandi wants to merge 1 commit into
mainfrom
amilandin/adk-telemetry-notice-before-emit
Open

adk-telemetry: show opt-out notice before the first emit (all paths)#244
amilandi wants to merge 1 commit into
mainfrom
amilandin/adk-telemetry-notice-before-emit

Conversation

@amilandi

Copy link
Copy Markdown
Contributor

Closes a notice-ordering gap discovered while investigating why real-customer /setup events dominate adk.capability.use.

The gap

For git-cloned installs (no bootstrap installer), the prior /setup flow emitted the first adk.capability.use before the maker had any chance to see the opt-out notice:

1. fetch_and_setup.py auth + emit_capability_use("setup", block=True)  <-- SEND
2. setup.py (subprocess) -> maybe_print_notice() on stderr             <-- NOTICE
3. setup.py emit_agent_create(...)                                     <-- SEND

That doesn't meet the "notice at time of collection" bar (GDPR Art. 13 / Microsoft privacy-review standard for pseudonymous OII on a legitimate-interest basis).

Installer-bootstrapped installs already do this right — setup/telemetry/install-telemetry.ps1 (and the .sh mirror) call Show-EssTelemetryNotice before ESSMakerKit.Installer.Start and persist noticeShown=true to ~/.adk/config. So this PR is a no-op for that path and closes the gap only for git-cloned installs that bypass the installer.

The fix

Route the notice through the single _emit choke point in adk_telemetry.py so every ADK event dispatch (capability_use, session_start, agent_create, agent_deploy, build_start/complete, api_call, flightcheck_run/result/error) shows the notice before the network round-trip.

New helper _notice_before_emit:

  • Idempotent via the existing noticeShown config flag — cheap read after the first call
  • Skips notice when telemetry is already opted out (nothing to disclose)
  • Best-effort — any IO error is swallowed so a notice-write can never break a caller's flow

Existing maybe_print_notice() call in scripts/setup.py is preserved as belt-and-suspenders; after this change it always finds the flag already set and no-ops.

Regression tests

  • test_emit_shows_notice_before_dispatch — spies on maybe_print_notice + _fc._post on a fresh install and asserts notice fires before post.
  • test_emit_skips_notice_when_opted_out — the opt-out env var suppresses both.
  • test_emit_notice_swallows_errors — a notice OSError doesn't break the emit.

Verification

  • tests/test_adk_telemetry.py69 passed
  • Broader touched suite (test_adk_telemetry.py + tests/flightcheck + tests/scripts/test_auth.py) — 1193 passed
  • Same 14 pre-existing failures (installer + foundation-checklist) remain, unrelated to this change.

ADO: https://o365exchange.visualstudio.com/O365%20Core/_workitems/edit/7764297

Fixes a notice-ordering gap discovered while investigating the /setup
capability skew:

Prior flow when a maker runs /setup on a direct-clone install (no installer):
  1. fetch_and_setup.py auth + emit_capability_use("setup") FIRES  <-- send
  2. setup.py subprocess -> maybe_print_notice() prints on stderr  <-- notice
  3. setup.py emit_agent_create(...)                                <-- send

The first emit sent an event before the maker was ever shown the opt-out
notice. That doesn't meet the "notice at time of collection" bar (GDPR
Art. 13 / Microsoft privacy review standard for pseudonymous OII on
legitimate-interest basis) for the direct-clone path.

Installer-bootstrapped installs already do this right: the PowerShell /
bash installer telemetry emitter (setup/telemetry/install-telemetry.{ps1,sh})
calls Show-EssTelemetryNotice BEFORE emitting ESSMakerKit.Installer.Start,
and persists ``noticeShown=true`` to ~/.adk/config. So this fix is a no-op
for them (the notice call in _emit finds the flag already set and returns
immediately) and closes the gap for git-cloned installs that bypass the
installer.

Change: route notice through the single ``_emit`` choke point so every ADK
event dispatch (capability_use, session_start, agent_create, agent_deploy,
build_start/complete, api_call, flightcheck_run/result/error) shows the
notice before the network round-trip. New helper ``_notice_before_emit``:

* Idempotent via the existing noticeShown config flag - cheap after 1st call
* Skips notice when telemetry is already opted out (nothing to disclose)
* Best-effort - any IO error is swallowed so a notice-write can never
  break a caller's flow

Existing setup.py maybe_print_notice() call at line 786 is preserved as
belt-and-suspenders; after this change it always finds the flag already
set and no-ops.

Regression tests:
* test_emit_shows_notice_before_dispatch - orders "notice" before "post"
  by spying on maybe_print_notice + _fc._post on a fresh install.
* test_emit_skips_notice_when_opted_out - opt-out env var suppresses both.
* test_emit_notice_swallows_errors - notice OSError doesn't break emit.

Tests: 69 passed in test_adk_telemetry; 1193 passed in broader touched suite
(tests/test_adk_telemetry.py + tests/flightcheck + tests/scripts/test_auth.py).

ADO: https://o365exchange.visualstudio.com/O365%20Core/_workitems/edit/7764297

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 70ddf006-7f8d-48e7-9afa-3fbae73b3864
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