Skip to content

Track and log changes to settings #17678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Aug 23, 2024
Merged
4 changes: 4 additions & 0 deletions src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,10 @@ namespace winrt::TerminalApp::implementation
return;
}
}
else
{
_settings.LogSettingChanges(true);
}

if (initialLoad)
{
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsEditor/MainPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

void MainPage::SaveButton_Click(const IInspectable& /*sender*/, const RoutedEventArgs& /*args*/)
{
_settingsClone.LogSettingChanges(false);
_settingsClone.WriteSettingsToDisk();
}

Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/TerminalSettingsModel/ActionMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
Json::Value ToJson() const;
Json::Value KeyBindingsToJson() const;
bool FixupsAppliedDuringLoad() const;
void LogSettingChanges(std::set<std::string_view>& changes, std::string_view& context) const;

// modification
bool RebindKeys(const Control::KeyChord& oldKeys, const Control::KeyChord& newKeys);
Expand Down Expand Up @@ -104,6 +105,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation

std::vector<Model::Command> _updateLocalSnippetCache(winrt::hstring currentWorkingDirectory);

void _logCommand(const Model::Command& cmd);

Windows::Foundation::Collections::IMap<hstring, Model::ActionAndArgs> _AvailableActionsCache{ nullptr };
Windows::Foundation::Collections::IMap<hstring, Model::Command> _NameMapCache{ nullptr };
Windows::Foundation::Collections::IMap<Control::KeyChord, Model::Command> _GlobalHotkeysCache{ nullptr };
Expand Down Expand Up @@ -138,6 +141,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation

til::shared_mutex<std::unordered_map<hstring, std::vector<Model::Command>>> _cwdLocalSnippetsCache{};

std::set<std::string_view> _changeLog;

friend class SettingsModelUnitTests::KeyBindingsTests;
friend class SettingsModelUnitTests::DeserializationTests;
friend class SettingsModelUnitTests::TerminalSettingsTests;
Expand Down
12 changes: 11 additions & 1 deletion src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// Now check if this is a command block
if (jsonBlock.isMember(JsonKey(CommandsKey)) || jsonBlock.isMember(JsonKey(ActionKey)))
{
AddAction(*Command::FromJson(jsonBlock, warnings, origin), keys);
auto command = Command::FromJson(jsonBlock, warnings, origin);
command->LogSettingChanges(_changeLog);
AddAction(*command, keys);

if (jsonBlock.isMember(JsonKey(KeysKey)))
{
Expand Down Expand Up @@ -156,4 +158,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation

return keybindingsList;
}

void ActionMap::LogSettingChanges(std::set<std::string_view>& changes, std::string_view& context) const
{
for (const auto& setting : _changeLog)
{
changes.emplace(fmt::format(FMT_COMPILE("{}.{}"), context, setting));
}
}
}
38 changes: 37 additions & 1 deletion src/cascadia/TerminalSettingsModel/AppearanceConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,27 +90,42 @@ Json::Value AppearanceConfig::ToJson() const
void AppearanceConfig::LayerJson(const Json::Value& json)
{
JsonUtils::GetValueForKey(json, ForegroundKey, _Foreground);
_logSettingIfSet(ForegroundKey, _Foreground.has_value());

JsonUtils::GetValueForKey(json, BackgroundKey, _Background);
_logSettingIfSet(BackgroundKey, _Background.has_value());

JsonUtils::GetValueForKey(json, SelectionBackgroundKey, _SelectionBackground);
_logSettingIfSet(SelectionBackgroundKey, _SelectionBackground.has_value());

JsonUtils::GetValueForKey(json, CursorColorKey, _CursorColor);
_logSettingIfSet(CursorColorKey, _CursorColor.has_value());

JsonUtils::GetValueForKey(json, LegacyAcrylicTransparencyKey, _Opacity);
JsonUtils::GetValueForKey(json, OpacityKey, _Opacity, JsonUtils::OptionalConverter<float, IntAsFloatPercentConversionTrait>{});
_logSettingIfSet(OpacityKey, _Opacity.has_value());

if (json["colorScheme"].isString())
{
// to make the UI happy, set ColorSchemeName.
JsonUtils::GetValueForKey(json, ColorSchemeKey, _DarkColorSchemeName);
_LightColorSchemeName = _DarkColorSchemeName;
_logSettingSet(ColorSchemeKey);
}
else if (json["colorScheme"].isObject())
{
// to make the UI happy, set ColorSchemeName to whatever the dark value is.
JsonUtils::GetValueForKey(json["colorScheme"], "dark", _DarkColorSchemeName);
JsonUtils::GetValueForKey(json["colorScheme"], "light", _LightColorSchemeName);

_logSettingSet("colorScheme.dark");
_logSettingSet("colorScheme.light");
}

#define APPEARANCE_SETTINGS_LAYER_JSON(type, name, jsonKey, ...) \
JsonUtils::GetValueForKey(json, jsonKey, _##name);
JsonUtils::GetValueForKey(json, jsonKey, _##name); \
_logSettingIfSet(jsonKey, _##name.has_value());

MTSM_APPEARANCE_SETTINGS(APPEARANCE_SETTINGS_LAYER_JSON)
#undef APPEARANCE_SETTINGS_LAYER_JSON
}
Expand Down Expand Up @@ -156,3 +171,24 @@ winrt::hstring AppearanceConfig::ExpandedBackgroundImagePath()
return winrt::hstring{ wil::ExpandEnvironmentStringsW<std::wstring>(path.c_str()) };
}
}

void AppearanceConfig::_logSettingSet(std::string_view setting)
{
_changeLog.emplace(setting);
}

void AppearanceConfig::_logSettingIfSet(std::string_view setting, const bool isSet)
{
if (isSet)
{
_logSettingSet(setting);
}
}

void AppearanceConfig::LogSettingChanges(std::set<std::string_view>& changes, std::string_view& context) const
{
for (const auto& setting : _changeLog)
{
changes.emplace(fmt::format(FMT_COMPILE("{}.{}"), context, setting));
}
}
5 changes: 5 additions & 0 deletions src/cascadia/TerminalSettingsModel/AppearanceConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
static winrt::com_ptr<AppearanceConfig> CopyAppearance(const AppearanceConfig* source, winrt::weak_ref<Profile> sourceProfile);
Json::Value ToJson() const;
void LayerJson(const Json::Value& json);
void LogSettingChanges(std::set<std::string_view>& changes, std::string_view& context) const;

Model::Profile SourceProfile();

Expand All @@ -52,5 +53,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation

private:
winrt::weak_ref<Profile> _sourceProfile;
std::set<std::string_view> _changeLog;

void _logSettingSet(std::string_view setting);
void _logSettingIfSet(std::string_view setting, const bool isSet);
};
}
6 changes: 6 additions & 0 deletions src/cascadia/TerminalSettingsModel/CascadiaSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
std::unordered_map<winrt::hstring, winrt::com_ptr<implementation::ColorScheme>> colorSchemes;
std::unordered_map<winrt::hstring, winrt::hstring> colorSchemeRemappings;
bool fixupsAppliedDuringLoad{ false };
std::set<std::string_view> themesChangeLog;

