Skip to content

Add setting to allow control over telemetry event writing #777

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.

Sign up for GitHub

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
JohnMcPMS merged 4 commits into from
Mar 5, 2021
Merged

Add setting to allow control over telemetry event writing #777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
717b4b6
Add setting to allow control over telemetry event writing
JohnMcPMS Mar 4, 2021
7a65810
PR feedback
JohnMcPMS Mar 5, 2021
ba6c448
Merge from master
JohnMcPMS Mar 5, 2021
d2598c3
Revert update to settings documentation until feature is released
JohnMcPMS Mar 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/actions/spelling/patterns.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ El proyecto .* diferentes
http://rfc3161.gtm.corp.microsoft.com/TSS/HttpTspServer

# schema regex
"pattern": .*$
"pattern": .*$
31 changes: 21 additions & 10 deletions src/AppInstallerCommonCore/AppInstallerTelemetry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "Public/AppInstallerRuntime.h"
#include "Public/AppInstallerSHA256.h"
#include "Public/AppInstallerStrings.h"
#include "winget/UserSettings.h"

#define AICLI_TraceLoggingStringView(_sv_,_name_) TraceLoggingCountedUtf8String(_sv_.data(), static_cast<ULONG>(_sv_.size()), _name_)

Expand Down Expand Up @@ -34,18 +35,10 @@ namespace AppInstaller::Logging
{
static const uint32_t s_RootExecutionId = 0;

// Used to disable telemetry on the fly.
std::atomic_bool s_isTelemetryEnabled{ true };

std::atomic_uint32_t s_executionStage{ 0 };

std::atomic_uint32_t s_subExecutionId{ s_RootExecutionId };

bool IsTelemetryEnabled()
{
return g_IsTelemetryProviderEnabled && s_isTelemetryEnabled;
}

void __stdcall wilResultLoggingCallback(const wil::FailureInfo& info) noexcept
{
Telemetry().LogFailure(info);
Expand All @@ -67,7 +60,10 @@ namespace AppInstaller::Logging

TelemetryTraceLogger::TelemetryTraceLogger()
{
// TODO: Needs to be made a singleton registration/removal in the future
RegisterTraceLogging();

m_isSettingEnabled = !Settings::User().Get<Settings::Setting::TelemetryDisable>();
}

TelemetryTraceLogger::~TelemetryTraceLogger()
Expand All @@ -81,6 +77,16 @@ namespace AppInstaller::Logging
return instance;
}

bool TelemetryTraceLogger::DisableRuntime()
{
return m_isRuntimeEnabled.exchange(false);
}

void TelemetryTraceLogger::EnableRuntime()
{
m_isRuntimeEnabled = true;
}

void TelemetryTraceLogger::LogFailure(const wil::FailureInfo& failure) const noexcept
{
if (IsTelemetryEnabled())
Expand Down Expand Up @@ -500,6 +506,11 @@ namespace AppInstaller::Logging
}
}

bool TelemetryTraceLogger::IsTelemetryEnabled() const noexcept
{
return g_IsTelemetryProviderEnabled && m_isSettingEnabled && m_isRuntimeEnabled;
}

#ifndef AICLI_DISABLE_TEST_HOOKS
static std::shared_ptr<TelemetryTraceLogger> s_TelemetryTraceLogger_TestOverride;
#endif
Expand All @@ -523,14 +534,14 @@ namespace AppInstaller::Logging

DisableTelemetryScope::DisableTelemetryScope()
{
m_token = s_isTelemetryEnabled.exchange(false);
m_token = Telemetry().DisableRuntime();
}

