Skip to content

Conversation

@JamesWTruher
Copy link
Collaborator

this is the implementation of https://github.com/PowerShell/PowerShell-RFC/blob/249e8d88eb779a6003fd138609ae94417ae13698/2-Draft-Accepted/RFC0036-AdditionalTelemetry.md

PR Summary

  • Add Telemetry in the following areas
    • When execution occurs, the ApplicationType will be captured as a metric
    • When a module is loaded, capture a metric of the module name if it's in the allow list.
    • When PowerShell.Create is used, send a metric
    • When an experimental feature is activated send a metric (this is at startup only)
    • Update startup telemetry to measure the type of shell startup (remote/ssh)
  • Added a couple of new fields to the StartUp telemetry
    • Channel allows us to understand when PS is being used in a docker instance and what the OS is.
    • Added a unique identifier which allows us to determine unique users. It's a generated guid which is persisted to disk.
    • Added a unique identifier for the shell session

I added an internal member to System.Management.Automation.Platform called CacheDirectory which makes it easy to find the location of the cache directory. I think this should probably be public, but that can be decided at a later time.

PR Context

We knew that our initial telemetry was just that. The bare minimum of information to determine whether the shell was actually being used and on what platform. The additional information we think we need is captured in the RFC.

PR Checklist

@iSazonov
Copy link
Collaborator

iSazonov commented Aug 9, 2019

Update startup telemetry to measure the type of shell startup (remote/ssh)

We will have -Login in days. That might be helpful too.

Copy link
Member

@TravisEz13 TravisEz13 left a comment

Choose a reason for hiding this comment

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

haven't reviewed everything

@ghost 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 Aug 9, 2019
@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Aug 12, 2019
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Aug 13, 2019
the entire assembly should be removed
Use Should -Exist rather than test-path
Fix test when environment variable is not set
fix case of '-Command'
address other codefactor issue
fix one more test to exclude profile
We put it in the PowerShell.Create code rather than in this location
Copy link
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

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

LGTM with one minor comment.

@iSazonov
Copy link
Collaborator

#10050 was merged and now it is possible to add telemetry in the code too.

Co-Authored-By: Ilya <darpa@yandex.ru>
@adityapatwardhan
Copy link
Member

@JamesWTruher All looks good and ready to merge. As @iSazonov said above, are the plans to add telemetry to pwsh -login?

@JamesWTruher
Copy link
Collaborator Author

@iSazonov @adityapatwardhan We can add additional telemetry for login to a subsequent PR if we decide it is worth knowing. I don't believe that we must do that now, though.

I don't need to keep the static strings of allowed module names or do convoluted checking of the experimental feature name.
Move the initialization of the HashSet with list of module names into the static constructor.
Ifdef a couple stopwatches out of the code (keeping the location tagged if we want to bring that back).
Add a comment about POWERSHELL_DISTRIBUTION_CHANNEL.
Change the type of the sessionid and uniqueid into strings so I don't repeatedly call ToString.
Optimize hashset creation to not create a new string array.
@adityapatwardhan adityapatwardhan self-assigned this Aug 15, 2019
@adityapatwardhan adityapatwardhan added the CL-Engine Indicates that a PR should be marked as an engine change in the Change Log label Aug 15, 2019
@adityapatwardhan adityapatwardhan merged commit fe2cc6a into PowerShell:master Aug 15, 2019
@TravisEz13 TravisEz13 added this to the 7.0.0-preview.3 milestone Aug 16, 2019
@ghost
Copy link

ghost commented Aug 20, 2019

🎉v7.0.0-preview.3 has been released which incorporates this pull request.:tada:

Handy links:

@JamesWTruher JamesWTruher deleted the telemetry20 branch September 23, 2023 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-Engine Indicates that a PR should be marked as an engine change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants