MSI installer: Add checkbox and MSI property DISABLE_TELEMETRY to optionally disable telemetry. #10725

bergmeister · 2019-10-07T22:12:08Z

PR Summary

Fixes #10578

Checkbox not ticked by default, i.e. telemetry is still on by default (via UI and silent install)

PR Context

To support community request in linked issue. Also a common enterprise scenario.

PR Checklist

…ionally disable telemetry. Not ticked by default, i.e. telemetry is still on by default.

kilasuit · 2020-05-21T16:45:52Z

@bergmeister this looks promising :-)

assets/Product.wxs

TravisEz13 · 2020-05-21T16:52:52Z

assets/Product.wxs

+                 Before="DisableTelemetry"
+                 Sequence="execute"
+                 Value="&quot;[$(var.ProductDirectoryName)]pwsh.exe&quot; -NoProfile -ExecutionPolicy Bypass -Command &quot;[\[]System.Environment[\]]::SetEnvironmentVariable('POWERSHELL_TELEMETRY_OPTOUT', '1',  [\[]System.EnvironmentVariableTarget[\]]::Machine)&quot;" />
+    <CustomAction Id="DisableTelemetry"


Can we use an Environment element

Good idea, I'd have to look into it in more detail.

Would the environment element be able to undo the change to env variable on uninstall?
Slightly off-topic: Because preview and stable use the same environment variable, I am wondering if there is a case where a user would want to turn telemetry off only for stable but allow for preview. WDYT? @kilasuit

Its possible that could be a potential use case though that should I think be asked in another issue

We would need a new feature in PowerShell for per channel disable. @JamesWTruher should be involved in that.

bergmeister · 2020-05-21T17:19:13Z

@kilasuit Please see my 2 recent review comments. I thought the change was easy to make but it turned out to be more complex than I initially thought and wasn't always working as expected, hence why it is still in draft, but thanks for the ping as I haven't looked at it for a few months.

…o disable_telemetry_option_msi

assets/Product.wxs

bergmeister · 2020-05-22T00:09:12Z

Update: I still need to fix the build failure due to the missing component registration (trivial but not on a PC atm) but apart from that I don't plan to push too many more changes.
Will post results and different scenarios of manual testing. What I think should happen at the moment is that if the checkbox is ticked (or MSI switch with value 1 is passed), then the environment variable will be set to 1. On uninstall, the environment variable should be removed. At the moment the GUID is hard coded, which means one would have to uninstall stable and preview to remove the environment variable because MSI keeps a component ref count. Maybe that's a reason why we should let WIX autogenerate a unique component guid for every installation/msi but that also mean we'd have to check/test what the behaviour is during MSI upgrade.

bergmeister · 2020-05-22T19:06:48Z

This is the final implementation and it's ready to be merged. I've tested it and scenarios are:

On uninstall, the environment variable gets removed
If the checkbox is ticked, then the env variable gets set to 1. If it is not ticked, no action is being performed, i.e. there is no action to remove/change the env variable if it is already pre-defined
Given one installs one version (e.g. stable) and disables telemetry and then installs preview but does not disable telemetry: If one then uninstalls stable, the telemetry env variable gets removed. This was surprising at first as I thought that it would only get removed once preview got uninstalled as well due to the component GUID being hard-coded but that is probably not the case because the parent directory is different. This is good behaviour and better than I expected.

TravisEz13 · 2020-05-22T20:06:01Z

cc @joeyaiello to review the language

JamesWTruher · 2020-05-27T17:24:53Z

my response to the tool-tip for telemetry is that it is over long and over wrought. The whole reboot pending sentence seems unneeded. Also, it's very engineer centric - we should be redirecting to documentation so those that want more information can get it. I think that we need to short 2 sentences: Processes may need to be restarted for the setting to take effect. See for more information.

My use of may is deliberate. We needn't talk about WM_SETTINGSCHANGE in the tool-tip.
I don't think this is ready as is.

bergmeister · 2020-05-27T17:47:08Z

