commit ad0337871964951de642bfc778bc558ee4190e00
parent 896424540b795753eba358b6405db203d907b4c5
Author: JohnMcPMS <johnmcp@microsoft.com>
Date: Fri, 8 Dec 2023 09:05:03 -0800
Allow user settings to control logging channels (#3955)
Add a user setting to control the enabled logging channels, then disable
the `SQL` by default. This channel is responsible for 95% of the log
lines in a verbose file, yet they are only helpful when debugging a very
specific set of issues that does not occur often.
Additionally, make the COM logging use the settings for logging (both
channels and level).
Diffstat:
10 files changed, 178 insertions(+), 44 deletions(-)
diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt
@@ -40,6 +40,7 @@ BEFACEF
BFirst
bght
BITMAPINFOHEADER
+bitspace
bkup
blargle
blockedbypolicy
diff --git a/doc/Settings.md b/doc/Settings.md
@@ -194,14 +194,31 @@ If set to true, the `telemetry.disable` setting will prevent any event from bein
## Logging
-The `logging` settings control the level of detail in log files. `--verbose-logs` will override this setting and always creates a verbose log.
-Defaults to `info` if value is not set or is invalid
+The `logging` settings control the level of detail in log files.
### level
+ `--verbose-logs` will override this setting and always creates a verbose log.
+Defaults to `info` if value is not set or is invalid.
+
+```json
+ "logging": {
+ "level": "verbose" | "info" | "warning" | "error" | "critical"
+ },
+```
+
+### channels
+
+The valid values in this array are defined in the function `GetChannelFromName` in the [logging code](../src/AppInstallerSharedLib/AppInstallerLogging.cpp). These align with the ***channel identifier*** found in the log files. For example, ***`CORE`*** in:
+```
+2023-12-06 19:17:07.988 [CORE] WinGet, version [1.7.0-preview], activity [{24A91EA8-46BE-47A1-B65C-CEBCE90B8675}]
+```
+
+In addition, there are special values that cover multiple channels. `default` is the default set of channels, while `all` is all of the channels. Invalid values are ignored.
+
```json
"logging": {
- "level": ["verbose", "info", "warning", "error", "critical"]
+ "channels": ["default"]
},
```
diff --git a/schemas/JSON/settings/settings.schema.0.2.json b/schemas/JSON/settings/settings.schema.0.2.json
@@ -50,6 +50,29 @@
"error",
"critical"
]
+ },
+ "channels": {
+ "description": "The logging channels to enable",
+ "type": "array",
+ "items": {
+ "uniqueItems": "true",
+ "type": "string",
+ "enum": [
+ "fail",
+ "cli",
+ "sql",
+ "repo",
+ "yaml",
+ "core",
+ "test",
+ "config",
+ "default",
+ "all"
+ ],
+ "maxLength": 20
+ },
+ "minItems": 0,
+ "maxItems": 20
}
}
},
diff --git a/src/AppInstallerCLICore/COMContext.cpp b/src/AppInstallerCLICore/COMContext.cpp
@@ -77,8 +77,8 @@ namespace AppInstaller::CLI::Execution
void COMContext::SetLoggers()
{
- Logging::Log().SetLevel(Logging::Level::Info);
- Logging::Log().EnableChannel(Logging::Channel::All);
+ Logging::Log().EnableChannel(Settings::User().Get<Settings::Setting::LoggingChannelPreference>());
+ Logging::Log().SetLevel(Settings::User().Get<Settings::Setting::LoggingLevelPreference>());
// TODO: Log to file for COM API calls only when debugging in visual studio
Logging::FileLogger::Add(s_comLogFileNamePrefix);
diff --git a/src/AppInstallerCLICore/Core.cpp b/src/AppInstallerCLICore/Core.cpp
@@ -64,8 +64,7 @@ namespace AppInstaller::CLI
Execution::Context context{ std::cout, std::cin };
auto previousThreadGlobals = context.SetForCurrentThread();
- // Enable all logging for this phase; we will update once we have the arguments
- Logging::Log().EnableChannel(Logging::Channel::All);
+ Logging::Log().EnableChannel(Settings::User().Get<Settings::Setting::LoggingChannelPreference>());
Logging::Log().SetLevel(Settings::User().Get<Settings::Setting::LoggingLevelPreference>());
Logging::FileLogger::Add();
Logging::EnableWilFailureTelemetry();
diff --git a/src/AppInstallerCLITests/UserSettings.cpp b/src/AppInstallerCLITests/UserSettings.cpp
@@ -576,3 +576,49 @@ TEST_CASE("SettingsMaxResumes", "[settings]")
REQUIRE(userSettingTest.Get<Setting::MaxResumes>() == 5);
}
}
+
+TEST_CASE("LoggingChannels", "[settings]")
+{
+ auto again = DeleteUserSettingsFiles();
+
+ SECTION("Not provided")
+ {
+ std::string_view json = R"({ })";
+ SetSetting(Stream::PrimaryUserSettings, json);
+ UserSettingsTest userSettingTest;
+
+ REQUIRE(userSettingTest.Get<Setting::LoggingChannelPreference>() == Channel::Defaults);
+ }
+ SECTION("No channels")
+ {
+ std::string_view json = R"({ "logging": { "channels": [] } })";
+ SetSetting(Stream::PrimaryUserSettings, json);
+ UserSettingsTest userSettingTest;
+
+ REQUIRE(userSettingTest.Get<Setting::LoggingChannelPreference>() == Channel::None);
+ }
+ SECTION("Default")
+ {
+ std::string_view json = R"({ "logging": { "channels": ["default"] } })";
+ SetSetting(Stream::PrimaryUserSettings, json);
+ UserSettingsTest userSettingTest;
+
+ REQUIRE(userSettingTest.Get<Setting::LoggingChannelPreference>() == Channel::Defaults);
+ }
+ SECTION("Multiple")
+ {
+ std::string_view json = R"({ "logging": { "channels": ["core","Repo","YAML"] } })";
+ SetSetting(Stream::PrimaryUserSettings, json);
+ UserSettingsTest userSettingTest;
+
+ REQUIRE(userSettingTest.Get<Setting::LoggingChannelPreference>() == (Channel::Core | Channel::Repo | Channel::YAML));
+ }
+ SECTION("Some invalid")
+ {
+ std::string_view json = R"({ "logging": { "channels": ["cli","sql","INVALID"] } })";
+ SetSetting(Stream::PrimaryUserSettings, json);
+ UserSettingsTest userSettingTest;
+
+ REQUIRE(userSettingTest.Get<Setting::LoggingChannelPreference>() == (Channel::CLI | Channel::SQL));
+ }
+}
diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h
@@ -97,6 +97,7 @@ namespace AppInstaller::Settings
NetworkWingetAlternateSourceURL,
// Logging
LoggingLevelPreference,
+ LoggingChannelPreference,
// Uninstall behavior
UninstallPurgePortablePackage,
// Download behavior
@@ -187,6 +188,7 @@ namespace AppInstaller::Settings
#endif
// Logging
SETTINGMAPPING_SPECIALIZATION(Setting::LoggingLevelPreference, std::string, Logging::Level, Logging::Level::Info, ".logging.level"sv);
+ SETTINGMAPPING_SPECIALIZATION(Setting::LoggingChannelPreference, std::vector<std::string>, Logging::Channel, Logging::Channel::Defaults, ".logging.channels"sv);
// Interactivity
SETTINGMAPPING_SPECIALIZATION(Setting::InteractivityDisable, bool, bool, false, ".interactivity.disable"sv);
diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp
@@ -435,6 +435,18 @@ namespace AppInstaller::Settings
}
return {};
}
+
+ WINGET_VALIDATE_SIGNATURE(LoggingChannelPreference)
+ {
+ Logging::Channel result = Logging::Channel::None;
+
+ for (auto const& entry : value)
+ {
+ result |= GetChannelFromName(entry);
+ }
+
+ return result;
+ }
}
#ifndef AICLI_DISABLE_TEST_HOOKS
diff --git a/src/AppInstallerSharedLib/AppInstallerLogging.cpp b/src/AppInstallerSharedLib/AppInstallerLogging.cpp
@@ -8,28 +8,7 @@
namespace AppInstaller::Logging
{
- namespace
- {
- template <typename E>
- std::underlying_type_t<E> AsNum(E e)
- {
- return static_cast<std::underlying_type_t<E>>(e);
- }
-
- uint64_t ConvertChannelToBitmask(Channel channel)
- {
- if (channel == Channel::All)
- {
- return std::numeric_limits<uint64_t>::max();
- }
- else
- {
- return (1ull << AsNum(channel));
- }
- }
- }
-
- char const* GetChannelName(Channel channel)
+ std::string_view GetChannelName(Channel channel)
{
switch(channel)
{
@@ -45,6 +24,54 @@ namespace AppInstaller::Logging
}
}
+ Channel GetChannelFromName(std::string_view channel)
+ {
+ std::string lowerChannel = Utility::ToLower(channel);
+
+ if (lowerChannel == "fail")
+ {
+ return Channel::Fail;
+ }
+ else if (lowerChannel == "cli")
+ {
+ return Channel::CLI;
+ }
+ else if (lowerChannel == "sql")
+ {
+ return Channel::SQL;
+ }
+ else if (lowerChannel == "repo")
+ {
+ return Channel::Repo;
+ }
+ else if (lowerChannel == "yaml")
+ {
+ return Channel::YAML;
+ }
+ else if (lowerChannel == "core")
+ {
+ return Channel::Core;
+ }
+ else if (lowerChannel == "test")
+ {
+ return Channel::Test;
+ }
+ else if (lowerChannel == "conf" || lowerChannel == "config")
+ {
+ return Channel::Config;
+ }
+ else if (lowerChannel == "default" || lowerChannel == "defaults")
+ {
+ return Channel::Defaults;
+ }
+ else if (lowerChannel == "all")
+ {
+ return Channel::All;
+ }
+
+ return Channel::None;
+ }
+
size_t GetMaxChannelNameLength() { return 4; }
void DiagnosticLogger::AddLogger(std::unique_ptr<ILogger>&& logger)
@@ -89,12 +116,12 @@ namespace AppInstaller::Logging
void DiagnosticLogger::EnableChannel(Channel channel)
{
- m_enabledChannels |= ConvertChannelToBitmask(channel);
+ WI_SetAllFlags(m_enabledChannels, channel);
}
void DiagnosticLogger::DisableChannel(Channel channel)
{
- m_enabledChannels &= ~ConvertChannelToBitmask(channel);
+ WI_ClearAllFlags(m_enabledChannels, channel);
}
void DiagnosticLogger::SetLevel(Level level)
@@ -110,8 +137,8 @@ namespace AppInstaller::Logging
bool DiagnosticLogger::IsEnabled(Channel channel, Level level) const
{
return (!m_loggers.empty() &&
- (m_enabledChannels & ConvertChannelToBitmask(channel)) != 0 &&
- (AsNum(level) >= AsNum(m_enabledLevel)));
+ WI_IsAnyFlagSet(m_enabledChannels, channel) &&
+ (ToIntegral(level) >= ToIntegral(m_enabledLevel)));
}
void DiagnosticLogger::Write(Channel channel, Level level, std::string_view message)
diff --git a/src/AppInstallerSharedLib/Public/AppInstallerLogging.h b/src/AppInstallerSharedLib/Public/AppInstallerLogging.h
@@ -48,19 +48,26 @@ namespace AppInstaller::Logging
// Channels enable large groups of logs to be enabled or disabled together.
enum class Channel : uint32_t
{
- Fail,
- CLI,
- SQL,
- Repo,
- YAML,
- Core,
- Test,
- Config,
- All,
+ Fail = 0x1,
+ CLI = 0x2,
+ SQL = 0x4,
+ Repo = 0x8,
+ YAML = 0x10,
+ Core = 0x20,
+ Test = 0x40,
+ Config = 0x80,
+ None = 0,
+ All = 0xFFFFFFFF,
+ Defaults = All & ~SQL,
};
+ DEFINE_ENUM_FLAG_OPERATORS(Channel);
+
// Gets the channel's name as a string.
- char const* GetChannelName(Channel channel);
+ std::string_view GetChannelName(Channel channel);
+
+ // Gets the channel from it's name.
+ Channel GetChannelFromName(std::string_view channel);
// Gets the maximum channel name length in characters.
size_t GetMaxChannelNameLength();
@@ -153,7 +160,7 @@ namespace AppInstaller::Logging
private:
std::vector<std::unique_ptr<ILogger>> m_loggers;
- uint64_t m_enabledChannels = 0;
+ Channel m_enabledChannels = Channel::None;
Level m_enabledLevel = Level::Info;
};
