From 8d37b213e20071fb3a24f277e5eeb241b4464e74 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Fri, 9 Sep 2022 15:57:49 +0200 Subject: [PATCH] Reduce amount of telemetry events (#13951) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit reduces the amount of telemetry during general usage by about half. 8 events that weren't really used anymore were removed. 1 new event was added ("AppInitialized") which will help us investigate #5907. During review 9 events were found that were incorrectly tagged as perf. data. * Launch Windows Terminal * The "latency" field "AppInitialized" matches the approx. launch time ✅ (cherry picked from commit 37c159abba3d3d34775e5f45a4198460bf332992) Service-Card-Id: 85548958 Service-Version: 1.15 --- src/cascadia/TerminalApp/AppLogic.cpp | 78 +++++++++---------- src/cascadia/TerminalApp/CommandPalette.cpp | 8 +- src/cascadia/TerminalApp/TabManagement.cpp | 27 ------- src/cascadia/TerminalApp/TerminalPage.cpp | 26 ++++--- .../TerminalConnection/ConptyConnection.cpp | 4 +- src/cascadia/WindowsTerminal/AppHost.cpp | 7 -- src/cascadia/WindowsTerminal/IslandWindow.cpp | 1 - src/cascadia/WindowsTerminal/main.cpp | 6 -- src/inc/WilErrorReporting.h | 15 ++-- 9 files changed, 65 insertions(+), 107 deletions(-) diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index 48300f08bf1..ccaf1ec6c0a 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -10,7 +10,7 @@ #include #include -#include +#include #include "../../types/inc/utils.hpp" @@ -296,6 +296,32 @@ namespace winrt::TerminalApp::implementation { _root->SetFullscreen(true); } + + FILETIME creationTime, exitTime, kernelTime, userTime, now; + if (GetThreadTimes(GetCurrentThread(), &creationTime, &exitTime, &kernelTime, &userTime)) + { + static constexpr auto asInteger = [](const FILETIME& f) { + ULARGE_INTEGER i; + i.LowPart = f.dwLowDateTime; + i.HighPart = f.dwHighDateTime; + return i.QuadPart; + }; + static constexpr auto asSeconds = [](uint64_t v) { + return v * 1e-7f; + }; + + GetSystemTimeAsFileTime(&now); + + const auto latency = asSeconds(asInteger(now) - asInteger(creationTime)); + + TraceLoggingWrite( + g_hTerminalAppProvider, + "AppInitialized", + TraceLoggingDescription("Event emitted once the app is initialized"), + TraceLoggingFloat32(latency, "latency"), + TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), + TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); + } }); _root->Create(); @@ -312,7 +338,7 @@ namespace winrt::TerminalApp::implementation TraceLoggingDescription("Event emitted when the application is started"), TraceLoggingBool(_settings.GlobalSettings().ShowTabsInTitlebar(), "TabsInTitlebar"), TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); + TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); } void AppLogic::Quit() @@ -504,26 +530,8 @@ namespace winrt::TerminalApp::implementation if (keyboardServiceIsDisabled) { _root->ShowKeyboardServiceWarning(); - - TraceLoggingWrite( - g_hTerminalAppProvider, - "KeyboardServiceWasDisabled", - TraceLoggingDescription("Event emitted when the keyboard service is disabled, and we warned them about it"), - TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); } } - else - { - // For when the warning was disabled in the settings - - TraceLoggingWrite( - g_hTerminalAppProvider, - "KeyboardServiceWarningWasDisabledBySetting", - TraceLoggingDescription("Event emitted when the user has disabled the KB service warning"), - TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); - } if (FAILED(_settingsLoadedResult)) { @@ -557,7 +565,7 @@ namespace winrt::TerminalApp::implementation // not be noisy with this dialog if we failed for some reason. // Open the service manager. This will return 0 if it failed. - wil::unique_schandle hManager{ OpenSCManager(nullptr, nullptr, 0) }; + wil::unique_schandle hManager{ OpenSCManagerW(nullptr, nullptr, 0) }; if (LOG_LAST_ERROR_IF(!hManager.is_valid())) { @@ -565,8 +573,12 @@ namespace winrt::TerminalApp::implementation } // Get a handle to the keyboard service - wil::unique_schandle hService{ OpenService(hManager.get(), TabletInputServiceKey.data(), SERVICE_QUERY_STATUS) }; - if (LOG_LAST_ERROR_IF(!hService.is_valid())) + wil::unique_schandle hService{ OpenServiceW(hManager.get(), TabletInputServiceKey.data(), SERVICE_QUERY_STATUS) }; + + // Windows 11 doesn't have a TabletInputService. + // (It was renamed to TextInputManagementService, because people kept thinking that a + // service called "tablet-something" is system-irrelevant on PCs and can be disabled.) + if (!hService.is_valid()) { return true; } @@ -851,15 +863,6 @@ namespace winrt::TerminalApp::implementation // happening during startup, it'll need to happen on a background thread. void AppLogic::LoadSettings() { - auto start = std::chrono::high_resolution_clock::now(); - - TraceLoggingWrite( - g_hTerminalAppProvider, - "SettingsLoadStarted", - TraceLoggingDescription("Event emitted before loading the settings"), - TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); - // Attempt to load the settings. // If it fails, // - use Default settings, @@ -875,17 +878,6 @@ namespace winrt::TerminalApp::implementation _settings = CascadiaSettings::LoadDefaults(); } - auto end = std::chrono::high_resolution_clock::now(); - std::chrono::duration delta = end - start; - - TraceLoggingWrite( - g_hTerminalAppProvider, - "SettingsLoadComplete", - TraceLoggingDescription("Event emitted when loading the settings is finished"), - TraceLoggingFloat64(delta.count(), "Duration"), - TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); - _loadedInitialSettings = true; // Register for directory change notification. diff --git a/src/cascadia/TerminalApp/CommandPalette.cpp b/src/cascadia/TerminalApp/CommandPalette.cpp index d05056b2543..c5be0ce09a4 100644 --- a/src/cascadia/TerminalApp/CommandPalette.cpp +++ b/src/cascadia/TerminalApp/CommandPalette.cpp @@ -70,7 +70,7 @@ namespace winrt::TerminalApp::implementation TraceLoggingDescription("Event emitted when the Command Palette is opened"), TraceLoggingWideString(L"Action", "Mode", "which mode the palette was opened in"), TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); + TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); } else { @@ -711,7 +711,7 @@ namespace winrt::TerminalApp::implementation TraceLoggingUInt32(searchTextLength, "SearchTextLength", "Number of characters in the search string"), TraceLoggingUInt32(nestedCommandDepth, "NestedCommandDepth", "the depth in the tree of commands for the dispatched action"), TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); + TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); } } } @@ -779,7 +779,7 @@ namespace winrt::TerminalApp::implementation "CommandPaletteDispatchedCommandline", TraceLoggingDescription("Event emitted when the user runs a commandline in the Command Palette"), TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); + TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); if (const auto commandLinePaletteItem{ filteredCommand.value().Item().try_as() }) { @@ -817,7 +817,7 @@ namespace winrt::TerminalApp::implementation "CommandPaletteDismissed", TraceLoggingDescription("Event emitted when the user dismisses the Command Palette without selecting an action"), TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); + TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); } // Method Description: diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp index 5ddcdc79cbe..414f3f519c1 100644 --- a/src/cascadia/TerminalApp/TabManagement.cpp +++ b/src/cascadia/TerminalApp/TabManagement.cpp @@ -87,33 +87,6 @@ namespace winrt::TerminalApp::implementation // This call to _MakePane won't return nullptr, we already checked that // case above with the _maybeElevate call. _CreateNewTabFromPane(_MakePane(newTerminalArgs, false, existingConnection)); - - const auto tabCount = _tabs.Size(); - const auto usedManualProfile = (newTerminalArgs != nullptr) && - (newTerminalArgs.ProfileIndex() != nullptr || - newTerminalArgs.Profile().empty()); - - // Lookup the name of the color scheme used by this profile. - const auto scheme = _settings.GetColorSchemeForProfile(profile); - // If they explicitly specified `null` as the scheme (indicating _no_ scheme), log - // that as the empty string. - const auto schemeName = scheme ? scheme.Name() : L"\0"; - - TraceLoggingWrite( - g_hTerminalAppProvider, // handle to TerminalApp tracelogging provider - "TabInformation", - TraceLoggingDescription("Event emitted upon new tab creation in TerminalApp"), - TraceLoggingUInt32(1u, "EventVer", "Version of this event"), - TraceLoggingUInt32(tabCount, "TabCount", "Count of tabs currently opened in TerminalApp"), - TraceLoggingBool(usedManualProfile, "ProfileSpecified", "Whether the new tab specified a profile explicitly"), - TraceLoggingGuid(profile.Guid(), "ProfileGuid", "The GUID of the profile spawned in the new tab"), - TraceLoggingBool(settings.DefaultSettings().UseAcrylic(), "UseAcrylic", "The acrylic preference from the settings"), - TraceLoggingFloat64(settings.DefaultSettings().Opacity(), "TintOpacity", "Opacity preference from the settings"), - TraceLoggingWideString(settings.DefaultSettings().FontFace().c_str(), "FontFace", "Font face chosen in the settings"), - TraceLoggingWideString(schemeName.data(), "SchemeName", "Color scheme set in the settings"), - TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); - return S_OK; } CATCH_RETURN(); diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 8c3dcad1113..01cda3a0df0 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -511,7 +511,7 @@ namespace winrt::TerminalApp::implementation "NewTabByDragDrop", TraceLoggingDescription("Event emitted when the user drag&drops onto the new tab button"), TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); + TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); } } @@ -1178,7 +1178,7 @@ namespace winrt::TerminalApp::implementation TraceLoggingGuid(profile.Guid(), "ProfileGuid", "The profile's GUID"), TraceLoggingGuid(sessionGuid, "SessionGuid", "The WT_SESSION's GUID"), TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); + TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); return connection; } @@ -3475,7 +3475,6 @@ namespace winrt::TerminalApp::implementation if (const auto infoBar = FindName(L"SetAsDefaultInfoBar").try_as()) { - TraceLoggingWrite(g_hTerminalAppProvider, "SetAsDefaultTipPresented", TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); infoBar.IsOpen(true); } } @@ -3500,7 +3499,7 @@ namespace winrt::TerminalApp::implementation return winrt::hstring{ TabletInputServiceKey }; } - wil::unique_schandle hManager{ OpenSCManager(nullptr, nullptr, 0) }; + wil::unique_schandle hManager{ OpenSCManagerW(nullptr, nullptr, 0) }; if (LOG_LAST_ERROR_IF(!hManager.is_valid())) { @@ -3508,15 +3507,24 @@ namespace winrt::TerminalApp::implementation } DWORD cchBuffer = 0; - GetServiceDisplayName(hManager.get(), TabletInputServiceKey.data(), nullptr, &cchBuffer); + const auto ok = GetServiceDisplayNameW(hManager.get(), TabletInputServiceKey.data(), nullptr, &cchBuffer); + + // Windows 11 doesn't have a TabletInputService. + // (It was renamed to TextInputManagementService, because people kept thinking that a + // service called "tablet-something" is system-irrelevant on PCs and can be disabled.) + if (ok || GetLastError() != ERROR_INSUFFICIENT_BUFFER) + { + return winrt::hstring{ TabletInputServiceKey }; + } + std::wstring buffer; cchBuffer += 1; // Add space for a null buffer.resize(cchBuffer); - if (LOG_LAST_ERROR_IF(!GetServiceDisplayName(hManager.get(), - TabletInputServiceKey.data(), - buffer.data(), - &cchBuffer))) + if (LOG_LAST_ERROR_IF(!GetServiceDisplayNameW(hManager.get(), + TabletInputServiceKey.data(), + buffer.data(), + &cchBuffer))) { return winrt::hstring{ TabletInputServiceKey }; } diff --git a/src/cascadia/TerminalConnection/ConptyConnection.cpp b/src/cascadia/TerminalConnection/ConptyConnection.cpp index 6fee6a797e0..a26e0a34968 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.cpp +++ b/src/cascadia/TerminalConnection/ConptyConnection.cpp @@ -192,7 +192,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation TraceLoggingGuid(_guid, "SessionGuid", "The WT_SESSION's GUID"), TraceLoggingWideString(_clientName.c_str(), "Client", "The attached client process"), TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); + TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); return S_OK; } @@ -336,7 +336,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation TraceLoggingGuid(_guid, "SessionGuid", "The WT_SESSION's GUID"), TraceLoggingWideString(_clientName.c_str(), "Client", "The attached client process"), TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); + TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); THROW_IF_FAILED(ConptyResizePseudoConsole(_hPC.get(), til::unwrap_coord_size(dimensions))); THROW_IF_FAILED(ConptyReparentPseudoConsole(_hPC.get(), reinterpret_cast(_initialParentHwnd))); diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index facd3fa1fb9..9ac3abc9ed7 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -683,13 +683,6 @@ void AppHost::_HandleCreateWindow(const HWND hwnd, til::rect proposedRect, Launc // If we can't resize the window, that's really okay. We can just go on with // the originally proposed window size. LOG_LAST_ERROR_IF(!succeeded); - - TraceLoggingWrite( - g_hWindowsTerminalProvider, - "WindowCreated", - TraceLoggingDescription("Event emitted upon creating the application window"), - TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); } // Method Description: diff --git a/src/cascadia/WindowsTerminal/IslandWindow.cpp b/src/cascadia/WindowsTerminal/IslandWindow.cpp index 3160ab2fa4e..d314f371a6a 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/IslandWindow.cpp @@ -1253,7 +1253,6 @@ bool IslandWindow::RegisterHotKey(const int index, const winrt::Microsoft::Termi // TODO GH#8888: We should display a warning of some kind if this fails. // This can fail if something else already bound this hotkey. const auto result = ::RegisterHotKey(_window.get(), index, hotkeyFlags, vkey); - LOG_IF_WIN32_BOOL_FALSE(result); TraceLoggingWrite(g_hWindowsTerminalProvider, "RegisterHotKey", diff --git a/src/cascadia/WindowsTerminal/main.cpp b/src/cascadia/WindowsTerminal/main.cpp index fe2353fbf07..a54d37faddc 100644 --- a/src/cascadia/WindowsTerminal/main.cpp +++ b/src/cascadia/WindowsTerminal/main.cpp @@ -99,12 +99,6 @@ static bool _messageIsAltSpaceKeypress(const MSG& message) int __stdcall wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int) { TraceLoggingRegister(g_hWindowsTerminalProvider); - TraceLoggingWrite( - g_hWindowsTerminalProvider, - "ExecutableStarted", - TraceLoggingDescription("Event emitted immediately on startup"), - TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance)); ::Microsoft::Console::ErrorReporting::EnableFallbackFailureReporting(g_hWindowsTerminalProvider); // If Terminal is spawned by a shortcut that requests that it run in a new process group diff --git a/src/inc/WilErrorReporting.h b/src/inc/WilErrorReporting.h index 41efd625e4d..35c31a464d3 100644 --- a/src/inc/WilErrorReporting.h +++ b/src/inc/WilErrorReporting.h @@ -33,7 +33,6 @@ namespace Microsoft::Console::ErrorReporting FallbackProvider, "FallbackError", TraceLoggingKeyword(MICROSOFT_KEYWORD_TELEMETRY), - TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage), TraceLoggingLevel(WINEVENT_LEVEL_ERROR), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingStruct(14, "wilResult", "wilResult"), @@ -44,13 +43,13 @@ namespace Microsoft::Console::ErrorReporting TraceLoggingUInt32(static_cast(failure.type), "failureType", "Indicates what type of failure was observed (exception, returned error, logged error or fail fast"), TraceLoggingWideString(failure.pszMessage, "message", "Custom message associated with the failure (if any)"), TraceLoggingUInt32(failure.threadId, "threadId", "Identifier of the thread the error occurred on"), - TraceLoggingString(failure.pszCallContext, "callContext", "List of containing this error"), - TraceLoggingUInt32(failure.callContextOriginating.contextId, "originatingContextId", "Identifier for the oldest activity containing this error"), - TraceLoggingString(failure.callContextOriginating.contextName, "originatingContextName", "Name of the oldest activity containing this error"), - TraceLoggingWideString(failure.callContextOriginating.contextMessage, "originatingContextMessage", "Custom message associated with the oldest activity containing this error (if any)"), - TraceLoggingUInt32(failure.callContextCurrent.contextId, "currentContextId", "Identifier for the newest activity containing this error"), - TraceLoggingString(failure.callContextCurrent.contextName, "currentContextName", "Name of the newest activity containing this error"), - TraceLoggingWideString(failure.callContextCurrent.contextMessage, "currentContextMessage", "Custom message associated with the newest activity containing this error (if any)")); + TraceLoggingString(failure.pszCallContext, "callContext", "List of telemetry activities containing this error"), + TraceLoggingUInt32(failure.callContextOriginating.contextId, "originatingContextId", "Identifier for the oldest telemetry activity containing this error"), + TraceLoggingString(failure.callContextOriginating.contextName, "originatingContextName", "Name of the oldest telemetry activity containing this error"), + TraceLoggingWideString(failure.callContextOriginating.contextMessage, "originatingContextMessage", "Custom message associated with the oldest telemetry activity containing this error (if any)"), + TraceLoggingUInt32(failure.callContextCurrent.contextId, "currentContextId", "Identifier for the newest telemetry activity containing this error"), + TraceLoggingString(failure.callContextCurrent.contextName, "currentContextName", "Name of the newest telemetry activity containing this error"), + TraceLoggingWideString(failure.callContextCurrent.contextMessage, "currentContextMessage", "Custom message associated with the newest telemetry activity containing this error (if any)")); } } catch (...)