Fair point @JamesWTruher about it being the message being to technical. Because not many people read the tooltip, I thought I'd be very explicit but I went a bit overboard with it but WiX/MSI at least provides line wrapping in case there is not enough horizontal space.
Is the text in parenthesis at least OK?
The problem with WiX is that it cannot provide a tooltip that can be copy-paste able or have a click-able URL because otherwise I'd have linked to this: https://wixtoolset.org/documentation/manual/v3/customactions/wixsettingchange.html
Do we still need the tooltip then? Happy to accept and apply any suggestion you have as-is.

WiX Toolset
WiX toolset
WixBroadcastSettingChange and WixBroadcastEnvironmentChange Custom Actions

TravisEz13

@bergmeister We need some time to review internally for some compliance issue(s). Sorry, the timeline on these things are unpredictable.

bergmeister · 2020-06-01T21:15:19Z

@heaths Can you please review as well from a WiX point of view. I see in your recent PR you removed some of the GUIDs but in this case, WiX would complain when removing the GUID or using an autogenerated one...

ghost · 2022-10-12T14:01:27Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

inoperable · 2022-10-12T16:55:48Z

Never ending story ... how serious is this whole debacle? I started another issue at least 3+ years ago about opt-out being ignored - and this is still the case: I can post Netlimiter prompts from pwsh.exe process to whomever is interested.
I was told back then that it has to be, quote: "some other component", despite the process trying to send "something" being pwsh.exe. How do I opt out of "some component", btw?

It would be cool if someone could explain to me what the problem is and why its taking you guys so long

inoperable · 2022-11-06T03:31:20Z

So how about it NOW?

TravisEz13 · 2023-02-13T20:52:03Z

@bergmeister We need some time to review internally for some compliance issue(s). Sorry, the timeline on these things are unpredictable.

I've pinged the PM team to get the compliance review to happen.

ghost · 2023-02-21T02:00:53Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

StevenBucher98 · 2023-03-01T20:55:46Z

Compliance has approved the wording, should be good to merge

StevenBucher98 · 2023-03-01T21:02:56Z

Note to test this with silent update

heaths

Why use two different variables if you're setting it on install and defaulting the old property based on the previous setting? The separate variable is unnecessary. Just have the search populate it. If someone passes the DISABLE_TELEMETRY property it will take precedence anyway.

This reverts commit 12efbf7.

…_option_msi

TravisEz13 · 2023-03-06T19:18:59Z

@heaths I think it was fine before looking at the tests. Some of the other properties, need the search pattern, but this one seems to persist even without the search, and I think simpler is better.

heaths · 2023-03-06T19:19:06Z

@TravisEz13 didn't you still want to read the telemetry setting into DISABLE_TELEMETRY, or was that a separate property? I see in the original PR it was something like POWERSHELL_TELEMETRY_OPTOUT.

pull-request-quantifier-deprecated · 2023-03-06T20:55:06Z

This PR has 29 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

Quantification details

Label      : Extra Small
Size       : +28 -1
Percentile : 11.6%

Total files changed: 2

Change summary by file extension:
.wxs : +7 -1
.ps1 : +21 -0

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

Fast and predictable releases to production:
- Optimal size changes are more likely to be reviewed faster with fewer
  iterations.
- Similarity in low PR complexity drives similar review times.
Review quality is likely higher as complexity is lower:
- Bugs are more likely to be detected.
- Code inconsistencies are more likely to be detected.
Knowledge sharing is improved within the participants:
- Small portions can be assimilated better.
Better engineering practices are exercised:
- Solving big problems by dividing them in well contained, smaller problems.
- Exercising separation of concerns within the code changes.

What can I do to optimize my changes

Use the PullRequestQuantifier to quantify your PR accurately
- Create a context profile for your repo using the context generator
- Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
- Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
- Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
Change your engineering behaviors
- For PRs that fall outside of the desired spectrum, review the details and check if:
  - Your PR could be split in smaller, self-contained PRs instead
  - Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

One line was added: +1 -0
One line was deleted: +0 -1
One line was modified: +1 -1 (git diff doesn't know about modified, it will
interpret that line like one addition plus one deletion)
Change percentiles: Change characteristics (addition, deletion, modification)
of this PR in relation to all other PRs within the repository.

Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

