[UrlParamFilter] Add applicable classifications metric
The new metric is intended to serve as a sanity check on what is received via component updater, especially if experiments are run. There is no intended user-facing behavioral change with this CL. Bug: 1327663 Change-Id: Icd69cbd8e366c5ef1f9f2767ea53475f68d858ba Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3696999 Reviewed-by: Ben Kelly <wanderview@chromium.org> Commit-Queue: Matt Reichhoff <mreichhoff@chromium.org> Cr-Commit-Position: refs/heads/main@{#1012662}
This commit is contained in:
Matt Reichhoff
committed by
Chromium LUCI CQ
Chromium LUCI CQ
parent
2212e2bf1f
commit
db919c1d55
chrome/browser/url_param_filter
components/url_param_filter/core
tools/metrics/histograms/metadata
@ -39,6 +39,10 @@ constexpr static const char kCrossOtrRefreshCountMetricName[] =
|
||||
"Navigation.CrossOtr.ContextMenu.RefreshCount";
|
||||
constexpr static const char kFilteredParamCountMetricName[] =
|
||||
"Navigation.UrlParamFilter.FilteredParamCount";
|
||||
constexpr static const char kApplicableSourceClassificationCount[] =
|
||||
"Navigation.UrlParamFilter.ApplicableClassificationCount.Source";
|
||||
constexpr static const char kApplicableDestinationClassificationCount[] =
|
||||
"Navigation.UrlParamFilter.ApplicableClassificationCount.Destination";
|
||||
constexpr static const char kDefaultTag[] = "default";
|
||||
|
||||
} // namespace
|
||||
@ -1002,14 +1006,13 @@ class ContextMenuIncognitoFilterComponentUpdaterBrowserTest
|
||||
}
|
||||
base::ScopedTempDir component_dir_;
|
||||
base::test::ScopedFeatureList scoped_feature_list_;
|
||||
base::HistogramTester histogram_tester_;
|
||||
};
|
||||
|
||||
// Enable "Open Link in Incognito Window" URL parameter filtering, and ensure
|
||||
// that it filters as expected.
|
||||
IN_PROC_BROWSER_TEST_F(ContextMenuIncognitoFilterComponentUpdaterBrowserTest,
|
||||
OpenIncognitoUrlParamFilter) {
|
||||
base::HistogramTester histogram_tester;
|
||||
|
||||
ui_test_utils::AllBrowserTabAddedWaiter add_tab;
|
||||
|
||||
ASSERT_TRUE(embedded_test_server()->Start());
|
||||
@ -1046,12 +1049,13 @@ IN_PROC_BROWSER_TEST_F(ContextMenuIncognitoFilterComponentUpdaterBrowserTest,
|
||||
|
||||
// The response was a 200 (params filtered => metrics collected), and the
|
||||
// navigation went from normal --> OTR browsing.
|
||||
histogram_tester.ExpectUniqueSample(
|
||||
histogram_tester_.ExpectUniqueSample(
|
||||
kCrossOtrResponseMetricName,
|
||||
net::HttpUtil::MapStatusCodeForHistogram(200), 1);
|
||||
// All params being removed are `default`, and no experiment override is
|
||||
// specified on the feature.
|
||||
histogram_tester.ExpectTotalCount(kCrossOtrResponseExperimentalMetricName, 0);
|
||||
histogram_tester_.ExpectTotalCount(kCrossOtrResponseExperimentalMetricName,
|
||||
0);
|
||||
}
|
||||
|
||||
class ContextMenuIncognitoFilterComponentUpdaterExperimentBrowserTest
|
||||
@ -1090,8 +1094,6 @@ class ContextMenuIncognitoFilterComponentUpdaterExperimentBrowserTest
|
||||
IN_PROC_BROWSER_TEST_F(
|
||||
ContextMenuIncognitoFilterComponentUpdaterExperimentBrowserTest,
|
||||
OpenIncognitoUrlParamFilter) {
|
||||
base::HistogramTester histogram_tester;
|
||||
|
||||
ui_test_utils::AllBrowserTabAddedWaiter add_tab;
|
||||
|
||||
ASSERT_TRUE(embedded_test_server()->Start());
|
||||
@ -1132,12 +1134,22 @@ IN_PROC_BROWSER_TEST_F(
|
||||
// The response was a 200 (params filtered => metrics collected), and the
|
||||
// navigation went from normal --> OTR browsing. Because an experimental param
|
||||
// was removed, we also validate that the segmented metric was written.
|
||||
histogram_tester.ExpectUniqueSample(
|
||||
histogram_tester_.ExpectUniqueSample(
|
||||
kCrossOtrResponseMetricName,
|
||||
net::HttpUtil::MapStatusCodeForHistogram(200), 1);
|
||||
histogram_tester.ExpectUniqueSample(
|
||||
histogram_tester_.ExpectUniqueSample(
|
||||
kCrossOtrResponseExperimentalMetricName,
|
||||
net::HttpUtil::MapStatusCodeForHistogram(200), 1);
|
||||
histogram_tester_.ExpectTotalCount(kApplicableSourceClassificationCount, 1);
|
||||
histogram_tester_.ExpectTotalCount(kApplicableDestinationClassificationCount,
|
||||
1);
|
||||
// Although additional classifications are passed, they are not applicable
|
||||
// given the experiment override.
|
||||
EXPECT_EQ(histogram_tester_.GetTotalSum(kApplicableSourceClassificationCount),
|
||||
1);
|
||||
EXPECT_EQ(
|
||||
histogram_tester_.GetTotalSum(kApplicableDestinationClassificationCount),
|
||||
0);
|
||||
}
|
||||
|
||||
class ContextMenuIncognitoFilterComponentUpdaterAdditiveExperimentBrowserTest
|
||||
@ -1179,8 +1191,6 @@ class ContextMenuIncognitoFilterComponentUpdaterAdditiveExperimentBrowserTest
|
||||
IN_PROC_BROWSER_TEST_F(
|
||||
ContextMenuIncognitoFilterComponentUpdaterAdditiveExperimentBrowserTest,
|
||||
OpenIncognitoUrlParamFilter) {
|
||||
base::HistogramTester histogram_tester;
|
||||
|
||||
ui_test_utils::AllBrowserTabAddedWaiter add_tab;
|
||||
|
||||
ASSERT_TRUE(embedded_test_server()->Start());
|
||||
@ -1221,19 +1231,25 @@ IN_PROC_BROWSER_TEST_F(
|
||||
// The response was a 200 (params filtered => metrics collected), and the
|
||||
// navigation went from normal --> OTR browsing. Because a param included in
|
||||
// the experiment was removed, we should also see that metric written.
|
||||
histogram_tester.ExpectUniqueSample(
|
||||
histogram_tester_.ExpectUniqueSample(
|
||||
kCrossOtrResponseMetricName,
|
||||
net::HttpUtil::MapStatusCodeForHistogram(200), 1);
|
||||
histogram_tester.ExpectUniqueSample(
|
||||
histogram_tester_.ExpectUniqueSample(
|
||||
kCrossOtrResponseExperimentalMetricName,
|
||||
net::HttpUtil::MapStatusCodeForHistogram(200), 1);
|
||||
histogram_tester_.ExpectTotalCount(kApplicableSourceClassificationCount, 1);
|
||||
histogram_tester_.ExpectTotalCount(kApplicableDestinationClassificationCount,
|
||||
1);
|
||||
EXPECT_EQ(histogram_tester_.GetTotalSum(kApplicableSourceClassificationCount),
|
||||
2);
|
||||
EXPECT_EQ(
|
||||
histogram_tester_.GetTotalSum(kApplicableDestinationClassificationCount),
|
||||
1);
|
||||
}
|
||||
|
||||
IN_PROC_BROWSER_TEST_F(
|
||||
ContextMenuIncognitoFilterComponentUpdaterAdditiveExperimentBrowserTest,
|
||||
OpenIncognitoUrlParamFilterNoExperimentalParamFiltered) {
|
||||
base::HistogramTester histogram_tester;
|
||||
|
||||
ui_test_utils::AllBrowserTabAddedWaiter add_tab;
|
||||
|
||||
ASSERT_TRUE(embedded_test_server()->Start());
|
||||
@ -1272,10 +1288,11 @@ IN_PROC_BROWSER_TEST_F(
|
||||
// The response was a 200 (params filtered => metrics collected), and the
|
||||
// navigation went from normal --> OTR browsing. Because no non-default param
|
||||
// was removed, the experimental metric should not be written.
|
||||
histogram_tester.ExpectUniqueSample(
|
||||
histogram_tester_.ExpectUniqueSample(
|
||||
kCrossOtrResponseMetricName,
|
||||
net::HttpUtil::MapStatusCodeForHistogram(200), 1);
|
||||
histogram_tester.ExpectTotalCount(kCrossOtrResponseExperimentalMetricName, 0);
|
||||
histogram_tester_.ExpectTotalCount(kCrossOtrResponseExperimentalMetricName,
|
||||
0);
|
||||
}
|
||||
|
||||
} // namespace url_param_filter
|
||||
|
||||
@ -9,6 +9,7 @@
|
||||
|
||||
#include "base/base64.h"
|
||||
#include "base/metrics/field_trial_params.h"
|
||||
#include "base/metrics/histogram_functions.h"
|
||||
#include "base/no_destructor.h"
|
||||
#include "base/sequence_checker.h"
|
||||
#include "base/strings/string_util.h"
|
||||
@ -50,12 +51,7 @@ void AppendParams(ClassificationMap& map,
|
||||
}
|
||||
|
||||
void ProcessClassification(ClassificationMap& map,
|
||||
const FilterClassification& classification,
|
||||
std::string experiment_identifier) {
|
||||
if (!experiment_identifier.empty() &&
|
||||
!HasExperimentTag(classification, experiment_identifier)) {
|
||||
return;
|
||||
}
|
||||
const FilterClassification& classification) {
|
||||
if (classification.use_cases_size() > 0) {
|
||||
for (int use_case : classification.use_cases()) {
|
||||
AppendParams(map, classification,
|
||||
@ -81,7 +77,7 @@ ClassificationMap GetClassificationsFromFeature(
|
||||
// When retrieving classifications from the feature, we do not allow
|
||||
// additional experiment overrides.
|
||||
DCHECK(i.experiment_tags().empty());
|
||||
ProcessClassification(map, i, "");
|
||||
ProcessClassification(map, i);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -96,15 +92,8 @@ ClassificationMap GetClassificationMap(
|
||||
if (!classifications.has_value())
|
||||
return ClassificationMap();
|
||||
ClassificationMap map;
|
||||
std::string experiment_identifier = base::GetFieldTrialParamValueByFeature(
|
||||
features::kIncognitoParamFilterEnabled, "experiment_identifier");
|
||||
// If there is no experiment identifier passed via the feature, then use the
|
||||
// classifications that are marked `default`.
|
||||
if (experiment_identifier.empty()) {
|
||||
experiment_identifier = DEFAULT_TAG;
|
||||
}
|
||||
for (const FilterClassification& classification : classifications.value()) {
|
||||
ProcessClassification(map, classification, experiment_identifier);
|
||||
ProcessClassification(map, classification);
|
||||
}
|
||||
return map;
|
||||
}
|
||||
@ -138,19 +127,42 @@ void ClassificationsLoader::ReadClassifications(
|
||||
|
||||
std::vector<FilterClassification> source_classifications,
|
||||
destination_classifications;
|
||||
int total_applicable_source_classifications = 0;
|
||||
int total_applicable_destination_classifications = 0;
|
||||
std::string experiment_identifier = base::GetFieldTrialParamValueByFeature(
|
||||
features::kIncognitoParamFilterEnabled, "experiment_identifier");
|
||||
// If there is no experiment identifier passed via the feature, then use the
|
||||
// classifications that are marked `default`.
|
||||
if (experiment_identifier.empty()) {
|
||||
experiment_identifier = DEFAULT_TAG;
|
||||
}
|
||||
for (const FilterClassification& fc : classification_list.classifications()) {
|
||||
DCHECK(fc.has_site());
|
||||
DCHECK(fc.has_site_role());
|
||||
if (fc.site_role() == FilterClassification_SiteRole_SOURCE)
|
||||
if (!HasExperimentTag(fc, experiment_identifier)) {
|
||||
continue;
|
||||
}
|
||||
if (fc.site_role() == FilterClassification_SiteRole_SOURCE) {
|
||||
source_classifications.push_back(fc);
|
||||
total_applicable_source_classifications++;
|
||||
}
|
||||
|
||||
if (fc.site_role() == FilterClassification_SiteRole_DESTINATION)
|
||||
if (fc.site_role() == FilterClassification_SiteRole_DESTINATION) {
|
||||
destination_classifications.push_back(fc);
|
||||
total_applicable_destination_classifications++;
|
||||
}
|
||||
}
|
||||
|
||||
component_source_classifications_ = std::move(source_classifications);
|
||||
component_destination_classifications_ =
|
||||
std::move(destination_classifications);
|
||||
|
||||
base::UmaHistogramCounts10000(
|
||||
"Navigation.UrlParamFilter.ApplicableClassificationCount.Source",
|
||||
total_applicable_source_classifications);
|
||||
base::UmaHistogramCounts10000(
|
||||
"Navigation.UrlParamFilter.ApplicableClassificationCount.Destination",
|
||||
total_applicable_destination_classifications);
|
||||
}
|
||||
|
||||
void ClassificationsLoader::ResetListsForTesting() {
|
||||
|
||||
@ -12,6 +12,7 @@
|
||||
#include "base/files/file_util.h"
|
||||
#include "base/files/scoped_temp_dir.h"
|
||||
#include "base/memory/raw_ptr.h"
|
||||
#include "base/test/metrics/histogram_tester.h"
|
||||
#include "base/test/scoped_feature_list.h"
|
||||
#include "components/url_param_filter/core/features.h"
|
||||
#include "components/url_param_filter/core/url_param_filter_classification.pb.h"
|
||||
@ -29,6 +30,12 @@ using ::testing::UnorderedElementsAre;
|
||||
|
||||
namespace url_param_filter {
|
||||
namespace {
|
||||
constexpr char kApplicableClassificationsSourceMetric[] =
|
||||
"Navigation.UrlParamFilter.ApplicableClassificationCount.Source";
|
||||
constexpr char kApplicableClassificationsDestinationMetric[] =
|
||||
"Navigation.UrlParamFilter.ApplicableClassificationCount.Destination";
|
||||
constexpr char kApplicableClassificationsInvalidMetric[] =
|
||||
"Navigation.UrlParamFilter.ApplicableClassificationCount.Invalid";
|
||||
|
||||
class UrlParamClassificationsLoaderTest : public ::testing::Test {
|
||||
public:
|
||||
@ -106,6 +113,7 @@ TEST_F(UrlParamClassificationsLoaderTest, ReadClassifications_EmptyList) {
|
||||
}
|
||||
|
||||
TEST_F(UrlParamClassificationsLoaderTest, ReadClassifications_OnlySources) {
|
||||
base::HistogramTester histogram_tester;
|
||||
FilterClassifications classifications = MakeClassificationsProtoFromMap(
|
||||
{{"source1.xyz", {"plzblock1"}}, {"source2.xyz", {"plzblock2"}}}, {});
|
||||
SetComponentFileContents(classifications.SerializeAsString());
|
||||
@ -127,10 +135,21 @@ TEST_F(UrlParamClassificationsLoaderTest, ReadClassifications_OnlySources) {
|
||||
"plzblock2",
|
||||
ClassificationExperimentStatus::NON_EXPERIMENTAL)))))));
|
||||
EXPECT_THAT(loader()->GetDestinationClassifications(), IsEmpty());
|
||||
|
||||
histogram_tester.ExpectTotalCount(kApplicableClassificationsSourceMetric, 1);
|
||||
ASSERT_EQ(
|
||||
histogram_tester.GetTotalSum(kApplicableClassificationsSourceMetric), 2);
|
||||
histogram_tester.ExpectTotalCount(kApplicableClassificationsDestinationMetric,
|
||||
1);
|
||||
ASSERT_EQ(
|
||||
histogram_tester.GetTotalSum(kApplicableClassificationsDestinationMetric),
|
||||
0);
|
||||
histogram_tester.ExpectTotalCount(kApplicableClassificationsInvalidMetric, 0);
|
||||
}
|
||||
|
||||
TEST_F(UrlParamClassificationsLoaderTest,
|
||||
ReadClassifications_OnlyDestinations) {
|
||||
base::HistogramTester histogram_tester;
|
||||
FilterClassifications classifications = MakeClassificationsProtoFromMap(
|
||||
{}, {{"destination1.xyz", {"plzblock1"}},
|
||||
{"destination2.xyz", {"plzblock2"}}});
|
||||
@ -154,10 +173,20 @@ TEST_F(UrlParamClassificationsLoaderTest,
|
||||
UnorderedElementsAre(Pair(
|
||||
"plzblock2",
|
||||
ClassificationExperimentStatus::NON_EXPERIMENTAL)))))));
|
||||
histogram_tester.ExpectTotalCount(kApplicableClassificationsSourceMetric, 1);
|
||||
ASSERT_EQ(
|
||||
histogram_tester.GetTotalSum(kApplicableClassificationsSourceMetric), 0);
|
||||
histogram_tester.ExpectTotalCount(kApplicableClassificationsDestinationMetric,
|
||||
1);
|
||||
ASSERT_EQ(
|
||||
histogram_tester.GetTotalSum(kApplicableClassificationsDestinationMetric),
|
||||
2);
|
||||
histogram_tester.ExpectTotalCount(kApplicableClassificationsInvalidMetric, 0);
|
||||
}
|
||||
|
||||
TEST_F(UrlParamClassificationsLoaderTest,
|
||||
ReadClassifications_SourcesAndDestinations) {
|
||||
base::HistogramTester histogram_tester;
|
||||
FilterClassifications classifications = MakeClassificationsProtoFromMap(
|
||||
{{"source1.xyz", {"plzblock1"}}}, {{"destination2.xyz", {"plzblock2"}}});
|
||||
|
||||
@ -182,6 +211,16 @@ TEST_F(UrlParamClassificationsLoaderTest,
|
||||
UnorderedElementsAre(Pair(
|
||||
"plzblock2",
|
||||
ClassificationExperimentStatus::NON_EXPERIMENTAL)))))));
|
||||
|
||||
histogram_tester.ExpectTotalCount(kApplicableClassificationsSourceMetric, 1);
|
||||
ASSERT_EQ(
|
||||
histogram_tester.GetTotalSum(kApplicableClassificationsSourceMetric), 1);
|
||||
histogram_tester.ExpectTotalCount(kApplicableClassificationsDestinationMetric,
|
||||
1);
|
||||
ASSERT_EQ(
|
||||
histogram_tester.GetTotalSum(kApplicableClassificationsDestinationMetric),
|
||||
1);
|
||||
histogram_tester.ExpectTotalCount(kApplicableClassificationsInvalidMetric, 0);
|
||||
}
|
||||
|
||||
TEST_F(UrlParamClassificationsLoaderTest,
|
||||
@ -215,6 +254,7 @@ TEST_F(UrlParamClassificationsLoaderTest,
|
||||
|
||||
TEST_F(UrlParamClassificationsLoaderTest,
|
||||
GetClassifications_ComponentOnlyWithExperiment) {
|
||||
base::HistogramTester histogram_tester;
|
||||
const std::string experiment_identifier = "mattwashere";
|
||||
base::test::ScopedFeatureList scoped_feature_list;
|
||||
base::FieldTrialParams params;
|
||||
@ -275,6 +315,18 @@ TEST_F(UrlParamClassificationsLoaderTest,
|
||||
UnorderedElementsAre(Pair(
|
||||
"plzblock7",
|
||||
ClassificationExperimentStatus::EXPERIMENTAL)))))));
|
||||
|
||||
// Although there are 6 total classifications, only one source and one
|
||||
// destination classification is applicable due to the experiment override.
|
||||
histogram_tester.ExpectTotalCount(kApplicableClassificationsSourceMetric, 1);
|
||||
ASSERT_EQ(
|
||||
histogram_tester.GetTotalSum(kApplicableClassificationsSourceMetric), 1);
|
||||
histogram_tester.ExpectTotalCount(kApplicableClassificationsDestinationMetric,
|
||||
1);
|
||||
ASSERT_EQ(
|
||||
histogram_tester.GetTotalSum(kApplicableClassificationsDestinationMetric),
|
||||
1);
|
||||
histogram_tester.ExpectTotalCount(kApplicableClassificationsInvalidMetric, 0);
|
||||
}
|
||||
|
||||
TEST_F(UrlParamClassificationsLoaderTest,
|
||||
|
||||
@ -1273,6 +1273,22 @@ Also used in tools/metrics/histograms/metadata/page/histograms.xml.
|
||||
</summary>
|
||||
</histogram>
|
||||
|
||||
<histogram
|
||||
name="Navigation.UrlParamFilter.ApplicableClassificationCount{UrlParamFilterClassificationRole}"
|
||||
units="ms" expires_after="2023-01-02">
|
||||
<owner>mreichhoff@chromium.org</owner>
|
||||
<owner>wanderview@chromium.org</owner>
|
||||
<owner>bcl@chromium.org</owner>
|
||||
<owner>dc-komics@google.com</owner>
|
||||
<summary>
|
||||
The number of FilterClassification objects received from component updater
|
||||
that are determined to be applicable given experiment settings. This metric
|
||||
is recorded when the component updater parsing is completed.
|
||||
</summary>
|
||||
<token key="UrlParamFilterClassificationRole"
|
||||
variants="UrlParamFilterClassificationRole"/>
|
||||
</histogram>
|
||||
|
||||
<histogram name="Navigation.UrlParamFilter.ClassificationListValidationResult"
|
||||
enum="ClassificationListValidationResult" expires_after="2022-11-20">
|
||||
<owner>kaklilu@chromium.org</owner>
|
||||
|
||||
@ -133,6 +133,16 @@ chromium-metrics-reviews@google.com.
|
||||
<variant name="WorkerProcess" summary=""/>
|
||||
</variants>
|
||||
|
||||
<variants name="UrlParamFilterClassificationRole">
|
||||
<variant name=".Destination"
|
||||
summary="parameters classified as having the destination role"/>
|
||||
<variant name=".Invalid"
|
||||
summary="parameters with an unknown role, likely indicating a
|
||||
compatibility issue"/>
|
||||
<variant name=".Source"
|
||||
summary="parameters classified as having the source role"/>
|
||||
</variants>
|
||||
|
||||
<variants name="VoiceIntentTargetVariant">
|
||||
<variant name="Assistant" summary="Assistant voice transcription."/>
|
||||
<variant name="System" summary="System voice transcription."/>
|
||||
|
||||
Reference in New Issue
Block a user