Skip to content

[analytics] propagate ENV to deeplink sub-tool calls#9836

Open
pq wants to merge 5 commits into
flutter:masterfrom
pq:analytics_envSet
Open

[analytics] propagate ENV to deeplink sub-tool calls#9836
pq wants to merge 5 commits into
flutter:masterfrom
pq:analytics_envSet

Conversation

@pq
Copy link
Copy Markdown
Contributor

@pq pq commented May 19, 2026

Implements environment variable propagation (DASH__SUPPRESS_ANALYTICS and DASH__TOOL) when DevTools spawns ecosystem sub-tools (such as the flutter CLI during deep link analysis).

This completes the DevTools-side changes for GitHub Issue #9702 and integrates with the new environment construction utilities in package:unified_analytics (v8.0.13+).

Fixes: #9702.


Why This Covers All Required Cases

A comprehensive audit of the repository was performed to identify all places where child processes are spawned:

  1. Ecosystem Sub-Tools (Covered):

    • DeeplinkManager (packages/devtools_shared): This is the only production runtime component that spawns ecosystem sub-tools (flutter analyze). We have updated it to extract the parent IDE (DASH__TOOL) and client opt-out telemetry state (DASH__SUPPRESS_ANALYTICS), propagating them to the child flutter process using package:unified_analytics's getEnvironment() builder.
  2. Exclusions & Why They Don't Require Special Handling:

    • Build-Time CLI Utilities (e.g., BuildExtensionCommand in devtools_extensions): These run directly in the developer's terminal session, naturally inheriting the shell's active environment variables without needing dynamic, server-negotiated overrides.
    • POSIX & OS Utilities (e.g., tar, git, which): These are not Dart/Flutter tools and do not integrate with the package:unified_analytics telemetry system, meaning analytics environment variables have no effect.
    • Test & Benchmark Infrastructure (e.g., spawning Chrome, ChromeDriver, mock DTD): Strictly for CI/testing where telemetry is globally suppressed by default; no user-facing query parameter flow exists.

Detailed Changes

1. Client-Side Updates (packages/devtools_app)

  • server.dart: Updated buildDevToolsServerRequestUri to declaratively append query parameters ide (parent tool) and suppress_analytics (opt-out status) to all server-bound HTTP API requests.
  • analytics_controller.dart: Added synchronous public getters isAnalyticsEnabled and isAnalyticsControllerInitialized to check client telemetry status safely and synchronously.
  • pubspec.yaml: Upgraded unified_analytics dependency to ^8.0.13 to align with devtools_shared and prevent monorepo version conflicts.

2. Server-Side Updates (packages/devtools_shared)

  • pubspec.yaml: Added package:unified_analytics: ^8.0.13 dependency, sorted alphabetically.
  • _deeplink.dart:
    • Added a clean Map helper extension to extract and parse incoming query parameters without duplication:
      extension on Map<String, String> {
        String? get ide => this['ide'];
        bool get suppressAnalytics => this['suppress_analytics'] == 'true';
      }
    • Updated all four deep link handlers to extract and forward these parameters to DeeplinkManager.
  • deeplink_manager.dart:
    • Updated runProcess to accept ide and suppressAnalytics parameters.
    • Added _ideToDashToolMap to map IDE query strings to DashTool enums with case-insensitive lookups and alias overrides (such as vscodePlugins or intellijPlugins) in O(1) constant time, falling back to the O(N) enum values list loop only for new unmapped values.
    • Explicitly override and propagate the environment map in Process.run.

3. Verification & Test Alignment

  • fakes.dart: Updated FakeDeeplinkManager method signatures to correctly match the base class overrides.
  • deeplink_manager_test.dart: Added a dedicated unit test to verify that both the parent IDE and analytics opt-out status are correctly populated and propagated down to the child process.

Pre-launch Checklist

General checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read the Flutter Style Guide recently, and have followed its advice.
  • I signed the CLA.
  • I updated/added relevant documentation (doc comments with ///).

Issues checklist

Tests checklist

  • I added new tests to check the change I am making...
  • OR there is a reason for not adding tests, which I explained in the PR description.

AI-tooling checklist

  • I did not use any AI tooling in creating this PR.
  • OR I did use AI tooling, and...
    • I read the AI contributions guidelines and agree to follow them.
    • I reviewed all AI-generated code before opening this PR.
    • I understand and am able to discuss the code in this PR.
    • I have verifed the accuracy of any AI-generated text included in the PR description.
    • I commit to verifying the accuracy of any AI-generated code or text that I upload in response to review comments.

Feature-change checklist

  • This PR does not change the DevTools UI or behavior and...
    • I added the release-notes-not-required label or left a comment requesting the label be added.
  • OR this PR does change the DevTools UI or behavior and...
    • I added an entry to packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md.
    • I included before/after screenshots and/or a GIF demo of the new UI to my PR description.
    • I ran the DevTools app locally to manually verify my changes.

build.yaml badge

If you need help, consider asking for help on Discord.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements the propagation of IDE information and analytics opt-out status from the DevTools client through the server to spawned subprocesses. It introduces new getters for analytics status, updates server request URI construction to include these parameters, and enhances the DeeplinkManager to configure the process environment using package:unified_analytics. A concern was raised regarding a redundant environment variable override in DeeplinkManager.runProcess that might conflict with the canonical naming expected by the analytics package.

Comment thread packages/devtools_shared/lib/src/deeplink/deeplink_manager.dart Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[analytics] Update devtools to set analytics ENV variables when invoking sub-tools

1 participant