TravisEz13 · 2023-03-06T20:58:15Z

@heaths DISABLE_TELEMETRY is the property. POWERSHELL_TELEMETRY_OPTOUT is the environment variable name. The tests say the existing environment setting is kept unless someone set's DISABLE_TELEMETRY to 1. My main concern is silent upgrades, and this gives the correct behavior. Reading the setting in, would just make the logic more complex for no gain in functionality for the scenario I'm concerned about.

ImportTaste · 2023-03-07T22:57:15Z

@TravisEz13 This appears to have made the Add 'Run with PowerShell 7' context menu for PowerShell files option disappear from the installer's GUI:

inoperable · 2023-03-10T03:16:45Z

It just took 4 years to make a telemetry opt-out visible for the users - almost available 🤣 😭

ghost · 2023-03-14T22:23:47Z

🎉v7.4.0-preview.2 has been released which incorporates this pull request.:tada:

Handy links:

Release Notes

bergmeister and others added 4 commits October 7, 2019 23:07

MSI installer: Add checkbox and MSI property DISABLE_TELEMETRY to opt…

0402ad6

…ionally disable telemetry. Not ticked by default, i.e. telemetry is still on by default.

Add test cases

5ba1546

Add missing closing brace

32c8da1

Escape brackets in WiX syntax

3236adf

SteveL-MSFT assigned TravisEz13 Nov 11, 2019

SteveL-MSFT requested a review from TravisEz13 November 11, 2019 20:04

TravisEz13 added the CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log label Nov 12, 2019

TravisEz13 requested changes May 21, 2020

View reviewed changes

ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels May 21, 2020

bergmeister added 3 commits May 21, 2020 21:18

Merge branch 'master' of https://github.com/PowerShell/PowerShell int…

320e62c

…o disable_telemetry_option_msi

Try use Environment element

e635843

Merge branch 'master' of https://github.com/PowerShell/Powershell int…

65b06b8

…o disable_telemetry_option_msi

PoshChan reviewed May 21, 2020

View reviewed changes

assets/Product.wxs Outdated Show resolved Hide resolved

use unique environment id

255ca75

TravisEz13 approved these changes May 21, 2020

View reviewed changes

bergmeister and others added 2 commits May 22, 2020 10:25

Register DisableTelemetry component

730be8d

tweak text and add tooltip

98fcf23

bergmeister marked this pull request as ready for review May 22, 2020 19:06

TravisEz13 approved these changes May 22, 2020

View reviewed changes

TravisEz13 requested changes May 27, 2020

View reviewed changes

ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label May 27, 2020

This comment was marked as off-topic.

Sign in to view

This comment was marked as spam.

Sign in to view

ghost removed the Review - Needed The PR is being reviewed label Feb 13, 2023

ghost added the Review - Needed The PR is being reviewed label Feb 21, 2023

Enable the installer to remember the setting

12efbf7

heaths reviewed Mar 6, 2023

View reviewed changes

TravisEz13 added 2 commits March 6, 2023 11:11

Revert "Enable the installer to remember the setting"

3c2872a

This reverts commit 12efbf7.

Merge remote-tracking branch 'upstream/master' into disable_telemetry…

7a41339

…_option_msi

TravisEz13 approved these changes Mar 6, 2023

View reviewed changes

ghost removed the Review - Needed The PR is being reviewed label Mar 6, 2023

pull-request-quantifier-deprecated bot added the Extra Small label Mar 6, 2023

TravisEz13 merged commit 6dfba03 into PowerShell:master Mar 6, 2023

xtqqczze mentioned this pull request Mar 8, 2023

Backport installer option DISABLE_TELEMETRY to 7.2 and 7.3 #19287

Closed

StevenBucher98 mentioned this pull request Mar 14, 2023

Fixing MSI checkbox #19325

Merged

22 tasks

ghost mentioned this pull request Mar 14, 2023

Add option to turn off telemetry as part of install process #10578

Closed

bergmeister mentioned this pull request Mar 21, 2023

