From 572f3959c61196c3d18c813724326e45794b83ba Mon Sep 17 00:00:00 2001 From: Yao Sun Date: Fri, 12 Nov 2021 21:29:49 -0800 Subject: [PATCH 1/9] snap1 --- src/AppInstallerCLICore/COMContext.cpp | 4 +- src/AppInstallerCLICore/COMContext.h | 2 +- .../Commands/CompleteCommand.cpp | 2 +- src/AppInstallerCLICore/ExecutionContext.cpp | 10 +-- src/AppInstallerCLICore/ExecutionContext.h | 12 +-- .../ExecutionContextData.h | 8 +- .../Workflows/DependenciesFlow.cpp | 3 +- .../Workflows/DependencyNodeProcessor.cpp | 4 +- .../Workflows/DependencyNodeProcessor.h | 46 ++++++------ .../Workflows/ImportExportFlow.cpp | 12 +-- .../Workflows/InstallFlow.cpp | 17 ++--- .../Workflows/UpdateFlow.cpp | 4 +- .../AppInstallerLogging.cpp | 2 +- .../AppInstallerTelemetry.cpp | 74 +++++++------------ .../Public/AppInstallerTelemetry.h | 43 ++++------- .../Public/winget/ThreadGlobals.h | 16 +++- src/AppInstallerCommonCore/ThreadGlobals.cpp | 24 +++++- src/AppInstallerCommonCore/TraceLogger.cpp | 4 +- 18 files changed, 137 insertions(+), 150 deletions(-) diff --git a/src/AppInstallerCLICore/COMContext.cpp b/src/AppInstallerCLICore/COMContext.cpp index 42f35b2b7d..d767710d40 100644 --- a/src/AppInstallerCLICore/COMContext.cpp +++ b/src/AppInstallerCLICore/COMContext.cpp @@ -44,11 +44,11 @@ namespace AppInstaller::CLI::Execution FireCallbacks(ReportType::EndProgress, 0, 0, ProgressType::None, m_executionStage); }; - void COMContext::SetExecutionStage(CLI::Workflow::ExecutionStage executionStage, bool) + void COMContext::SetExecutionStage(CLI::Workflow::ExecutionStage executionStage) { m_executionStage = executionStage; FireCallbacks(ReportType::ExecutionPhaseUpdate, 0, 0, ProgressType::None, m_executionStage); - Logging::SetExecutionStage(static_cast(m_executionStage)); + Logging::Telemetry().SetExecutionStage(static_cast(m_executionStage)); } void COMContext::SetContextLoggers(const std::wstring_view telemetryCorrelationJson, const std::string& caller) diff --git a/src/AppInstallerCLICore/COMContext.h b/src/AppInstallerCLICore/COMContext.h index 75dbf53a62..0508e9981c 100644 --- a/src/AppInstallerCLICore/COMContext.h +++ b/src/AppInstallerCLICore/COMContext.h @@ -56,7 +56,7 @@ namespace AppInstaller::CLI::Execution void EndProgress(bool) override; //Execution::Context - void SetExecutionStage(CLI::Workflow::ExecutionStage executionPhase, bool); + void SetExecutionStage(CLI::Workflow::ExecutionStage executionPhase); CLI::Workflow::ExecutionStage GetExecutionStage() const { return m_executionStage; } diff --git a/src/AppInstallerCLICore/Commands/CompleteCommand.cpp b/src/AppInstallerCLICore/Commands/CompleteCommand.cpp index e762367c3e..700a9d75fe 100644 --- a/src/AppInstallerCLICore/Commands/CompleteCommand.cpp +++ b/src/AppInstallerCLICore/Commands/CompleteCommand.cpp @@ -53,7 +53,7 @@ namespace AppInstaller::CLI } // Create a new Context to execute the Complete from - auto subContextPtr = context.Clone(); + auto subContextPtr = context.CreateSubContext(); Context& subContext = *subContextPtr; subContext.Reporter.SetChannel(Execution::Reporter::Channel::Completion); subContext.Add(std::move(data)); diff --git a/src/AppInstallerCLICore/ExecutionContext.cpp b/src/AppInstallerCLICore/ExecutionContext.cpp index 6d03e5b6f3..65e197f5b3 100644 --- a/src/AppInstallerCLICore/ExecutionContext.cpp +++ b/src/AppInstallerCLICore/ExecutionContext.cpp @@ -118,9 +118,9 @@ namespace AppInstaller::CLI::Execution } } - std::unique_ptr Context::Clone() + std::unique_ptr Context::CreateSubContext() { - auto clone = std::make_unique(Reporter); + auto clone = std::make_unique(Reporter, m_threadGlobals); clone->m_flags = m_flags; // If the parent is hooked up to the CTRL signal, have the clone be as well if (m_disableCtrlHandlerOnExit) @@ -192,19 +192,19 @@ namespace AppInstaller::CLI::Execution Reporter.CancelInProgressTask(bypassUser); } - void Context::SetExecutionStage(Workflow::ExecutionStage stage, bool allowBackward) + void Context::SetExecutionStage(Workflow::ExecutionStage stage) { if (m_executionStage == stage) { return; } - else if (m_executionStage > stage && !allowBackward) + else if (m_executionStage > stage) { THROW_HR_MSG(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), "Reporting ExecutionStage to an earlier Stage without allowBackward as true"); } m_executionStage = stage; - Logging::SetExecutionStage(static_cast(m_executionStage)); + Logging::Telemetry().SetExecutionStage(static_cast(m_executionStage)); } AppInstaller::ThreadLocalStorage::ThreadGlobals& Context::GetThreadGlobals() diff --git a/src/AppInstallerCLICore/ExecutionContext.h b/src/AppInstallerCLICore/ExecutionContext.h index 24482799c7..53d30f1d8a 100644 --- a/src/AppInstallerCLICore/ExecutionContext.h +++ b/src/AppInstallerCLICore/ExecutionContext.h @@ -70,8 +70,10 @@ namespace AppInstaller::CLI::Execution { Context(std::ostream& out, std::istream& in) : Reporter(out, in) {} - // Clone the reporter for this constructor. - Context(Execution::Reporter& reporter) : Reporter(reporter, Execution::Reporter::clone_t{}) {} + // Constructor for creating a sub-context. + Context(Execution::Reporter& reporter, const ThreadLocalStorage::ThreadGlobals& threadGlobals) : + Reporter(reporter, Execution::Reporter::clone_t{}), + m_threadGlobals(threadGlobals, ThreadLocalStorage::ThreadGlobals::create_sub_thread_globals_t{}) {} virtual ~Context(); @@ -81,8 +83,8 @@ namespace AppInstaller::CLI::Execution // The arguments given to execute with. Args Args; - // Creates a copy of this context as it was at construction. - virtual std::unique_ptr Clone(); + // Creates a child of this context. + virtual std::unique_ptr CreateSubContext(); // Enables reception of CTRL signals. void EnableCtrlHandler(bool enabled = true); @@ -125,7 +127,7 @@ namespace AppInstaller::CLI::Execution WI_ClearAllFlags(m_flags, flags); } - virtual void SetExecutionStage(Workflow::ExecutionStage stage, bool); + virtual void SetExecutionStage(Workflow::ExecutionStage stage); // Get Globals for Current Thread AppInstaller::ThreadLocalStorage::ThreadGlobals& GetThreadGlobals(); diff --git a/src/AppInstallerCLICore/ExecutionContextData.h b/src/AppInstallerCLICore/ExecutionContextData.h index 0c06bc9024..8ba149cda0 100644 --- a/src/AppInstallerCLICore/ExecutionContextData.h +++ b/src/AppInstallerCLICore/ExecutionContextData.h @@ -53,10 +53,12 @@ namespace AppInstaller::CLI::Execution Max }; + struct Context; + // Contains all the information needed to install a package. // This is used when installing multiple packages to pass all the // data to a sub-context. - struct PackageToInstall + /*struct PackageToInstall { PackageToInstall( std::shared_ptr&& packageVersion, @@ -82,7 +84,7 @@ namespace AppInstaller::CLI::Execution // Use this sub execution id when installing this package so that // install telemetry is captured with the same sub execution id as other events in Search phase. uint32_t PackageSubExecutionId = 0; - }; + };*/ namespace details { @@ -203,7 +205,7 @@ namespace AppInstaller::CLI::Execution template <> struct DataMapping { - using value_t = std::vector; + using value_t = std::vector>; }; template <> diff --git a/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp b/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp index 6a269be806..e05fd93292 100644 --- a/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp @@ -146,13 +146,12 @@ namespace AppInstaller::CLI::Workflow AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INTERNAL_ERROR); } - std::map idToPackageMap; bool foundError = false; DependencyGraph dependencyGraph(rootAsDependency, rootDependencies, [&](Dependency node) { DependencyNodeProcessor nodeProcessor(context); - + auto result = nodeProcessor.EvaluateDependencies(node); DependencyList list = nodeProcessor.GetDependencyList(); foundError = foundError || (result == DependencyNodeProcessorResult::Error); diff --git a/src/AppInstallerCLICore/Workflows/DependencyNodeProcessor.cpp b/src/AppInstallerCLICore/Workflows/DependencyNodeProcessor.cpp index 271e33cf13..6592c9efc3 100644 --- a/src/AppInstallerCLICore/Workflows/DependencyNodeProcessor.cpp +++ b/src/AppInstallerCLICore/Workflows/DependencyNodeProcessor.cpp @@ -9,8 +9,8 @@ using namespace AppInstaller::Repository; namespace AppInstaller::CLI::Workflow { - DependencyNodeProcessor::DependencyNodeProcessor(Execution::Context& context) - : m_context(context) {} + DependencyNodeProcessor::DependencyNodeProcessor(Execution::Context& context) + : m_context(context) {} DependencyNodeProcessorResult DependencyNodeProcessor::EvaluateDependencies(Dependency& dependencyNode) { diff --git a/src/AppInstallerCLICore/Workflows/DependencyNodeProcessor.h b/src/AppInstallerCLICore/Workflows/DependencyNodeProcessor.h index cfbebbeb54..099bce7472 100644 --- a/src/AppInstallerCLICore/Workflows/DependencyNodeProcessor.h +++ b/src/AppInstallerCLICore/Workflows/DependencyNodeProcessor.h @@ -11,35 +11,35 @@ using namespace AppInstaller::Repository; namespace AppInstaller::CLI::Workflow { - enum DependencyNodeProcessorResult - { - Error, - Success, - Skipped, - }; + enum DependencyNodeProcessorResult + { + Error, + Success, + Skipped, + }; - struct DependencyNodeProcessor - { - DependencyNodeProcessor(Execution::Context& context); + struct DependencyNodeProcessor + { + DependencyNodeProcessor(Execution::Context& context); - DependencyNodeProcessorResult EvaluateDependencies(Dependency& dependencyNode); + DependencyNodeProcessorResult EvaluateDependencies(Dependency& dependencyNode); - DependencyList GetDependencyList() { return m_dependenciesList; } + DependencyList GetDependencyList() { return m_dependenciesList; } - std::shared_ptr GetPackageLatestVersion() { return m_nodePackageLatestVersion; } + std::shared_ptr GetPackageLatestVersion() { return m_nodePackageLatestVersion; } - std::shared_ptr GetPackageInstalledVersion() { return m_nodePackageInstalledVersion; } + std::shared_ptr GetPackageInstalledVersion() { return m_nodePackageInstalledVersion; } - Manifest::Manifest GetManifest() { return m_nodeManifest; } + Manifest::Manifest GetManifest() { return m_nodeManifest; } - Manifest::ManifestInstaller GetPreferredInstaller() { return m_installer; } + Manifest::ManifestInstaller GetPreferredInstaller() { return m_installer; } - private: - Execution::Context& m_context; - DependencyList m_dependenciesList; - std::shared_ptr m_nodePackageLatestVersion; - std::shared_ptr m_nodePackageInstalledVersion; - Manifest::ManifestInstaller m_installer; - Manifest::Manifest m_nodeManifest; - }; + private: + Execution::Context& m_context; + DependencyList m_dependenciesList; + std::shared_ptr m_nodePackageLatestVersion; + std::shared_ptr m_nodePackageInstalledVersion; + Manifest::ManifestInstaller m_installer; + Manifest::Manifest m_nodeManifest; + }; } \ No newline at end of file diff --git a/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp b/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp index b3c58852bb..b85b24470b 100644 --- a/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp @@ -246,7 +246,7 @@ namespace AppInstaller::CLI::Workflow void SearchPackagesForImport(Execution::Context& context) { const auto& sources = context.Get(); - std::vector packagesToInstall = {}; + std::vector> packagesToInstall; bool foundAll = true; // Look for the packages needed from each source independently. @@ -273,7 +273,7 @@ namespace AppInstaller::CLI::Workflow SearchRequest searchRequest; searchRequest.Filters.emplace_back(PackageMatchFilter(PackageMatchField::Id, MatchType::CaseInsensitive, packageRequest.Id.get())); - auto searchContextPtr = context.Clone(); + auto searchContextPtr = context.CreateSubContext(); Execution::Context& searchContext = *searchContextPtr; searchContext.Add(source); searchContext.Add(source.Search(searchRequest)); @@ -320,13 +320,7 @@ namespace AppInstaller::CLI::Workflow } } - packagesToInstall.emplace_back( - std::move(searchContext.Get()), - std::move(searchContext.Get()), - std::move(searchContext.Get()), - std::move(searchContext.Get().value()), - packageRequest.Scope, - subExecution.GetCurrentSubExecutionId()); + packagesToInstall.emplace_back(std::move(searchContextPtr)); } } diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index 8d3e1448b9..140fe76957 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -189,23 +189,18 @@ namespace AppInstaller::CLI::Workflow void EnsurePackageAgreementsAcceptanceForMultipleInstallers(Execution::Context& context) { bool hasPackageAgreements = false; - for (auto package : context.Get()) + for (auto& packageContext : context.Get()) { - // Show agreements for each package in a sub-context - auto showContextPtr = context.Clone(); - Execution::Context& showContext = *showContextPtr; - - showContext.Add(package.Manifest); - - showContext << + // Show agreements for each package in the sub-context + *packageContext << Workflow::ReportManifestIdentityWithVersion << Workflow::ShowPackageAgreements(/* ensureAcceptance */ false); - if (showContext.IsTerminated()) + if (packageContext->IsTerminated()) { - AICLI_TERMINATE_CONTEXT(showContext.GetTerminationHR()); + AICLI_TERMINATE_CONTEXT(packageContext->GetTerminationHR()); } - hasPackageAgreements |= !package.Manifest.CurrentLocalization.Get().empty(); + hasPackageAgreements |= !packageContext->Get().CurrentLocalization.Get().empty(); } // If any package has agreements, ensure they are accepted diff --git a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp index f1b18fe5da..a2c1fb43df 100644 --- a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp @@ -118,7 +118,7 @@ namespace AppInstaller::CLI::Workflow void UpdateAllApplicable(Execution::Context& context) { const auto& matches = context.Get().Matches; - std::vector packagesToInstall; + std::vector> packagesToInstall; bool updateAllFoundUpdate = false; for (const auto& match : matches) @@ -126,7 +126,7 @@ namespace AppInstaller::CLI::Workflow Logging::SubExecutionTelemetryScope subExecution; // We want to do best effort to update all applicable updates regardless on previous update failure - auto updateContextPtr = context.Clone(); + auto updateContextPtr = context.CreateSubContext(); Execution::Context& updateContext = *updateContextPtr; updateContext.Add(match.Package); diff --git a/src/AppInstallerCommonCore/AppInstallerLogging.cpp b/src/AppInstallerCommonCore/AppInstallerLogging.cpp index b19e243cbc..d3da54d03d 100644 --- a/src/AppInstallerCommonCore/AppInstallerLogging.cpp +++ b/src/AppInstallerCommonCore/AppInstallerLogging.cpp @@ -141,7 +141,7 @@ namespace AppInstaller::Logging DiagnosticLogger& Log() { ThreadLocalStorage::ThreadGlobals* pThreadGlobals = ThreadLocalStorage::ThreadGlobals::GetForCurrentThread(); - if (pThreadGlobals) + if (pThreadGlobals && pThreadGlobals->ContainsDiagnosticLogger()) { return pThreadGlobals->GetDiagnosticLogger(); } diff --git a/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp b/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp index 92663e4c4e..c93e43e2b3 100644 --- a/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp +++ b/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp @@ -44,11 +44,11 @@ namespace AppInstaller::Logging namespace { + // TODO: This and all usages should be removed after transition to summary event in back end. static const uint32_t s_RootExecutionId = 0; + static std::atomic_uint32_t s_subExecutionId{ s_RootExecutionId }; - std::atomic_uint32_t s_executionStage{ 0 }; - - std::atomic_uint32_t s_subExecutionId{ s_RootExecutionId }; + static std::atomic_bool s_isRuntimeEnabled{ true }; constexpr std::wstring_view s_UserProfileReplacement = L"%USERPROFILE%"sv; @@ -68,14 +68,9 @@ namespace AppInstaller::Logging return &m_activityId; } - bool TelemetryTraceLogger::DisableRuntime() - { - return m_isRuntimeEnabled.exchange(false); - } - - void TelemetryTraceLogger::EnableRuntime() + const GUID* TelemetryTraceLogger::GetParentActivityId() const { - m_isRuntimeEnabled = true; + return &m_parentActivityId; } void TelemetryTraceLogger::Initialize() @@ -132,6 +127,20 @@ namespace AppInstaller::Logging m_caller = caller; } + void TelemetryTraceLogger::SetExecutionStage(uint32_t stage) noexcept + { + m_executionStage = stage; + } + + std::unique_ptr TelemetryTraceLogger::CreateSubTraceLogger() const + { + auto subTraceLogger = std::make_unique(*this); + subTraceLogger->m_parentActivityId = this->m_activityId; + std::ignore = CoCreateGuid(&subTraceLogger->m_activityId); + subTraceLogger->m_subExecutionId = s_subExecutionId++; + return subTraceLogger; + } + void TelemetryTraceLogger::LogFailure(const wil::FailureInfo& failure) const noexcept { if (IsTelemetryEnabled()) @@ -148,7 +157,7 @@ namespace AppInstaller::Logging TraceLoggingUInt32(static_cast(failure.type), "Type"), TraceLoggingString(failure.pszFile, "File"), TraceLoggingUInt32(failure.uLineNumber, "Line"), - TraceLoggingUInt32(s_executionStage, "ExecutionStage"), + TraceLoggingUInt32(m_executionStage, "ExecutionStage"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); } @@ -229,7 +238,7 @@ namespace AppInstaller::Logging TraceLoggingHResult(hr, "HResult"), AICLI_TraceLoggingStringView(file, "File"), TraceLoggingUInt64(static_cast(line), "Line"), - TraceLoggingUInt32(s_executionStage, "ExecutionStage"), + TraceLoggingUInt32(m_executionStage, "ExecutionStage"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); } @@ -248,7 +257,7 @@ namespace AppInstaller::Logging TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), AICLI_TraceLoggingStringView(type, "Type"), AICLI_TraceLoggingWStringView(anonMessage, "Message"), - TraceLoggingUInt32(s_executionStage, "ExecutionStage"), + TraceLoggingUInt32(m_executionStage, "ExecutionStage"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); } @@ -536,7 +545,7 @@ namespace AppInstaller::Logging bool TelemetryTraceLogger::IsTelemetryEnabled() const noexcept { - return g_IsTelemetryProviderEnabled && m_isInitialized && m_isSettingEnabled && m_isRuntimeEnabled; + return g_IsTelemetryProviderEnabled && m_isInitialized && m_isSettingEnabled && s_isRuntimeEnabled; } void TelemetryTraceLogger::InitializeInternal(const AppInstaller::Settings::UserSettings& userSettings) @@ -570,7 +579,7 @@ namespace AppInstaller::Logging } #endif ThreadLocalStorage::ThreadGlobals* pThreadGlobals = ThreadLocalStorage::ThreadGlobals::GetForCurrentThread(); - if (pThreadGlobals) + if (pThreadGlobals && pThreadGlobals->ContainsTelemetryLogger()) { return pThreadGlobals->GetTelemetryLogger(); } @@ -589,48 +598,17 @@ namespace AppInstaller::Logging DisableTelemetryScope::DisableTelemetryScope() { - m_token = Telemetry().DisableRuntime(); + m_token = s_isRuntimeEnabled.exchange(false); } DisableTelemetryScope::~DisableTelemetryScope() { if (m_token) { - Telemetry().EnableRuntime(); + s_isRuntimeEnabled = true; } } - void SetExecutionStage(uint32_t stage) - { - s_executionStage = stage; - } - - std::atomic_uint32_t SubExecutionTelemetryScope::m_sessionId{ s_RootExecutionId }; - - SubExecutionTelemetryScope::SubExecutionTelemetryScope() - { - auto expected = s_RootExecutionId; - THROW_HR_IF_MSG(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !s_subExecutionId.compare_exchange_strong(expected, ++m_sessionId), - "Cannot create a sub execution telemetry session when a previous session exists."); - } - - SubExecutionTelemetryScope::SubExecutionTelemetryScope(uint32_t sessionId) - { - auto expected = s_RootExecutionId; - THROW_HR_IF_MSG(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !s_subExecutionId.compare_exchange_strong(expected, sessionId), - "Cannot create a sub execution telemetry session when a previous session exists."); - } - - uint32_t SubExecutionTelemetryScope::GetCurrentSubExecutionId() const - { - return (uint32_t)s_subExecutionId; - } - - SubExecutionTelemetryScope::~SubExecutionTelemetryScope() - { - s_subExecutionId = s_RootExecutionId; - } - #ifndef AICLI_DISABLE_TEST_HOOKS // Replace this test hook with context telemetry when it gets moved over void TestHook_SetTelemetryOverride(std::shared_ptr ttl) diff --git a/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h b/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h index 37fda40f8f..711b55d34e 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h @@ -31,13 +31,12 @@ namespace AppInstaller::Logging TelemetryTraceLogger(TelemetryTraceLogger&&) = default; TelemetryTraceLogger& operator=(TelemetryTraceLogger&&) = default; - // Control whether this trace logger is enabled at runtime. - bool DisableRuntime(); - void EnableRuntime(); - // Return address of m_activityId const GUID* GetActivityId() const; + // Return address of m_parentActivityId + const GUID* GetParentActivityId() const; + // Capture if UserSettings is enabled and set user profile path void Initialize(); @@ -50,6 +49,10 @@ namespace AppInstaller::Logging // Store the passed in Telemetry Correlation Json for COM calls void SetTelemetryCorrelationJson(const std::wstring_view jsonStr_view) noexcept; + void SetExecutionStage(uint32_t stage) noexcept; + + std::unique_ptr CreateSubTraceLogger() const; + // Logs the failure info. void LogFailure(const wil::FailureInfo& failure) const noexcept; @@ -147,15 +150,20 @@ namespace AppInstaller::Logging 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 }; + std::atomic_uint32_t m_executionStage{ 0 }; + GUID m_activityId = GUID_NULL; + GUID m_parentActivityId = GUID_NULL; std::wstring m_telemetryCorrelationJsonW = L"{}"; std::string m_caller; // Data that is needed by AnonymizeString std::wstring m_userProfile; + + // TODO: This and all related code could be removed after transition to summary event in back end. + uint32_t m_subExecutionId; }; // Helper to make the call sites look clean. @@ -181,29 +189,4 @@ namespace AppInstaller::Logging private: DestructionToken m_token; }; - - // Sets an execution stage to be reported when failures occur. - void SetExecutionStage(uint32_t stage); - - // An RAII object to log telemetry as sub execution. - // Does not support nested sub execution. - struct SubExecutionTelemetryScope - { - SubExecutionTelemetryScope(); - - SubExecutionTelemetryScope(uint32_t sessionId); - - SubExecutionTelemetryScope(const SubExecutionTelemetryScope&) = delete; - SubExecutionTelemetryScope& operator=(const SubExecutionTelemetryScope&) = delete; - - SubExecutionTelemetryScope(SubExecutionTelemetryScope&&) = default; - SubExecutionTelemetryScope& operator=(SubExecutionTelemetryScope&&) = default; - - uint32_t GetCurrentSubExecutionId() const; - - ~SubExecutionTelemetryScope(); - - private: - static std::atomic_uint32_t m_sessionId; - }; } diff --git a/src/AppInstallerCommonCore/Public/winget/ThreadGlobals.h b/src/AppInstallerCommonCore/Public/winget/ThreadGlobals.h index 83864f0e71..0c463c7a5f 100644 --- a/src/AppInstallerCommonCore/Public/winget/ThreadGlobals.h +++ b/src/AppInstallerCommonCore/Public/winget/ThreadGlobals.h @@ -14,8 +14,20 @@ namespace AppInstaller::ThreadLocalStorage ThreadGlobals() = default; ~ThreadGlobals() = default; + // Constructor to create a ThreadGlobals with given diagnostic and telemetry loggers. + // During SetForCurrentThread, no new diagnostic or telemetry loggers will be created. + ThreadGlobals( + std::shared_ptr diagnosticLogger, + std::unique_ptr&& telemetryLogger); + + // Request that a sub ThreadGlobals be constructed from the given parent. + struct create_sub_thread_globals_t {}; + ThreadGlobals(const ThreadGlobals& parent, create_sub_thread_globals_t); + + bool ContainsDiagnosticLogger() const; AppInstaller::Logging::DiagnosticLogger& GetDiagnosticLogger(); + bool ContainsTelemetryLogger() const; AppInstaller::Logging::TelemetryTraceLogger& GetTelemetryLogger(); // Set Globals for Current Thread @@ -29,9 +41,9 @@ namespace AppInstaller::ThreadLocalStorage void Initialize(); - std::unique_ptr m_pDiagnosticLogger; + std::shared_ptr m_pDiagnosticLogger; std::unique_ptr m_pTelemetryLogger; - std::once_flag loggerInitOnceFlag; + std::once_flag m_loggerInitOnceFlag; }; struct PreviousThreadGlobals diff --git a/src/AppInstallerCommonCore/ThreadGlobals.cpp b/src/AppInstallerCommonCore/ThreadGlobals.cpp index 2853ee50df..248ef15619 100644 --- a/src/AppInstallerCommonCore/ThreadGlobals.cpp +++ b/src/AppInstallerCommonCore/ThreadGlobals.cpp @@ -8,11 +8,33 @@ namespace AppInstaller::ThreadLocalStorage // Set and return Globals for Current Thread static ThreadGlobals* SetOrGetThreadGlobals(bool setThreadGlobals, ThreadGlobals* pThreadGlobals = nullptr); + ThreadGlobals::ThreadGlobals(std::shared_ptr diagnosticLogger, std::unique_ptr&& telemetryLogger) + : m_pDiagnosticLogger(std::move(diagnosticLogger)), m_pTelemetryLogger(std::move(telemetryLogger)) + { + // Flip the initialization flag + std::call_once(m_loggerInitOnceFlag, []() {}); + } + + ThreadGlobals::ThreadGlobals(const ThreadGlobals& parent, create_sub_thread_globals_t) + : ThreadGlobals(parent.m_pDiagnosticLogger, parent.m_pTelemetryLogger ? parent.m_pTelemetryLogger->CreateSubTraceLogger() : nullptr) + { + } + + bool ThreadGlobals::ContainsDiagnosticLogger() const + { + return m_pDiagnosticLogger != nullptr; + } + DiagnosticLogger& ThreadGlobals::GetDiagnosticLogger() { return *(m_pDiagnosticLogger); } + bool ThreadGlobals::ContainsTelemetryLogger() const + { + return m_pTelemetryLogger != nullptr; + } + TelemetryTraceLogger& ThreadGlobals::GetTelemetryLogger() { return *(m_pTelemetryLogger); @@ -31,7 +53,7 @@ namespace AppInstaller::ThreadLocalStorage { try { - std::call_once(loggerInitOnceFlag, [this]() + std::call_once(m_loggerInitOnceFlag, [this]() { m_pDiagnosticLogger = std::make_unique(); m_pTelemetryLogger = std::make_unique(); diff --git a/src/AppInstallerCommonCore/TraceLogger.cpp b/src/AppInstallerCommonCore/TraceLogger.cpp index f77f7b9821..f11f6849d1 100644 --- a/src/AppInstallerCommonCore/TraceLogger.cpp +++ b/src/AppInstallerCommonCore/TraceLogger.cpp @@ -16,7 +16,7 @@ namespace AppInstaller::Logging TraceLoggingWriteActivity(g_hTraceProvider, "Diagnostics", Telemetry().GetActivityId(), - nullptr, + Telemetry().GetParentActivityId(), TraceLoggingString(strstr.str().c_str(), "LogMessage")); } catch (...) @@ -29,7 +29,7 @@ namespace AppInstaller::Logging TraceLoggingWriteActivity(g_hTraceProvider, "Diagnostics", Telemetry().GetActivityId(), - nullptr, + Telemetry().GetParentActivityId(), TraceLoggingCountedUtf8String(message.data(), static_cast(message.size()), "LogMessage")); } catch (...) From 622135683912f911321e227bc96ee241c7457918 Mon Sep 17 00:00:00 2001 From: Yao Sun Date: Sun, 14 Nov 2021 17:58:01 -0800 Subject: [PATCH 2/9] infra done --- .../Commands/CompleteCommand.cpp | 2 + src/AppInstallerCLICore/Core.cpp | 2 +- src/AppInstallerCLICore/ExecutionContext.h | 6 +++ .../ExecutionContextData.h | 31 ----------- .../Workflows/DependenciesFlow.cpp | 53 ++++++++++++++----- .../Workflows/ImportExportFlow.cpp | 3 +- .../Workflows/InstallFlow.cpp | 31 +++++------ .../Workflows/UpdateFlow.cpp | 22 +++----- .../Workflows/WorkflowBase.cpp | 2 +- .../Workflows/WorkflowBase.h | 3 +- .../AppInstallerTelemetry.cpp | 35 +++++++++--- .../Public/AppInstallerTelemetry.h | 5 ++ 12 files changed, 107 insertions(+), 88 deletions(-) diff --git a/src/AppInstallerCLICore/Commands/CompleteCommand.cpp b/src/AppInstallerCLICore/Commands/CompleteCommand.cpp index 700a9d75fe..f0ebd156c0 100644 --- a/src/AppInstallerCLICore/Commands/CompleteCommand.cpp +++ b/src/AppInstallerCLICore/Commands/CompleteCommand.cpp @@ -55,6 +55,8 @@ namespace AppInstaller::CLI // Create a new Context to execute the Complete from auto subContextPtr = context.CreateSubContext(); Context& subContext = *subContextPtr; + auto previousThreadGlobals = subContext.GetThreadGlobals().SetForCurrentThread(); + subContext.Reporter.SetChannel(Execution::Reporter::Channel::Completion); subContext.Add(std::move(data)); diff --git a/src/AppInstallerCLICore/Core.cpp b/src/AppInstallerCLICore/Core.cpp index e2ac443969..78e78de5f4 100644 --- a/src/AppInstallerCLICore/Core.cpp +++ b/src/AppInstallerCLICore/Core.cpp @@ -61,7 +61,7 @@ namespace AppInstaller::CLI // Initiate the background cleanup of the log file location. Logging::BeginLogFileCleanup(); - Execution::Context context{ std::cout, std::cin }; + Execution::Context context{ std::cout, std::cin, nullptr /* Command execution always use global diagnostic logger */, Logging::Telemetry().CreateSubTraceLogger() }; context.EnableCtrlHandler(); context << Workflow::ReportExecutionStage(Workflow::ExecutionStage::ParseArgs); diff --git a/src/AppInstallerCLICore/ExecutionContext.h b/src/AppInstallerCLICore/ExecutionContext.h index 53d30f1d8a..54a213c463 100644 --- a/src/AppInstallerCLICore/ExecutionContext.h +++ b/src/AppInstallerCLICore/ExecutionContext.h @@ -70,6 +70,12 @@ namespace AppInstaller::CLI::Execution { Context(std::ostream& out, std::istream& in) : Reporter(out, in) {} + Context( + std::ostream& out, + std::istream& in, + std::shared_ptr diagnosticLogger, + std::unique_ptr&& telemetryLogger) : Reporter(out, in), m_threadGlobals(diagnosticLogger, std::move(telemetryLogger)) {} + // Constructor for creating a sub-context. Context(Execution::Reporter& reporter, const ThreadLocalStorage::ThreadGlobals& threadGlobals) : Reporter(reporter, Execution::Reporter::clone_t{}), diff --git a/src/AppInstallerCLICore/ExecutionContextData.h b/src/AppInstallerCLICore/ExecutionContextData.h index 8ba149cda0..d37abc7b12 100644 --- a/src/AppInstallerCLICore/ExecutionContextData.h +++ b/src/AppInstallerCLICore/ExecutionContextData.h @@ -55,37 +55,6 @@ namespace AppInstaller::CLI::Execution struct Context; - // Contains all the information needed to install a package. - // This is used when installing multiple packages to pass all the - // data to a sub-context. - /*struct PackageToInstall - { - PackageToInstall( - std::shared_ptr&& packageVersion, - std::shared_ptr&& installedPackageVersion, - Manifest::Manifest&& manifest, - Manifest::ManifestInstaller&& installer, - Manifest::ScopeEnum scope = Manifest::ScopeEnum::Unknown, - uint32_t packageSubExecutionId = 0) - : PackageVersion(std::move(packageVersion)), InstalledPackageVersion(std::move(installedPackageVersion)), Manifest(std::move(manifest)), Installer(std::move(installer)), Scope(scope), PackageSubExecutionId(packageSubExecutionId) { } - - std::shared_ptr PackageVersion; - - // Used to uninstall the old version if needed. - std::shared_ptr InstalledPackageVersion; - - // Use this instead of the PackageVersion->GetManifest() as the locale was - // applied when selecting the installer. - Manifest::Manifest Manifest; - - Manifest::ManifestInstaller Installer; - Manifest::ScopeEnum Scope = Manifest::ScopeEnum::Unknown; - - // Use this sub execution id when installing this package so that - // install telemetry is captured with the same sub execution id as other events in Search phase. - uint32_t PackageSubExecutionId = 0; - };*/ - namespace details { template diff --git a/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp b/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp index e05fd93292..ac1f2ac2b1 100644 --- a/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp @@ -13,6 +13,25 @@ using namespace AppInstaller::Manifest; namespace AppInstaller::CLI::Workflow { + namespace + { + // Contains all the information needed to install a dependency package. + struct DependencyPackageCandidate + { + DependencyPackageCandidate( + std::shared_ptr&& packageVersion, + std::shared_ptr&& installedPackageVersion, + Manifest::Manifest&& manifest, + Manifest::ManifestInstaller&& installer) + : PackageVersion(std::move(packageVersion)), InstalledPackageVersion(std::move(installedPackageVersion)), Manifest(std::move(manifest)), Installer(std::move(installer)) { } + + std::shared_ptr PackageVersion; + std::shared_ptr InstalledPackageVersion; + Manifest::Manifest Manifest; + Manifest::ManifestInstaller Installer; + }; + } + void ReportDependencies::operator()(Execution::Context& context) const { if (!Settings::ExperimentalFeature::IsEnabled(Settings::ExperimentalFeature::Feature::Dependencies)) @@ -20,7 +39,7 @@ namespace AppInstaller::CLI::Workflow return; } auto info = context.Reporter.Info(); - + const auto& dependencies = context.Get(); if (dependencies.HasAny()) { @@ -114,7 +133,6 @@ namespace AppInstaller::CLI::Workflow } } - void ManagePackageDependencies::operator()(Execution::Context& context) const { if (!Settings::ExperimentalFeature::IsEnabled(Settings::ExperimentalFeature::Feature::Dependencies)) @@ -127,7 +145,7 @@ namespace AppInstaller::CLI::Workflow const auto& rootManifest = context.Get(); Dependency rootAsDependency = Dependency(DependencyType::Package, rootManifest.Id, rootManifest.Version); - + const auto& rootInstaller = context.Get(); const auto& rootDependencies = rootInstaller->Dependencies; @@ -146,10 +164,11 @@ namespace AppInstaller::CLI::Workflow AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INTERNAL_ERROR); } - std::map idToPackageMap; + std::map idToPackageMap; bool foundError = false; DependencyGraph dependencyGraph(rootAsDependency, rootDependencies, - [&](Dependency node) { + [&](Dependency node) + { DependencyNodeProcessor nodeProcessor(context); auto result = nodeProcessor.EvaluateDependencies(node); @@ -158,12 +177,12 @@ namespace AppInstaller::CLI::Workflow if (result == DependencyNodeProcessorResult::Success) { - Execution::PackageToInstall packageToInstall{ + DependencyPackageCandidate dependencyPackageCandidate{ std::move(nodeProcessor.GetPackageLatestVersion()), std::move(nodeProcessor.GetPackageInstalledVersion()), std::move(nodeProcessor.GetManifest()), std::move(nodeProcessor.GetPreferredInstaller()) }; - idToPackageMap.emplace(node.Id, std::move(packageToInstall)); + idToPackageMap.emplace(node.Id, std::move(dependencyPackageCandidate)); }; return list; @@ -184,21 +203,31 @@ namespace AppInstaller::CLI::Workflow const auto& installationOrder = dependencyGraph.GetInstallationOrder(); - std::vector installers; + std::vector> dependencyPackageContexts; for (auto const& node : installationOrder) - { + { auto itr = idToPackageMap.find(node.Id); // if the package was already installed (with a useful version) or is the root // then there will be no installer for it on the map. if (itr != idToPackageMap.end()) { - installers.push_back(std::move(itr->second)); + auto dependencyContextPtr = context.CreateSubContext(); + Execution::Context& dependencyContext = *dependencyContextPtr; + auto previousThreadGlobals = dependencyContext.GetThreadGlobals().SetForCurrentThread(); + + // Extract the data needed for installing + dependencyContext.Add(itr->second.PackageVersion); + dependencyContext.Add(itr->second.Manifest); + dependencyContext.Add(itr->second.InstalledPackageVersion); + dependencyContext.Add(itr->second.Installer); + + dependencyPackageContexts.emplace_back(std::move(dependencyContextPtr)); } } - + // Install dependencies in the correct order - context.Add(installers); + context.Add(std::move(dependencyPackageContexts)); context << Workflow::InstallMultiplePackages(m_dependencyReportMessage, APPINSTALLER_CLI_ERROR_INSTALL_DEPENDENCIES, {}, false, true); } } \ No newline at end of file diff --git a/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp b/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp index b85b24470b..f41b986dc4 100644 --- a/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp @@ -266,7 +266,6 @@ namespace AppInstaller::CLI::Workflow AICLI_LOG(CLI, Info, << "Searching for packages requested from source [" << requiredSource.Details.Identifier << "]"); for (const auto& packageRequest : requiredSource.Packages) { - Logging::SubExecutionTelemetryScope subExecution; AICLI_LOG(CLI, Info, << "Searching for package [" << packageRequest.Id << "]"); // Search for the current package @@ -275,6 +274,8 @@ namespace AppInstaller::CLI::Workflow auto searchContextPtr = context.CreateSubContext(); Execution::Context& searchContext = *searchContextPtr; + auto previousThreadGlobals = searchContext.GetThreadGlobals().SetForCurrentThread(); + searchContext.Add(source); searchContext.Add(source.Search(searchRequest)); diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index 140fe76957..90a5ed9acd 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -191,16 +191,19 @@ namespace AppInstaller::CLI::Workflow bool hasPackageAgreements = false; for (auto& packageContext : context.Get()) { - // Show agreements for each package in the sub-context - *packageContext << + // Show agreements for each package + Execution::Context& showContext = *packageContext; + auto previousThreadGlobals = showContext.GetThreadGlobals().SetForCurrentThread(); + + showContext << Workflow::ReportManifestIdentityWithVersion << Workflow::ShowPackageAgreements(/* ensureAcceptance */ false); - if (packageContext->IsTerminated()) + if (showContext.IsTerminated()) { - AICLI_TERMINATE_CONTEXT(packageContext->GetTerminationHR()); + AICLI_TERMINATE_CONTEXT(showContext.GetTerminationHR()); } - hasPackageAgreements |= !packageContext->Get().CurrentLocalization.Get().empty(); + hasPackageAgreements |= !showContext.Get().CurrentLocalization.Get().empty(); } // If any package has agreements, ensure they are accepted @@ -406,9 +409,9 @@ namespace AppInstaller::CLI::Workflow if (Settings::ExperimentalFeature::IsEnabled(Settings::ExperimentalFeature::Feature::Dependencies)) { DependencyList allDependencies; - for (auto package : context.Get()) + for (auto& packageContext : context.Get()) { - allDependencies.Add(package.Installer.Dependencies); + allDependencies.Add(packageContext->Get().value().Dependencies); } context.Add(allDependencies); @@ -419,22 +422,14 @@ namespace AppInstaller::CLI::Workflow size_t packagesCount = context.Get().size(); size_t packagesProgress = 0; - for (auto package : context.Get()) + for (auto& packageContext : context.Get()) { - Logging::SubExecutionTelemetryScope subExecution{ package.PackageSubExecutionId }; - packagesProgress++; context.Reporter.Info() << "(" << packagesProgress << "/" << packagesCount << ") "; // We want to do best effort to install all packages regardless of previous failures - auto installContextPtr = context.Clone(); - Execution::Context& installContext = *installContextPtr; - - // Extract the data needed for installing - installContext.Add(package.PackageVersion); - installContext.Add(package.Manifest); - installContext.Add(package.InstalledPackageVersion); - installContext.Add(package.Installer); + Execution::Context& installContext = *packageContext; + auto previousThreadGlobals = installContext.GetThreadGlobals().SetForCurrentThread(); installContext << Workflow::ReportIdentityAndInstallationDisclaimer; if (!m_ignorePackageDependencies) diff --git a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp index a2c1fb43df..51a523e097 100644 --- a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp @@ -19,19 +19,19 @@ namespace AppInstaller::CLI::Workflow return (installedVersion < updateVersion || updateVersion.IsLatest()); } - void AddToPackagesToInstallIfNotPresent(std::vector& packagesToInstall, Execution::PackageToInstall&& package) + void AddToPackagesToInstallIfNotPresent(std::vector>& packagesToInstall, std::unique_ptr packageContext) { for (auto const& existing : packagesToInstall) { - if (existing.Manifest.Id == package.Manifest.Id && - existing.Manifest.Version == package.Manifest.Version && - existing.PackageVersion->GetProperty(PackageVersionProperty::SourceIdentifier) == package.PackageVersion->GetProperty(PackageVersionProperty::SourceIdentifier)) + if (existing->Get().Id == packageContext->Get().Id && + existing->Get().Version == packageContext->Get().Version && + existing->Get()->GetProperty(PackageVersionProperty::SourceIdentifier) == packageContext->Get()->GetProperty(PackageVersionProperty::SourceIdentifier)) { return; } } - packagesToInstall.emplace_back(std::move(package)); + packagesToInstall.emplace_back(std::move(packageContext)); } } @@ -123,11 +123,10 @@ namespace AppInstaller::CLI::Workflow for (const auto& match : matches) { - Logging::SubExecutionTelemetryScope subExecution; - // We want to do best effort to update all applicable updates regardless on previous update failure auto updateContextPtr = context.CreateSubContext(); Execution::Context& updateContext = *updateContextPtr; + auto previousThreadGlobals = updateContext.GetThreadGlobals().SetForCurrentThread(); updateContext.Add(match.Package); @@ -143,14 +142,7 @@ namespace AppInstaller::CLI::Workflow updateAllFoundUpdate = true; - Execution::PackageToInstall package{ - std::move(updateContext.Get()), - std::move(updateContext.Get()), - std::move(updateContext.Get()), - std::move(updateContext.Get().value()) }; - package.PackageSubExecutionId = subExecution.GetCurrentSubExecutionId(); - - AddToPackagesToInstallIfNotPresent(packagesToInstall, std::move(package)); + AddToPackagesToInstallIfNotPresent(packagesToInstall, std::move(updateContextPtr)); } if (!updateAllFoundUpdate) diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index 8b26adb99e..1c091c2590 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -1052,7 +1052,7 @@ namespace AppInstaller::CLI::Workflow void ReportExecutionStage::operator()(Execution::Context& context) const { - context.SetExecutionStage(m_stage, m_allowBackward); + context.SetExecutionStage(m_stage); } void HandleSourceAgreements::operator()(Execution::Context& context) const diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.h b/src/AppInstallerCLICore/Workflows/WorkflowBase.h index 81ec9a067b..ab526554db 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.h +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.h @@ -352,13 +352,12 @@ namespace AppInstaller::CLI::Workflow // Outputs: ExecutionStage struct ReportExecutionStage : public WorkflowTask { - ReportExecutionStage(ExecutionStage stage, bool allowBackward = false) : WorkflowTask("ReportExecutionStage"), m_stage(stage), m_allowBackward(allowBackward) {} + ReportExecutionStage(ExecutionStage stage) : WorkflowTask("ReportExecutionStage"), m_stage(stage) {} void operator()(Execution::Context& context) const override; private: ExecutionStage m_stage; - bool m_allowBackward; }; // Handles all opened source(s) agreements if needed. diff --git a/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp b/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp index c93e43e2b3..9864c9ca5b 100644 --- a/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp +++ b/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp @@ -48,8 +48,6 @@ namespace AppInstaller::Logging static const uint32_t s_RootExecutionId = 0; static std::atomic_uint32_t s_subExecutionId{ s_RootExecutionId }; - static std::atomic_bool s_isRuntimeEnabled{ true }; - constexpr std::wstring_view s_UserProfileReplacement = L"%USERPROFILE%"sv; void __stdcall wilResultLoggingCallback(const wil::FailureInfo& info) noexcept @@ -61,6 +59,7 @@ namespace AppInstaller::Logging TelemetryTraceLogger::TelemetryTraceLogger() { std::ignore = CoCreateGuid(&m_activityId); + m_subExecutionId = s_RootExecutionId; } const GUID* TelemetryTraceLogger::GetActivityId() const @@ -73,6 +72,16 @@ namespace AppInstaller::Logging return &m_parentActivityId; } + bool TelemetryTraceLogger::DisableRuntime() + { + return m_isRuntimeEnabled.exchange(false); + } + + void TelemetryTraceLogger::EnableRuntime() + { + m_isRuntimeEnabled = true; + } + void TelemetryTraceLogger::Initialize() { if (!m_isInitialized) @@ -134,10 +143,22 @@ namespace AppInstaller::Logging std::unique_ptr TelemetryTraceLogger::CreateSubTraceLogger() const { - auto subTraceLogger = std::make_unique(*this); + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !this->m_isInitialized); + + auto subTraceLogger = std::make_unique(); + subTraceLogger->m_parentActivityId = this->m_activityId; - std::ignore = CoCreateGuid(&subTraceLogger->m_activityId); subTraceLogger->m_subExecutionId = s_subExecutionId++; + + // Copy remaining fields from parent. + subTraceLogger->m_isSettingEnabled = this->m_isSettingEnabled; + subTraceLogger->m_isRuntimeEnabled = this->m_isRuntimeEnabled.load(); + subTraceLogger->m_isInitialized = this->m_isInitialized.load(); + subTraceLogger->m_executionStage = this->m_executionStage.load(); + subTraceLogger->m_telemetryCorrelationJsonW = this->m_telemetryCorrelationJsonW; + subTraceLogger->m_caller = this->m_caller; + subTraceLogger->m_userProfile = this->m_userProfile; + return subTraceLogger; } @@ -545,7 +566,7 @@ namespace AppInstaller::Logging bool TelemetryTraceLogger::IsTelemetryEnabled() const noexcept { - return g_IsTelemetryProviderEnabled && m_isInitialized && m_isSettingEnabled && s_isRuntimeEnabled; + return g_IsTelemetryProviderEnabled && m_isInitialized && m_isSettingEnabled && m_isRuntimeEnabled; } void TelemetryTraceLogger::InitializeInternal(const AppInstaller::Settings::UserSettings& userSettings) @@ -598,14 +619,14 @@ namespace AppInstaller::Logging DisableTelemetryScope::DisableTelemetryScope() { - m_token = s_isRuntimeEnabled.exchange(false); + m_token = Telemetry().DisableRuntime(); } DisableTelemetryScope::~DisableTelemetryScope() { if (m_token) { - s_isRuntimeEnabled = true; + Telemetry().EnableRuntime(); } } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h b/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h index 711b55d34e..30e3fa2bc8 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h @@ -31,6 +31,10 @@ namespace AppInstaller::Logging TelemetryTraceLogger(TelemetryTraceLogger&&) = default; TelemetryTraceLogger& operator=(TelemetryTraceLogger&&) = default; + // Control whether this trace logger is enabled at runtime. + bool DisableRuntime(); + void EnableRuntime(); + // Return address of m_activityId const GUID* GetActivityId() const; @@ -150,6 +154,7 @@ namespace AppInstaller::Logging 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 }; std::atomic_uint32_t m_executionStage{ 0 }; From 253078a2edbd7ed439f5fac504474913eeb358bb Mon Sep 17 00:00:00 2001 From: Yao Sun Date: Mon, 15 Nov 2021 18:19:34 -0800 Subject: [PATCH 3/9] Log summary event --- src/AppInstallerCLICore/Core.cpp | 2 + .../Workflows/WorkflowBase.cpp | 10 +- .../AppInstallerTelemetry.cpp | 180 ++++++++++++++++-- .../Public/AppInstallerTelemetry.h | 110 ++++++++++- 4 files changed, 275 insertions(+), 27 deletions(-) diff --git a/src/AppInstallerCLICore/Core.cpp b/src/AppInstallerCLICore/Core.cpp index 78e78de5f4..799167c0ca 100644 --- a/src/AppInstallerCLICore/Core.cpp +++ b/src/AppInstallerCLICore/Core.cpp @@ -46,6 +46,8 @@ namespace AppInstaller::CLI { init_apartment(); + Logging::UseGlobalTelemetryLoggerActivityIdOnly(); + // Enable all logging for this phase; we will update once we have the arguments Logging::Log().EnableChannel(Logging::Channel::All); Logging::Log().SetLevel(Logging::Level::Info); diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index 1c091c2590..e0561033aa 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -247,7 +247,7 @@ namespace AppInstaller::CLI::Workflow catch (const wil::ResultException& re) { // Even though they are logged at their source, log again here for completeness. - Logging::Telemetry().LogException("wil::ResultException", re.what()); + Logging::Telemetry().LogException(Logging::FailureTypeResultException, re.what()); context.Reporter.Error() << Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << GetUserPresentableMessage(re) << std::endl; @@ -256,7 +256,7 @@ namespace AppInstaller::CLI::Workflow catch (const winrt::hresult_error& hre) { std::string message = GetUserPresentableMessage(hre); - Logging::Telemetry().LogException("winrt::hresult_error", message); + Logging::Telemetry().LogException(Logging::FailureTypeWinrtHResultError, message); context.Reporter.Error() << Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << message << std::endl; @@ -270,13 +270,13 @@ namespace AppInstaller::CLI::Workflow } catch (const Resource::ResourceOpenException& e) { - Logging::Telemetry().LogException("ResourceOpenException", e.what()); + Logging::Telemetry().LogException(Logging::FailureTypeResourceOpen, e.what()); context.Reporter.Error() << GetUserPresentableMessage(e) << std::endl; return APPINSTALLER_CLI_ERROR_MISSING_RESOURCE_FILE; } catch (const std::exception& e) { - Logging::Telemetry().LogException("std::exception", e.what()); + Logging::Telemetry().LogException(Logging::FailureTypeStdException, e.what()); context.Reporter.Error() << Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << GetUserPresentableMessage(e) << std::endl; @@ -285,7 +285,7 @@ namespace AppInstaller::CLI::Workflow catch (...) { LOG_CAUGHT_EXCEPTION(); - Logging::Telemetry().LogException("unknown", {}); + Logging::Telemetry().LogException(Logging::FailureTypeUnknown, {}); context.Reporter.Error() << Resource::String::UnexpectedErrorExecutingCommand << " ???"_liv << std::endl; return APPINSTALLER_CLI_ERROR_COMMAND_FAILED; diff --git a/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp b/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp index 9864c9ca5b..d9ba39ba6f 100644 --- a/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp +++ b/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp @@ -15,7 +15,7 @@ #define AICLI_TraceLoggingWriteActivity(_eventName_,...) TraceLoggingWriteActivity(\ g_hTraceProvider,\ _eventName_,\ -GetActivityId(),\ +s_useGlobalTelemetryActivityId ? s_globalTelemetryLoggerActivityId : GetActivityId(),\ nullptr,\ TraceLoggingCountedUtf8String(m_caller.c_str(), static_cast(m_caller.size()), "Caller"),\ TraceLoggingPackedFieldEx(m_telemetryCorrelationJsonW.c_str(), static_cast((m_telemetryCorrelationJsonW.size() + 1) * sizeof(wchar_t)), TlgInUNICODESTRING, TlgOutJSON, "CvJson"),\ @@ -50,10 +50,49 @@ namespace AppInstaller::Logging constexpr std::wstring_view s_UserProfileReplacement = L"%USERPROFILE%"sv; + // TODO: Temporary code to keep existing telemetry behavior + static bool s_useGlobalTelemetryActivityId = false; + static const GUID* s_globalTelemetryLoggerActivityId = nullptr; + void __stdcall wilResultLoggingCallback(const wil::FailureInfo& info) noexcept { Telemetry().LogFailure(info); } + + UINT32 ConvertWilFailureTypeToFailureType(wil::FailureType failureType) + { + switch (failureType) + { + case wil::FailureType::Exception: + return FailureTypeResultException; + case wil::FailureType::Return: + return FailureTypeResultReturn; + case wil::FailureType::Log: + return FailureTypeResultLog; + case wil::FailureType::FailFast: + return FailureTypeResultFailFast; + default: + return FailureTypeUnknown; + } + } + + std::string_view LogExceptionTypeToString(UINT32 exceptionType) + { + switch (exceptionType) + { + case FailureTypeResultException: + return "wil::ResultException"sv; + case FailureTypeWinrtHResultError: + return "winrt::hresult_error"sv; + case FailureTypeResourceOpen: + return "ResourceOpenException"sv; + case FailureTypeStdException: + return "std::exception"sv; + case FailureTypeUnknown: + default: + return "unknown"sv; + } + } } TelemetryTraceLogger::TelemetryTraceLogger() @@ -158,6 +197,7 @@ namespace AppInstaller::Logging subTraceLogger->m_telemetryCorrelationJsonW = this->m_telemetryCorrelationJsonW; subTraceLogger->m_caller = this->m_caller; subTraceLogger->m_userProfile = this->m_userProfile; + subTraceLogger->m_summary = this->m_summary; return subTraceLogger; } @@ -170,7 +210,7 @@ namespace AppInstaller::Logging AICLI_TraceLoggingWriteActivity( "FailureInfo", - TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), + TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), TraceLoggingHResult(failure.hr, "HResult"), AICLI_TraceLoggingWStringView(anonMessage, "Message"), TraceLoggingString(failure.pszModule, "Module"), @@ -181,6 +221,14 @@ namespace AppInstaller::Logging TraceLoggingUInt32(m_executionStage, "ExecutionStage"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.FailureHResult = failure.hr; + m_summary.FailureMessage = anonMessage; + m_summary.FailureModule = failure.pszModule; + m_summary.FailureThreadId = failure.threadId; + m_summary.FailureType = ConvertWilFailureTypeToFailureType(failure.type); + m_summary.FailureFile = failure.pszFile; + m_summary.FailureLine = failure.uLineNumber; } // Also send failure to the log @@ -209,6 +257,8 @@ namespace AppInstaller::Logging TraceLoggingCountedString(packageVersion->c_str(), static_cast(packageVersion->size()), "PackageVersion"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.IsCOMCall = isCOMCall; } AICLI_LOG(Core, Info, << "WinGet, version [" << version << "], activity [" << *GetActivityId() << ']'); @@ -230,6 +280,8 @@ namespace AppInstaller::Logging AICLI_TraceLoggingStringView(commandName, "Command"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.Command = commandName; } AICLI_LOG(CLI, Info, << "Leaf command to execute: " << commandName); @@ -244,6 +296,8 @@ namespace AppInstaller::Logging AICLI_TraceLoggingStringView(commandName, "Command"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.CommandSuccess = true; } AICLI_LOG(CLI, Info, << "Leaf command succeeded: " << commandName); @@ -255,35 +309,44 @@ namespace AppInstaller::Logging { AICLI_TraceLoggingWriteActivity( "CommandTermination", - TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), + TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), TraceLoggingHResult(hr, "HResult"), AICLI_TraceLoggingStringView(file, "File"), TraceLoggingUInt64(static_cast(line), "Line"), TraceLoggingUInt32(m_executionStage, "ExecutionStage"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.CommandTerminationHResult = hr; + m_summary.CommandTerminationFile = file; + m_summary.CommandTerminationLine = static_cast(line); } AICLI_LOG(CLI, Error, << "Terminating context: 0x" << SetHRFormat << hr << " at " << file << ":" << line); } - void TelemetryTraceLogger::LogException(std::string_view type, std::string_view message) const noexcept + void TelemetryTraceLogger::LogException(UINT32 type, std::string_view message) const noexcept { + auto exceptionTypeString = LogExceptionTypeToString(type); + if (IsTelemetryEnabled()) { auto anonMessage = AnonymizeString(Utility::ConvertToUTF16(message)); AICLI_TraceLoggingWriteActivity( "Exception", - TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), - AICLI_TraceLoggingStringView(type, "Type"), + TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), + AICLI_TraceLoggingStringView(exceptionTypeString, "Type"), AICLI_TraceLoggingWStringView(anonMessage, "Message"), TraceLoggingUInt32(m_executionStage, "ExecutionStage"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.FailureType = type; + m_summary.FailureMessage = anonMessage; } - AICLI_LOG(CLI, Error, << "Caught " << type << ": " << message); + AICLI_LOG(CLI, Error, << "Caught " << exceptionTypeString << ": " << message); } void TelemetryTraceLogger::LogIsManifestLocal(bool isLocalManifest) const noexcept @@ -292,10 +355,12 @@ namespace AppInstaller::Logging { AICLI_TraceLoggingWriteActivity( "GetManifest", - TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), + TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), TraceLoggingBool(isLocalManifest, "IsManifestLocal"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.IsManifestLocal = isLocalManifest; } } @@ -305,12 +370,16 @@ namespace AppInstaller::Logging { AICLI_TraceLoggingWriteActivity( "ManifestFields", - TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), + TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), AICLI_TraceLoggingStringView(id, "Id"), AICLI_TraceLoggingStringView(name, "Name"), AICLI_TraceLoggingStringView(version, "Version"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.PackageIdentifier = id; + m_summary.PackageName = name; + m_summary.PackageVersion = version; } AICLI_LOG(CLI, Info, << "Manifest fields: Name [" << name << "], Version [" << version << ']'); @@ -322,9 +391,11 @@ namespace AppInstaller::Logging { AICLI_TraceLoggingWriteActivity( "NoAppMatch", - TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), + TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.NoAppMatch = true; } AICLI_LOG(CLI, Info, << "No app found matching input criteria"); @@ -336,9 +407,11 @@ namespace AppInstaller::Logging { AICLI_TraceLoggingWriteActivity( "MultiAppMatch", - TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), + TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.MultipleAppMatch = true; } AICLI_LOG(CLI, Info, << "Multiple apps found matching input criteria"); @@ -350,11 +423,14 @@ namespace AppInstaller::Logging { AICLI_TraceLoggingWriteActivity( "AppFound", - TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), + TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), AICLI_TraceLoggingStringView(name, "Name"), AICLI_TraceLoggingStringView(id, "Id"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.PackageIdentifier = id; + m_summary.PackageName = name; } AICLI_LOG(CLI, Info, << "Found one app. App id: " << id << " App name: " << name); @@ -366,7 +442,7 @@ namespace AppInstaller::Logging { AICLI_TraceLoggingWriteActivity( "SelectedInstaller", - TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), + TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), TraceLoggingInt32(arch, "Arch"), AICLI_TraceLoggingStringView(url, "Url"), AICLI_TraceLoggingStringView(installerType, "InstallerType"), @@ -374,6 +450,12 @@ namespace AppInstaller::Logging AICLI_TraceLoggingStringView(language, "Language"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.InstallerArchitecture = arch; + m_summary.InstallerUrl = url; + m_summary.InstallerType = installerType; + m_summary.InstallerScope = scope; + m_summary.InstallerLocale = language; } AICLI_LOG(CLI, Info, << "Completed installer selection."); @@ -399,7 +481,7 @@ namespace AppInstaller::Logging { AICLI_TraceLoggingWriteActivity( "SearchRequest", - TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), + TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), AICLI_TraceLoggingStringView(type, "Type"), AICLI_TraceLoggingStringView(query, "Query"), AICLI_TraceLoggingStringView(id, "Id"), @@ -411,6 +493,16 @@ namespace AppInstaller::Logging AICLI_TraceLoggingStringView(request, "Request"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.SearchType = type; + m_summary.SearchQuery = query; + m_summary.SearchId = id; + m_summary.SearchName = name; + m_summary.SearchMoniker = moniker; + m_summary.SearchTag = tag; + m_summary.SearchCommand = command; + m_summary.SearchMaximum = static_cast(maximum); + m_summary.SearchRequest = request; } } @@ -420,10 +512,12 @@ namespace AppInstaller::Logging { AICLI_TraceLoggingWriteActivity( "SearchResultCount", - TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), + TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), TraceLoggingUInt64(resultCount, "ResultCount"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.SearchResultCount = resultCount; } } @@ -439,15 +533,22 @@ namespace AppInstaller::Logging { AICLI_TraceLoggingWriteActivity( "HashMismatch", - TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), + TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), AICLI_TraceLoggingStringView(id, "Id"), AICLI_TraceLoggingStringView(version, "Version"), AICLI_TraceLoggingStringView(channel, "Channel"), TraceLoggingBinary(expected.data(), static_cast(expected.size()), "Expected"), TraceLoggingBinary(actual.data(), static_cast(actual.size()), "Actual"), - TraceLoggingValue(overrideHashMismatch, "Override"), + TraceLoggingBool(overrideHashMismatch, "Override"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.PackageIdentifier = id; + m_summary.PackageVersion = version; + m_summary.Channel = channel; + m_summary.HashMismatchExpected = expected; + m_summary.HashMismatchActual = actual; + m_summary.HashMismatchOverride = overrideHashMismatch; } AICLI_LOG(CLI, Error, @@ -464,7 +565,7 @@ namespace AppInstaller::Logging { AICLI_TraceLoggingWriteActivity( "InstallerFailure", - TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), + TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), AICLI_TraceLoggingStringView(id, "Id"), AICLI_TraceLoggingStringView(version, "Version"), AICLI_TraceLoggingStringView(channel, "Channel"), @@ -472,6 +573,12 @@ namespace AppInstaller::Logging TraceLoggingUInt32(errorCode, "ErrorCode"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.PackageIdentifier = id; + m_summary.PackageVersion = version; + m_summary.Channel = channel; + m_summary.InstallerExecutionType = type; + m_summary.InstallerErrorCode = errorCode; } AICLI_LOG(CLI, Error, << type << " installer failed: " << errorCode); @@ -483,13 +590,18 @@ namespace AppInstaller::Logging { AICLI_TraceLoggingWriteActivity( "UninstallerFailure", - TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), + TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), AICLI_TraceLoggingStringView(id, "Id"), AICLI_TraceLoggingStringView(version, "Version"), AICLI_TraceLoggingStringView(type, "Type"), TraceLoggingUInt32(errorCode, "ErrorCode"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.PackageIdentifier = id; + m_summary.PackageVersion = version; + m_summary.UninstallerExecutionType = type; + m_summary.UninstallerErrorCode = errorCode; } AICLI_LOG(CLI, Error, << type << " uninstaller failed: " << errorCode); @@ -521,7 +633,7 @@ namespace AppInstaller::Logging AICLI_TraceLoggingWriteActivity( "InstallARPChange", - TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), + TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), AICLI_TraceLoggingStringView(sourceIdentifier, "SourceIdentifier"), AICLI_TraceLoggingStringView(packageIdentifier, "PackageIdentifier"), AICLI_TraceLoggingStringView(packageVersion, "PackageVersion"), @@ -535,6 +647,18 @@ namespace AppInstaller::Logging TraceLoggingUInt64(static_cast(languageNumber), "ARPLanguage"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage | PDT_SoftwareSetupAndInventory), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); + + m_summary.SourceIdentifier = sourceIdentifier; + m_summary.PackageIdentifier = packageIdentifier; + m_summary.PackageVersion = packageVersion; + m_summary.Channel = packageChannel; + m_summary.ChangesToARP = static_cast(changesToARP); + m_summary.MatchesInARP = static_cast(matchesInARP); + m_summary.ChangesThatMatch = static_cast(countOfIntersectionOfChangesAndMatches); + m_summary.ARPName = arpName; + m_summary.ARPVersion = arpVersion; + m_summary.ARPPublisher = arpPublisher; + m_summary.ARPLanguage = static_cast(languageNumber); } AICLI_LOG(CLI, Info, << "During package install, " << changesToARP << " changes to ARP were observed, " @@ -556,11 +680,14 @@ namespace AppInstaller::Logging { AICLI_TraceLoggingWriteActivity( "NonFatalDOError", - TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), + TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), AICLI_TraceLoggingStringView(url, "Url"), TraceLoggingHResult(hr, "HResult"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES)); + + m_summary.DOUrl = url; + m_summary.DOHResult = hr; } } @@ -608,6 +735,12 @@ namespace AppInstaller::Logging { static TelemetryTraceLogger processGlobalTelemetry; processGlobalTelemetry.TryInitialize(); + + if (s_useGlobalTelemetryActivityId && s_globalTelemetryLoggerActivityId == nullptr) + { + s_globalTelemetryLoggerActivityId = processGlobalTelemetry.GetActivityId(); + } + return processGlobalTelemetry; } } @@ -617,6 +750,11 @@ namespace AppInstaller::Logging wil::SetResultLoggingCallback(wilResultLoggingCallback); } + void UseGlobalTelemetryLoggerActivityIdOnly() + { + s_useGlobalTelemetryActivityId = true; + } + DisableTelemetryScope::DisableTelemetryScope() { m_token = Telemetry().DisableRuntime(); diff --git a/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h b/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h index 30e3fa2bc8..46e83c2c52 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h @@ -15,6 +15,109 @@ namespace AppInstaller::Settings namespace AppInstaller::Logging { + static constexpr UINT32 FailureTypeNone = 0; + // Failure type from FailureInfo in result_macros.h + static constexpr UINT32 FailureTypeResultException = 1; // THROW_... + static constexpr UINT32 FailureTypeResultReturn = 2; // RETURN_..._LOG or RETURN_..._MSG + static constexpr UINT32 FailureTypeResultLog = 3; // LOG_... + static constexpr UINT32 FailureTypeResultFailFast = 4; // FAIL_FAST_... + // Other failure types from LogException() + static constexpr UINT32 FailureTypeWinrtHResultError = 101; + static constexpr UINT32 FailureTypeResourceOpen = 102; + static constexpr UINT32 FailureTypeStdException = 103; + static constexpr UINT32 FailureTypeUnknown = 104; + + // Contains all fields logged through the TelemetryTraceLogger. Last write wins. + // This will be used to report a summary event upon destruction of the TelemetryTraceLogger. + struct TelemetrySummary + { + // Log wil failure, exception + HRESULT FailureHResult = S_OK; + std::wstring FailureMessage; + std::string FailureModule; + UINT32 FailureThreadId = 0; + UINT32 FailureType = FailureTypeNone; + std::string FailureFile; + UINT32 FailureLine = 0; + + // LogCommandTermination + HRESULT CommandTerminationHResult = S_OK; + std::string CommandTerminationFile; + UINT32 CommandTerminationLine = 0; + + // LogStartup + bool IsCOMCall = false; + + // LogCommand + std::string Command; + + // LogCommandSuccess + bool CommandSuccess = false; + + // LogIsManifestLocal + bool IsManifestLocal = false; + + // LogManifestFields, LogAppFound + std::string PackageIdentifier; + std::string PackageName; + std::string PackageVersion; + std::string Channel; + std::string SourceIdentifier; + + // LogNoAppMatch + bool NoAppMatch = false; + + // LogMultipleAppMatch + bool MultipleAppMatch = false; + + // LogSelectedInstaller + INT32 InstallerArchitecture = -1; + std::string InstallerUrl; + std::string InstallerType; + std::string InstallerScope; + std::string InstallerLocale; + + // LogSearchRequest + std::string SearchType; + std::string SearchQuery; + std::string SearchId; + std::string SearchName; + std::string SearchMoniker; + std::string SearchTag; + std::string SearchCommand; + UINT64 SearchMaximum = 0; + std::string SearchRequest; + + // LogSearchResultCount + UINT64 SearchResultCount = 0; + + // LogInstallerHashMismatch + std::vector HashMismatchExpected; + std::vector HashMismatchActual; + bool HashMismatchOverride = false; + + // LogInstallerFailure + std::string InstallerExecutionType; + UINT32 InstallerErrorCode = 0; + + // LogUninstallerFailure + std::string UninstallerExecutionType; + UINT32 UninstallerErrorCode = 0; + + // LogSuccessfulInstallARPChange + UINT64 ChangesToARP = 0; + UINT64 MatchesInARP = 0; + UINT64 ChangesThatMatch = 0; + UINT64 ARPLanguage = 0; + std::string ARPName; + std::string ARPVersion; + std::string ARPPublisher; + + // LogNonFatalDOError + std::string DOUrl; + HRESULT DOHResult = S_OK; + }; + // This type contains the registration lifetime of the telemetry trace logging provider. // Due to the nature of trace logging, specific methods should be added per desired trace. // As there should not be a significantly large number of individual telemetry events, @@ -73,7 +176,7 @@ namespace AppInstaller::Logging void LogCommandTermination(HRESULT hr, std::string_view file, size_t line) const noexcept; // Logs the invoked command termination. - void LogException(std::string_view type, std::string_view message) const noexcept; + void LogException(UINT32 type, std::string_view message) const noexcept; // Logs whether the manifest used in workflow is local void LogIsManifestLocal(bool isLocalManifest) const noexcept; @@ -167,6 +270,8 @@ namespace AppInstaller::Logging // Data that is needed by AnonymizeString std::wstring m_userProfile; + mutable TelemetrySummary m_summary; + // TODO: This and all related code could be removed after transition to summary event in back end. uint32_t m_subExecutionId; }; @@ -177,6 +282,9 @@ namespace AppInstaller::Logging // Turns on wil failure telemetry and logging. void EnableWilFailureTelemetry(); + // TODO: Temporary code to keep existing telemetry behavior for command execution cases. + void UseGlobalTelemetryLoggerActivityIdOnly(); + // An RAII object to disable telemetry during its lifetime. // Primarily used by the complete command to prevent messy input from spamming us. struct DisableTelemetryScope From bf27a32b8b02684705907a9b51fda9f2fecab479 Mon Sep 17 00:00:00 2001 From: Yao Sun Date: Mon, 15 Nov 2021 20:55:56 -0800 Subject: [PATCH 4/9] Send summary event --- .../AppInstallerTelemetry.cpp | 81 +++++++++++++++++++ .../Public/AppInstallerTelemetry.h | 4 +- 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp b/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp index d9ba39ba6f..88f7279c2b 100644 --- a/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp +++ b/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp @@ -691,6 +691,87 @@ namespace AppInstaller::Logging } } + TelemetryTraceLogger::~TelemetryTraceLogger() + { + if (IsTelemetryEnabled()) + { + LocIndString version = Runtime::GetClientVersion(); + LocIndString packageVersion; + if (Runtime::IsRunningInPackagedContext()) + { + packageVersion = Runtime::GetPackageVersion(); + } + + TraceLoggingWriteActivity( + g_hTraceProvider, + "Summary", + GetActivityId(), + GetParentActivityId(), + // From member fields or program info. + AICLI_TraceLoggingStringView(m_caller, "Caller"), + TraceLoggingPackedFieldEx(m_telemetryCorrelationJsonW.c_str(), static_cast((m_telemetryCorrelationJsonW.size() + 1) * sizeof(wchar_t)), TlgInUNICODESTRING, TlgOutJSON, "CvJson"), + TraceLoggingCountedString(version->c_str(), static_cast(version->size()), "ClientVersion"), + TraceLoggingCountedString(packageVersion->c_str(), static_cast(packageVersion->size()), "PackageVersion"), + TraceLoggingBool(Runtime::IsReleaseBuild(), "IsReleaseBuild"), + TraceLoggingUInt32(m_executionStage, "ExecutionStage"), + // From TelemetrySummary + TraceLoggingHResult(m_summary.FailureHResult, "FailureHResult"), + AICLI_TraceLoggingWStringView(m_summary.FailureMessage, "FailureMessage"), + AICLI_TraceLoggingStringView(m_summary.FailureModule, "FailureModule"), + TraceLoggingUInt32(m_summary.FailureThreadId, "FailureThreadId"), + TraceLoggingUInt32(m_summary.FailureType, "FailureType"), + AICLI_TraceLoggingStringView(m_summary.FailureFile, "FailureFile"), + TraceLoggingUInt32(m_summary.FailureLine, "FailureLine"), + TraceLoggingHResult(m_summary.CommandTerminationHResult, "CommandTerminationHResult"), + AICLI_TraceLoggingStringView(m_summary.CommandTerminationFile, "CommandTerminationFile"), + TraceLoggingUInt64(m_summary.CommandTerminationLine, "CommandTerminationLine"), + TraceLoggingBool(m_summary.IsCOMCall, "IsCOMCall"), + AICLI_TraceLoggingStringView(m_summary.Command, "Command"), + TraceLoggingBool(m_summary.CommandSuccess, "CommandSuccess"), + TraceLoggingBool(m_summary.IsManifestLocal, "IsManifestLocal"), + AICLI_TraceLoggingStringView(m_summary.PackageIdentifier, "PackageIdentifier"), + AICLI_TraceLoggingStringView(m_summary.PackageName, "PackageName"), + AICLI_TraceLoggingStringView(m_summary.PackageVersion, "PackageVersion"), + AICLI_TraceLoggingStringView(m_summary.Channel, "Channel"), + AICLI_TraceLoggingStringView(m_summary.SourceIdentifier, "SourceIdentifier"), + TraceLoggingBool(m_summary.NoAppMatch, "NoAppMatch"), + TraceLoggingBool(m_summary.MultipleAppMatch, "MultipleAppMatch"), + TraceLoggingInt32(m_summary.InstallerArchitecture, "InstallerArchitecture"), + AICLI_TraceLoggingStringView(m_summary.InstallerUrl, "InstallerUrl"), + AICLI_TraceLoggingStringView(m_summary.InstallerType, "InstallerType"), + AICLI_TraceLoggingStringView(m_summary.InstallerScope, "InstallerScope"), + AICLI_TraceLoggingStringView(m_summary.InstallerLocale, "InstallerLocale"), + AICLI_TraceLoggingStringView(m_summary.SearchType, "SearchType"), + AICLI_TraceLoggingStringView(m_summary.SearchQuery, "SearchQuery"), + AICLI_TraceLoggingStringView(m_summary.SearchId, "SearchId"), + AICLI_TraceLoggingStringView(m_summary.SearchName, "SearchName"), + AICLI_TraceLoggingStringView(m_summary.SearchMoniker, "SearchMoniker"), + AICLI_TraceLoggingStringView(m_summary.SearchTag, "SearchTag"), + AICLI_TraceLoggingStringView(m_summary.SearchCommand, "SearchCommand"), + TraceLoggingUInt64(m_summary.SearchMaximum, "SearchMaximum"), + AICLI_TraceLoggingStringView(m_summary.SearchRequest, "SearchRequest"), + TraceLoggingUInt64(m_summary.SearchResultCount, "SearchResultCount"), + TraceLoggingBinary(m_summary.HashMismatchExpected.data(), static_cast(m_summary.HashMismatchExpected.size()), "HashMismatchExpected"), + TraceLoggingBinary(m_summary.HashMismatchActual.data(), static_cast(m_summary.HashMismatchActual.size()), "HashMismatchActual"), + TraceLoggingBool(m_summary.HashMismatchOverride, "HashMismatchOverride"), + AICLI_TraceLoggingStringView(m_summary.InstallerExecutionType, "InstallerExecutionType"), + TraceLoggingUInt32(m_summary.InstallerErrorCode, "InstallerErrorCode"), + AICLI_TraceLoggingStringView(m_summary.UninstallerExecutionType, "UninstallerExecutionType"), + TraceLoggingUInt32(m_summary.UninstallerErrorCode, "UninstallerErrorCode"), + TraceLoggingUInt64(m_summary.ChangesToARP, "ChangesToARP"), + TraceLoggingUInt64(m_summary.MatchesInARP, "MatchesInARP"), + TraceLoggingUInt64(m_summary.ChangesThatMatch, "ChangesThatMatch"), + TraceLoggingUInt64(m_summary.ARPLanguage, "ARPLanguage"), + AICLI_TraceLoggingStringView(m_summary.ARPName, "ARPName"), + AICLI_TraceLoggingStringView(m_summary.ARPVersion, "ARPVersion"), + AICLI_TraceLoggingStringView(m_summary.ARPPublisher, "ARPPublisher"), + AICLI_TraceLoggingStringView(m_summary.DOUrl, "DOUrl"), + TraceLoggingHResult(m_summary.DOHResult, "DOHResult"), + TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage | PDT_SoftwareSetupAndInventory), + TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES)); + } + } + bool TelemetryTraceLogger::IsTelemetryEnabled() const noexcept { return g_IsTelemetryProviderEnabled && m_isInitialized && m_isSettingEnabled && m_isRuntimeEnabled; diff --git a/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h b/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h index 46e83c2c52..c0c4e28763 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h @@ -43,7 +43,7 @@ namespace AppInstaller::Logging // LogCommandTermination HRESULT CommandTerminationHResult = S_OK; std::string CommandTerminationFile; - UINT32 CommandTerminationLine = 0; + UINT64 CommandTerminationLine = 0; // LogStartup bool IsCOMCall = false; @@ -126,7 +126,7 @@ namespace AppInstaller::Logging { TelemetryTraceLogger(); - ~TelemetryTraceLogger() = default; + ~TelemetryTraceLogger(); TelemetryTraceLogger(const TelemetryTraceLogger&) = default; TelemetryTraceLogger& operator=(const TelemetryTraceLogger&) = default; From fe6fb0b7ef5b2b291f049d01880a8dac9f77a15c Mon Sep 17 00:00:00 2001 From: Yao Sun Date: Tue, 16 Nov 2021 00:32:27 -0800 Subject: [PATCH 5/9] fix tests --- src/AppInstallerCLITests/WorkFlow.cpp | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp index a42c1fab2d..aa51d74125 100644 --- a/src/AppInstallerCLITests/WorkFlow.cpp +++ b/src/AppInstallerCLITests/WorkFlow.cpp @@ -391,7 +391,7 @@ namespace m_overrides->emplace_back(wto); } - std::unique_ptr Clone() override + std::unique_ptr CreateSubContext() override { auto clone = std::make_unique(m_out, m_in, true, m_overrides); clone->SetFlags(this->GetFlags()); @@ -1184,9 +1184,9 @@ TEST_CASE("DependencyGraph_SkipInstalled", "[InstallFlow][workflow][dependencyGr context << ManagePackageDependencies(Resource::String::InstallAndUpgradeCommandsReportDependencies); - std::vector installers = context.Get(); + auto& dependencyPackages = context.Get(); REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::DependenciesFlowContainsLoop)) == std::string::npos); - REQUIRE(installers.size() == 0); + REQUIRE(dependencyPackages.size() == 0); } TEST_CASE("DependencyGraph_validMinVersions", "[InstallFlow][workflow][dependencyGraph][dependencies]") @@ -1208,13 +1208,13 @@ TEST_CASE("DependencyGraph_validMinVersions", "[InstallFlow][workflow][dependenc context << ManagePackageDependencies(Resource::String::InstallAndUpgradeCommandsReportDependencies); - std::vector installers = context.Get(); + auto& dependencyPackages = context.Get(); REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::DependenciesFlowContainsLoop)) == std::string::npos); - REQUIRE(installers.size() == 1); - REQUIRE(installers.at(0).Manifest.Id == "minVersion"); + REQUIRE(dependencyPackages.size() == 1); + REQUIRE(dependencyPackages.at(0)->Get().Id == "minVersion"); // minVersion 1.5 is available but this requires 1.0 so that version is installed - REQUIRE(installers.at(0).Manifest.Version == "1.0"); + REQUIRE(dependencyPackages.at(0)->Get().Version == "1.0"); } TEST_CASE("DependencyGraph_PathNoLoop", "[InstallFlow][workflow][dependencyGraph][dependencies]", ) @@ -1236,16 +1236,16 @@ TEST_CASE("DependencyGraph_PathNoLoop", "[InstallFlow][workflow][dependencyGraph context << ManagePackageDependencies(Resource::String::InstallAndUpgradeCommandsReportDependencies); - std::vector installers = context.Get(); + auto& dependencyPackages = context.Get(); REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::DependenciesFlowContainsLoop)) == std::string::npos); // Verify installers are called in order - REQUIRE(installers.size() == 4); - REQUIRE(installers.at(0).Manifest.Id == "B"); - REQUIRE(installers.at(1).Manifest.Id == "C"); - REQUIRE(installers.at(2).Manifest.Id == "G"); - REQUIRE(installers.at(3).Manifest.Id == "H"); + REQUIRE(dependencyPackages.size() == 4); + REQUIRE(dependencyPackages.at(0)->Get().Id == "B"); + REQUIRE(dependencyPackages.at(1)->Get().Id == "C"); + REQUIRE(dependencyPackages.at(2)->Get().Id == "G"); + REQUIRE(dependencyPackages.at(3)->Get().Id == "H"); } TEST_CASE("UpdateFlow_UpdateWithManifest", "[UpdateFlow][workflow]") From 1be7c87c87bedd98b6c150186766f2e25d95c819 Mon Sep 17 00:00:00 2001 From: Yao Sun Date: Tue, 16 Nov 2021 01:58:48 -0800 Subject: [PATCH 6/9] Fix selected installer telemetry --- .../Workflows/DependenciesFlow.cpp | 12 ++++++++++++ .../Workflows/ManifestComparator.cpp | 7 ------- src/AppInstallerCLICore/Workflows/UpdateFlow.cpp | 12 ++++++++++++ src/AppInstallerCLICore/Workflows/WorkflowBase.cpp | 10 ++++++++++ 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp b/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp index ac1f2ac2b1..ec2e34ae0b 100644 --- a/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp @@ -216,6 +216,18 @@ namespace AppInstaller::CLI::Workflow Execution::Context& dependencyContext = *dependencyContextPtr; auto previousThreadGlobals = dependencyContext.GetThreadGlobals().SetForCurrentThread(); + Logging::Telemetry().LogSelectedInstaller( + static_cast(itr->second.Installer.Arch), + itr->second.Installer.Url, + Manifest::InstallerTypeToString(itr->second.Installer.InstallerType), + Manifest::ScopeToString(itr->second.Installer.Scope), + itr->second.Installer.Locale); + + Logging::Telemetry().LogManifestFields( + itr->second.Manifest.Id, + itr->second.Manifest.DefaultLocalization.Get(), + itr->second.Manifest.Version); + // Extract the data needed for installing dependencyContext.Add(itr->second.PackageVersion); dependencyContext.Add(itr->second.Manifest); diff --git a/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp b/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp index 213599b9f8..162edb9e47 100644 --- a/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp +++ b/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp @@ -562,13 +562,6 @@ namespace AppInstaller::CLI::Workflow return { {}, std::move(inapplicabilitiesInstallers) }; } - Logging::Telemetry().LogSelectedInstaller( - static_cast(result->Arch), - result->Url, - Manifest::InstallerTypeToString(result->InstallerType), - Manifest::ScopeToString(result->Scope), - result->Locale); - return { *result, std::move(inapplicabilitiesInstallers) }; } diff --git a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp index 51a523e097..945191a7bb 100644 --- a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp @@ -68,6 +68,18 @@ namespace AppInstaller::CLI::Workflow continue; } + Logging::Telemetry().LogSelectedInstaller( + static_cast(installer->Arch), + installer->Url, + Manifest::InstallerTypeToString(installer->InstallerType), + Manifest::ScopeToString(installer->Scope), + installer->Locale); + + Logging::Telemetry().LogManifestFields( + manifest.Id, + manifest.DefaultLocalization.Get(), + manifest.Version); + // Since we already did installer selection, just populate the context Data manifest.ApplyLocale(installer->Locale); context.Add(std::move(manifest)); diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index e0561033aa..ccc87252da 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -983,6 +983,16 @@ namespace AppInstaller::CLI::Workflow } } + if (installer.has_value()) + { + Logging::Telemetry().LogSelectedInstaller( + static_cast(installer->Arch), + installer->Url, + Manifest::InstallerTypeToString(installer->InstallerType), + Manifest::ScopeToString(installer->Scope), + installer->Locale); + } + context.Add(installer); } From a984cdc23667a08459a145e6f56f80f7d4f8515d Mon Sep 17 00:00:00 2001 From: Yao Sun Date: Thu, 18 Nov 2021 15:53:11 -0800 Subject: [PATCH 7/9] pr comments --- src/AppInstallerCLICore/COMContext.cpp | 4 +- .../Commands/CompleteCommand.cpp | 2 +- .../ContextOrchestrator.cpp | 2 +- src/AppInstallerCLICore/Core.cpp | 7 +- src/AppInstallerCLICore/ExecutionContext.cpp | 7 +- src/AppInstallerCLICore/ExecutionContext.h | 12 +- .../Workflows/DependenciesFlow.cpp | 2 +- .../Workflows/ImportExportFlow.cpp | 2 +- .../Workflows/InstallFlow.cpp | 4 +- .../Workflows/UpdateFlow.cpp | 2 +- .../Workflows/WorkflowBase.cpp | 15 +- src/AppInstallerCLITests/WorkFlow.cpp | 161 ++++++++++++++---- .../AppInstallerLogging.cpp | 2 +- .../AppInstallerTelemetry.cpp | 76 ++++----- .../Public/AppInstallerLanguageUtilities.h | 10 ++ .../Public/AppInstallerTelemetry.h | 62 ++++--- .../Public/winget/ThreadGlobals.h | 10 +- src/AppInstallerCommonCore/ThreadGlobals.cpp | 25 +-- 18 files changed, 238 insertions(+), 167 deletions(-) diff --git a/src/AppInstallerCLICore/COMContext.cpp b/src/AppInstallerCLICore/COMContext.cpp index d767710d40..216bc07bb3 100644 --- a/src/AppInstallerCLICore/COMContext.cpp +++ b/src/AppInstallerCLICore/COMContext.cpp @@ -48,14 +48,14 @@ namespace AppInstaller::CLI::Execution { m_executionStage = executionStage; FireCallbacks(ReportType::ExecutionPhaseUpdate, 0, 0, ProgressType::None, m_executionStage); - Logging::Telemetry().SetExecutionStage(static_cast(m_executionStage)); + GetThreadGlobals().GetTelemetryLogger().SetExecutionStage(static_cast(m_executionStage)); } void COMContext::SetContextLoggers(const std::wstring_view telemetryCorrelationJson, const std::string& caller) { m_correlationData = telemetryCorrelationJson; - std::unique_ptr setThreadGlobalsToPreviousState = GetThreadGlobals().SetForCurrentThread(); + std::unique_ptr setThreadGlobalsToPreviousState = this->SetForCurrentThread(); SetLoggers(); GetThreadGlobals().GetTelemetryLogger().SetTelemetryCorrelationJson(telemetryCorrelationJson); diff --git a/src/AppInstallerCLICore/Commands/CompleteCommand.cpp b/src/AppInstallerCLICore/Commands/CompleteCommand.cpp index f0ebd156c0..7c6a22d7e3 100644 --- a/src/AppInstallerCLICore/Commands/CompleteCommand.cpp +++ b/src/AppInstallerCLICore/Commands/CompleteCommand.cpp @@ -55,7 +55,7 @@ namespace AppInstaller::CLI // Create a new Context to execute the Complete from auto subContextPtr = context.CreateSubContext(); Context& subContext = *subContextPtr; - auto previousThreadGlobals = subContext.GetThreadGlobals().SetForCurrentThread(); + auto previousThreadGlobals = subContext.SetForCurrentThread(); subContext.Reporter.SetChannel(Execution::Reporter::Channel::Completion); subContext.Add(std::move(data)); diff --git a/src/AppInstallerCLICore/ContextOrchestrator.cpp b/src/AppInstallerCLICore/ContextOrchestrator.cpp index ccc6e35b95..d5da60e9b6 100644 --- a/src/AppInstallerCLICore/ContextOrchestrator.cpp +++ b/src/AppInstallerCLICore/ContextOrchestrator.cpp @@ -110,7 +110,7 @@ namespace AppInstaller::CLI::Execution { std::unique_ptr command = item->PopNextCommand(); - std::unique_ptr setThreadGlobalsToPreviousState = item->GetContext().GetThreadGlobals().SetForCurrentThread(); + std::unique_ptr setThreadGlobalsToPreviousState = item->GetContext().SetForCurrentThread(); item->GetContext().GetThreadGlobals().GetTelemetryLogger().LogCommand(command->FullName()); command->ValidateArguments(item->GetContext().Args); diff --git a/src/AppInstallerCLICore/Core.cpp b/src/AppInstallerCLICore/Core.cpp index 799167c0ca..3097beb3dd 100644 --- a/src/AppInstallerCLICore/Core.cpp +++ b/src/AppInstallerCLICore/Core.cpp @@ -48,6 +48,10 @@ namespace AppInstaller::CLI Logging::UseGlobalTelemetryLoggerActivityIdOnly(); + Execution::Context context{ std::cout, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); + context.EnableCtrlHandler(); + // Enable all logging for this phase; we will update once we have the arguments Logging::Log().EnableChannel(Logging::Channel::All); Logging::Log().SetLevel(Logging::Level::Info); @@ -63,9 +67,6 @@ namespace AppInstaller::CLI // Initiate the background cleanup of the log file location. Logging::BeginLogFileCleanup(); - Execution::Context context{ std::cout, std::cin, nullptr /* Command execution always use global diagnostic logger */, Logging::Telemetry().CreateSubTraceLogger() }; - context.EnableCtrlHandler(); - context << Workflow::ReportExecutionStage(Workflow::ExecutionStage::ParseArgs); // Convert incoming wide char args to UTF8 diff --git a/src/AppInstallerCLICore/ExecutionContext.cpp b/src/AppInstallerCLICore/ExecutionContext.cpp index 65e197f5b3..6e592467c3 100644 --- a/src/AppInstallerCLICore/ExecutionContext.cpp +++ b/src/AppInstallerCLICore/ExecutionContext.cpp @@ -204,7 +204,7 @@ namespace AppInstaller::CLI::Execution } m_executionStage = stage; - Logging::Telemetry().SetExecutionStage(static_cast(m_executionStage)); + GetThreadGlobals().GetTelemetryLogger().SetExecutionStage(static_cast(m_executionStage)); } AppInstaller::ThreadLocalStorage::ThreadGlobals& Context::GetThreadGlobals() @@ -212,6 +212,11 @@ namespace AppInstaller::CLI::Execution return m_threadGlobals; } + std::unique_ptr Context::SetForCurrentThread() + { + return m_threadGlobals.SetForCurrentThread(); + } + #ifndef AICLI_DISABLE_TEST_HOOKS bool Context::ShouldExecuteWorkflowTask(const Workflow::WorkflowTask& task) { diff --git a/src/AppInstallerCLICore/ExecutionContext.h b/src/AppInstallerCLICore/ExecutionContext.h index 54a213c463..d087c8836f 100644 --- a/src/AppInstallerCLICore/ExecutionContext.h +++ b/src/AppInstallerCLICore/ExecutionContext.h @@ -70,14 +70,8 @@ namespace AppInstaller::CLI::Execution { Context(std::ostream& out, std::istream& in) : Reporter(out, in) {} - Context( - std::ostream& out, - std::istream& in, - std::shared_ptr diagnosticLogger, - std::unique_ptr&& telemetryLogger) : Reporter(out, in), m_threadGlobals(diagnosticLogger, std::move(telemetryLogger)) {} - // Constructor for creating a sub-context. - Context(Execution::Reporter& reporter, const ThreadLocalStorage::ThreadGlobals& threadGlobals) : + Context(Execution::Reporter& reporter, ThreadLocalStorage::ThreadGlobals& threadGlobals) : Reporter(reporter, Execution::Reporter::clone_t{}), m_threadGlobals(threadGlobals, ThreadLocalStorage::ThreadGlobals::create_sub_thread_globals_t{}) {} @@ -135,9 +129,11 @@ namespace AppInstaller::CLI::Execution virtual void SetExecutionStage(Workflow::ExecutionStage stage); - // Get Globals for Current Thread + // Get Globals for Current Context AppInstaller::ThreadLocalStorage::ThreadGlobals& GetThreadGlobals(); + std::unique_ptr SetForCurrentThread(); + #ifndef AICLI_DISABLE_TEST_HOOKS // Enable tests to override behavior bool ShouldExecuteWorkflowTask(const Workflow::WorkflowTask& task); diff --git a/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp b/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp index ec2e34ae0b..40efcaf708 100644 --- a/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp @@ -214,7 +214,7 @@ namespace AppInstaller::CLI::Workflow { auto dependencyContextPtr = context.CreateSubContext(); Execution::Context& dependencyContext = *dependencyContextPtr; - auto previousThreadGlobals = dependencyContext.GetThreadGlobals().SetForCurrentThread(); + auto previousThreadGlobals = dependencyContext.SetForCurrentThread(); Logging::Telemetry().LogSelectedInstaller( static_cast(itr->second.Installer.Arch), diff --git a/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp b/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp index f41b986dc4..f734799e66 100644 --- a/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp @@ -274,7 +274,7 @@ namespace AppInstaller::CLI::Workflow auto searchContextPtr = context.CreateSubContext(); Execution::Context& searchContext = *searchContextPtr; - auto previousThreadGlobals = searchContext.GetThreadGlobals().SetForCurrentThread(); + auto previousThreadGlobals = searchContext.SetForCurrentThread(); searchContext.Add(source); searchContext.Add(source.Search(searchRequest)); diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index 90a5ed9acd..9ed21b9324 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -193,7 +193,7 @@ namespace AppInstaller::CLI::Workflow { // Show agreements for each package Execution::Context& showContext = *packageContext; - auto previousThreadGlobals = showContext.GetThreadGlobals().SetForCurrentThread(); + auto previousThreadGlobals = showContext.SetForCurrentThread(); showContext << Workflow::ReportManifestIdentityWithVersion << @@ -429,7 +429,7 @@ namespace AppInstaller::CLI::Workflow // We want to do best effort to install all packages regardless of previous failures Execution::Context& installContext = *packageContext; - auto previousThreadGlobals = installContext.GetThreadGlobals().SetForCurrentThread(); + auto previousThreadGlobals = installContext.SetForCurrentThread(); installContext << Workflow::ReportIdentityAndInstallationDisclaimer; if (!m_ignorePackageDependencies) diff --git a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp index 945191a7bb..a64924d1e3 100644 --- a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp @@ -138,7 +138,7 @@ namespace AppInstaller::CLI::Workflow // We want to do best effort to update all applicable updates regardless on previous update failure auto updateContextPtr = context.CreateSubContext(); Execution::Context& updateContext = *updateContextPtr; - auto previousThreadGlobals = updateContext.GetThreadGlobals().SetForCurrentThread(); + auto previousThreadGlobals = updateContext.SetForCurrentThread(); updateContext.Add(match.Package); diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index ccc87252da..8a7b377c2c 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -247,7 +247,7 @@ namespace AppInstaller::CLI::Workflow catch (const wil::ResultException& re) { // Even though they are logged at their source, log again here for completeness. - Logging::Telemetry().LogException(Logging::FailureTypeResultException, re.what()); + Logging::Telemetry().LogException(Logging::FailureTypeEnum::ResultException, re.what()); context.Reporter.Error() << Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << GetUserPresentableMessage(re) << std::endl; @@ -256,7 +256,7 @@ namespace AppInstaller::CLI::Workflow catch (const winrt::hresult_error& hre) { std::string message = GetUserPresentableMessage(hre); - Logging::Telemetry().LogException(Logging::FailureTypeWinrtHResultError, message); + Logging::Telemetry().LogException(Logging::FailureTypeEnum::WinrtHResultError, message); context.Reporter.Error() << Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << message << std::endl; @@ -270,13 +270,13 @@ namespace AppInstaller::CLI::Workflow } catch (const Resource::ResourceOpenException& e) { - Logging::Telemetry().LogException(Logging::FailureTypeResourceOpen, e.what()); + Logging::Telemetry().LogException(Logging::FailureTypeEnum::ResourceOpen, e.what()); context.Reporter.Error() << GetUserPresentableMessage(e) << std::endl; return APPINSTALLER_CLI_ERROR_MISSING_RESOURCE_FILE; } catch (const std::exception& e) { - Logging::Telemetry().LogException(Logging::FailureTypeStdException, e.what()); + Logging::Telemetry().LogException(Logging::FailureTypeEnum::StdException, e.what()); context.Reporter.Error() << Resource::String::UnexpectedErrorExecutingCommand << ' ' << std::endl << GetUserPresentableMessage(e) << std::endl; @@ -285,7 +285,7 @@ namespace AppInstaller::CLI::Workflow catch (...) { LOG_CAUGHT_EXCEPTION(); - Logging::Telemetry().LogException(Logging::FailureTypeUnknown, {}); + Logging::Telemetry().LogException(Logging::FailureTypeEnum::Unknown, {}); context.Reporter.Error() << Resource::String::UnexpectedErrorExecutingCommand << " ???"_liv << std::endl; return APPINSTALLER_CLI_ERROR_COMMAND_FAILED; @@ -545,7 +545,6 @@ namespace AppInstaller::CLI::Workflow void ReportSearchResult(Execution::Context& context) { auto& searchResult = context.Get(); - Logging::Telemetry().LogSearchResultCount(searchResult.Matches.size()); bool sourceIsComposite = context.Get().IsComposite(); Execution::TableOutput<5> table(context.Reporter, @@ -621,7 +620,7 @@ namespace AppInstaller::CLI::Workflow } } - AICLI_TERMINATE_CONTEXT(overallHR); + context.SetTerminationHR(overallHR); } } } @@ -774,6 +773,8 @@ namespace AppInstaller::CLI::Workflow { auto& searchResult = context.Get(); + Logging::Telemetry().LogSearchResultCount(searchResult.Matches.size()); + if (searchResult.Matches.size() == 0) { Logging::Telemetry().LogNoAppMatch(); diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp index aa51d74125..5de3d30afa 100644 --- a/src/AppInstallerCLITests/WorkFlow.cpp +++ b/src/AppInstallerCLITests/WorkFlow.cpp @@ -672,6 +672,7 @@ TEST_CASE("ExeInstallFlowWithTestManifest", "[InstallFlow][workflow]") std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Exe.yaml").GetPath().u8string()); @@ -695,6 +696,7 @@ TEST_CASE("InstallFlowNonZeroExitCode", "[InstallFlow][workflow]") std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_NonZeroExitCode.yaml").GetPath().u8string()); @@ -719,6 +721,7 @@ TEST_CASE("InstallFlow_ExpectedReturnCodes", "[InstallFlow][workflow]") std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_ExpectedReturnCodes.yaml").GetPath().u8string()); context.Args.AddArg(Execution::Args::Type::Override, "/ExitCode 8"sv); @@ -739,6 +742,7 @@ TEST_CASE("InstallFlowWithNonApplicableArchitecture", "[InstallFlow][workflow]") std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_NoApplicableArchitecture.yaml").GetPath().u8string()); InstallCommand install({}); @@ -757,6 +761,7 @@ TEST_CASE("MSStoreInstallFlowWithTestManifest", "[InstallFlow][workflow]") std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForMSStore(context, false); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_MSStore.yaml").GetPath().u8string()); @@ -779,6 +784,7 @@ TEST_CASE("MsixInstallFlow_DownloadFlow", "[InstallFlow][workflow]") std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForMSIX(context); OverrideForUpdateInstallerMotw(context); // Todo: point to files from our repo when the repo goes public @@ -804,6 +810,7 @@ TEST_CASE("MsixInstallFlow_StreamingFlow", "[InstallFlow][workflow]") std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForMSIX(context); OverrideForCheckExistingInstaller(context); // Todo: point to files from our repo when the repo goes public @@ -832,6 +839,7 @@ TEST_CASE("MsiInstallFlow_DirectMsi", "[InstallFlow][workflow]") std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForDirectMsi(context); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallerArgTest_Msi_NoSwitches.yaml").GetPath().u8string()); context.Args.AddArg(Execution::Args::Type::Silent); @@ -854,6 +862,7 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs", "[InstallFlow][workflow]") { std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); // Default Msi type with no args passed in, no switches specified in manifest auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallerArgTest_Msi_NoSwitches.yaml")); context.Add(manifest); @@ -870,6 +879,7 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs", "[InstallFlow][workflow]") { std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); // Msi type with /silent and /log and /custom and /installlocation, no switches specified in manifest auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallerArgTest_Msi_NoSwitches.yaml")); context.Args.AddArg(Execution::Args::Type::Silent); @@ -887,6 +897,7 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs", "[InstallFlow][workflow]") { std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); // Msi type with /silent and /log and /custom and /installlocation, switches specified in manifest auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallerArgTest_Msi_WithSwitches.yaml")); context.Args.AddArg(Execution::Args::Type::Silent); @@ -905,6 +916,7 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs", "[InstallFlow][workflow]") { std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); // Default Inno type with no args passed in, no switches specified in manifest auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallerArgTest_Inno_NoSwitches.yaml")); context.Add(manifest); @@ -921,6 +933,7 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs", "[InstallFlow][workflow]") { std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); // Inno type with /silent and /log and /custom and /installlocation, no switches specified in manifest auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallerArgTest_Inno_NoSwitches.yaml")); context.Args.AddArg(Execution::Args::Type::Silent); @@ -938,6 +951,7 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs", "[InstallFlow][workflow]") { std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); // Inno type with /silent and /log and /custom and /installlocation, switches specified in manifest auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallerArgTest_Inno_WithSwitches.yaml")); context.Args.AddArg(Execution::Args::Type::Silent); @@ -956,6 +970,7 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs", "[InstallFlow][workflow]") { std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); // Override switch specified. The whole arg passed to installer is overridden. auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallerArgTest_Inno_WithSwitches.yaml")); context.Args.AddArg(Execution::Args::Type::Silent); @@ -976,6 +991,7 @@ TEST_CASE("InstallFlow_SearchAndInstall", "[InstallFlow][workflow]") std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForOpenSource(context); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::Query, "TestQueryReturnOne"sv); @@ -998,6 +1014,7 @@ TEST_CASE("InstallFlow_SearchFoundNoApp", "[InstallFlow][workflow]") { std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForOpenSource(context); context.Args.AddArg(Execution::Args::Type::Query, "TestQueryReturnZero"sv); @@ -1013,6 +1030,7 @@ TEST_CASE("InstallFlow_SearchFoundMultipleApp", "[InstallFlow][workflow]") { std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForOpenSource(context); context.Args.AddArg(Execution::Args::Type::Query, "TestQueryReturnTwo"sv); @@ -1030,6 +1048,7 @@ TEST_CASE("InstallFlow_LicenseAgreement", "[InstallFlow][workflow]") std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_LicenseAgreement.yaml").GetPath().u8string()); context.Args.AddArg(Execution::Args::Type::AcceptPackageAgreements); @@ -1057,6 +1076,7 @@ TEST_CASE("InstallFlow_LicenseAgreement_Prompt", "[InstallFlow][workflow]") std::ostringstream installOutput; TestContext context{ installOutput, installInput }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_LicenseAgreement.yaml").GetPath().u8string()); @@ -1086,6 +1106,7 @@ TEST_CASE("InstallFlow_LicenseAgreement_NotAccepted", "[InstallFlow][workflow]") std::ostringstream installOutput; TestContext context{ installOutput, installInput }; + auto previousThreadGlobals = context.SetForCurrentThread(); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_LicenseAgreement.yaml").GetPath().u8string()); InstallCommand install({}); @@ -1108,6 +1129,7 @@ TEST_CASE("ShowFlow_SearchAndShowAppInfo", "[ShowFlow][workflow]") { std::ostringstream showOutput; TestContext context{ showOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForOpenSource(context); context.Args.AddArg(Execution::Args::Type::Query, "TestQueryReturnOne"sv); @@ -1126,6 +1148,7 @@ TEST_CASE("ShowFlow_SearchAndShowAppVersion", "[ShowFlow][workflow]") { std::ostringstream showOutput; TestContext context{ showOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForOpenSource(context); context.Args.AddArg(Execution::Args::Type::Query, "TestQueryReturnOne"sv); context.Args.AddArg(Execution::Args::Type::ListVersions); @@ -1144,6 +1167,7 @@ TEST_CASE("ShowFlow_Dependencies", "[ShowFlow][workflow][dependencies]") { std::ostringstream showOutput; TestContext context{ showOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("Manifest-Good-AllDependencyTypes.yaml").GetPath().u8string()); TestUserSettings settings; @@ -1170,6 +1194,7 @@ TEST_CASE("DependencyGraph_SkipInstalled", "[InstallFlow][workflow][dependencyGr std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); Manifest manifest = CreateFakeManifestWithDependencies("DependenciesInstalled"); OverrideOpenDependencySource(context); @@ -1195,6 +1220,7 @@ TEST_CASE("DependencyGraph_validMinVersions", "[InstallFlow][workflow][dependenc std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); Manifest manifest = CreateFakeManifestWithDependencies("DependenciesValidMinVersions"); OverrideOpenDependencySource(context); OverrideForInstallMultiplePackages(context); @@ -1223,6 +1249,7 @@ TEST_CASE("DependencyGraph_PathNoLoop", "[InstallFlow][workflow][dependencyGraph std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); Manifest manifest = CreateFakeManifestWithDependencies("PathBetweenBranchesButNoLoop"); OverrideOpenDependencySource(context); OverrideForInstallMultiplePackages(context); @@ -1254,6 +1281,7 @@ TEST_CASE("UpdateFlow_UpdateWithManifest", "[UpdateFlow][workflow]") std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("UpdateFlowTest_Exe.yaml").GetPath().u8string()); @@ -1278,6 +1306,7 @@ TEST_CASE("UpdateFlow_UpdateWithManifestMSStore", "[UpdateFlow][workflow]") std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); OverrideForMSStore(context, true); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_MSStore.yaml").GetPath().u8string()); @@ -1301,6 +1330,7 @@ TEST_CASE("UpdateFlow_UpdateWithManifestAppNotInstalled", "[UpdateFlow][workflow std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallerArgTest_Inno_NoSwitches.yaml").GetPath().u8string()); @@ -1320,6 +1350,7 @@ TEST_CASE("UpdateFlow_UpdateWithManifestVersionAlreadyInstalled", "[UpdateFlow][ std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Exe.yaml").GetPath().u8string()); @@ -1339,6 +1370,7 @@ TEST_CASE("UpdateFlow_UpdateExe", "[UpdateFlow][workflow]") std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller"sv); @@ -1365,6 +1397,7 @@ TEST_CASE("UpdateFlow_UpdateMsix", "[UpdateFlow][workflow]") std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); OverrideForMSIX(context); context.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestMsixInstaller"sv); @@ -1383,6 +1416,7 @@ TEST_CASE("UpdateFlow_UpdateMSStore", "[UpdateFlow][workflow]") std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); OverrideForMSStore(context, true); context.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestMSStoreInstaller"sv); @@ -1406,6 +1440,7 @@ TEST_CASE("UpdateFlow_UpdateExeLatestAlreadyInstalled", "[UpdateFlow][workflow]" std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); context.Args.AddArg(Execution::Args::Type::Query, "TestExeInstallerWithLatestInstalled"sv); @@ -1425,6 +1460,7 @@ TEST_CASE("UpdateFlow_UpdateExeInstallerTypeNotApplicable", "[UpdateFlow][workfl std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); context.Args.AddArg(Execution::Args::Type::Query, "TestExeInstallerWithIncompatibleInstallerType"sv); @@ -1444,6 +1480,7 @@ TEST_CASE("UpdateFlow_UpdateExeInstallerTypeNotApplicableSpecificVersion", "[Upd std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); context.Args.AddArg(Execution::Args::Type::Query, "TestExeInstallerWithIncompatibleInstallerType"sv); context.Args.AddArg(Execution::Args::Type::Version, "2.0.0.0"sv); @@ -1464,6 +1501,7 @@ TEST_CASE("UpdateFlow_UpdateExeSpecificVersionNotFound", "[UpdateFlow][workflow] std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); context.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller"sv); context.Args.AddArg(Execution::Args::Type::Version, "1.2.3.4"sv); @@ -1484,6 +1522,7 @@ TEST_CASE("UpdateFlow_UpdateExeSpecificVersionNotApplicable", "[UpdateFlow][work std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); context.Args.AddArg(Execution::Args::Type::Query, "TestExeInstallerWithIncompatibleInstallerType"sv); context.Args.AddArg(Execution::Args::Type::Version, "1.0.0.0"sv); @@ -1506,6 +1545,7 @@ TEST_CASE("UpdateFlow_UpdateAllApplicable", "[UpdateFlow][workflow]") std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); OverrideForShellExecute(context); OverrideForMSIX(context); @@ -1528,6 +1568,7 @@ TEST_CASE("UpdateFlow_UpgradeWithDuplicateUpgradeItemsFound", "[UpdateFlow][work std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); // Installer should only be run once since the 2 upgrade items are same. OverrideForShellExecute(context, 1); @@ -1548,6 +1589,7 @@ TEST_CASE("UpdateFlow_Dependencies", "[UpdateFlow][workflow][dependencies]") std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller.Dependencies"sv); @@ -1573,6 +1615,7 @@ TEST_CASE("UpdateFlow_LicenseAgreement", "[UpdateFlow][workflow]") std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::Query, "TestInstallerWithLicenseAgreement"sv); @@ -1599,6 +1642,7 @@ TEST_CASE("UpdateFlow_LicenseAgreement_NotAccepted", "[UpdateFlow][workflow]") std::ostringstream updateOutput; TestContext context{ updateOutput, updateInput }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); context.Args.AddArg(Execution::Args::Type::Query, "TestInstallerWithLicenseAgreement"sv); @@ -1624,6 +1668,7 @@ TEST_CASE("UpdateFlow_All_LicenseAgreement", "[UpdateFlow][workflow]") std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context, /* upgradeUsesLicenses */ true); OverrideForShellExecute(context); OverrideForMSIX(context); @@ -1658,6 +1703,7 @@ TEST_CASE("UpdateFlow_All_LicenseAgreement_NotAccepted", "[UpdateFlow][workflow] std::ostringstream updateOutput; TestContext context{ updateOutput, updateInput }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context, /* upgradeUsesLicenses */ true); context.Args.AddArg(Execution::Args::Type::All); @@ -1684,6 +1730,7 @@ TEST_CASE("UninstallFlow_UninstallExe", "[UninstallFlow][workflow]") std::ostringstream uninstallOutput; TestContext context{ uninstallOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); OverrideForExeUninstall(context); context.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller"sv); @@ -1709,6 +1756,7 @@ TEST_CASE("UninstallFlow_UninstallMsix", "[UninstallFlow][workflow]") std::ostringstream uninstallOutput; TestContext context{ uninstallOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); OverrideForMSIXUninstall(context); context.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestMsixInstaller"sv); @@ -1732,6 +1780,7 @@ TEST_CASE("UninstallFlow_UninstallMSStore", "[UninstallFlow][workflow]") std::ostringstream uninstallOutput; TestContext context{ uninstallOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); OverrideForMSIXUninstall(context); context.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestMSStoreInstaller"sv); @@ -1755,6 +1804,7 @@ TEST_CASE("UninstallFlow_UninstallExeNotFound", "[UninstallFlow][workflow]") std::ostringstream uninstallOutput; TestContext context{ uninstallOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); context.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.MissingApp"sv); context.Args.AddArg(Execution::Args::Type::Silent); @@ -1775,6 +1825,7 @@ TEST_CASE("ExportFlow_ExportAll", "[ExportFlow][workflow]") std::ostringstream exportOutput; TestContext context{ exportOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); context.Args.AddArg(Execution::Args::Type::OutputFile, exportResultPath); @@ -1809,6 +1860,7 @@ TEST_CASE("ExportFlow_ExportAll_WithVersions", "[ExportFlow][workflow]") std::ostringstream exportOutput; TestContext context{ exportOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(context); context.Args.AddArg(Execution::Args::Type::OutputFile, exportResultPath); context.Args.AddArg(Execution::Args::Type::IncludeVersions); @@ -1845,6 +1897,7 @@ TEST_CASE("ImportFlow_Successful", "[ImportFlow][workflow]") std::ostringstream importOutput; TestContext context{ importOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForImportSource(context); OverrideForMSIX(context); OverrideForShellExecute(context); @@ -1865,6 +1918,7 @@ TEST_CASE("ImportFlow_PackageAlreadyInstalled", "[ImportFlow][workflow]") std::ostringstream importOutput; TestContext context{ importOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForImportSource(context, true); context.Args.AddArg(Execution::Args::Type::ImportFile, TestDataFile("ImportFile-Good-AlreadyInstalled.json").GetPath().string()); @@ -1883,6 +1937,7 @@ TEST_CASE("ImportFlow_IgnoreVersions", "[ImportFlow][workflow]") std::ostringstream importOutput; TestContext context{ importOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForImportSource(context); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::ImportFile, TestDataFile("ImportFile-Good-AlreadyInstalled.json").GetPath().string()); @@ -1902,6 +1957,7 @@ TEST_CASE("ImportFlow_MissingSource", "[ImportFlow][workflow]") std::ostringstream importOutput; TestContext context{ importOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); context.Args.AddArg(Execution::Args::Type::ImportFile, TestDataFile("ImportFile-Bad-UnknownSource.json").GetPath().string()); ImportCommand importCommand({}); @@ -1920,6 +1976,7 @@ TEST_CASE("ImportFlow_MissingPackage", "[ImportFlow][workflow]") std::ostringstream importOutput; TestContext context{ importOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForImportSource(context); context.Args.AddArg(Execution::Args::Type::ImportFile, TestDataFile("ImportFile-Bad-UnknownPackage.json").GetPath().string()); @@ -1939,6 +1996,7 @@ TEST_CASE("ImportFlow_IgnoreMissingPackage", "[ImportFlow][workflow]") std::ostringstream importOutput; TestContext context{ importOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForImportSource(context); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::ImportFile, TestDataFile("ImportFile-Bad-UnknownPackage.json").GetPath().string()); @@ -1959,6 +2017,7 @@ TEST_CASE("ImportFlow_MissingVersion", "[ImportFlow][workflow]") std::ostringstream importOutput; TestContext context{ importOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForImportSource(context); context.Args.AddArg(Execution::Args::Type::ImportFile, TestDataFile("ImportFile-Bad-UnknownPackageVersion.json").GetPath().string()); @@ -1976,6 +2035,7 @@ TEST_CASE("ImportFlow_MalformedJsonFile", "[ImportFlow][workflow]") { std::ostringstream importOutput; TestContext context{ importOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); context.Args.AddArg(Execution::Args::Type::ImportFile, TestDataFile("ImportFile-Bad-Malformed.json").GetPath().string()); ImportCommand importCommand({}); @@ -1990,6 +2050,7 @@ TEST_CASE("ImportFlow_InvalidJsonFile", "[ImportFlow][workflow]") { std::ostringstream importOutput; TestContext context{ importOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); context.Args.AddArg(Execution::Args::Type::ImportFile, TestDataFile("ImportFile-Bad-Invalid.json").GetPath().string()); ImportCommand importCommand({}); @@ -2006,6 +2067,7 @@ TEST_CASE("ImportFlow_MachineScope", "[ImportFlow][workflow]") std::ostringstream importOutput; TestContext context{ importOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForImportSource(context); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::ImportFile, TestDataFile("ImportFile-Good-MachineScope.json").GetPath().string()); @@ -2030,6 +2092,7 @@ TEST_CASE("ImportFlow_Dependencies", "[ImportFlow][workflow][dependencies]") std::ostringstream importOutput; TestContext context{ importOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForImportSource(context); OverrideForMSIX(context); OverrideForShellExecute(context); @@ -2055,6 +2118,7 @@ TEST_CASE("ImportFlow_LicenseAgreement", "[ImportFlow][workflow]") std::ostringstream importOutput; TestContext context{ importOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForImportSource(context); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::ImportFile, TestDataFile("ImportFile-Good-WithLicenseAgreement.json").GetPath().string()); @@ -2079,6 +2143,7 @@ TEST_CASE("ImportFlow_LicenseAgreement_NotAccepted", "[ImportFlow][workflow]") std::ostringstream importOutput; TestContext context{ importOutput, importInput }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForImportSource(context); context.Args.AddArg(Execution::Args::Type::ImportFile, TestDataFile("ImportFile-Good-WithLicenseAgreement.json").GetPath().string()); @@ -2119,6 +2184,7 @@ TEST_CASE("VerifyInstallerTrustLevelAndUpdateInstallerFileMotw", "[DownloadInsta std::ostringstream updateMotwOutput; TestContext context{ updateMotwOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); context.Add({ {}, {} }); context.Add(testInstallerPath); auto packageVersion = std::make_shared(Manifest{}); @@ -2149,6 +2215,7 @@ TEST_CASE("InstallFlowMultiLocale_RequirementNotSatisfied", "[InstallFlow][workf std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("Manifest-Good-MultiLocale.yaml").GetPath().u8string()); context.Args.AddArg(Execution::Args::Type::Locale, "en-US"sv); @@ -2168,6 +2235,7 @@ TEST_CASE("InstallFlowMultiLocale_RequirementSatisfied", "[InstallFlow][workflow std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("Manifest-Good-MultiLocale.yaml").GetPath().u8string()); context.Args.AddArg(Execution::Args::Type::Locale, "fr-FR"sv); @@ -2191,6 +2259,7 @@ TEST_CASE("InstallFlowMultiLocale_PreferenceNoBetterLocale", "[InstallFlow][work std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("Manifest-Good-MultiLocale.yaml").GetPath().u8string()); @@ -2216,6 +2285,7 @@ TEST_CASE("InstallFlowMultiLocale_PreferenceWithBetterLocale", "[InstallFlow][wo std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("Manifest-Good-MultiLocale.yaml").GetPath().u8string()); @@ -2244,6 +2314,7 @@ TEST_CASE("SourceAddFlow_Agreement", "[SourceAddFlow][workflow]") { std::ostringstream sourceAddOutput; TestContext context{ sourceAddOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForSourceAddWithAgreements(context); context.Args.AddArg(Execution::Args::Type::SourceName, "TestSource"sv); context.Args.AddArg(Execution::Args::Type::SourceType, "Microsoft.Test"sv); @@ -2270,6 +2341,7 @@ TEST_CASE("SourceAddFlow_Agreement_Prompt_Yes", "[SourceAddFlow][workflow]") std::istringstream sourceAddInput{ "y" }; std::ostringstream sourceAddOutput; TestContext context{ sourceAddOutput, sourceAddInput }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForSourceAddWithAgreements(context); context.Args.AddArg(Execution::Args::Type::SourceName, "TestSource"sv); context.Args.AddArg(Execution::Args::Type::SourceType, "Microsoft.Test"sv); @@ -2295,6 +2367,7 @@ TEST_CASE("SourceAddFlow_Agreement_Prompt_No", "[SourceAddFlow][workflow]") std::istringstream sourceAddInput{ "n" }; std::ostringstream sourceAddOutput; TestContext context{ sourceAddOutput, sourceAddInput }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForSourceAddWithAgreements(context, false); context.Args.AddArg(Execution::Args::Type::SourceName, "TestSource"sv); context.Args.AddArg(Execution::Args::Type::SourceType, "Microsoft.Test"sv); @@ -2318,6 +2391,7 @@ TEST_CASE("ValidateCommand_Dependencies", "[workflow][dependencies]") { std::ostringstream validateOutput; TestContext context{ validateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); context.Args.AddArg(Execution::Args::Type::ValidateManifest, TestDataFile("Manifest-Good-AllDependencyTypes.yaml").GetPath().u8string()); TestUserSettings settings; @@ -2345,6 +2419,7 @@ TEST_CASE("DependencyGraph_StackOrderIsOk", "[InstallFlow][workflow][dependencyG std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideOpenSourceForDependencies(context); OverrideForShellExecute(context, installationOrder); @@ -2372,6 +2447,7 @@ TEST_CASE("InstallerWithoutDependencies_RootDependenciesAreUsed", "[dependencies std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForShellExecute(context); OverrideDependencySource(context); @@ -2395,6 +2471,7 @@ TEST_CASE("DependenciesMultideclaration_InstallerDependenciesPreference", "[depe std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForShellExecute(context); OverrideDependencySource(context); @@ -2420,6 +2497,7 @@ TEST_CASE("InstallFlow_Dependencies", "[InstallFlow][workflow][dependencies]") std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForShellExecute(context); OverrideDependencySource(context); @@ -2464,6 +2542,7 @@ TEST_CASE("OpenSource_WithCustomHeader", "[OpenSource][CustomHeader]") std::ostringstream output; TestContext context{ output, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); context.Args.AddArg(Execution::Args::Type::Query, "TestQuery"sv); context.Args.AddArg(Execution::Args::Type::CustomHeader, customHeader); context.Args.AddArg(Execution::Args::Type::Source, details.Name); @@ -2476,39 +2555,51 @@ TEST_CASE("AdminSetting_LocalManifestFiles", "[LocalManifests][workflow]") { RemoveSetting(Stream::AdminSettings); - // If there's no admin setting, using local manifest should fail. - Execution::Args args; - args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Exe.yaml").GetPath().u8string()); - InstallCommand installCommand({}); - REQUIRE_THROWS(installCommand.ValidateArguments(args)); - - // Using settings command to enable local manifests - std::ostringstream settingsOutput; - TestContext context{ settingsOutput, std::cin }; - context.Args.AddArg(Execution::Args::Type::AdminSettingEnable, "LocalManifestFiles"sv); - context.Override({ EnsureRunningAsAdmin, [](TestContext&){} }); - SettingsCommand settings({}); - settings.Execute(context); - INFO(settingsOutput.str()); - - // Now using local manifests should succeed - Execution::Args args2; - args2.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Exe.yaml").GetPath().u8string()); - InstallCommand installCommand2({}); - REQUIRE_NOTHROW(installCommand2.ValidateArguments(args2)); - - // Using settings command to disable local manifests - std::ostringstream settingsOutput2; - TestContext context2{ settingsOutput2, std::cin }; - context2.Args.AddArg(Execution::Args::Type::AdminSettingDisable, "LocalManifestFiles"sv); - context2.Override({ EnsureRunningAsAdmin, [](TestContext&) {} }); - SettingsCommand settings2({}); - settings2.Execute(context2); - INFO(settingsOutput2.str()); - - // Now using local manifests should fail - Execution::Args args3; - args3.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Exe.yaml").GetPath().u8string()); - InstallCommand installCommand3({}); - REQUIRE_THROWS(installCommand3.ValidateArguments(args3)); + { + // If there's no admin setting, using local manifest should fail. + Execution::Args args; + args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Exe.yaml").GetPath().u8string()); + InstallCommand installCommand({}); + REQUIRE_THROWS(installCommand.ValidateArguments(args)); + } + + { + // Using settings command to enable local manifests + std::ostringstream settingsOutput; + TestContext context{ settingsOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); + context.Args.AddArg(Execution::Args::Type::AdminSettingEnable, "LocalManifestFiles"sv); + context.Override({ EnsureRunningAsAdmin, [](TestContext&) {} }); + SettingsCommand settings({}); + settings.Execute(context); + INFO(settingsOutput.str()); + } + + { + // Now using local manifests should succeed + Execution::Args args2; + args2.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Exe.yaml").GetPath().u8string()); + InstallCommand installCommand2({}); + REQUIRE_NOTHROW(installCommand2.ValidateArguments(args2)); + } + + { + // Using settings command to disable local manifests + std::ostringstream settingsOutput2; + TestContext context2{ settingsOutput2, std::cin }; + auto previousThreadGlobals = context2.SetForCurrentThread(); + context2.Args.AddArg(Execution::Args::Type::AdminSettingDisable, "LocalManifestFiles"sv); + context2.Override({ EnsureRunningAsAdmin, [](TestContext&) {} }); + SettingsCommand settings2({}); + settings2.Execute(context2); + INFO(settingsOutput2.str()); + } + + { + // Now using local manifests should fail + Execution::Args args3; + args3.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Exe.yaml").GetPath().u8string()); + InstallCommand installCommand3({}); + REQUIRE_THROWS(installCommand3.ValidateArguments(args3)); + } } diff --git a/src/AppInstallerCommonCore/AppInstallerLogging.cpp b/src/AppInstallerCommonCore/AppInstallerLogging.cpp index d3da54d03d..b19e243cbc 100644 --- a/src/AppInstallerCommonCore/AppInstallerLogging.cpp +++ b/src/AppInstallerCommonCore/AppInstallerLogging.cpp @@ -141,7 +141,7 @@ namespace AppInstaller::Logging DiagnosticLogger& Log() { ThreadLocalStorage::ThreadGlobals* pThreadGlobals = ThreadLocalStorage::ThreadGlobals::GetForCurrentThread(); - if (pThreadGlobals && pThreadGlobals->ContainsDiagnosticLogger()) + if (pThreadGlobals) { return pThreadGlobals->GetDiagnosticLogger(); } diff --git a/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp b/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp index 88f7279c2b..bec0fea927 100644 --- a/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp +++ b/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp @@ -15,7 +15,7 @@ #define AICLI_TraceLoggingWriteActivity(_eventName_,...) TraceLoggingWriteActivity(\ g_hTraceProvider,\ _eventName_,\ -s_useGlobalTelemetryActivityId ? s_globalTelemetryLoggerActivityId : GetActivityId(),\ +s_useGlobalTelemetryActivityId ? &s_globalTelemetryLoggerActivityId : GetActivityId(),\ nullptr,\ TraceLoggingCountedUtf8String(m_caller.c_str(), static_cast(m_caller.size()), "Caller"),\ TraceLoggingPackedFieldEx(m_telemetryCorrelationJsonW.c_str(), static_cast((m_telemetryCorrelationJsonW.size() + 1) * sizeof(wchar_t)), TlgInUNICODESTRING, TlgOutJSON, "CvJson"),\ @@ -52,49 +52,54 @@ namespace AppInstaller::Logging // TODO: Temporary code to keep existing telemetry behavior static bool s_useGlobalTelemetryActivityId = false; - static const GUID* s_globalTelemetryLoggerActivityId = nullptr; + static GUID s_globalTelemetryLoggerActivityId = GUID_NULL; void __stdcall wilResultLoggingCallback(const wil::FailureInfo& info) noexcept { Telemetry().LogFailure(info); } - UINT32 ConvertWilFailureTypeToFailureType(wil::FailureType failureType) + FailureTypeEnum ConvertWilFailureTypeToFailureType(wil::FailureType failureType) { switch (failureType) { case wil::FailureType::Exception: - return FailureTypeResultException; + return FailureTypeEnum::ResultException; case wil::FailureType::Return: - return FailureTypeResultReturn; + return FailureTypeEnum::ResultReturn; case wil::FailureType::Log: - return FailureTypeResultLog; + return FailureTypeEnum::ResultLog; case wil::FailureType::FailFast: - return FailureTypeResultFailFast; + return FailureTypeEnum::ResultFailFast; default: - return FailureTypeUnknown; + return FailureTypeEnum::Unknown; } } - std::string_view LogExceptionTypeToString(UINT32 exceptionType) + std::string_view LogExceptionTypeToString(FailureTypeEnum exceptionType) { switch (exceptionType) { - case FailureTypeResultException: + case FailureTypeEnum::ResultException: return "wil::ResultException"sv; - case FailureTypeWinrtHResultError: + case FailureTypeEnum::WinrtHResultError: return "winrt::hresult_error"sv; - case FailureTypeResourceOpen: + case FailureTypeEnum::ResourceOpen: return "ResourceOpenException"sv; - case FailureTypeStdException: + case FailureTypeEnum::StdException: return "std::exception"sv; - case FailureTypeUnknown: + case FailureTypeEnum::Unknown: default: return "unknown"sv; } } } + TelemetrySummary::TelemetrySummary(const TelemetrySummary& other) + { + this->IsCOMCall = other.IsCOMCall; + } + TelemetryTraceLogger::TelemetryTraceLogger() { std::ignore = CoCreateGuid(&m_activityId); @@ -184,21 +189,11 @@ namespace AppInstaller::Logging { THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !this->m_isInitialized); - auto subTraceLogger = std::make_unique(); + auto subTraceLogger = std::make_unique(*this); subTraceLogger->m_parentActivityId = this->m_activityId; subTraceLogger->m_subExecutionId = s_subExecutionId++; - // Copy remaining fields from parent. - subTraceLogger->m_isSettingEnabled = this->m_isSettingEnabled; - subTraceLogger->m_isRuntimeEnabled = this->m_isRuntimeEnabled.load(); - subTraceLogger->m_isInitialized = this->m_isInitialized.load(); - subTraceLogger->m_executionStage = this->m_executionStage.load(); - subTraceLogger->m_telemetryCorrelationJsonW = this->m_telemetryCorrelationJsonW; - subTraceLogger->m_caller = this->m_caller; - subTraceLogger->m_userProfile = this->m_userProfile; - subTraceLogger->m_summary = this->m_summary; - return subTraceLogger; } @@ -317,15 +312,16 @@ namespace AppInstaller::Logging TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); - m_summary.CommandTerminationHResult = hr; - m_summary.CommandTerminationFile = file; - m_summary.CommandTerminationLine = static_cast(line); + m_summary.FailureHResult = hr; + m_summary.FailureType = FailureTypeEnum::CommandTermination; + m_summary.FailureFile = file; + m_summary.FailureLine = static_cast(line); } AICLI_LOG(CLI, Error, << "Terminating context: 0x" << SetHRFormat << hr << " at " << file << ":" << line); } - void TelemetryTraceLogger::LogException(UINT32 type, std::string_view message) const noexcept + void TelemetryTraceLogger::LogException(FailureTypeEnum type, std::string_view message) const noexcept { auto exceptionTypeString = LogExceptionTypeToString(type); @@ -394,8 +390,6 @@ namespace AppInstaller::Logging TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); - - m_summary.NoAppMatch = true; } AICLI_LOG(CLI, Info, << "No app found matching input criteria"); @@ -410,8 +404,6 @@ namespace AppInstaller::Logging TraceLoggingUInt32(m_subExecutionId, "SubExecutionId"), TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); - - m_summary.MultipleAppMatch = true; } AICLI_LOG(CLI, Info, << "Multiple apps found matching input criteria"); @@ -719,12 +711,9 @@ namespace AppInstaller::Logging AICLI_TraceLoggingWStringView(m_summary.FailureMessage, "FailureMessage"), AICLI_TraceLoggingStringView(m_summary.FailureModule, "FailureModule"), TraceLoggingUInt32(m_summary.FailureThreadId, "FailureThreadId"), - TraceLoggingUInt32(m_summary.FailureType, "FailureType"), + TraceLoggingUInt32(static_cast(m_summary.FailureType), "FailureType"), AICLI_TraceLoggingStringView(m_summary.FailureFile, "FailureFile"), TraceLoggingUInt32(m_summary.FailureLine, "FailureLine"), - TraceLoggingHResult(m_summary.CommandTerminationHResult, "CommandTerminationHResult"), - AICLI_TraceLoggingStringView(m_summary.CommandTerminationFile, "CommandTerminationFile"), - TraceLoggingUInt64(m_summary.CommandTerminationLine, "CommandTerminationLine"), TraceLoggingBool(m_summary.IsCOMCall, "IsCOMCall"), AICLI_TraceLoggingStringView(m_summary.Command, "Command"), TraceLoggingBool(m_summary.CommandSuccess, "CommandSuccess"), @@ -734,8 +723,6 @@ namespace AppInstaller::Logging AICLI_TraceLoggingStringView(m_summary.PackageVersion, "PackageVersion"), AICLI_TraceLoggingStringView(m_summary.Channel, "Channel"), AICLI_TraceLoggingStringView(m_summary.SourceIdentifier, "SourceIdentifier"), - TraceLoggingBool(m_summary.NoAppMatch, "NoAppMatch"), - TraceLoggingBool(m_summary.MultipleAppMatch, "MultipleAppMatch"), TraceLoggingInt32(m_summary.InstallerArchitecture, "InstallerArchitecture"), AICLI_TraceLoggingStringView(m_summary.InstallerUrl, "InstallerUrl"), AICLI_TraceLoggingStringView(m_summary.InstallerType, "InstallerType"), @@ -808,7 +795,7 @@ namespace AppInstaller::Logging } #endif ThreadLocalStorage::ThreadGlobals* pThreadGlobals = ThreadLocalStorage::ThreadGlobals::GetForCurrentThread(); - if (pThreadGlobals && pThreadGlobals->ContainsTelemetryLogger()) + if (pThreadGlobals) { return pThreadGlobals->GetTelemetryLogger(); } @@ -816,12 +803,6 @@ namespace AppInstaller::Logging { static TelemetryTraceLogger processGlobalTelemetry; processGlobalTelemetry.TryInitialize(); - - if (s_useGlobalTelemetryActivityId && s_globalTelemetryLoggerActivityId == nullptr) - { - s_globalTelemetryLoggerActivityId = processGlobalTelemetry.GetActivityId(); - } - return processGlobalTelemetry; } } @@ -834,6 +815,7 @@ namespace AppInstaller::Logging void UseGlobalTelemetryLoggerActivityIdOnly() { s_useGlobalTelemetryActivityId = true; + std::ignore = CoCreateGuid(&s_globalTelemetryLoggerActivityId); } DisableTelemetryScope::DisableTelemetryScope() @@ -855,5 +837,5 @@ namespace AppInstaller::Logging { s_TelemetryTraceLogger_TestOverride = std::move(ttl); } -#endif +#endif } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerLanguageUtilities.h b/src/AppInstallerCommonCore/Public/AppInstallerLanguageUtilities.h index 0b673d346c..2a8f2c92fb 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerLanguageUtilities.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerLanguageUtilities.h @@ -152,3 +152,13 @@ std::enable_if_t, std::ostream&> operator<<(std::ostream& out, { return out << AppInstaller::ToIntegral(e); } + +template +struct CopyConstructibleAtomic : public std::atomic +{ + using std::atomic::atomic; + using std::atomic::operator=; + + CopyConstructibleAtomic(const CopyConstructibleAtomic& other) : + std::atomic(other.load()) {} +}; diff --git a/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h b/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h index c0c4e28763..5c8da57590 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h @@ -15,36 +15,48 @@ namespace AppInstaller::Settings namespace AppInstaller::Logging { - static constexpr UINT32 FailureTypeNone = 0; - // Failure type from FailureInfo in result_macros.h - static constexpr UINT32 FailureTypeResultException = 1; // THROW_... - static constexpr UINT32 FailureTypeResultReturn = 2; // RETURN_..._LOG or RETURN_..._MSG - static constexpr UINT32 FailureTypeResultLog = 3; // LOG_... - static constexpr UINT32 FailureTypeResultFailFast = 4; // FAIL_FAST_... - // Other failure types from LogException() - static constexpr UINT32 FailureTypeWinrtHResultError = 101; - static constexpr UINT32 FailureTypeResourceOpen = 102; - static constexpr UINT32 FailureTypeStdException = 103; - static constexpr UINT32 FailureTypeUnknown = 104; + enum class FailureTypeEnum : UINT32 + { + None = 0x0, + + // Failure type from FailureInfo in result_macros.h + ResultException = 0x1, // THROW_... + ResultReturn = 0x2, // RETURN_..._LOG or RETURN_..._MSG + ResultLog = 0x3, // LOG_... + ResultFailFast = 0x4, // FAIL_FAST_... + + // Other failure types from LogException() + Unknown = 0x10000, + WinrtHResultError = 0x10001, + ResourceOpen = 0x10002, + StdException = 0x10003, + + // Command termination + CommandTermination = 0x20000, + }; // Contains all fields logged through the TelemetryTraceLogger. Last write wins. // This will be used to report a summary event upon destruction of the TelemetryTraceLogger. struct TelemetrySummary { - // Log wil failure, exception + TelemetrySummary() = default; + + // Selectively copy member fields for copy constructor; + TelemetrySummary(const TelemetrySummary& other); + TelemetrySummary& operator=(const TelemetrySummary&) = default; + + TelemetrySummary(TelemetrySummary&&) = default; + TelemetrySummary& operator=(TelemetrySummary&&) = default; + + // Log wil failure, exception, command termination HRESULT FailureHResult = S_OK; std::wstring FailureMessage; std::string FailureModule; UINT32 FailureThreadId = 0; - UINT32 FailureType = FailureTypeNone; + FailureTypeEnum FailureType = FailureTypeEnum::None; std::string FailureFile; UINT32 FailureLine = 0; - // LogCommandTermination - HRESULT CommandTerminationHResult = S_OK; - std::string CommandTerminationFile; - UINT64 CommandTerminationLine = 0; - // LogStartup bool IsCOMCall = false; @@ -64,12 +76,6 @@ namespace AppInstaller::Logging std::string Channel; std::string SourceIdentifier; - // LogNoAppMatch - bool NoAppMatch = false; - - // LogMultipleAppMatch - bool MultipleAppMatch = false; - // LogSelectedInstaller INT32 InstallerArchitecture = -1; std::string InstallerUrl; @@ -176,7 +182,7 @@ namespace AppInstaller::Logging void LogCommandTermination(HRESULT hr, std::string_view file, size_t line) const noexcept; // Logs the invoked command termination. - void LogException(UINT32 type, std::string_view message) const noexcept; + void LogException(FailureTypeEnum type, std::string_view message) const noexcept; // Logs whether the manifest used in workflow is local void LogIsManifestLocal(bool isLocalManifest) const noexcept; @@ -257,10 +263,10 @@ namespace AppInstaller::Logging 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 }; + CopyConstructibleAtomic m_isRuntimeEnabled{ true }; + CopyConstructibleAtomic m_isInitialized{ false }; - std::atomic_uint32_t m_executionStage{ 0 }; + CopyConstructibleAtomic m_executionStage{ 0 }; GUID m_activityId = GUID_NULL; GUID m_parentActivityId = GUID_NULL; diff --git a/src/AppInstallerCommonCore/Public/winget/ThreadGlobals.h b/src/AppInstallerCommonCore/Public/winget/ThreadGlobals.h index 0c463c7a5f..aa07178400 100644 --- a/src/AppInstallerCommonCore/Public/winget/ThreadGlobals.h +++ b/src/AppInstallerCommonCore/Public/winget/ThreadGlobals.h @@ -14,20 +14,12 @@ namespace AppInstaller::ThreadLocalStorage ThreadGlobals() = default; ~ThreadGlobals() = default; - // Constructor to create a ThreadGlobals with given diagnostic and telemetry loggers. - // During SetForCurrentThread, no new diagnostic or telemetry loggers will be created. - ThreadGlobals( - std::shared_ptr diagnosticLogger, - std::unique_ptr&& telemetryLogger); - // Request that a sub ThreadGlobals be constructed from the given parent. struct create_sub_thread_globals_t {}; - ThreadGlobals(const ThreadGlobals& parent, create_sub_thread_globals_t); + ThreadGlobals(ThreadGlobals& parent, create_sub_thread_globals_t); - bool ContainsDiagnosticLogger() const; AppInstaller::Logging::DiagnosticLogger& GetDiagnosticLogger(); - bool ContainsTelemetryLogger() const; AppInstaller::Logging::TelemetryTraceLogger& GetTelemetryLogger(); // Set Globals for Current Thread diff --git a/src/AppInstallerCommonCore/ThreadGlobals.cpp b/src/AppInstallerCommonCore/ThreadGlobals.cpp index 248ef15619..13bec7c1fd 100644 --- a/src/AppInstallerCommonCore/ThreadGlobals.cpp +++ b/src/AppInstallerCommonCore/ThreadGlobals.cpp @@ -6,33 +6,20 @@ namespace AppInstaller::ThreadLocalStorage using namespace AppInstaller::Logging; // Set and return Globals for Current Thread - static ThreadGlobals* SetOrGetThreadGlobals(bool setThreadGlobals, ThreadGlobals* pThreadGlobals = nullptr); - - ThreadGlobals::ThreadGlobals(std::shared_ptr diagnosticLogger, std::unique_ptr&& telemetryLogger) - : m_pDiagnosticLogger(std::move(diagnosticLogger)), m_pTelemetryLogger(std::move(telemetryLogger)) + static ThreadGlobals* SetOrGetThreadGlobals(bool setThreadGlobals, ThreadGlobals* pThreadGlobals = nullptr); + + ThreadGlobals::ThreadGlobals(ThreadGlobals& parent, create_sub_thread_globals_t) { + parent.Initialize(); + m_pDiagnosticLogger = parent.m_pDiagnosticLogger; + m_pTelemetryLogger = parent.m_pTelemetryLogger->CreateSubTraceLogger(); // Flip the initialization flag std::call_once(m_loggerInitOnceFlag, []() {}); } - ThreadGlobals::ThreadGlobals(const ThreadGlobals& parent, create_sub_thread_globals_t) - : ThreadGlobals(parent.m_pDiagnosticLogger, parent.m_pTelemetryLogger ? parent.m_pTelemetryLogger->CreateSubTraceLogger() : nullptr) - { - } - - bool ThreadGlobals::ContainsDiagnosticLogger() const - { - return m_pDiagnosticLogger != nullptr; - } - DiagnosticLogger& ThreadGlobals::GetDiagnosticLogger() { return *(m_pDiagnosticLogger); - } - - bool ThreadGlobals::ContainsTelemetryLogger() const - { - return m_pTelemetryLogger != nullptr; } TelemetryTraceLogger& ThreadGlobals::GetTelemetryLogger() From 739e2a5f12080de832d72fa1c3b34c64c0c6002c Mon Sep 17 00:00:00 2001 From: Yao Sun Date: Fri, 19 Nov 2021 12:46:10 -0800 Subject: [PATCH 8/9] Fix e2e tests --- src/AppInstallerCLICore/ExecutionContext.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/AppInstallerCLICore/ExecutionContext.cpp b/src/AppInstallerCLICore/ExecutionContext.cpp index 6e592467c3..e2ff82c821 100644 --- a/src/AppInstallerCLICore/ExecutionContext.cpp +++ b/src/AppInstallerCLICore/ExecutionContext.cpp @@ -184,6 +184,7 @@ namespace AppInstaller::CLI::Execution void Context::SetTerminationHR(HRESULT hr) { m_terminationHR = hr; + m_isTerminated = FAILED(hr); } void Context::Cancel(bool exitIfStuck, bool bypassUser) From fb2f4e5a96ab3ce2801f540350d90c98162a00b8 Mon Sep 17 00:00:00 2001 From: Yao Sun Date: Fri, 19 Nov 2021 17:08:16 -0800 Subject: [PATCH 9/9] pr comments --- src/AppInstallerCLICore/Core.cpp | 14 +++++++------- src/AppInstallerCLICore/ExecutionContext.cpp | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/AppInstallerCLICore/Core.cpp b/src/AppInstallerCLICore/Core.cpp index 5a77335298..9baa0b3936 100644 --- a/src/AppInstallerCLICore/Core.cpp +++ b/src/AppInstallerCLICore/Core.cpp @@ -51,6 +51,13 @@ namespace AppInstaller::CLI { init_apartment(); +#ifndef AICLI_DISABLE_TEST_HOOKS + if (Settings::User().Get()) + { + Debugging::EnableSelfInitiatedMinidump(); + } +#endif + Logging::UseGlobalTelemetryLoggerActivityIdOnly(); Execution::Context context{ std::cout, std::cin }; @@ -63,13 +70,6 @@ namespace AppInstaller::CLI Logging::AddFileLogger(); Logging::EnableWilFailureTelemetry(); -#ifndef AICLI_DISABLE_TEST_HOOKS - if (Settings::User().Get()) - { - Debugging::EnableSelfInitiatedMinidump(); - } -#endif - // Set output to UTF8 ConsoleOutputCPRestore utf8CP(CP_UTF8); diff --git a/src/AppInstallerCLICore/ExecutionContext.cpp b/src/AppInstallerCLICore/ExecutionContext.cpp index e2ff82c821..0447b156ee 100644 --- a/src/AppInstallerCLICore/ExecutionContext.cpp +++ b/src/AppInstallerCLICore/ExecutionContext.cpp @@ -184,7 +184,7 @@ namespace AppInstaller::CLI::Execution void Context::SetTerminationHR(HRESULT hr) { m_terminationHR = hr; - m_isTerminated = FAILED(hr); + m_isTerminated = true; } void Context::Cancel(bool exitIfStuck, bool bypassUser)