From 93f3747eca174f1c6954f8ac8d48def4ef6abc48 Mon Sep 17 00:00:00 2001 From: tsaichristine Date: Tue, 24 Jan 2023 11:50:48 -0800 Subject: Add check on sampled_what_field that it's a subset of dimensions_in_what Test: atest statsd_test Bug: 259164633 Change-Id: I110e6e168a775dcd1dfe9146dc8604a5b3c88714 --- .../src/guardrail/invalid_config_reason_enum.proto | 1 + .../metrics/parsing_utils/metrics_manager_util.cpp | 18 ++++++++++++------ .../parsing_utils/metrics_manager_util_test.cpp | 21 +++++++++++++++++++++ 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/statsd/src/guardrail/invalid_config_reason_enum.proto b/statsd/src/guardrail/invalid_config_reason_enum.proto index 74e32c2a..c56e39c1 100644 --- a/statsd/src/guardrail/invalid_config_reason_enum.proto +++ b/statsd/src/guardrail/invalid_config_reason_enum.proto @@ -105,4 +105,5 @@ enum InvalidConfigReasonEnum { INVALID_CONFIG_REASON_METRIC_DIMENSIONAL_SAMPLING_INFO_INCORRECT_SHARD_COUNT = 80; INVALID_CONFIG_REASON_METRIC_DIMENSIONAL_SAMPLING_INFO_MISSING_SAMPLED_FIELD = 81; INVALID_CONFIG_REASON_METRIC_SAMPLED_FIELD_INCORRECT_SIZE = 82; + INVALID_CONFIG_REASON_METRIC_SAMPLED_FIELDS_NOT_SUBSET_DIM_IN_WHAT = 83; }; diff --git a/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp b/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp index 250b93e2..c44c8186 100644 --- a/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp +++ b/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp @@ -251,7 +251,7 @@ optional handleMetricWithStateLink(const int64_t metricId, optional handleMetricWithSampling( const int64_t metricId, const DimensionalSamplingInfo& dimSamplingInfo, - SamplingInfo& samplingInfo) { + const vector& dimensionsInWhat, SamplingInfo& samplingInfo) { if (!dimSamplingInfo.has_sampled_what_field()) { ALOGE("metric DimensionalSamplingInfo missing sampledWhatField"); return InvalidConfigReason( @@ -280,6 +280,10 @@ optional handleMetricWithSampling( return InvalidConfigReason(INVALID_CONFIG_REASON_METRIC_SAMPLED_FIELD_INCORRECT_SIZE, metricId); } + if (!subsetDimensions(samplingInfo.sampledWhatFields, dimensionsInWhat)) { + return InvalidConfigReason( + INVALID_CONFIG_REASON_METRIC_SAMPLED_FIELDS_NOT_SUBSET_DIM_IN_WHAT, metricId); + } return nullopt; } @@ -530,7 +534,7 @@ optional> createCountMetricProducerAndUpdateMetadata( SamplingInfo samplingInfo; if (metric.has_dimensional_sampling_info()) { invalidConfigReason = handleMetricWithSampling( - metric.id(), metric.dimensional_sampling_info(), samplingInfo); + metric.id(), metric.dimensional_sampling_info(), dimensionsInWhat, samplingInfo); if (invalidConfigReason.has_value()) { return nullopt; } @@ -715,7 +719,7 @@ optional> createDurationMetricProducerAndUpdateMetadata( SamplingInfo samplingInfo; if (metric.has_dimensional_sampling_info()) { invalidConfigReason = handleMetricWithSampling( - metric.id(), metric.dimensional_sampling_info(), samplingInfo); + metric.id(), metric.dimensional_sampling_info(), dimensionsInWhat, samplingInfo); if (invalidConfigReason.has_value()) { return nullopt; } @@ -939,7 +943,7 @@ optional> createNumericValueMetricProducerAndUpdateMetadata( SamplingInfo samplingInfo; if (metric.has_dimensional_sampling_info()) { invalidConfigReason = handleMetricWithSampling( - metric.id(), metric.dimensional_sampling_info(), samplingInfo); + metric.id(), metric.dimensional_sampling_info(), dimensionsInWhat, samplingInfo); if (invalidConfigReason.has_value()) { return nullopt; } @@ -1090,7 +1094,7 @@ optional> createKllMetricProducerAndUpdateMetadata( SamplingInfo samplingInfo; if (metric.has_dimensional_sampling_info()) { invalidConfigReason = handleMetricWithSampling( - metric.id(), metric.dimensional_sampling_info(), samplingInfo); + metric.id(), metric.dimensional_sampling_info(), dimensionsInWhat, samplingInfo); if (invalidConfigReason.has_value()) { return nullopt; } @@ -1232,9 +1236,11 @@ optional> createGaugeMetricProducerAndUpdateMetadata( dimensionHardLimit); SamplingInfo samplingInfo; + std::vector dimensionsInWhat; + translateFieldMatcher(metric.dimensions_in_what(), &dimensionsInWhat); if (metric.has_dimensional_sampling_info()) { invalidConfigReason = handleMetricWithSampling( - metric.id(), metric.dimensional_sampling_info(), samplingInfo); + metric.id(), metric.dimensional_sampling_info(), dimensionsInWhat, samplingInfo); if (invalidConfigReason.has_value()) { return nullopt; } diff --git a/statsd/tests/metrics/parsing_utils/metrics_manager_util_test.cpp b/statsd/tests/metrics/parsing_utils/metrics_manager_util_test.cpp index 25679363..a5732b30 100644 --- a/statsd/tests/metrics/parsing_utils/metrics_manager_util_test.cpp +++ b/statsd/tests/metrics/parsing_utils/metrics_manager_util_test.cpp @@ -1761,6 +1761,27 @@ TEST_F(MetricsManagerUtilTest, TestMetricHasRepeatedSampledField_PositionANY) { metric.id())); } +TEST_F(MetricsManagerUtilTest, TestMetricSampledFieldNotSubsetDimension) { + AtomMatcher appCrashMatcher = + CreateSimpleAtomMatcher("APP_CRASH_OCCURRED", util::APP_CRASH_OCCURRED); + + StatsdConfig config; + config.add_allowed_log_source("AID_ROOT"); + *config.add_atom_matcher() = appCrashMatcher; + + CountMetric metric = + createCountMetric("CountSampledAppCrashesPerUid", appCrashMatcher.id(), nullopt, {}); + *metric.mutable_dimensional_sampling_info()->mutable_sampled_what_field() = + CreateDimensions(util::APP_CRASH_OCCURRED, {1 /*uid*/}); + metric.mutable_dimensional_sampling_info()->set_shard_count(2); + *config.add_count_metric() = metric; + + EXPECT_EQ( + initConfig(config), + InvalidConfigReason(INVALID_CONFIG_REASON_METRIC_SAMPLED_FIELDS_NOT_SUBSET_DIM_IN_WHAT, + metric.id())); +} + } // namespace statsd } // namespace os } // namespace android -- cgit v1.2.3