void clear();
};
Expand Down Expand Up @@ -96,6 +97,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
void _executeGenerator(const IDynamicProfileGenerator& generator);

std::unordered_set<std::wstring_view> _ignoredNamespaces;
std::set<std::string_view> themesChangeLog;
// See _getNonUserOriginProfiles().
size_t _userProfileCount = 0;
};
Expand Down Expand Up @@ -150,6 +152,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation

void ExpandCommands();

void LogSettingChanges(bool isJsonLoad) const;

private:
static const std::filesystem::path& _settingsPath();
static const std::filesystem::path& _releaseSettingsPath();
Expand All @@ -173,13 +177,15 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
void _validateThemeExists();

void _researchOnLoad();
std::set<std::string_view> _logSettingChanges() const;

// user settings
winrt::hstring _hash;
winrt::com_ptr<implementation::GlobalAppSettings> _globals = winrt::make_self<implementation::GlobalAppSettings>();
winrt::com_ptr<implementation::Profile> _baseLayerProfile = winrt::make_self<implementation::Profile>();
winrt::Windows::Foundation::Collections::IObservableVector<Model::Profile> _allProfiles = winrt::single_threaded_observable_vector<Model::Profile>();
winrt::Windows::Foundation::Collections::IObservableVector<Model::Profile> _activeProfiles = winrt::single_threaded_observable_vector<Model::Profile>();
std::set<std::string_view> _themesChangeLog{};

// load errors
winrt::Windows::Foundation::Collections::IVector<Model::SettingsLoadWarnings> _warnings = winrt::single_threaded_vector<Model::SettingsLoadWarnings>();
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/CascadiaSettings.idl
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ namespace Microsoft.Terminal.Settings.Model

CascadiaSettings Copy();
void WriteSettingsToDisk();
void LogSettingChanges(Boolean isJsonLoad);

String Hash { get; };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ void ParsedSettings::clear()
profilesByGuid.clear();
colorSchemes.clear();
fixupsAppliedDuringLoad = false;
themesChangeLog.clear();
}

// This is a convenience method used by the CascadiaSettings constructor.
Expand Down Expand Up @@ -642,6 +643,12 @@ void SettingsLoader::_parse(const OriginTag origin, const winrt::hstring& source
// versions of these themes overriding the built-in ones.
continue;
}

if (origin != OriginTag::InBox)
{
static std::string_view themesContext{ "themes" };
theme->LogSettingChanges(settings.themesChangeLog, themesContext);
}
settings.globals->AddTheme(*theme);
}
}
Expand Down Expand Up @@ -1206,6 +1213,7 @@ CascadiaSettings::CascadiaSettings(SettingsLoader&& loader) :
_allProfiles = winrt::single_threaded_observable_vector(std::move(allProfiles));
_activeProfiles = winrt::single_threaded_observable_vector(std::move(activeProfiles));
_warnings = winrt::single_threaded_vector(std::move(warnings));
_themesChangeLog = std::move(loader.userSettings.themesChangeLog);

