commit 7ee678403a6cd082f73cae47be9a98bfda3c5973
parent 959aacb3cb6573931d6aea30f591dfeb8bfd872c
Author: yao-msft <50888816+yao-msft@users.noreply.github.com>
Date: Tue, 20 Oct 2020 18:05:14 -0700
Telemetry reporting improvements (#617)
Diffstat:
10 files changed, 167 insertions(+), 7 deletions(-)
diff --git a/src/AppInstallerCLICore/Commands/InstallCommand.cpp b/src/AppInstallerCLICore/Commands/InstallCommand.cpp
@@ -97,13 +97,17 @@ namespace AppInstaller::CLI
void InstallCommand::ExecuteInternal(Execution::Context& context) const
{
context <<
+ Workflow::ReportExecutionStage(ExecutionStage::Discovery) <<
Workflow::GetManifest <<
Workflow::EnsureMinOSVersion <<
Workflow::SelectInstaller <<
Workflow::EnsureApplicableInstaller <<
Workflow::ShowInstallationDisclaimer <<
+ Workflow::ReportExecutionStage(ExecutionStage::Download) <<
Workflow::DownloadInstaller <<
+ Workflow::ReportExecutionStage(ExecutionStage::Execution) <<
Workflow::ExecuteInstaller <<
+ Workflow::ReportExecutionStage(ExecutionStage::PostExecution) <<
Workflow::RemoveInstaller;
}
}
diff --git a/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp b/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp
@@ -116,6 +116,7 @@ namespace AppInstaller::CLI
WI_SetFlag(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate);
context <<
+ Workflow::ReportExecutionStage(ExecutionStage::Discovery) <<
OpenSource <<
OpenCompositeSource(Repository::PredefinedSource::Installed);
@@ -149,8 +150,11 @@ namespace AppInstaller::CLI
SelectInstaller <<
EnsureApplicableInstaller <<
ShowInstallationDisclaimer <<
+ Workflow::ReportExecutionStage(ExecutionStage::Download) <<
DownloadInstaller <<
+ Workflow::ReportExecutionStage(ExecutionStage::Execution) <<
ExecuteInstaller <<
+ Workflow::ReportExecutionStage(ExecutionStage::PostExecution) <<
RemoveInstaller;
}
else
@@ -182,8 +186,11 @@ namespace AppInstaller::CLI
context <<
ShowInstallationDisclaimer <<
+ Workflow::ReportExecutionStage(ExecutionStage::Download) <<
DownloadInstaller <<
+ Workflow::ReportExecutionStage(ExecutionStage::Execution) <<
ExecuteInstaller <<
+ Workflow::ReportExecutionStage(ExecutionStage::PostExecution) <<
RemoveInstaller;
}
}
diff --git a/src/AppInstallerCLICore/Core.cpp b/src/AppInstallerCLICore/Core.cpp
@@ -4,6 +4,7 @@
#include "Public/AppInstallerCLICore.h"
#include "Commands/RootCommand.h"
#include "ExecutionContext.h"
+#include "Workflows/WorkflowBase.h"
#include <winget/UserSettings.h>
using namespace winrt;
@@ -61,6 +62,8 @@ namespace AppInstaller::CLI
Execution::Context context{ std::cout, std::cin };
context.EnableCtrlHandler();
+ context << Workflow::ReportExecutionStage(Workflow::ExecutionStage::ParseArgs);
+
// Convert incoming wide char args to UTF8
std::vector<std::string> utf8Args;
for (int i = 1; i < argc; ++i)
diff --git a/src/AppInstallerCLICore/ExecutionContext.h b/src/AppInstallerCLICore/ExecutionContext.h
@@ -33,6 +33,7 @@
namespace AppInstaller::CLI::Workflow
{
struct WorkflowTask;
+ enum class ExecutionStage : uint32_t;
}
namespace AppInstaller::CLI::Execution
@@ -53,6 +54,7 @@ namespace AppInstaller::CLI::Execution
InstallerArgs,
CompletionData,
InstalledPackageVersion,
+ ExecutionStage,
Max
};
@@ -139,6 +141,12 @@ namespace AppInstaller::CLI::Execution
using value_t = std::shared_ptr<Repository::IPackageVersion>;
};
+ template <>
+ struct DataMapping<Data::ExecutionStage>
+ {
+ using value_t = Workflow::ExecutionStage;
+ };
+
// Used to deduce the DataVariant type; making a variant that includes std::monostate and all DataMapping types.
template <size_t... I>
inline auto Deduce(std::index_sequence<I...>) { return std::variant<std::monostate, DataMapping<static_cast<Data>(I)>::value_t...>{}; }
@@ -194,6 +202,11 @@ namespace AppInstaller::CLI::Execution
{
m_data[D].emplace<details::DataIndex(D)>(std::forward<typename details::DataMapping<D>::value_t>(v));
}
+ template <Data D>
+ void Add(const typename details::DataMapping<D>::value_t& v)
+ {
+ m_data[D].emplace<details::DataIndex(D)>(v);
+ }
// Return a value indicating whether the given data type is stored in the context.
bool Contains(Data d) { return (m_data.find(d) != m_data.end()); }
diff --git a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp
@@ -105,6 +105,8 @@ namespace AppInstaller::CLI::Workflow
bool updateAllHasFailure = false;
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.Clone();
Execution::Context& updateContext = *updateContextPtr;
@@ -113,10 +115,14 @@ namespace AppInstaller::CLI::Workflow
updateContext.Add<Execution::Data::InstalledPackageVersion>(match.Package->GetInstalledVersion());
updateContext <<
+ Workflow::ReportExecutionStage(ExecutionStage::Discovery) <<
SelectLatestApplicableUpdate(*(match.Package)) <<
ShowInstallationDisclaimer <<
+ Workflow::ReportExecutionStage(ExecutionStage::Download) <<
DownloadInstaller <<
+ Workflow::ReportExecutionStage(ExecutionStage::Execution) <<
ExecuteInstaller <<
+ Workflow::ReportExecutionStage(ExecutionStage::PostExecution) <<
RemoveInstaller;
if (updateContext.GetTerminationHR() != S_OK &&
diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp
@@ -458,7 +458,7 @@ namespace AppInstaller::CLI::Workflow
AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_NO_MANIFEST_FOUND);
}
- Logging::Telemetry().LogManifestFields(manifest->Id, manifest->Name, manifest->Version, false);
+ Logging::Telemetry().LogManifestFields(manifest->Id, manifest->Name, manifest->Version);
context.Add<Execution::Data::Manifest>(std::move(manifest.value()));
}
@@ -481,12 +481,14 @@ namespace AppInstaller::CLI::Workflow
void GetManifestFromArg(Execution::Context& context)
{
+ Logging::Telemetry().LogIsManifestLocal(true);
+
context <<
VerifyFile(Execution::Args::Type::Manifest) <<
[](Execution::Context& context)
{
Manifest::Manifest manifest = Manifest::YamlParser::CreateFromPath(Utility::ConvertToUTF16(context.Args.GetArg(Execution::Args::Type::Manifest)));
- Logging::Telemetry().LogManifestFields(manifest.Id, manifest.Name, manifest.Version, true);
+ Logging::Telemetry().LogManifestFields(manifest.Id, manifest.Name, manifest.Version);
context.Add<Execution::Data::Manifest>(std::move(manifest));
};
}
@@ -600,6 +602,28 @@ namespace AppInstaller::CLI::Workflow
const auto& searchResult = context.Get<Execution::Data::SearchResult>();
context.Add<Execution::Data::InstalledPackageVersion>(searchResult.Matches.at(0).Package->GetInstalledVersion());
}
+
+ void ReportExecutionStage::operator()(Execution::Context& context) const
+ {
+ if (!context.Contains(Execution::Data::ExecutionStage))
+ {
+ context.Add<Execution::Data::ExecutionStage>(m_stage);
+ }
+ else if (context.Get<Execution::Data::ExecutionStage>() == m_stage)
+ {
+ return;
+ }
+ else if (context.Get<Execution::Data::ExecutionStage>() < m_stage || m_allowBackward)
+ {
+ context.Get<Execution::Data::ExecutionStage>() = m_stage;
+ }
+ else
+ {
+ THROW_HR_MSG(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), "Reporting ExecutionStage to an earlier Stage without allowBackward as true");
+ }
+
+ Logging::SetExecutionStage(static_cast<uint32_t>(context.Get<Execution::Data::ExecutionStage>()));
+ }
}
AppInstaller::CLI::Execution::Context& operator<<(AppInstaller::CLI::Execution::Context& context, AppInstaller::CLI::Workflow::WorkflowTask::Func f)
diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.h b/src/AppInstallerCLICore/Workflows/WorkflowBase.h
@@ -18,6 +18,16 @@ namespace AppInstaller::CLI::Workflow
{
static const char* s_InstallationMetadata_Key_InstallerType = "InstallerType";
+ // Values are ordered in a typical workflow stages
+ enum class ExecutionStage : uint32_t
+ {
+ ParseArgs = 1000,
+ Discovery = 2000,
+ Download = 3000,
+ Execution = 4000,
+ PostExecution = 5000,
+ };
+
// A task in the workflow.
struct WorkflowTask
{
@@ -233,6 +243,21 @@ namespace AppInstaller::CLI::Workflow
// Inputs: SearchResult
// Outputs: InstalledPackageVersion
void GetInstalledPackageVersion(Execution::Context& context);
+
+ // Reports execution stage in a workflow
+ // Required Args: ExecutionStage
+ // Inputs: ExecutionStage?
+ // Outputs: ExecutionStage
+ struct ReportExecutionStage : public WorkflowTask
+ {
+ ReportExecutionStage(ExecutionStage stage, bool allowBackward = false) : WorkflowTask("ReportExecutionStage"), m_stage(stage), m_allowBackward(allowBackward) {}
+
+ void operator()(Execution::Context& context) const override;
+
+ private:
+ ExecutionStage m_stage;
+ bool m_allowBackward;
+ };
}
// Passes the context to the function if it has not been terminated; returns the context.
diff --git a/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp b/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp
@@ -32,9 +32,15 @@ namespace AppInstaller::Logging
namespace
{
+ 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;
@@ -83,6 +89,7 @@ namespace AppInstaller::Logging
"FailureInfo",
GetActivityId(),
nullptr,
+ TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"),
TraceLoggingHResult(failure.hr, "HResult"),
TraceLoggingWideString(failure.pszMessage, "Message"),
TraceLoggingString(failure.pszModule, "Module"),
@@ -90,6 +97,7 @@ namespace AppInstaller::Logging
TraceLoggingUInt32(static_cast<uint32_t>(failure.type), "Type"),
TraceLoggingString(failure.pszFile, "File"),
TraceLoggingUInt32(failure.uLineNumber, "Line"),
+ TraceLoggingUInt32(s_executionStage, "ExecutionStage"),
TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance),
TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA));
}
@@ -172,9 +180,11 @@ namespace AppInstaller::Logging
"CommandTermination",
GetActivityId(),
nullptr,
+ TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"),
TraceLoggingHResult(hr, "HResult"),
AICLI_TraceLoggingStringView(file, "File"),
TraceLoggingUInt64(static_cast<UINT64>(line), "Line"),
+ TraceLoggingUInt32(s_executionStage, "ExecutionStage"),
TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance),
TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA));
}
@@ -190,9 +200,11 @@ namespace AppInstaller::Logging
"Exception",
GetActivityId(),
nullptr,
+ TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"),
AICLI_TraceLoggingStringView(commandName, "Command"),
AICLI_TraceLoggingStringView(type, "Type"),
AICLI_TraceLoggingStringView(message, "Message"),
+ TraceLoggingUInt32(s_executionStage, "ExecutionStage"),
TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance),
TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA));
}
@@ -200,7 +212,22 @@ namespace AppInstaller::Logging
AICLI_LOG(CLI, Error, << "Caught " << type << ": " << message);
}
- void TelemetryTraceLogger::LogManifestFields(std::string_view id, std::string_view name, std::string_view version, bool localManifest) noexcept
+ void TelemetryTraceLogger::LogIsManifestLocal(bool isLocalManifest) noexcept
+ {
+ if (IsTelemetryEnabled())
+ {
+ TraceLoggingWriteActivity(g_hTelemetryProvider,
+ "GetManifest",
+ GetActivityId(),
+ nullptr,
+ TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"),
+ TraceLoggingBool(isLocalManifest, "IsManifestLocal"),
+ TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage),
+ TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA));
+ }
+ }
+
+ void TelemetryTraceLogger::LogManifestFields(std::string_view id, std::string_view name, std::string_view version) noexcept
{
if (IsTelemetryEnabled())
{
@@ -208,10 +235,10 @@ namespace AppInstaller::Logging
"ManifestFields",
GetActivityId(),
nullptr,
+ TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"),
AICLI_TraceLoggingStringView(id, "Id"),
AICLI_TraceLoggingStringView(name,"Name"),
AICLI_TraceLoggingStringView(version, "Version"),
- TraceLoggingBool(localManifest, "IsManifestLocal"),
TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance|PDT_ProductAndServiceUsage),
TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA));
}
@@ -227,6 +254,7 @@ namespace AppInstaller::Logging
"NoAppMatch",
GetActivityId(),
nullptr,
+ TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"),
TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage),
TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA));
}
@@ -242,6 +270,7 @@ namespace AppInstaller::Logging
"MultiAppMatch",
GetActivityId(),
nullptr,
+ TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"),
TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage),
TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA));
}
@@ -257,6 +286,7 @@ namespace AppInstaller::Logging
"AppFound",
GetActivityId(),
nullptr,
+ TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"),
AICLI_TraceLoggingStringView(name, "Name"),
AICLI_TraceLoggingStringView(id, "Id"),
TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage),
@@ -274,6 +304,7 @@ namespace AppInstaller::Logging
"SelectedInstaller",
GetActivityId(),
nullptr,
+ TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"),
TraceLoggingInt32(arch, "Arch"),
AICLI_TraceLoggingStringView(url, "Url"),
AICLI_TraceLoggingStringView(installerType, "InstallerType"),
@@ -308,6 +339,7 @@ namespace AppInstaller::Logging
"SearchRequest",
GetActivityId(),
nullptr,
+ TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"),
AICLI_TraceLoggingStringView(type, "Type"),
AICLI_TraceLoggingStringView(query, "Query"),
AICLI_TraceLoggingStringView(id, "Id"),
@@ -330,6 +362,7 @@ namespace AppInstaller::Logging
"SearchResultCount",
GetActivityId(),
nullptr,
+ TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"),
TraceLoggingUInt64(resultCount, "ResultCount"),
TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage),
TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA));
@@ -350,6 +383,7 @@ namespace AppInstaller::Logging
"HashMismatch",
GetActivityId(),
nullptr,
+ TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"),
AICLI_TraceLoggingStringView(id, "Id"),
AICLI_TraceLoggingStringView(version, "Version"),
AICLI_TraceLoggingStringView(channel, "Channel"),
@@ -376,6 +410,7 @@ namespace AppInstaller::Logging
"InstallerFailure",
GetActivityId(),
nullptr,
+ TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"),
AICLI_TraceLoggingStringView(id, "Id"),
AICLI_TraceLoggingStringView(version, "Version"),
AICLI_TraceLoggingStringView(channel, "Channel"),
@@ -405,4 +440,23 @@ namespace AppInstaller::Logging
s_isTelemetryEnabled = 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()
+ {
+ s_subExecutionId = s_RootExecutionId;
+ }
}
\ No newline at end of file
diff --git a/src/AppInstallerCommonCore/Manifest/YamlParser.cpp b/src/AppInstallerCommonCore/Manifest/YamlParser.cpp
@@ -433,7 +433,7 @@ namespace AppInstaller::Manifest
{
{ManifestInstaller::InstallerSwitchType::Silent, ManifestInstaller::string_t("/S")},
{ManifestInstaller::InstallerSwitchType::SilentWithProgress, ManifestInstaller::string_t("/S")},
- {ManifestInstaller::InstallerSwitchType::InstallLocation, ManifestInstaller::string_t("/D=\"" + std::string(ARG_TOKEN_INSTALLPATH) + "\"")}
+ {ManifestInstaller::InstallerSwitchType::InstallLocation, ManifestInstaller::string_t("/D=" + std::string(ARG_TOKEN_INSTALLPATH))}
};
case ManifestInstaller::InstallerTypeEnum::Inno:
return
diff --git a/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h b/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h
@@ -44,9 +44,12 @@ namespace AppInstaller::Logging
// Logs the invoked command termination.
void LogException(std::string_view commandName, std::string_view type, std::string_view message) noexcept;
+ // Logs whether the manifest used in workflow is local
+ void LogIsManifestLocal(bool isLocalManifest) noexcept;
+
// Logs the Manifest fields.
- void LogManifestFields(std::string_view id, std::string_view name, std::string_view version, bool localManifest) noexcept;
-
+ void LogManifestFields(std::string_view id, std::string_view name, std::string_view version) noexcept;
+
// Logs when there is no matching App found for search
void LogNoAppMatch() noexcept;
@@ -116,4 +119,25 @@ 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(const SubExecutionTelemetryScope&) = delete;
+ SubExecutionTelemetryScope& operator=(const SubExecutionTelemetryScope&) = delete;
+
+ SubExecutionTelemetryScope(SubExecutionTelemetryScope&&) = default;
+ SubExecutionTelemetryScope& operator=(SubExecutionTelemetryScope&&) = default;
+
+ ~SubExecutionTelemetryScope();
+
+ private:
+ static std::atomic_uint32_t m_sessionId;
+ };
}
