From 3fa3fb5e0e1e7eb068990a50c258693bd24b0fd7 Mon Sep 17 00:00:00 2001 From: Victor Romero Date: Mon, 12 Sep 2022 14:08:51 -0700 Subject: [PATCH] [telemetry] Refactor `track_property()` (#685) * initialize timestamp in metrics message * refactor track_property() * Update src/vcpkg/vcpkgpaths.cpp * Remove `track_option()` * Renamed SetMetric to DefineMetric * Never trust VS's `Ctrl+R,Ctrl+R` * Add missing metric name * format * Partially address Robert's comments * Change `track_property()` call sites * Review comments and tests * Update src/vcpkg-test/metrics.cpp Co-authored-by: Robert Schumacher * simplify tests * Remove `track_property()` overrides Co-authored-by: Robert Schumacher --- include/vcpkg/metrics.h | 77 ++++++++++++++++++-- src/vcpkg-test/metrics.cpp | 48 +++++++++++++ src/vcpkg.cpp | 13 ++-- src/vcpkg/binarycaching.cpp | 29 ++++++-- src/vcpkg/build.cpp | 4 +- src/vcpkg/commands.add.cpp | 8 +-- src/vcpkg/commands.find.cpp | 8 +-- src/vcpkg/commands.integrate.cpp | 4 +- src/vcpkg/commands.setinstalled.cpp | 2 +- src/vcpkg/input.cpp | 3 +- src/vcpkg/install.cpp | 13 ++-- src/vcpkg/metrics.cpp | 107 ++++++++++++++++++++++++---- src/vcpkg/portfileprovider.cpp | 2 +- src/vcpkg/registries.cpp | 15 ++-- src/vcpkg/sourceparagraph.cpp | 10 +-- src/vcpkg/vcpkgcmdarguments.cpp | 22 +++--- src/vcpkg/vcpkglib.cpp | 2 +- src/vcpkg/vcpkgpaths.cpp | 13 ++-- 18 files changed, 299 insertions(+), 81 deletions(-) create mode 100644 src/vcpkg-test/metrics.cpp diff --git a/include/vcpkg/metrics.h b/include/vcpkg/metrics.h index 4e67fe7809..fc5e269f91 100644 --- a/include/vcpkg/metrics.h +++ b/include/vcpkg/metrics.h @@ -8,6 +8,68 @@ namespace vcpkg { + template + struct MetricEntry + { + T metric; + StringLiteral name; + }; + + enum class DefineMetric + { + AssetSource, + BinaryCachingAws, + BinaryCachingAzBlob, + BinaryCachingCos, + BinaryCachingDefault, + BinaryCachingFiles, + BinaryCachingGcs, + BinaryCachingHttp, + BinaryCachingNuget, + BinaryCachingSource, + ErrorVersioningDisabled, + ErrorVersioningNoBaseline, + GitHubRepository, + ManifestBaseline, + ManifestOverrides, + ManifestVersionConstraint, + RegistriesErrorCouldNotFindBaseline, + RegistriesErrorNoVersionsAtCommit, + VcpkgBinarySources, + VcpkgDefaultBinaryCache, + VcpkgNugetRepository, + VersioningErrorBaseline, + VersioningErrorVersion, + X_VcpkgRegistriesCache, + X_WriteNugetPackagesConfig, + COUNT // always keep COUNT last + }; + + enum class StringMetric + { + BuildError, + CommandArgs, + CommandContext, + CommandName, + Error, + InstallPlan_1, + ListFile, + RegistriesDefaultRegistryKind, + RegistriesKindsUsed, + Title, + UserMac, + VcpkgVersion, + Warning, + COUNT // always keep COUNT last + }; + + enum class BoolMetric + { + InstallManifestMode, + OptionOverlayPorts, + COUNT // always keep COUNT last + }; + struct Metrics { Metrics() = default; @@ -22,18 +84,23 @@ namespace vcpkg void track_metric(const std::string& name, double value); void track_buildtime(const std::string& name, double value); - void track_property(const std::string& name, const std::string& value); - void track_property(const std::string& name, bool value); + + void track_define_property(DefineMetric metric); + void track_string_property(StringMetric metric, StringView value); + void track_bool_property(BoolMetric metric, bool value); + void track_feature(const std::string& feature, bool value); - void track_option(const std::string& option, bool value); bool metrics_enabled(); void upload(const std::string& payload); void flush(Filesystem& fs); - }; - Optional find_first_nonzero_mac(StringView sv); + // exposed for testing + static View> get_define_metrics(); + static View> get_string_metrics(); + static View> get_bool_metrics(); + }; extern LockGuarded g_metrics; } diff --git a/src/vcpkg-test/metrics.cpp b/src/vcpkg-test/metrics.cpp new file mode 100644 index 0000000000..36360358b0 --- /dev/null +++ b/src/vcpkg-test/metrics.cpp @@ -0,0 +1,48 @@ +#include + +#include + +#include + +using namespace vcpkg; + +template +void validate_enum_values_and_names(View> entries, const size_t expected_size) +{ + REQUIRE(expected_size == entries.size()); + + size_t enum_value = 0; + std::set used_names; + for (auto&& m : entries) + { + // fails when a metric is not in the right order in the entries array + // - check that there are no duplicate or skipped metric entries + // - check that the order in Metrics::get__metrics() and in the Metric enum is the same + REQUIRE(static_cast(m.metric) == enum_value); + ++enum_value; + + // fails when there's a repeated metric name + REQUIRE(!m.name.empty()); + auto it_names = used_names.find(m.name); + REQUIRE(it_names == used_names.end()); + used_names.insert(m.name); + } +} + +TEST_CASE ("Check metric enum types", "[metrics]") +{ + SECTION ("define metrics") + { + validate_enum_values_and_names(Metrics::get_define_metrics(), static_cast(DefineMetric::COUNT)); + } + + SECTION ("string metrics") + { + validate_enum_values_and_names(Metrics::get_string_metrics(), static_cast(StringMetric::COUNT)); + } + + SECTION ("bool metrics") + { + validate_enum_values_and_names(Metrics::get_bool_metrics(), static_cast(BoolMetric::COUNT)); + } +} \ No newline at end of file diff --git a/src/vcpkg.cpp b/src/vcpkg.cpp index 51573a67ae..e523833904 100644 --- a/src/vcpkg.cpp +++ b/src/vcpkg.cpp @@ -44,7 +44,8 @@ static void invalid_command(const std::string& cmd) static void inner(vcpkg::Filesystem& fs, const VcpkgCmdArguments& args) { // track version on each invocation - LockGuardPtr(g_metrics)->track_property("vcpkg_version", Commands::Version::version.to_string()); + LockGuardPtr(g_metrics)->track_string_property(StringMetric::VcpkgVersion, + Commands::Version::version.to_string()); if (args.command.empty()) { @@ -67,11 +68,11 @@ static void inner(vcpkg::Filesystem& fs, const VcpkgCmdArguments& args) } }; - LockGuardPtr(g_metrics)->track_option("overlay_ports", !args.overlay_ports.empty()); + LockGuardPtr(g_metrics)->track_bool_property(BoolMetric::OptionOverlayPorts, !args.overlay_ports.empty()); if (const auto command_function = find_command(Commands::get_available_basic_commands())) { - LockGuardPtr(g_metrics)->track_property("command_name", command_function->name); + LockGuardPtr(g_metrics)->track_string_property(StringMetric::CommandName, command_function->name); return command_function->function->perform_and_exit(args, fs); } @@ -82,7 +83,7 @@ static void inner(vcpkg::Filesystem& fs, const VcpkgCmdArguments& args) if (const auto command_function = find_command(Commands::get_available_paths_commands())) { - LockGuardPtr(g_metrics)->track_property("command_name", command_function->name); + LockGuardPtr(g_metrics)->track_string_property(StringMetric::CommandName, command_function->name); return command_function->function->perform_and_exit(args, paths); } @@ -93,7 +94,7 @@ static void inner(vcpkg::Filesystem& fs, const VcpkgCmdArguments& args) if (const auto command_function = find_command(Commands::get_available_triplet_commands())) { - LockGuardPtr(g_metrics)->track_property("command_name", command_function->name); + LockGuardPtr(g_metrics)->track_string_property(StringMetric::CommandName, command_function->name); return command_function->function->perform_and_exit(args, paths, default_triplet, host_triplet); } @@ -316,7 +317,7 @@ int main(const int argc, const char* const* const argv) exc_msg = "unknown error(...)"; } - LockGuardPtr(g_metrics)->track_property("error", exc_msg); + LockGuardPtr(g_metrics)->track_string_property(StringMetric::Error, exc_msg); fflush(stdout); msg::println(msgVcpkgHasCrashed); diff --git a/src/vcpkg/binarycaching.cpp b/src/vcpkg/binarycaching.cpp index 5161c79cb8..b766a9413c 100644 --- a/src/vcpkg/binarycaching.cpp +++ b/src/vcpkg/binarycaching.cpp @@ -1579,7 +1579,7 @@ namespace auto maybe_cachepath = get_environment_variable("VCPKG_DEFAULT_BINARY_CACHE"); if (auto p_str = maybe_cachepath.get()) { - LockGuardPtr(g_metrics)->track_property("VCPKG_DEFAULT_BINARY_CACHE", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::VcpkgDefaultBinaryCache); Path path = *p_str; path.make_preferred(); if (!get_real_filesystem().is_directory(path)) @@ -1997,9 +1997,24 @@ namespace return add_error(msg::format(msgUnknownBinaryProviderType), segments[0].first); } + static const std::map metric_names{ + {"aws", DefineMetric::BinaryCachingAws}, + {"azblob", DefineMetric::BinaryCachingAzBlob}, + {"cos", DefineMetric::BinaryCachingCos}, + {"default", DefineMetric::BinaryCachingDefault}, + {"files", DefineMetric::BinaryCachingFiles}, + {"gcs", DefineMetric::BinaryCachingGcs}, + {"http", DefineMetric::BinaryCachingHttp}, + {"nuget", DefineMetric::BinaryCachingNuget}, + }; + auto metrics = LockGuardPtr(g_metrics); for (const auto& cache_provider : state->binary_cache_providers) { - LockGuardPtr(g_metrics)->track_property("binarycaching_" + cache_provider, "defined"); + auto it = metric_names.find(cache_provider); + if (it != metric_names.end()) + { + metrics->track_define_property(it->second); + } } } }; @@ -2132,7 +2147,7 @@ ExpectedS vcpkg::parse_download_configuration(const Optio { if (!arg || arg.get()->empty()) return DownloadManagerConfig{}; - LockGuardPtr(g_metrics)->track_property("asset-source", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::AssetSource); AssetSourcesState s; AssetSourcesParser parser(*arg.get(), Strings::concat("$", VcpkgCmdArguments::ASSET_SOURCES_ENV), &s); @@ -2187,12 +2202,12 @@ ExpectedS vcpkg::create_binary_providers_from_configs_p LockGuardPtr metrics(g_metrics); if (!env_string.empty()) { - metrics->track_property("VCPKG_BINARY_SOURCES", "defined"); + metrics->track_define_property(DefineMetric::VcpkgBinarySources); } if (args.size() != 0) { - metrics->track_property("binarycaching-source", "defined"); + metrics->track_define_property(DefineMetric::BinaryCachingSource); } } @@ -2331,7 +2346,7 @@ details::NuGetRepoInfo details::get_nuget_repo_info_from_env() auto vcpkg_nuget_repository = get_environment_variable("VCPKG_NUGET_REPOSITORY"); if (auto p = vcpkg_nuget_repository.get()) { - LockGuardPtr(g_metrics)->track_property("VCPKG_NUGET_REPOSITORY", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::VcpkgNugetRepository); return {std::move(*p)}; } @@ -2347,7 +2362,7 @@ details::NuGetRepoInfo details::get_nuget_repo_info_from_env() return {}; } - LockGuardPtr(g_metrics)->track_property("GITHUB_REPOSITORY", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::GitHubRepository); return {Strings::concat(gh_server, '/', gh_repo, ".git"), get_environment_variable("GITHUB_REF").value_or(""), get_environment_variable("GITHUB_SHA").value_or("")}; diff --git a/src/vcpkg/build.cpp b/src/vcpkg/build.cpp index 783751b7d1..1daee0ed76 100644 --- a/src/vcpkg/build.cpp +++ b/src/vcpkg/build.cpp @@ -947,8 +947,8 @@ namespace vcpkg buildtimeus); if (!succeeded(return_code)) { - metrics->track_property("error", "build failed"); - metrics->track_property("build_error", spec_string); + metrics->track_string_property(StringMetric::Error, "build failed"); + metrics->track_string_property(StringMetric::BuildError, spec_string); const auto logs = buildpath / Strings::concat("error-logs-", action.spec.triplet(), ".txt"); std::vector error_logs; if (fs.exists(logs, VCPKG_LINE_INFO)) diff --git a/src/vcpkg/commands.add.cpp b/src/vcpkg/commands.add.cpp index cf6411e42d..e4d1704be4 100644 --- a/src/vcpkg/commands.add.cpp +++ b/src/vcpkg/commands.add.cpp @@ -46,8 +46,8 @@ namespace vcpkg::Commands auto artifact_hash = Hash::get_string_hash(artifact_name, Hash::Algorithm::Sha256); { auto metrics = LockGuardPtr(g_metrics); - metrics->track_property("command_context", "artifact"); - metrics->track_property("command_args", artifact_hash); + metrics->track_string_property(StringMetric::CommandContext, "artifact"); + metrics->track_string_property(StringMetric::CommandArgs, artifact_hash); } // unlock g_metrics std::string ce_args[] = {"add", artifact_name}; @@ -122,8 +122,8 @@ namespace vcpkg::Commands })); { auto metrics = LockGuardPtr(g_metrics); - metrics->track_property("command_context", "port"); - metrics->track_property("command_args", command_args_hash); + metrics->track_string_property(StringMetric::CommandContext, "port"); + metrics->track_string_property(StringMetric::CommandArgs, command_args_hash); } // unlock metrics Checks::exit_success(VCPKG_LINE_INFO); diff --git a/src/vcpkg/commands.find.cpp b/src/vcpkg/commands.find.cpp index 273895fac1..63062c24df 100644 --- a/src/vcpkg/commands.find.cpp +++ b/src/vcpkg/commands.find.cpp @@ -237,10 +237,10 @@ namespace vcpkg::Commands auto args_hash = Hash::get_string_hash(filter.value_or_exit(VCPKG_LINE_INFO), Hash::Algorithm::Sha256); { auto metrics = LockGuardPtr(g_metrics); - metrics->track_property("command_context", "artifact"); + metrics->track_string_property(StringMetric::CommandContext, "artifact"); if (auto p_filter_hash = filter_hash.get()) { - metrics->track_property("command_args", *p_filter_hash); + metrics->track_string_property(StringMetric::CommandArgs, *p_filter_hash); } } // unlock metrics @@ -252,10 +252,10 @@ namespace vcpkg::Commands Optional filter_hash = filter.map(Hash::get_string_sha256); { auto metrics = LockGuardPtr(g_metrics); - metrics->track_property("command_context", "port"); + metrics->track_string_property(StringMetric::CommandContext, "port"); if (auto p_filter_hash = filter_hash.get()) { - metrics->track_property("command_args", *p_filter_hash); + metrics->track_string_property(StringMetric::CommandArgs, *p_filter_hash); } } // unlock metrics diff --git a/src/vcpkg/commands.integrate.cpp b/src/vcpkg/commands.integrate.cpp index bb27d4e75c..a30b1e5d56 100644 --- a/src/vcpkg/commands.integrate.cpp +++ b/src/vcpkg/commands.integrate.cpp @@ -472,8 +472,8 @@ namespace vcpkg::Commands::Integrate .append_raw("\n" + script_path.generic_u8string())); { auto locked_metrics = LockGuardPtr(g_metrics); - locked_metrics->track_property("error", "powershell script failed"); - locked_metrics->track_property("title", TITLE.to_string()); + locked_metrics->track_string_property(StringMetric::Error, "powershell script failed"); + locked_metrics->track_string_property(StringMetric::Title, TITLE.to_string()); } } diff --git a/src/vcpkg/commands.setinstalled.cpp b/src/vcpkg/commands.setinstalled.cpp index 964340f6fa..a44c5b35b3 100644 --- a/src/vcpkg/commands.setinstalled.cpp +++ b/src/vcpkg/commands.setinstalled.cpp @@ -164,7 +164,7 @@ namespace vcpkg::Commands::SetInstalled auto it_pkgsconfig = options.settings.find(OPTION_WRITE_PACKAGES_CONFIG); if (it_pkgsconfig != options.settings.end()) { - LockGuardPtr(g_metrics)->track_property("x-write-nuget-packages-config", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::X_WriteNugetPackagesConfig); pkgsconfig = it_pkgsconfig->second; } diff --git a/src/vcpkg/input.cpp b/src/vcpkg/input.cpp index 91b8fb1bb0..3c1a06210f 100644 --- a/src/vcpkg/input.cpp +++ b/src/vcpkg/input.cpp @@ -35,7 +35,8 @@ namespace vcpkg if (!paths.is_valid_triplet(t)) { print2(Color::error, "Error: invalid triplet: ", t, '\n'); - LockGuardPtr(g_metrics)->track_property("error", "invalid triplet: " + t.to_string()); + LockGuardPtr(g_metrics)->track_string_property(StringMetric::Error, + "invalid triplet: " + t.to_string()); Help::help_topic_valid_triplet(paths); Checks::exit_fail(VCPKG_LINE_INFO); } diff --git a/src/vcpkg/install.cpp b/src/vcpkg/install.cpp index babe0a98e5..c1c566cf5b 100644 --- a/src/vcpkg/install.cpp +++ b/src/vcpkg/install.cpp @@ -873,7 +873,8 @@ namespace vcpkg : UnsupportedPortAction::Error; const bool no_print_usage = Util::Sets::contains(options.switches, OPTION_NO_PRINT_USAGE); - LockGuardPtr(g_metrics)->track_property("install_manifest_mode", paths.manifest_mode_enabled()); + LockGuardPtr(g_metrics)->track_bool_property(BoolMetric::InstallManifestMode, + paths.manifest_mode_enabled()); if (auto p = paths.get_manifest().get()) { @@ -963,7 +964,7 @@ namespace vcpkg auto it_pkgsconfig = options.settings.find(OPTION_WRITE_PACKAGES_CONFIG); if (it_pkgsconfig != options.settings.end()) { - LockGuardPtr(g_metrics)->track_property("x-write-nuget-packages-config", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::X_WriteNugetPackagesConfig); pkgsconfig = Path(it_pkgsconfig->second); } auto maybe_manifest_scf = @@ -1031,12 +1032,12 @@ namespace vcpkg return dep.constraint.type != VersionConstraintKind::None; })) { - LockGuardPtr(g_metrics)->track_property("manifest_version_constraint", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::ManifestVersionConstraint); } if (!manifest_core.overrides.empty()) { - LockGuardPtr(g_metrics)->track_property("manifest_overrides", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::ManifestOverrides); } auto verprovider = make_versioned_portfile_provider(paths); @@ -1166,7 +1167,7 @@ namespace vcpkg auto it_pkgsconfig = options.settings.find(OPTION_WRITE_PACKAGES_CONFIG); if (it_pkgsconfig != options.settings.end()) { - LockGuardPtr(g_metrics)->track_property("x-write-nuget-packages-config", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::X_WriteNugetPackagesConfig); compute_all_abis(paths, action_plan, var_provider, status_db); auto pkgsconfig_path = paths.original_cwd / it_pkgsconfig->second; @@ -1315,6 +1316,6 @@ namespace vcpkg Hash::get_string_hash(version_as_string, Hash::Algorithm::Sha256)); } - LockGuardPtr(g_metrics)->track_property("installplan_1", specs_string); + LockGuardPtr(g_metrics)->track_string_property(StringMetric::InstallPlan_1, specs_string); } } diff --git a/src/vcpkg/metrics.cpp b/src/vcpkg/metrics.cpp index 0de1962dd0..616a1bca93 100644 --- a/src/vcpkg/metrics.cpp +++ b/src/vcpkg/metrics.cpp @@ -3,9 +3,11 @@ #include #include #include +#include #include #include #include +#include #include #include @@ -17,10 +19,88 @@ #pragma comment(lib, "winhttp") #endif +namespace +{ + using namespace vcpkg; + + template + StringLiteral get_metric_name(const T metric, View> entries) + { + auto metric_index = static_cast(metric); + if (metric_index < entries.size()) + { + return entries[metric_index].name; + } + // abort() is used because Checks:: will call back into metrics machinery. + abort(); + } +} + namespace vcpkg { LockGuarded g_metrics; + View> Metrics::get_define_metrics() + { + static constexpr std::array, static_cast(DefineMetric::COUNT)> ENTRIES{{ + {DefineMetric::AssetSource, "asset-source"}, + {DefineMetric::BinaryCachingAws, "binarycaching_aws"}, + {DefineMetric::BinaryCachingAzBlob, "binarycaching_azblob"}, + {DefineMetric::BinaryCachingCos, "binarycaching_cos"}, + {DefineMetric::BinaryCachingDefault, "binarycaching_default"}, + {DefineMetric::BinaryCachingFiles, "binarycaching_files"}, + {DefineMetric::BinaryCachingGcs, "binarycaching_gcs"}, + {DefineMetric::BinaryCachingHttp, "binarycaching_http"}, + {DefineMetric::BinaryCachingNuget, "binarycaching_nuget"}, + {DefineMetric::BinaryCachingSource, "binarycaching-source"}, + {DefineMetric::ErrorVersioningDisabled, "error-versioning-disabled"}, + {DefineMetric::ErrorVersioningNoBaseline, "error-versioning-no-baseline"}, + {DefineMetric::GitHubRepository, "GITHUB_REPOSITORY"}, + {DefineMetric::ManifestBaseline, "manifest_baseline"}, + {DefineMetric::ManifestOverrides, "manifest_overrides"}, + {DefineMetric::ManifestVersionConstraint, "manifest_version_constraint"}, + {DefineMetric::RegistriesErrorCouldNotFindBaseline, "registries-error-could-not-find-baseline"}, + {DefineMetric::RegistriesErrorNoVersionsAtCommit, "registries-error-no-versions-at-commit"}, + {DefineMetric::VcpkgBinarySources, "VCPKG_BINARY_SOURCES"}, + {DefineMetric::VcpkgDefaultBinaryCache, "VCPKG_DEFAULT_BINARY_CACHE"}, + {DefineMetric::VcpkgNugetRepository, "VCPKG_NUGET_REPOSITORY"}, + {DefineMetric::VersioningErrorBaseline, "versioning-error-baseline"}, + {DefineMetric::VersioningErrorVersion, "versioning-error-version"}, + {DefineMetric::X_VcpkgRegistriesCache, "X_VCPKG_REGISTRIES_CACHE"}, + {DefineMetric::X_WriteNugetPackagesConfig, "x-write-nuget-packages-config"}, + }}; + return {ENTRIES.data(), ENTRIES.size()}; + } + + View> Metrics::get_string_metrics() + { + static constexpr std::array, static_cast(StringMetric::COUNT)> ENTRIES{{ + {StringMetric::BuildError, "build_error"}, + {StringMetric::CommandArgs, "command_args"}, + {StringMetric::CommandContext, "command_context"}, + {StringMetric::CommandName, "command_name"}, + {StringMetric::Error, "error"}, + {StringMetric::InstallPlan_1, "installplan_1"}, + {StringMetric::ListFile, "listfile"}, + {StringMetric::RegistriesDefaultRegistryKind, "registries-default-registry-kind"}, + {StringMetric::RegistriesKindsUsed, "registries-kinds-used"}, + {StringMetric::Title, "title"}, + {StringMetric::UserMac, "user_mac"}, + {StringMetric::VcpkgVersion, "vcpkg_version"}, + {StringMetric::Warning, "warning"}, + }}; + return {ENTRIES.data(), ENTRIES.size()}; + } + + View> Metrics::get_bool_metrics() + { + static constexpr std::array, static_cast(BoolMetric::COUNT)> ENTRIES{{ + {BoolMetric::InstallManifestMode, "install_manifest_mode"}, + {BoolMetric::OptionOverlayPorts, "option_overlay_ports"}, + }}; + return {ENTRIES.data(), ENTRIES.size()}; + } + static std::string get_current_date_time_string() { auto maybe_time = CTime::get_current_date_time(); @@ -81,12 +161,12 @@ namespace vcpkg Json::Array buildtime_names; Json::Array buildtime_times; - void track_property(StringView name, const std::string& value) + void track_string(StringView name, StringView value) { properties.insert_or_replace(name, Json::Value::string(value)); } - void track_property(StringView name, bool value) + void track_bool(StringView name, bool value) { properties.insert_or_replace(name, Json::Value::boolean(value)); } @@ -106,11 +186,6 @@ namespace vcpkg properties.insert(Strings::concat("feature-flag-", name), Json::Value::boolean(value)); } - void track_option(StringView name, bool value) - { - properties.insert(Strings::concat("option_", name), Json::Value::boolean(value)); - } - std::string format_event_data_template() const { auto props_plus_buildtimes = properties; @@ -231,7 +306,7 @@ namespace vcpkg g_metricmessage.user_id = config.user_id; g_metricmessage.user_timestamp = config.user_time; - metrics->track_property("user_mac", config.user_mac); + metrics->track_string_property(StringMetric::UserMac, config.user_mac); g_metrics_disabled = false; } @@ -246,16 +321,22 @@ namespace vcpkg g_metricmessage.track_buildtime(name, value); } - void Metrics::track_property(const std::string& name, const std::string& value) + void Metrics::track_define_property(DefineMetric metric) { - g_metricmessage.track_property(name, value); + g_metricmessage.track_string(get_metric_name(metric, get_define_metrics()), "defined"); } - void Metrics::track_property(const std::string& name, bool value) { g_metricmessage.track_property(name, value); } + void Metrics::track_string_property(StringMetric metric, StringView value) + { + g_metricmessage.track_string(get_metric_name(metric, get_string_metrics()), value); + } - void Metrics::track_feature(const std::string& name, bool value) { g_metricmessage.track_feature(name, value); } + void Metrics::track_bool_property(BoolMetric metric, bool value) + { + g_metricmessage.track_bool(get_metric_name(metric, get_bool_metrics()), value); + } - void Metrics::track_option(const std::string& name, bool value) { g_metricmessage.track_option(name, value); } + void Metrics::track_feature(const std::string& name, bool value) { g_metricmessage.track_feature(name, value); } void Metrics::upload(const std::string& payload) { diff --git a/src/vcpkg/portfileprovider.cpp b/src/vcpkg/portfileprovider.cpp index d4c956f314..d1cef89f9c 100644 --- a/src/vcpkg/portfileprovider.cpp +++ b/src/vcpkg/portfileprovider.cpp @@ -204,7 +204,7 @@ namespace vcpkg } else { - LockGuardPtr(g_metrics)->track_property("versioning-error-version", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::VersioningErrorVersion); return maybe_path.error(); } } diff --git a/src/vcpkg/registries.cpp b/src/vcpkg/registries.cpp index 43cb77c314..ac1eb1a326 100644 --- a/src/vcpkg/registries.cpp +++ b/src/vcpkg/registries.cpp @@ -85,8 +85,8 @@ namespace e.commit_id(), registry_versions_dir_name.to_string()); if (!maybe_tree) { - LockGuardPtr(g_metrics)->track_property("registries-error-no-versions-at-commit", - "defined"); + LockGuardPtr(g_metrics)->track_define_property( + DefineMetric::RegistriesErrorNoVersionsAtCommit); Checks::exit_with_message( VCPKG_LINE_INFO, "Error: could not find the git tree for `versions` in repo `%s` at commit `%s`: %s", @@ -661,8 +661,8 @@ namespace auto maybe_err = m_paths.git_fetch(m_repo, m_baseline_identifier); if (!maybe_err) { - LockGuardPtr(g_metrics)->track_property("registries-error-could-not-find-baseline", - "defined"); + LockGuardPtr(g_metrics)->track_define_property( + DefineMetric::RegistriesErrorCouldNotFindBaseline); Checks::exit_with_message( VCPKG_LINE_INFO, "Error: Couldn't find baseline `\"%s\"` for repo %s:\n%s\nError: Failed to fetch %s:\n%s", @@ -678,7 +678,8 @@ namespace if (!maybe_contents) { - LockGuardPtr(g_metrics)->track_property("registries-error-could-not-find-baseline", "defined"); + LockGuardPtr(g_metrics)->track_define_property( + DefineMetric::RegistriesErrorCouldNotFindBaseline); Checks::exit_with_message(VCPKG_LINE_INFO, "Error: Couldn't find baseline in commit `\"%s\"` from repo %s:\n%s\n", m_baseline_identifier, @@ -696,8 +697,8 @@ namespace } else { - LockGuardPtr(g_metrics)->track_property("registries-error-could-not-find-baseline", - "defined"); + LockGuardPtr(g_metrics)->track_define_property( + DefineMetric::RegistriesErrorCouldNotFindBaseline); Checks::exit_maybe_upgrade( VCPKG_LINE_INFO, "The baseline.json from commit `\"%s\"` in the repo %s did not contain a \"default\" field.", diff --git a/src/vcpkg/sourceparagraph.cpp b/src/vcpkg/sourceparagraph.cpp index 3619c7ab90..49e0eb800b 100644 --- a/src/vcpkg/sourceparagraph.cpp +++ b/src/vcpkg/sourceparagraph.cpp @@ -1290,7 +1290,7 @@ namespace vcpkg { if (dep.constraint.type != VersionConstraintKind::None) { - LockGuardPtr(g_metrics)->track_property("error-versioning-disabled", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::ErrorVersioningDisabled); return Strings::concat( origin, " was rejected because it uses constraints and the `", @@ -1311,7 +1311,7 @@ namespace vcpkg if (core_paragraph->overrides.size() != 0) { - LockGuardPtr(g_metrics)->track_property("error-versioning-disabled", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::ErrorVersioningDisabled); return Strings::concat( origin, format_error_message(ManifestDeserializer::OVERRIDES, VcpkgCmdArguments::VERSIONS_FEATURE), @@ -1320,7 +1320,7 @@ namespace vcpkg if (core_paragraph->builtin_baseline.has_value()) { - LockGuardPtr(g_metrics)->track_property("error-versioning-disabled", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::ErrorVersioningDisabled); return Strings::concat( origin, format_error_message(ManifestDeserializer::BUILTIN_BASELINE, VcpkgCmdArguments::VERSIONS_FEATURE), @@ -1337,7 +1337,7 @@ namespace vcpkg return dependency.constraint.type != VersionConstraintKind::None; })) { - LockGuardPtr(g_metrics)->track_property("error-versioning-no-baseline", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::ErrorVersioningNoBaseline); return Strings::concat( origin, " was rejected because it uses \"version>=\" and does not have a \"builtin-baseline\".\n", @@ -1346,7 +1346,7 @@ namespace vcpkg if (!core_paragraph->overrides.empty()) { - LockGuardPtr(g_metrics)->track_property("error-versioning-no-baseline", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::ErrorVersioningNoBaseline); return Strings::concat( origin, " was rejected because it uses \"overrides\" and does not have a \"builtin-baseline\".\n", diff --git a/src/vcpkg/vcpkgcmdarguments.cpp b/src/vcpkg/vcpkgcmdarguments.cpp index bed7ae97df..e382cb5627 100644 --- a/src/vcpkg/vcpkgcmdarguments.cpp +++ b/src/vcpkg/vcpkgcmdarguments.cpp @@ -39,8 +39,8 @@ namespace vcpkg if (place.has_value()) { msg::println_error(msgTwoFeatureFlagsSpecified, msg::value = flag); - LockGuardPtr(g_metrics)->track_property("error", - "error feature flag +-" + flag.to_string()); + LockGuardPtr(g_metrics)->track_string_property(StringMetric::Error, + "error feature flag +-" + flag.to_string()); Checks::exit_fail(VCPKG_LINE_INFO); } @@ -79,7 +79,8 @@ namespace vcpkg if (nullptr != option_field) { msg::println_error(msgDuplicateOptions, msg::value = option_name); - LockGuardPtr(g_metrics)->track_property("error", "error option specified multiple times"); + LockGuardPtr(g_metrics)->track_string_property(StringMetric::Error, + "error option specified multiple times"); print_usage(); Checks::exit_fail(VCPKG_LINE_INFO); } @@ -92,7 +93,7 @@ namespace vcpkg if (option_field && option_field != new_setting) { msg::println_error(msgConflictingValuesForOption, msg::option = option_name); - LockGuardPtr(g_metrics)->track_property("error", "error conflicting switches"); + LockGuardPtr(g_metrics)->track_string_property(StringMetric::Error, "error conflicting switches"); print_usage(); Checks::exit_fail(VCPKG_LINE_INFO); } @@ -106,7 +107,7 @@ namespace vcpkg if (new_value.size() == 0) { msg::println_error(msgExpectedValueForOption, msg::option = option_name); - LockGuardPtr(g_metrics)->track_property("error", "error option name"); + LockGuardPtr(g_metrics)->track_string_property(StringMetric::Error, "error option name"); print_usage(); Checks::exit_fail(VCPKG_LINE_INFO); } @@ -121,7 +122,7 @@ namespace vcpkg if (new_value.size() == 0) { msg::println_error(msgExpectedValueForOption, msg::option = option_name); - LockGuardPtr(g_metrics)->track_property("error", "error option name"); + LockGuardPtr(g_metrics)->track_string_property(StringMetric::Error, "error option name"); print_usage(); Checks::exit_fail(VCPKG_LINE_INFO); } @@ -188,7 +189,7 @@ namespace vcpkg } msg::println_error(msgExpectedValueForOption, msg::option = option); - LockGuardPtr(g_metrics)->track_property("error", "error option name"); + LockGuardPtr(g_metrics)->track_string_property(StringMetric::Error, "error option name"); print_usage(); Checks::exit_fail(VCPKG_LINE_INFO); } @@ -261,7 +262,8 @@ namespace vcpkg if (basic_arg.size() >= 2 && basic_arg[0] == '-' && basic_arg[1] != '-') { - LockGuardPtr(g_metrics)->track_property("error", "error short options are not supported"); + LockGuardPtr(g_metrics)->track_string_property(StringMetric::Error, + "error short options are not supported"); Checks::msg_exit_with_message(VCPKG_LINE_INFO, msgUnsupportedShortOptions, msg::value = basic_arg); } @@ -834,8 +836,8 @@ namespace vcpkg msg::println_warning( msgSpecifiedFeatureTurnedOff, msg::command_name = el.flag, msg::option = el.option); msg::println_warning(msgDefaultFlag, msg::option = el.flag); - LockGuardPtr(g_metrics)->track_property( - "warning", Strings::format("warning %s alongside %s", el.flag, el.option)); + LockGuardPtr(g_metrics)->track_string_property( + StringMetric::Warning, Strings::format("warning %s alongside %s", el.flag, el.option)); } } } diff --git a/src/vcpkg/vcpkglib.cpp b/src/vcpkg/vcpkglib.cpp index 662faa39ce..27e7c87386 100644 --- a/src/vcpkg/vcpkglib.cpp +++ b/src/vcpkg/vcpkglib.cpp @@ -111,7 +111,7 @@ namespace vcpkg if (!was_tracked) { was_tracked = true; - LockGuardPtr(g_metrics)->track_property("listfile", "update to new format"); + LockGuardPtr(g_metrics)->track_string_property(StringMetric::ListFile, "update to new format"); } // The files are sorted such that directories are placed just before the files they contain diff --git a/src/vcpkg/vcpkgpaths.cpp b/src/vcpkg/vcpkgpaths.cpp index 6832248563..49d48cef08 100644 --- a/src/vcpkg/vcpkgpaths.cpp +++ b/src/vcpkg/vcpkgpaths.cpp @@ -187,10 +187,10 @@ namespace vcpkg { if (auto p_baseline = manifest->builtin_baseline.get()) { - LockGuardPtr(g_metrics)->track_property("manifest_baseline", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::ManifestBaseline); if (!is_git_commit_sha(*p_baseline)) { - LockGuardPtr(g_metrics)->track_property("versioning-error-baseline", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::VersioningErrorBaseline); Checks::exit_maybe_upgrade(VCPKG_LINE_INFO, "Error: the top-level builtin-baseline%s was not a valid commit sha: " "expected 40 hexadecimal characters.\n%s\n", @@ -325,7 +325,7 @@ namespace vcpkg Path ret; if (args.registries_cache_dir) { - LockGuardPtr(g_metrics)->track_property("X_VCPKG_REGISTRIES_CACHE", "defined"); + LockGuardPtr(g_metrics)->track_define_property(DefineMetric::X_VcpkgRegistriesCache); ret = *args.registries_cache_dir; const auto status = get_real_filesystem().status(ret, VCPKG_LINE_INFO); if (!vcpkg::exists(status)) @@ -666,11 +666,12 @@ namespace vcpkg LockGuardPtr metrics(g_metrics); if (default_registry) { - metrics->track_property("registries-default-registry-kind", default_registry->kind().to_string()); + metrics->track_string_property(StringMetric::RegistriesDefaultRegistryKind, + default_registry->kind().to_string()); } else { - metrics->track_property("registries-default-registry-kind", "disabled"); + metrics->track_string_property(StringMetric::RegistriesDefaultRegistryKind, "disabled"); } if (other_registries.size() != 0) @@ -681,7 +682,7 @@ namespace vcpkg registry_kinds.push_back(reg.implementation().kind()); } Util::sort_unique_erase(registry_kinds); - metrics->track_property("registries-kinds-used", Strings::join(",", registry_kinds)); + metrics->track_string_property(StringMetric::RegistriesKindsUsed, Strings::join(",", registry_kinds)); } } }