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
@pq
Copy link
Copy Markdown
Contributor Author

pq commented May 19, 2026

UPDATE: I needed to rollback our bump to a new unified_analytics package meaning we can't use the newly added helpers. Here's the rationale...

Rationale for the Manual Environment Construction

To resolve Issue #9702, DevTools must propagate the parent IDE (DASH__TOOL) and analytics opt-out status (DASH__SUPPRESS_ANALYTICS) down to spawned child processes (like flutter analyze during deep link verification).

While package:unified_analytics (v8.0.13+) introduced helper utilities (getEnvironment() and enums) for this, integrating those helpers required bumping the package dependency constraint. However, unified_analytics v8.0.13+ introduced a minimum Dart SDK constraint of ^3.10.0 stable. Because the repository's pinned Flutter candidate SDK currently uses a development pre-release (3.10.0-28.0.dev), which Dart's semver treats as older than 3.10.0 stable, this upgrade triggered a version solver conflict that blocked dependency resolution on CI.

To bypass this SDK-bound version solving conflict while fully delivering the required telemetry propagation, we opted to construct the environment variables map manually using String constants inside DeeplinkManager. This manual implementation is 100% functionally equivalent to the package helper output, preserves case-insensitive IDE alias lookups in O(1) constant time, and keeps the shared backend package (devtools_shared) completely decoupled from the new dependency, ensuring zero version conflicts across all CI/CD runner environments.

@pq
Copy link
Copy Markdown
Contributor Author

pq commented May 19, 2026

@kenzieschmoll: I think this is ready for review. The integration test failures seem unrelated?

@pq
Copy link
Copy Markdown
Contributor Author

pq commented May 19, 2026

(Re-running w/ debugging enabled in case that gives us any clues.)

@pq
Copy link
Copy Markdown
Contributor Author

pq commented May 20, 2026

Interestingly a few retries got the integration tests green. (Maybe they should be configured to retry?)

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