8000 Remove UTC and SQM telemetry code by JamesWTruher · Pull Request #4190 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]

Skip to content

Remove UTC and SQM telemetry code #4190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By cl 8000 icking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

JamesWTruher
Copy link
Collaborator

Implementation of #3807

The historical telemetry code uses WindowsUTC which is not applicable to non-Windows platforms and is superseded by our ApplicationInsight telemetry. #if LEGACYTELEMETRY was used to make it easier to track when we add our next round of telemetry.

Change should not be visible to the user, unless they were inspecting the eventlog looking for it. Any SQM telemetry appears to have been disabled in June 2016.

@lzybkr lzybkr self-assigned this Jul 5, 2017
@@ -44,8 +46,12 @@ internal sealed partial class ConsoleHost
:
PSHost,
IDisposable,
#if LEGACYTELEMETRY
IHostSupportsInteractiveSession,
IHostProvidesTelemetryData
Copy link
Member

Choose a reason for hiding this comment

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

Can you rearrange the order to avoid the #else?

This is done via `#if LEGACYTELEMETRY` to make it easier to bring back at a later time
remove the #else so it's a little cleaner
@JamesWTruher JamesWTruher force-pushed the jameswtruher/RemoveLegacyTelemetry01 branch from ee63f52 to f91ec0b Compare July 12, 2017 22:02
@@ -44,8 +46,12 @@ internal sealed partial class ConsoleHost
:
PSHost,
IDisposable,
IHostSupportsInteractiveSession
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be:

#if LEGACYTELEMETRY
  IHostProvidesTelemetryData,
#endif
  IHostSupportsInteractiveSession

Copy link
Member
@SteveL-MSFT SteveL-MSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@daxian-dbw
Copy link
Member

The old telemetry implementation are still kept in code base (PSTelemetryMethods.cs, PSTelemetryWrapper.cs). Shall we delete them in this PR?

@SteveL-MSFT
Copy link
Member

@daxian-dbw current thinking is to keep it for now. We'll likely add more telemetry in 6.1.0 and we can see what we can reuse from the old telemetry and clean up the code completely at that time.

@daxian-dbw daxian-dbw merged commit 71392a2 into PowerShell:master Jul 17, 2017
@JamesWTruher JamesWTruher added the Breaking-Change breaking change that may affect users label Aug 3, 2017
@JamesWTruher JamesWTruher deleted the jameswtruher/RemoveLegacyTelemetry01 branch September 23, 2023 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking-Change breaking change that may affect users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0