_resolveDefaultProfile();
_resolveNewTabMenuProfiles();
Expand Down Expand Up @@ -1580,3 +1588,70 @@ void CascadiaSettings::_resolveNewTabMenuProfilesSet(const IVector<Model::NewTab
}
}
}

std::set<std::string_view> CascadiaSettings::_logSettingChanges() const
{
// aggregate setting changes
std::set<std::string_view> changes;
static std::string_view globalContext{ "global" };
_globals->LogSettingChanges(changes, globalContext);

static std::string_view actionContext{ "action" };
winrt::get_self<implementation::ActionMap>(_globals->ActionMap())->LogSettingChanges(changes, actionContext);

static std::string_view profileContext{ "profile" };
for (const auto& profile : _allProfiles)
{
winrt::get_self<Profile>(profile)->LogSettingChanges(changes, profileContext);
}

static std::string_view profileDefaultsContext{ "profileDefaults" };
_baseLayerProfile->LogSettingChanges(changes, profileDefaultsContext);

// DO NOT CALL Theme::LogSettingChanges!!
// We already collected the changes when we loaded the JSON
for (const auto& change : _themesChangeLog)
{
changes.insert(change);
}

return changes;
}

void CascadiaSettings::LogSettingChanges(bool isJsonLoad) const
{
// Only do this if we're actually being sampled
if (!TraceLoggingProviderEnabled(g_hSettingsModelProvider, 0, MICROSOFT_KEYWORD_MEASURES))
{
return;
}

const auto changes = _logSettingChanges();

// report changes
for (const auto& change : changes)
{
// A `isJsonLoad ? "JsonSettingsChanged" : "UISettingsChanged"`
// would be nice, but that apparently isn't allowed in the macro below.
// Also, there's guidance to not send too much data all in one event,
// so we'll be sending a ton of events here.
if (isJsonLoad)
{
TraceLoggingWrite(g_hSettingsModelProvider,
"JsonSettingsChanged",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we may get flagged with a "garrulous event". this is a lot of data. there might be a way to TraceLog an array??? i don't actually know.

my concern is that we'll get absolutely whacked with data for any even moderately complicated settings file

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a limit of 65535 bytes per event so that's my concern (though idk how valid it may be) in packing it all together into one.

Even then though, if we sent it over as an array, won't that exclude the stuff we actually want to learn? Like, if we received events with a payload of...

  • globals.initialRows, globals.wordDelimiters, globals.copyOnSelect
  • globals.wordDelimiters, globals.copyOnSelect

We would just see the data as 2 hits with a payload of an array each whereas we want to track "how many hits each setting got (i.e. globals.initialRows=1, globals.wordDelimiters=2, globals.copyOnSelect=2). Right?

TraceLoggingDescription("Event emitted when settings change"),
TraceLoggingValue(change.data()),
TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));
}
else
{
TraceLoggingWrite(g_hSettingsModelProvider,
"UISettingsChanged",
TraceLoggingDescription("Event emitted when settings change"),
TraceLoggingValue(change.data()),
TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));
}
}
}
53 changes: 53 additions & 0 deletions src/cascadia/TerminalSettingsModel/Command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -741,4 +741,57 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation

return winrt::single_threaded_vector<Model::Command>(std::move(result));
}

void Command::LogSettingChanges(std::set<std::string_view>& changes)
{
if (_IterateOn != ExpandCommandType::None)
{
switch (_IterateOn)
{
case ExpandCommandType::Profiles:
changes.insert(fmt::format(FMT_COMPILE("{}.{}"), IterateOnKey, "profiles"));
break;
case ExpandCommandType::ColorSchemes:
changes.insert(fmt::format(FMT_COMPILE("{}.{}"), IterateOnKey, "schemes"));
break;
}
}

if (!_Description.empty())
{
changes.insert(fmt::format(FMT_COMPILE("{}"), DescriptionKey));
}

if (IsNestedCommand())
{
changes.insert(fmt::format(FMT_COMPILE("{}"), CommandsKey));
}
else
{
const auto json{ ActionAndArgs::ToJson(ActionAndArgs()) };
if (json.isString())
{
// covers actions w/out args
// - "command": "unbound" --> "unbound"
// - "command": "copy" --> "copy"
changes.insert(fmt::format(FMT_COMPILE("{}"), json.asString()));
}
else
{
// covers actions w/ args
// - "command": { "action": "copy", "singleLine": true } --> "copy.singleLine"
// - "command": { "action": "copy", "singleLine": true, "dismissSelection": true } --> "copy.singleLine", "copy.dismissSelection"

std::string_view shortcutActionName{ json[JsonKey("action")].asString() };

auto members = json.getMemberNames();
members.erase(std::remove_if(members.begin(), members.end(), [](const auto& member) { return member == "action"; }), members.end());

for (const auto& actionArg : members)
{
changes.insert(fmt::format(FMT_COMPILE("{}.{}"), shortcutActionName, actionArg));
}
}
}
}
}
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/Command.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
const Json::Value& json,
const OriginTag origin);
Json::Value ToJson() const;
void LogSettingChanges(std::set<std::string_view>& changes);

bool HasNestedCommands() const;
bool IsNestedCommand() const noexcept;
Expand Down
Loading
Loading