diff --git a/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp b/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp index a924938544..92663e4c4e 100644 --- a/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp +++ b/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp @@ -80,8 +80,24 @@ namespace AppInstaller::Logging void TelemetryTraceLogger::Initialize() { - m_isSettingEnabled = !Settings::User().Get(); - m_userProfile = Runtime::GetPathTo(Runtime::PathName::UserProfile).wstring(); + if (!m_isInitialized) + { + InitializeInternal(Settings::User()); + } + } + + bool TelemetryTraceLogger::TryInitialize() + { + if (!m_isInitialized) + { + auto userSettings = Settings::TryGetUser(); + if (userSettings) + { + InitializeInternal(*userSettings); + } + } + + return m_isInitialized; } void TelemetryTraceLogger::SetTelemetryCorrelationJson(const std::wstring_view jsonStr_view) noexcept @@ -520,7 +536,14 @@ namespace AppInstaller::Logging bool TelemetryTraceLogger::IsTelemetryEnabled() const noexcept { - return g_IsTelemetryProviderEnabled && m_isSettingEnabled && m_isRuntimeEnabled; + return g_IsTelemetryProviderEnabled && m_isInitialized && m_isSettingEnabled && m_isRuntimeEnabled; + } + + void TelemetryTraceLogger::InitializeInternal(const AppInstaller::Settings::UserSettings& userSettings) + { + m_isSettingEnabled = !userSettings.Get(); + m_userProfile = Runtime::GetPathTo(Runtime::PathName::UserProfile).wstring(); + m_isInitialized = true; } std::wstring TelemetryTraceLogger::AnonymizeString(const wchar_t* input) const noexcept @@ -553,7 +576,8 @@ namespace AppInstaller::Logging } else { - static GlobalTelemetryTraceLogger processGlobalTelemetry; + static TelemetryTraceLogger processGlobalTelemetry; + processGlobalTelemetry.TryInitialize(); return processGlobalTelemetry; } } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h b/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h index ff96de8521..37fda40f8f 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h @@ -8,6 +8,11 @@ #include #include +namespace AppInstaller::Settings +{ + struct UserSettings; +} + namespace AppInstaller::Logging { // This type contains the registration lifetime of the telemetry trace logging provider. @@ -36,6 +41,9 @@ namespace AppInstaller::Logging // Capture if UserSettings is enabled and set user profile path void Initialize(); + // Try to capture if UserSettings is enabled and set user profile path, returns whether the action is successfully completed. + bool TryInitialize(); + // Store the passed in name of the Caller for COM calls void SetCaller(const std::string& caller); @@ -131,6 +139,8 @@ namespace AppInstaller::Logging protected: bool IsTelemetryEnabled() const noexcept; + void InitializeInternal(const AppInstaller::Settings::UserSettings& userSettings); + // Used to anonymize a string to the best of our ability. // Should primarily be used on failure messages or paths if needed. std::wstring AnonymizeString(const wchar_t* input) const noexcept; @@ -138,6 +148,7 @@ namespace AppInstaller::Logging bool m_isSettingEnabled = true; std::atomic_bool m_isRuntimeEnabled{ true }; + std::atomic_bool m_isInitialized{ false }; GUID m_activityId = GUID_NULL; std::wstring m_telemetryCorrelationJsonW = L"{}"; @@ -147,13 +158,6 @@ namespace AppInstaller::Logging std::wstring m_userProfile; }; - struct GlobalTelemetryTraceLogger : TelemetryTraceLogger - { - GlobalTelemetryTraceLogger() { Initialize(); } - - ~GlobalTelemetryTraceLogger() = default; - }; - // Helper to make the call sites look clean. TelemetryTraceLogger& Telemetry(); diff --git a/src/AppInstallerCommonCore/Public/winget/ThreadGlobals.h b/src/AppInstallerCommonCore/Public/winget/ThreadGlobals.h index 6267c39711..83864f0e71 100644 --- a/src/AppInstallerCommonCore/Public/winget/ThreadGlobals.h +++ b/src/AppInstallerCommonCore/Public/winget/ThreadGlobals.h @@ -26,13 +26,12 @@ namespace AppInstaller::ThreadLocalStorage static ThreadGlobals* GetForCurrentThread(); private: - + void Initialize(); std::unique_ptr m_pDiagnosticLogger; std::unique_ptr m_pTelemetryLogger; std::once_flag loggerInitOnceFlag; - }; struct PreviousThreadGlobals diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h index fbbbdf5f06..5c6b745cb5 100644 --- a/src/AppInstallerCommonCore/Public/winget/UserSettings.h +++ b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -188,8 +188,6 @@ namespace AppInstaller::Settings ~UserSettings() = default; }; - inline UserSettings const& User() - { - return UserSettings::Instance(); - } + const UserSettings* TryGetUser(); + UserSettings const& User(); } diff --git a/src/AppInstallerCommonCore/ThreadGlobals.cpp b/src/AppInstallerCommonCore/ThreadGlobals.cpp index a93822beb4..2853ee50df 100644 --- a/src/AppInstallerCommonCore/ThreadGlobals.cpp +++ b/src/AppInstallerCommonCore/ThreadGlobals.cpp @@ -35,7 +35,7 @@ namespace AppInstaller::ThreadLocalStorage { m_pDiagnosticLogger = std::make_unique(); m_pTelemetryLogger = std::make_unique(); - + // The above make_unique for TelemetryTraceLogger will either create an object or will throw which is caught below. m_pTelemetryLogger->Initialize(); }); diff --git a/src/AppInstallerCommonCore/TraceLogger.cpp b/src/AppInstallerCommonCore/TraceLogger.cpp index a7bb4efce8..f77f7b9821 100644 --- a/src/AppInstallerCommonCore/TraceLogger.cpp +++ b/src/AppInstallerCommonCore/TraceLogger.cpp @@ -15,7 +15,7 @@ namespace AppInstaller::Logging TraceLoggingWriteActivity(g_hTraceProvider, "Diagnostics", - nullptr, // TODO: ActivityId of the Global and COMContext telemetry to be logged in future + Telemetry().GetActivityId(), nullptr, TraceLoggingString(strstr.str().c_str(), "LogMessage")); } @@ -28,7 +28,7 @@ namespace AppInstaller::Logging { TraceLoggingWriteActivity(g_hTraceProvider, "Diagnostics", - nullptr, // TODO: ActivityId of the Global and COMContext telemetry to be logged in future + Telemetry().GetActivityId(), nullptr, TraceLoggingCountedUtf8String(message.data(), static_cast(message.size()), "LogMessage")); } diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index 7f9a3dc2f5..22ac71f297 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -305,20 +305,50 @@ namespace AppInstaller::Settings } #endif + static std::atomic_bool s_userSettingsInitialized{ false }; + static std::atomic_bool s_userSettingsInInitialization{ false }; + UserSettings const& UserSettings::Instance() { - static UserSettings userSettings; - #ifndef AICLI_DISABLE_TEST_HOOKS if (s_UserSettings_Override) { return *s_UserSettings_Override; } #endif + if (!s_userSettingsInitialized) + { + s_userSettingsInInitialization = true; + } + + static UserSettings userSettings; + s_userSettingsInitialized = true; + s_userSettingsInInitialization = false; return userSettings; } + const UserSettings* TryGetUser() + { + if (s_userSettingsInitialized) + { + return &UserSettings::Instance(); + } + + // Try to initialize UserSettings, return nullptr if it's already in initialization. + if (s_userSettingsInInitialization) + { + return nullptr; + } + + return &UserSettings::Instance(); + } + + UserSettings const& User() + { + return UserSettings::Instance(); + } + UserSettings::UserSettings() : m_type(UserSettingsType::Default) { Json::Value settingsRoot = Json::Value::nullSingleton(); diff --git a/src/WinGetUtil/Exports.cpp b/src/WinGetUtil/Exports.cpp index 54ad45d305..c943b66464 100644 --- a/src/WinGetUtil/Exports.cpp +++ b/src/WinGetUtil/Exports.cpp @@ -10,6 +10,7 @@ #include #include #include +#include using namespace AppInstaller::Utility; using namespace AppInstaller::Manifest; @@ -21,8 +22,13 @@ extern "C" { THROW_HR_IF(E_INVALIDARG, !logPath); - static std::once_flag s_initLogging; - std::call_once(s_initLogging, []() { + thread_local AppInstaller::ThreadLocalStorage::ThreadGlobals threadGlobals; + thread_local std::once_flag initLogging; + + std::call_once(initLogging, []() { + std::unique_ptr previous = threadGlobals.SetForCurrentThread(); + // Intentionally release to leave the local ThreadGlobals. + previous.release(); // Enable all logs for now. AppInstaller::Logging::Log().EnableChannel(AppInstaller::Logging::Channel::All); AppInstaller::Logging::Log().SetLevel(AppInstaller::Logging::Level::Verbose);