Prevent circular dependency in global telemetry logger creation (#1674)

src/AppInstallerCommonCore/AppInstallerTelemetry.cpp

@@ -80,8 +80,24 @@ namespace AppInstaller::Logging
 
     void TelemetryTraceLogger::Initialize()
     {
-        m_isSettingEnabled = !Settings::User().Get<Settings::Setting::TelemetryDisable>();
-        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<Settings::Setting::TelemetryDisable>();
+        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;
         }
     }

src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h

@@ -8,6 +8,11 @@
 #include <vector>
 #include <cguid.h>
 
+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,13 +139,16 @@ 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;
         std::wstring AnonymizeString(std::wstring_view input) const noexcept;
 
         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();
 

src/AppInstallerCommonCore/Public/winget/ThreadGlobals.h

@@ -26,13 +26,12 @@ namespace AppInstaller::ThreadLocalStorage
         static ThreadGlobals* GetForCurrentThread();
 
     private:
-        
+
         void Initialize();
 
         std::unique_ptr<AppInstaller::Logging::DiagnosticLogger> m_pDiagnosticLogger;
         std::unique_ptr<AppInstaller::Logging::TelemetryTraceLogger> m_pTelemetryLogger;
         std::once_flag loggerInitOnceFlag;
-
     };
 
     struct PreviousThreadGlobals

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();
 }

src/AppInstallerCommonCore/ThreadGlobals.cpp

@@ -35,7 +35,7 @@ namespace AppInstaller::ThreadLocalStorage
                 {
                     m_pDiagnosticLogger = std::make_unique<DiagnosticLogger>();
                     m_pTelemetryLogger = std::make_unique<TelemetryTraceLogger>();
-                    
+
                     // The above make_unique for TelemetryTraceLogger will either create an object or will throw which is caught below.
                     m_pTelemetryLogger->Initialize();
                 });

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<ULONG>(message.size()), "LogMessage"));
     }

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();

src/WinGetUtil/Exports.cpp

@@ -10,6 +10,7 @@
 #include <AppInstallerTelemetry.h>
 #include <Microsoft/SQLiteIndex.h>
 #include <winget/ManifestYamlParser.h>
+#include <winget/ThreadGlobals.h>
 
 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<AppInstaller::ThreadLocalStorage::PreviousThreadGlobals> 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);