commit c20c7177c496c0c06c049b0d8f13c4cf85d131fd parent ec46ec319b5b76c8552b3482418101c5653dede5 Author: JohnMcPMS <johnmcp@microsoft.com> Date: Mon, 3 Mar 2025 09:53:54 -0800 Fix logging channel setting (#5261) ## Issue The user setting to control the logging channels was no longer being respected after a previous change to enable all channels during initialization (and to send the initialization logs to the debug stream). ## Change `EnableChannel` for logging has always been semantically "add the given channel(s) to the list of enabled channels". Every place that we were calling `EnableChannel` was really semantically wanting to set the list of enabled channels rather than add to it. This change creates a new function `SetEnabledChannels` that behaves that way; it simply sets the enabled channel mask to the given value. All callers of `EnableChannel` were changed to use this new function. Diffstat:
10 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/src/AppInstallerCLICore/COMContext.cpp b/src/AppInstallerCLICore/COMContext.cpp @@ -80,10 +80,10 @@ namespace AppInstaller::CLI::Execution { // Set up debug string logging during initialization Logging::OutputDebugStringLogger::Add(); - Logging::Log().EnableChannel(Logging::Channel::All); + Logging::Log().SetEnabledChannels(Logging::Channel::All); Logging::Log().SetLevel(Logging::Level::Verbose); - Logging::Log().EnableChannel(channel.has_value() ? channel.value() : Settings::User().Get<Settings::Setting::LoggingChannelPreference>()); + Logging::Log().SetEnabledChannels(channel.has_value() ? channel.value() : Settings::User().Get<Settings::Setting::LoggingChannelPreference>()); Logging::Log().SetLevel(level.has_value() ? level.value() : Settings::User().Get<Settings::Setting::LoggingLevelPreference>()); // TODO: Log to file for COM API calls only when debugging in visual studio diff --git a/src/AppInstallerCLICore/Core.cpp b/src/AppInstallerCLICore/Core.cpp @@ -70,7 +70,7 @@ namespace AppInstaller::CLI #ifndef AICLI_DISABLE_TEST_HOOKS // We have to do this here so the auto minidump config initialization gets caught Logging::OutputDebugStringLogger::Add(); - Logging::Log().EnableChannel(Logging::Channel::All); + Logging::Log().SetEnabledChannels(Logging::Channel::All); Logging::Log().SetLevel(Logging::Level::Verbose); if (Settings::User().Get<Settings::Setting::EnableSelfInitiatedMinidump>()) @@ -88,10 +88,10 @@ namespace AppInstaller::CLI // Set up debug string logging during initialization Logging::OutputDebugStringLogger::Add(); - Logging::Log().EnableChannel(Logging::Channel::All); + Logging::Log().SetEnabledChannels(Logging::Channel::All); Logging::Log().SetLevel(Logging::Level::Verbose); - Logging::Log().EnableChannel(Settings::User().Get<Settings::Setting::LoggingChannelPreference>()); + Logging::Log().SetEnabledChannels(Settings::User().Get<Settings::Setting::LoggingChannelPreference>()); Logging::Log().SetLevel(Settings::User().Get<Settings::Setting::LoggingLevelPreference>()); Logging::FileLogger::Add(); Logging::OutputDebugStringLogger::Remove(); @@ -192,7 +192,7 @@ namespace AppInstaller::CLI #ifndef AICLI_DISABLE_TEST_HOOKS // We have to do this here so the auto minidump config initialization gets caught Logging::OutputDebugStringLogger::Add(); - Logging::Log().EnableChannel(Logging::Channel::All); + Logging::Log().SetEnabledChannels(Logging::Channel::All); Logging::Log().SetLevel(Logging::Level::Verbose); if (Settings::User().Get<Settings::Setting::EnableSelfInitiatedMinidump>()) @@ -211,7 +211,7 @@ namespace AppInstaller::CLI #ifndef AICLI_DISABLE_TEST_HOOKS // We have to do this here so the auto minidump config initialization gets caught Logging::OutputDebugStringLogger::Add(); - Logging::Log().EnableChannel(Logging::Channel::All); + Logging::Log().SetEnabledChannels(Logging::Channel::All); Logging::Log().SetLevel(Logging::Level::Verbose); if (Settings::User().Get<Settings::Setting::EnableSelfInitiatedMinidump>()) diff --git a/src/AppInstallerCLITests/main.cpp b/src/AppInstallerCLITests/main.cpp @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #define CATCH_CONFIG_RUNNER #include <catch.hpp> @@ -137,7 +137,7 @@ int main(int argc, char** argv) // Disable SQL by default, as it generates 10s of MBs of log file and // increases the full test run time by 60% or more. // By not creating a log target, it will all be thrown away. - Logging::Log().EnableChannel(Logging::Channel::All); + Logging::Log().SetEnabledChannels(Logging::Channel::All); if (!keepSQLLogging) { Logging::Log().DisableChannel(Logging::Channel::SQL); diff --git a/src/AppInstallerRepositoryCore/InstallerMetadataCollectionContext.cpp b/src/AppInstallerRepositoryCore/InstallerMetadataCollectionContext.cpp @@ -981,7 +981,7 @@ namespace AppInstaller::Repository::Metadata auto threadGlobalsLifetime = threadGlobals.SetForCurrentThread(); Logging::Log().SetLevel(Logging::Level::Info); - Logging::Log().EnableChannel(Logging::Channel::All); + Logging::Log().SetEnabledChannels(Logging::Channel::All); Logging::EnableWilFailureTelemetry(); Logging::TraceLogger::Add(); diff --git a/src/AppInstallerSharedLib/AppInstallerLogging.cpp b/src/AppInstallerSharedLib/AppInstallerLogging.cpp @@ -124,6 +124,11 @@ namespace AppInstaller::Logging WI_SetAllFlags(m_enabledChannels, channel); } + void DiagnosticLogger::SetEnabledChannels(Channel channel) + { + m_enabledChannels = channel; + } + void DiagnosticLogger::DisableChannel(Channel channel) { WI_ClearAllFlags(m_enabledChannels, channel); diff --git a/src/AppInstallerSharedLib/Public/AppInstallerLogging.h b/src/AppInstallerSharedLib/Public/AppInstallerLogging.h @@ -134,9 +134,12 @@ namespace AppInstaller::Logging // Removes all loggers. void RemoveAllLoggers(); - // Enables the given channel. + // Enables the given channel(s), in addition to the currently enabled channels. void EnableChannel(Channel channel); + // The given channel mask will become the only enabled channels. + void SetEnabledChannels(Channel channel); + // Disables the given channel. void DisableChannel(Channel channel); diff --git a/src/Microsoft.Management.Configuration/ConfigurationProcessor.cpp b/src/Microsoft.Management.Configuration/ConfigurationProcessor.cpp @@ -119,7 +119,7 @@ namespace winrt::Microsoft::Management::Configuration::implementation THROW_HR_IF(APPINSTALLER_CLI_ERROR_BLOCKED_BY_POLICY, !::AppInstaller::Settings::GroupPolicies().IsEnabled(::AppInstaller::Settings::TogglePolicy::Policy::Configuration)); AppInstaller::Logging::DiagnosticLogger& logger = m_threadGlobals.GetDiagnosticLogger(); - logger.EnableChannel(AppInstaller::Logging::Channel::All); + logger.SetEnabledChannels(AppInstaller::Logging::Channel::All); logger.SetLevel(AppInstaller::Logging::Level::Verbose); logger.AddLogger(std::make_unique<ConfigurationProcessorDiagnosticsLogger>(*this)); } diff --git a/src/Microsoft.Management.Deployment/PackageManagerSettings.cpp b/src/Microsoft.Management.Deployment/PackageManagerSettings.cpp @@ -50,7 +50,7 @@ namespace winrt::Microsoft::Management::Deployment::implementation success = AppInstaller::Settings::TryInitializeCustomUserSettings(AppInstaller::Utility::ConvertToUTF8(settingsContent)); if (success) { - AppInstaller::Logging::Log().EnableChannel(AppInstaller::Settings::User().Get<AppInstaller::Settings::Setting::LoggingChannelPreference>()); + AppInstaller::Logging::Log().SetEnabledChannels(AppInstaller::Settings::User().Get<AppInstaller::Settings::Setting::LoggingChannelPreference>()); AppInstaller::Logging::Log().SetLevel(AppInstaller::Settings::User().Get<AppInstaller::Settings::Setting::LoggingLevelPreference>()); } }); diff --git a/src/WinGetUtil/Exports.cpp b/src/WinGetUtil/Exports.cpp @@ -55,7 +55,7 @@ extern "C" // Intentionally release to leave the local ThreadGlobals. previous.release(); // Enable all logs for now. - AppInstaller::Logging::Log().EnableChannel(AppInstaller::Logging::Channel::All); + AppInstaller::Logging::Log().SetEnabledChannels(AppInstaller::Logging::Channel::All); AppInstaller::Logging::Log().SetLevel(AppInstaller::Logging::Level::Verbose); AppInstaller::Logging::EnableWilFailureTelemetry(); }); diff --git a/src/WindowsPackageManager/ConfigurationStaticFunctions.cpp b/src/WindowsPackageManager/ConfigurationStaticFunctions.cpp @@ -49,7 +49,7 @@ namespace ConfigurationShim { auto threadGlobalsRestore = m_threadGlobals.SetForCurrentThread(); auto& diagnosticsLogger = m_threadGlobals.GetDiagnosticLogger(); - diagnosticsLogger.EnableChannel(AppInstaller::Logging::Channel::All); + diagnosticsLogger.SetEnabledChannels(AppInstaller::Logging::Channel::All); diagnosticsLogger.SetLevel(AppInstaller::Logging::Level::Verbose); diagnosticsLogger.AddLogger(std::make_unique<AppInstaller::Logging::FileLogger>("ConfigStatics"sv));