DisableTelemetryScope::~DisableTelemetryScope()
{
if (m_token)
{
s_isTelemetryEnabled = true;
Telemetry().EnableRuntime();
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ namespace AppInstaller::Logging
// Gets the singleton instance of this type.
static TelemetryTraceLogger& GetInstance();

// Control whether this trace logger is enabled at runtime.
bool DisableRuntime();
void EnableRuntime();

// Logs the failure info.
void LogFailure(const wil::FailureInfo& failure) const noexcept;

Expand Down Expand Up @@ -112,6 +116,11 @@ namespace AppInstaller::Logging

protected:
TelemetryTraceLogger();

bool IsTelemetryEnabled() const noexcept;

bool m_isSettingEnabled = true;
std::atomic_bool m_isRuntimeEnabled{ true };
};

// Helper to make the call sites look clean.
Expand Down
8 changes: 5 additions & 3 deletions src/AppInstallerCommonCore/Public/winget/UserSettings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
#include <variant>
#include <vector>

using namespace std::chrono_literals;
using namespace std::string_view_literals;

namespace AppInstaller::Settings
{

@yao-msft yao-msft Mar 4, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why we move usings outside for this one specifically?

@JohnMcPMS JohnMcPMS Mar 5, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Putting the using statements inside the namespace imports them into that namespace. So AppInstaller::Settings would now have definitions for sv, ms, etc.

using namespace std::chrono_literals;
using namespace std::string_view_literals;

// The type of argument.
enum class UserSettingsType
{
Expand Down Expand Up @@ -54,6 +54,7 @@ namespace AppInstaller::Settings
EFUninstall,
EFImport,
EFExport,
TelemetryDisable,
Max
};

Expand Down Expand Up @@ -91,6 +92,7 @@ namespace AppInstaller::Settings
SETTINGMAPPING_SPECIALIZATION(Setting::EFUninstall, bool, bool, false, ".experimentalFeatures.uninstall"sv);
SETTINGMAPPING_SPECIALIZATION(Setting::EFImport, bool, bool, false, ".experimentalFeatures.import"sv);
SETTINGMAPPING_SPECIALIZATION(Setting::EFExport, bool, bool, false, ".experimentalFeatures.export"sv);
SETTINGMAPPING_SPECIALIZATION(Setting::TelemetryDisable, bool, bool, false, ".telemetry.disable"sv);

// Used to deduce the SettingVariant type; making a variant that includes std::monostate and all SettingMapping types.
template <size_t... I>
Expand Down
64 changes: 17 additions & 47 deletions src/AppInstallerCommonCore/UserSettings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ namespace AppInstaller::Settings

namespace details
{
// Stamps out a validate function that simply returns the input value.
#define WINGET_VALIDATE_PASS_THROUGH(_setting_) \
std::optional<SettingMapping<Setting::_setting_>::value_t> \
SettingMapping<Setting::_setting_>::Validate(const SettingMapping<Setting::_setting_>::json_t& value) \
{ \
return value; \
}

std::optional<SettingMapping<Setting::AutoUpdateTimeInMinutes>::value_t>
SettingMapping<Setting::AutoUpdateTimeInMinutes>::Validate(const SettingMapping<Setting::AutoUpdateTimeInMinutes>::json_t& value)
{
Expand Down Expand Up @@ -181,53 +189,15 @@ namespace AppInstaller::Settings
return {};
}

std::optional<SettingMapping<Setting::EFExperimentalCmd>::value_t>
SettingMapping<Setting::EFExperimentalCmd>::Validate(const SettingMapping<Setting::EFExperimentalCmd>::json_t& value)
{
return value;
}

std::optional<SettingMapping<Setting::EFExperimentalArg>::value_t>
SettingMapping<Setting::EFExperimentalArg>::Validate(const SettingMapping<Setting::EFExperimentalArg>::json_t& value)
{
return value;
}

std::optional<SettingMapping<Setting::EFExperimentalMSStore>::value_t>
SettingMapping<Setting::EFExperimentalMSStore>::Validate(const SettingMapping<Setting::EFExperimentalMSStore>::json_t& value)
{
return value;
}

std::optional<SettingMapping<Setting::EFList>::value_t>
SettingMapping<Setting::EFList>::Validate(const SettingMapping<Setting::EFList>::json_t& value)
{
return value;
}

std::optional<SettingMapping<Setting::EFExperimentalUpgrade>::value_t>
SettingMapping<Setting::EFExperimentalUpgrade>::Validate(const SettingMapping<Setting::EFExperimentalUpgrade>::json_t& value)
{
return value;
}

std::optional<SettingMapping<Setting::EFUninstall>::value_t>
SettingMapping<Setting::EFUninstall>::Validate(const SettingMapping<Setting::EFUninstall>::json_t& value)
{
return value;
}

std::optional<SettingMapping<Setting::EFImport>::value_t>
SettingMapping<Setting::EFImport>::Validate(const SettingMapping<Setting::EFImport>::json_t& value)
{
return value;
}

std::optional<SettingMapping<Setting::EFExport>::value_t>
SettingMapping<Setting::EFExport>::Validate(const SettingMapping<Setting::EFExport>::json_t& value)
{
return value;
}
WINGET_VALIDATE_PASS_THROUGH(EFExperimentalCmd)
WINGET_VALIDATE_PASS_THROUGH(EFExperimentalArg)
WINGET_VALIDATE_PASS_THROUGH(EFExperimentalMSStore)
WINGET_VALIDATE_PASS_THROUGH(EFList)
WINGET_VALIDATE_PASS_THROUGH(EFExperimentalUpgrade)
WINGET_VALIDATE_PASS_THROUGH(EFUninstall)
WINGET_VALIDATE_PASS_THROUGH(EFImport)
WINGET_VALIDATE_PASS_THROUGH(EFExport)
WINGET_VALIDATE_PASS_THROUGH(TelemetryDisable)
}

UserSettings::UserSettings() : m_type(UserSettingsType::Default)
Expand Down