Update Installing-PowerShell-on-Windows.md to add DISABLE_TELEMETRY MSI property MicrosoftDocs/PowerShell-Docs#9935

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MSI installer: Add checkbox and MSI property DISABLE_TELEMETRY to optionally disable telemetry. #10725

MSI installer: Add checkbox and MSI property DISABLE_TELEMETRY to optionally disable telemetry. #10725

bergmeister commented Oct 7, 2019 •

edited

kilasuit commented May 21, 2020

TravisEz13 May 21, 2020

bergmeister May 21, 2020

bergmeister May 21, 2020

kilasuit May 21, 2020

TravisEz13 May 21, 2020

bergmeister commented May 21, 2020

bergmeister commented May 22, 2020 •

edited

bergmeister commented May 22, 2020 •

edited

TravisEz13 commented May 22, 2020

JamesWTruher commented May 27, 2020

bergmeister commented May 27, 2020 •

edited by unfurl-links bot

TravisEz13 left a comment •

edited

bergmeister commented Jun 1, 2020

ghost commented Oct 12, 2022

inoperable commented Oct 12, 2022 •

edited

This comment was marked as off-topic.

inoperable commented Nov 6, 2022

This comment was marked as spam.

TravisEz13 commented Feb 13, 2023

ghost commented Feb 21, 2023

StevenBucher98 commented Mar 1, 2023

StevenBucher98 commented Mar 1, 2023

heaths left a comment

TravisEz13 commented Mar 6, 2023

heaths commented Mar 6, 2023

pull-request-quantifier-deprecated bot commented Mar 6, 2023

What can I do to optimize my changes

How to interpret the change counts in git diff output

TravisEz13 commented Mar 6, 2023

ImportTaste commented Mar 7, 2023

inoperable commented Mar 10, 2023 via email •

edited by ghost

ghost commented Mar 14, 2023

MSI installer: Add checkbox and MSI property DISABLE_TELEMETRY to optionally disable telemetry. #10725

MSI installer: Add checkbox and MSI property DISABLE_TELEMETRY to optionally disable telemetry. #10725

Conversation

bergmeister commented Oct 7, 2019 • edited

PR Summary

PR Context

PR Checklist

kilasuit commented May 21, 2020

TravisEz13 May 21, 2020

Choose a reason for hiding this comment

bergmeister May 21, 2020

Choose a reason for hiding this comment

bergmeister May 21, 2020

Choose a reason for hiding this comment

kilasuit May 21, 2020

Choose a reason for hiding this comment

TravisEz13 May 21, 2020

Choose a reason for hiding this comment

bergmeister commented May 21, 2020

bergmeister commented May 22, 2020 • edited

bergmeister commented May 22, 2020 • edited

TravisEz13 commented May 22, 2020

JamesWTruher commented May 27, 2020

bergmeister commented May 27, 2020 • edited by unfurl-links bot

TravisEz13 left a comment • edited

Choose a reason for hiding this comment

bergmeister commented Jun 1, 2020

ghost commented Oct 12, 2022

inoperable commented Oct 12, 2022 • edited

This comment was marked as off-topic.

inoperable commented Nov 6, 2022

This comment was marked as spam.

TravisEz13 commented Feb 13, 2023

ghost commented Feb 21, 2023

StevenBucher98 commented Mar 1, 2023

StevenBucher98 commented Mar 1, 2023

heaths left a comment

Choose a reason for hiding this comment

TravisEz13 commented Mar 6, 2023

heaths commented Mar 6, 2023

pull-request-quantifier-deprecated bot commented Mar 6, 2023

What can I do to optimize my changes

How to interpret the change counts in git diff output

TravisEz13 commented Mar 6, 2023

ImportTaste commented Mar 7, 2023

inoperable commented Mar 10, 2023 via email • edited by ghost

ghost commented Mar 14, 2023

bergmeister commented Oct 7, 2019 •

edited

bergmeister commented May 22, 2020 •

edited

bergmeister commented May 22, 2020 •

edited

bergmeister commented May 27, 2020 •

edited by unfurl-links bot

TravisEz13 left a comment •

edited

inoperable commented Oct 12, 2022 •

edited

inoperable commented Mar 10, 2023 via email •

edited by ghost