From 466ec4e632857572397409c901332d9bae299602 Mon Sep 17 00:00:00 2001 From: Adam Stone Date: Thu, 27 Jul 2017 15:29:02 -0700 Subject: [PATCH 1/2] Create two new metric types to simplify metrics. This is part one of a mult-part change to revise some metrics. Several metrics are currently EventMetric type when they should be a simpler type. Test: Added unit tests for the new types. Also, re-ran existing tests. Verified playback works with Google Play, and re-ran Widevine GTS tests. Bug: 36220619 Change-Id: I2ec8fc355f66ad4834dd722aacd22541fb9c94ad --- .../build_and_run_all_unit_tests.sh | 6 +- libwvdrmengine/cdm/Android.mk | 2 + .../cdm/metrics/include/counter_metric.h | 205 ++++++++++++++++++ .../cdm/metrics/include/event_metric.h | 166 +++----------- .../cdm/metrics/include/field_tuples.h | 125 +++++++++++ .../cdm/metrics/include/value_metric.h | 100 +++++++++ .../cdm/metrics/src/counter_metric.cpp | 33 +++ .../cdm/metrics/src/value_metric.cpp | 115 ++++++++++ .../metrics/test/counter_metric_unittest.cpp | 177 +++++++++++++++ ...ion_test.cpp => distribution_unittest.cpp} | 0 ...ric_test.cpp => event_metric_unittest.cpp} | 0 ...t.cpp => metrics_collections_unittest.cpp} | 2 +- .../metrics/test/value_metric_unittest.cpp | 77 +++++++ libwvdrmengine/cdm/test/Android.mk | 24 +- libwvdrmengine/run_all_unit_tests.sh | 6 +- tests/Android.mk | 6 +- 16 files changed, 891 insertions(+), 153 deletions(-) create mode 100644 libwvdrmengine/cdm/metrics/include/counter_metric.h create mode 100644 libwvdrmengine/cdm/metrics/include/field_tuples.h create mode 100644 libwvdrmengine/cdm/metrics/include/value_metric.h create mode 100644 libwvdrmengine/cdm/metrics/src/counter_metric.cpp create mode 100644 libwvdrmengine/cdm/metrics/src/value_metric.cpp create mode 100644 libwvdrmengine/cdm/metrics/test/counter_metric_unittest.cpp rename libwvdrmengine/cdm/metrics/test/{distribution_test.cpp => distribution_unittest.cpp} (100%) rename libwvdrmengine/cdm/metrics/test/{event_metric_test.cpp => event_metric_unittest.cpp} (100%) rename libwvdrmengine/cdm/metrics/test/{metrics_collections_test.cpp => metrics_collections_unittest.cpp} (99%) create mode 100644 libwvdrmengine/cdm/metrics/test/value_metric_unittest.cpp diff --git a/libwvdrmengine/build_and_run_all_unit_tests.sh b/libwvdrmengine/build_and_run_all_unit_tests.sh index a2ea2fc4..65b9ad81 100755 --- a/libwvdrmengine/build_and_run_all_unit_tests.sh +++ b/libwvdrmengine/build_and_run_all_unit_tests.sh @@ -81,10 +81,11 @@ try_adb_push cdm_engine_test try_adb_push cdm_feature_test try_adb_push cdm_extended_duration_test try_adb_push cdm_session_unittest +try_adb_push counter_metric_unittest try_adb_push crypto_session_unittest try_adb_push device_files_unittest -try_adb_push distribution_test -try_adb_push event_metric_test +try_adb_push distribution_unittest +try_adb_push event_metric_unittest try_adb_push file_store_unittest try_adb_push file_utils_unittest try_adb_push http_socket_test @@ -104,6 +105,7 @@ try_adb_push request_license_test try_adb_push service_certificate_unittest try_adb_push timer_unittest try_adb_push usage_table_header_unittest +try_adb_push value_metric_unittest try_adb_push wv_cdm_metrics_test # Run the tests using run_all_unit_tests.sh diff --git a/libwvdrmengine/cdm/Android.mk b/libwvdrmengine/cdm/Android.mk index c6060edc..954cc5d2 100644 --- a/libwvdrmengine/cdm/Android.mk +++ b/libwvdrmengine/cdm/Android.mk @@ -40,10 +40,12 @@ LOCAL_SRC_FILES := \ $(CORE_SRC_DIR)/service_certificate.cpp \ $(CORE_SRC_DIR)/usage_table_header.cpp \ $(SRC_DIR)/wv_content_decryption_module.cpp \ + $(METRICS_SRC_DIR)/counter_metric.cpp \ $(METRICS_SRC_DIR)/distribution.cpp \ $(METRICS_SRC_DIR)/event_metric.cpp \ $(METRICS_SRC_DIR)/metrics_collections.cpp \ $(METRICS_SRC_DIR)/timer_metric.cpp \ + $(METRICS_SRC_DIR)/value_metric.cpp LOCAL_MODULE := libcdm diff --git a/libwvdrmengine/cdm/metrics/include/counter_metric.h b/libwvdrmengine/cdm/metrics/include/counter_metric.h new file mode 100644 index 00000000..c66e9996 --- /dev/null +++ b/libwvdrmengine/cdm/metrics/include/counter_metric.h @@ -0,0 +1,205 @@ +// Copyright 2017 Google Inc. All Rights Reserved. +// +// This file contains the declarations for the Metric class and related +// types. +#ifndef WVCDM_METRICS_COUNTER_METRIC_H_ +#define WVCDM_METRICS_COUNTER_METRIC_H_ + +#include +#include +#include +#include +#include + +#include "field_tuples.h" +#include "lock.h" +#include "metric_serialization.h" + +namespace wvcdm { +namespace metrics { + +class CounterMetricTest; + +// This base class provides the common defintion used by all templated +// instances of CounterMetric. +class BaseCounterMetric : public MetricSerializable { + public: + // Send metric values to the MetricSerializer. |serializer| must + // not be null and is owned by the caller. + virtual void Serialize(MetricSerializer* serializer); + + protected: + // Instantiates a BaseCounterMetric. + BaseCounterMetric(const std::string& metric_name) + : metric_name_(metric_name) {} + + virtual ~BaseCounterMetric() {} + + // Increment will look for an existing instance of the field names and + // add the new value to the existing value. If the instance does not exist, + // this method will create it. + void Increment(const std::string& field_names_values, int64_t value); + + private: + friend class CounterMetricTest; + const std::string metric_name_; + // value_map_ contains a mapping from the field name/value pairs to the + // counter(int64_t) instance. + std::map value_map_; + + /* + * This locks the internal state of the counter metric to ensure safety across + * multiple threads preventing the caller from worrying about locking. + */ + Lock internal_lock_; +}; + +// The CounterMetric class is used to track a counter such as the number of +// times a method is called. It can also track a delta that could be positive +// or negative. +// +// The CounterMetric class supports the ability to keep track of multiple +// variations of the count based on certain "field" values. E.g. keep track of +// the counts of success and failure counts for a method call. Or keep track of +// number of times the method was called with a particular parameter. +// Fields define what variations to track. Each Field is a separate dimension. +// The count for each combination of field values is tracked independently. +// +// Example usage: +// +// CounterMetric my_metric("multiple/fields/metric", +// "request_type", // Field name. +// "error_code"); // Field name. +// +// my_metric.Increment(1, 7, 23); // (counter value, request type, error code). +// +// The CounterMetric supports serialization. A call to Serialize will serialize +// all values to the provided MetricsSerializer instance. +// +// example: +// +// class MyMetricSerializer : public MetricSerializer { +// // Add implementation here. +// } +// +// MyMetricSerializer serializer; +// my_metric.Serialize(&serializer); +template +class CounterMetric : public BaseCounterMetric { + public: + // Create a CounterMetric instance with the name |metric_name|. + CounterMetric(const std::string& metric_name); + + // Overloaded constructors with variable field name arguments. The number + // of |field_name| arguments must match the number of used Field type + // arguments. + CounterMetric(const std::string& metric_name, + const char* field_name); + CounterMetric(const std::string& metric_name, + const char* field_name1, + const char* field_name2); + CounterMetric(const std::string& metric_name, + const char* field_name1, + const char* field_name2, + const char* field_name3); + CounterMetric(const std::string& metric_name, + const char* field_name1, + const char* field_name2, + const char* field_name3, + const char* field_name4); + + // Increment will update the counter value associated with the provided + // field values. + void Increment(F1 field1 = util::Unused(), F2 field2 = util::Unused(), + F3 field3 = util::Unused(), F4 field4 = util::Unused()); + + // Increment will add the value to the counter associated with the provided + // field values. + void Increment(int64_t value, + F1 field1 = util::Unused(), F2 field2 = util::Unused(), + F3 field3 = util::Unused(), F4 field4 = util::Unused()); + + private: + friend class CounterMetricTest; + std::vector field_names_; +}; + +// Overloaded template constructor implementations for CounterMetric. +template +CounterMetric::CounterMetric( + const std::string& metric_name) + : BaseCounterMetric(metric_name) { + ASSERT_METRIC_UNUSED_START_FROM(1); +} + +template +CounterMetric::CounterMetric( + const std::string& metric_name, + const char* field_name) + : BaseCounterMetric(metric_name) { + ASSERT_METRIC_UNUSED_START_FROM(2); + util::AppendFieldNames(&field_names_, + util::FirstUnusedType::value - 1, + field_name); +} +template +CounterMetric::CounterMetric( + const std::string& metric_name, + const char* field_name1, + const char* field_name2) + : BaseCounterMetric(metric_name) { + ASSERT_METRIC_UNUSED_START_FROM(3); + util::AppendFieldNames(&field_names_, + util::FirstUnusedType::value - 1, + field_name1, field_name2); +} +template +CounterMetric::CounterMetric( + const std::string& metric_name, + const char* field_name1, + const char* field_name2, + const char* field_name3) + : BaseCounterMetric(metric_name) { + ASSERT_METRIC_UNUSED_START_FROM(4); + util::AppendFieldNames(&field_names_, + util::FirstUnusedType::value - 1, + field_name1, field_name2, field_name3); +} +template +CounterMetric::CounterMetric( + const std::string& metric_name, + const char* field_name1, + const char* field_name2, + const char* field_name3, + const char* field_name4) + : BaseCounterMetric(metric_name) { + ASSERT_METRIC_UNUSED_START_FROM(5); + util::AppendFieldNames(&field_names_, + util::FirstUnusedType::value - 1, + field_name1, field_name2, + field_name3, field_name4); +} + +template +void CounterMetric::Increment( + F1 field1, F2 field2, F3 field3, F4 field4) { + std::string field_name_values = + util::MakeFieldNameString(field_names_, field1, field2, field3, field4); + BaseCounterMetric::Increment(field_name_values, 1); +} + +template +void CounterMetric::Increment( + int64_t value, F1 field1, F2 field2, F3 field3, F4 field4) { + std::string field_name_values = + util::MakeFieldNameString(field_names_, field1, field2, field3, field4); + BaseCounterMetric::Increment(field_name_values, value); +} + +} // namespace metrics +} // namespace wvcdm + +#endif // WVCDM_METRICS_COUNTER_METRIC_H_ diff --git a/libwvdrmengine/cdm/metrics/include/event_metric.h b/libwvdrmengine/cdm/metrics/include/event_metric.h index 4e81ad62..dcfd5223 100644 --- a/libwvdrmengine/cdm/metrics/include/event_metric.h +++ b/libwvdrmengine/cdm/metrics/include/event_metric.h @@ -6,18 +6,13 @@ #define WVCDM_METRICS_EVENT_METRIC_H_ #include -#include #include -#include -#include -#include -#include #include #include -#include "lock.h" - #include "distribution.h" +#include "field_tuples.h" +#include "lock.h" #include "metric_serialization.h" namespace wvcdm { @@ -60,14 +55,6 @@ class BaseEventMetric : public MetricSerializable { Lock internal_lock_; }; -// This is a placeholder type for unused type parameters. -struct Unused { - // Required for compilation. Should never be used. - inline friend std::ostream& operator<< (std::ostream& out, const Unused&) - { return out; } -}; - - // This class converts the size_t value into the highest power of two // below the value. E.g. for 7, the value is 4. For 11, the value is 8. // This class is intended to simplify the use of EventMetric Fields that may @@ -113,7 +100,7 @@ class Pow2Bucket { // on certain "field" values. For example, if a particular operation can run in // one of two modes, it's useful to track the latency of the operation in each // mode separately. You can use Fields to define how to breakdown the -// statistics. Each Field is a separate dimention. The statistics for each +// statistics. Each Field is a separate dimension. The statistics for each // combination of field values are tracked independently. // // Example usage: @@ -123,19 +110,22 @@ class Pow2Bucket { // // my_metric.Record(1, 7, 23); // (latency value, request type, error code). // -// A MetricNotification may be used to allow the EventMetric instance to -// notify that an update occurred and provide the updated value. +// The EventMetric supports serialization. A call to Serialize will +// serialize all values to the provided MetricsSerializer instance. // -// class MyMetricNotification : public MetricNotification { +// example: +// +// class MyMetricSerializer : public MetricSerializer { // // Add implementation here. // } // -// MyMetricNotification notification; -// my_metric.Publish(notification); -template +// MyMetricSerializer serializer; +// my_metric.Serialize(&serializer); +// +template class EventMetric : public BaseEventMetric { public: // Create an EventMetric instance with the name |metric_name|. @@ -162,116 +152,16 @@ class EventMetric : public BaseEventMetric { // Record will update the statistics of the EventMetric broken down by the // given field values. void Record(double value, - F1 field1 = Unused(), - F2 field2 = Unused(), - F3 field3 = Unused(), - F4 field4 = Unused()); + F1 field1 = util::Unused(), + F2 field2 = util::Unused(), + F3 field3 = util::Unused(), + F4 field4 = util::Unused()); private: friend class EventMetricTest; std::vector field_names_; }; -// This is an internal namespace for helper functions only. -namespace impl { - -// This method formats the collection of field name/value pairs. -// The format of the string is: -// -// [{field:value[&field:value]*}] -// -// If there are no pairs, returns a blank string. -// -// TODO(blueeyes): Add a check for the count of field_names and the count -// of field values (check that the fields are not type Unused). -template -std::string MakeFieldNameString(const std::vector& field_names, - const F1 field1, const F2 field2, - const F3 field3, const F4 field4) { - std::stringstream field_name_and_values; - std::vector::const_iterator field_name_iterator = - field_names.begin(); - if (field_name_iterator == field_names.end()) { - return field_name_and_values.str(); - } - // There is at least one name/value pair. prepend open brace. - field_name_and_values << "{"; - field_name_and_values << *field_name_iterator << ':' << field1; - if (++field_name_iterator == field_names.end()) { - field_name_and_values << "}"; - return field_name_and_values.str(); - } - field_name_and_values << '&' << *field_name_iterator << ':' << field2; - if (++field_name_iterator == field_names.end()) { - field_name_and_values << "}"; - return field_name_and_values.str(); - } - field_name_and_values << '&' << *field_name_iterator << ':' << field3; - if (++field_name_iterator == field_names.end()) { - field_name_and_values << "}"; - return field_name_and_values.str(); - } - field_name_and_values << '&' << *field_name_iterator << ':' << field4; - field_name_and_values << "}"; - return field_name_and_values.str(); -} - -// This specialization of the helper method is a shortcut for EventMetric -// instances with no fields. -template<> -inline std::string MakeFieldNameString( - const std::vector& /* field_names */, - const Unused /* unused1 */, const Unused /* unused2 */, - const Unused /* unused3 */, const Unused /* unused4 */) { - return ""; -} - -// This helper function appends the field names to a vector of strings. -inline void AppendFieldNames(std::vector* field_name_vector, - int field_count, ...) { - va_list field_names; - - va_start(field_names, field_count); - for (int x = 0; x < field_count; x++) { - field_name_vector->push_back(va_arg(field_names, const char*)); - } - va_end(field_names); -} - -// These helper methods and FirstUnusedType assure that there is no mismatch -// between the specified types for EventMetrics and the constructors and -// methods used for the specializations. -template -struct CompileAssert {}; -#define COMPILE_ASSERT(expr, msg) \ - typedef impl::CompileAssert<(bool(expr))> msg[bool(expr) ? 1 : -1] - -template struct is_unused { static const bool value = false; }; -template <> struct is_unused { static const bool value = true; }; - -template -class FirstUnusedType { - static const bool a = is_unused::value; - static const bool b = is_unused::value; - static const bool c = is_unused::value; - static const bool d = is_unused::value; - // Check that all types after the first Unused are also Unused. - COMPILE_ASSERT(a <= b, Invalid_Unused_At_Position_2); - COMPILE_ASSERT(b <= c, Invalid_Unused_At_Position_3); - COMPILE_ASSERT(c <= d, Invalid_Unused_At_Position_4); - - public: - static const int value = 5 - (a + b + c + d); -}; - -// Asserts that no Unused types exist before N; after N, are all Unused types. -#define ASSERT_METRIC_UNUSED_START_FROM(N) \ - COMPILE_ASSERT((\ - impl::FirstUnusedType::value) == N, \ - Unused_Start_From_##N) - -} // namespace impl - // Overloaded template constructor implementations for EventMetric. template EventMetric::EventMetric( @@ -286,8 +176,8 @@ EventMetric::EventMetric( const char* field_name) : BaseEventMetric(metric_name) { ASSERT_METRIC_UNUSED_START_FROM(2); - impl::AppendFieldNames(&field_names_, - impl::FirstUnusedType::value - 1, + util::AppendFieldNames(&field_names_, + util::FirstUnusedType::value - 1, field_name); } template @@ -297,8 +187,8 @@ EventMetric::EventMetric( const char* field_name2) : BaseEventMetric(metric_name) { ASSERT_METRIC_UNUSED_START_FROM(3); - impl::AppendFieldNames(&field_names_, - impl::FirstUnusedType::value - 1, + util::AppendFieldNames(&field_names_, + util::FirstUnusedType::value - 1, field_name1, field_name2); } template @@ -309,8 +199,8 @@ EventMetric::EventMetric( const char* field_name3) : BaseEventMetric(metric_name) { ASSERT_METRIC_UNUSED_START_FROM(4); - impl::AppendFieldNames(&field_names_, - impl::FirstUnusedType::value - 1, + util::AppendFieldNames(&field_names_, + util::FirstUnusedType::value - 1, field_name1, field_name2, field_name3); } template @@ -322,8 +212,8 @@ EventMetric::EventMetric( const char* field_name4) : BaseEventMetric(metric_name) { ASSERT_METRIC_UNUSED_START_FROM(5); - impl::AppendFieldNames(&field_names_, - impl::FirstUnusedType::value - 1, + util::AppendFieldNames(&field_names_, + util::FirstUnusedType::value - 1, field_name1, field_name2, field_name3, field_name4); } @@ -332,7 +222,7 @@ template void EventMetric::Record( double value, F1 field1, F2 field2, F3 field3, F4 field4) { std::string field_name_values = - impl::MakeFieldNameString(field_names_, field1, field2, field3, field4); + util::MakeFieldNameString(field_names_, field1, field2, field3, field4); BaseEventMetric::Record(field_name_values, value); } diff --git a/libwvdrmengine/cdm/metrics/include/field_tuples.h b/libwvdrmengine/cdm/metrics/include/field_tuples.h new file mode 100644 index 00000000..495ccefe --- /dev/null +++ b/libwvdrmengine/cdm/metrics/include/field_tuples.h @@ -0,0 +1,125 @@ +// Copyright 2017 Google Inc. All Rights Reserved. +// +// This file contains the helper classes and methods for using field tuples +// used by metrics classes to record variations of a single metric. +#ifndef WVCDM_METRICS_FIELD_TUPLES_H_ +#define WVCDM_METRICS_FIELD_TUPLES_H_ + +#include +#include +#include +#include +#include +#include +#include + +namespace wvcdm { +namespace metrics { +namespace util { + +// This is a placeholder type for unused type parameters. It aids in supporting +// templated classes with "variable" type arguments. +struct Unused { + // Required for compilation. Should never be used. + inline friend std::ostream& operator<< (std::ostream& out, const Unused&) + { return out; } +}; + +// This utility method formats the collection of field name/value pairs. +// The format of the string is: +// +// [{field:value[&field:value]*}] +// +// If there are no pairs, returns a blank string. +template +std::string MakeFieldNameString(const std::vector& field_names, + const F1 field1, const F2 field2, + const F3 field3, const F4 field4) { + std::stringstream field_name_and_values; + std::vector::const_iterator field_name_iterator = + field_names.begin(); + if (field_name_iterator == field_names.end()) { + return field_name_and_values.str(); + } + // There is at least one name/value pair. Prepend open brace. + field_name_and_values << "{"; + field_name_and_values << *field_name_iterator << ':' << field1; + if (++field_name_iterator == field_names.end()) { + field_name_and_values << "}"; + return field_name_and_values.str(); + } + field_name_and_values << '&' << *field_name_iterator << ':' << field2; + if (++field_name_iterator == field_names.end()) { + field_name_and_values << "}"; + return field_name_and_values.str(); + } + field_name_and_values << '&' << *field_name_iterator << ':' << field3; + if (++field_name_iterator == field_names.end()) { + field_name_and_values << "}"; + return field_name_and_values.str(); + } + field_name_and_values << '&' << *field_name_iterator << ':' << field4; + field_name_and_values << "}"; + return field_name_and_values.str(); +} + +// This specialization of the helper method is a shortcut for class +// instances with no fields. +template<> +inline std::string MakeFieldNameString( + const std::vector& /* field_names */, + const Unused /* unused1 */, const Unused /* unused2 */, + const Unused /* unused3 */, const Unused /* unused4 */) { + return ""; +} + +// This helper function appends the field names to a vector of strings. +inline void AppendFieldNames(std::vector* field_name_vector, + int field_count, ...) { + va_list field_names; + + va_start(field_names, field_count); + for (int x = 0; x < field_count; x++) { + field_name_vector->push_back(va_arg(field_names, const char*)); + } + va_end(field_names); +} + +// These helper methods and FirstUnusedType assure that there is no mismatch +// between the specified types for metrics type parameters and the constructors +// and methods used for the specializations. +template +struct CompileAssert {}; +#define COMPILE_ASSERT(expr, msg) \ + typedef util::CompileAssert<(bool(expr))> msg[bool(expr) ? 1 : -1] + +template struct is_unused { static const bool value = false; }; +template <> struct is_unused { static const bool value = true; }; + +template +class FirstUnusedType { + static const bool a = is_unused::value; + static const bool b = is_unused::value; + static const bool c = is_unused::value; + static const bool d = is_unused::value; + // Check that all types after the first Unused are also Unused. + COMPILE_ASSERT(a <= b, Invalid_Unused_At_Position_2); + COMPILE_ASSERT(b <= c, Invalid_Unused_At_Position_3); + COMPILE_ASSERT(c <= d, Invalid_Unused_At_Position_4); + + public: + static const int value = 5 - (a + b + c + d); +}; + +// Asserts that no Unused types exist before N; after N, are all Unused types. +#define ASSERT_METRIC_UNUSED_START_FROM(N) \ + COMPILE_ASSERT((\ + util::FirstUnusedType::value) == N, \ + Unused_Start_From_##N) + +} // namespace util +} // namespace metrics +} // namespace wvcdm + +#endif // WVCDM_METRICS_FIELD_TUPLES_H_ + diff --git a/libwvdrmengine/cdm/metrics/include/value_metric.h b/libwvdrmengine/cdm/metrics/include/value_metric.h new file mode 100644 index 00000000..6de010ee --- /dev/null +++ b/libwvdrmengine/cdm/metrics/include/value_metric.h @@ -0,0 +1,100 @@ +// Copyright 2017 Google Inc. All Rights Reserved. +// +// This file contains the declarations for the Metric class and related +// types. +#ifndef WVCDM_METRICS_VALUE_METRIC_H_ +#define WVCDM_METRICS_VALUE_METRIC_H_ + +#include +#include + +#include "metric_serialization.h" + +namespace wvcdm { +namespace metrics { + +// Private namespace for some helper implementation functions. +namespace impl { + +// These helper functions map the templated ValueMetric class +// Serialize call to the MetricSerializer explicit calls. +template +void Serialize(MetricSerializer* serializer, + const std::string& metric_name, const T& t); + +inline void SerializeError(MetricSerializer* serializer, + const std::string& metric_name, + const int& error_code) { + serializer->SetInt32(metric_name + "/error", error_code); +} + +} // namespace impl + +// The Metric class supports storing a single value which can be overwritten. +// the Metric class also supports the MetricSerializer interface through +// which the value can be serialized. If the value was never given a value +// or an error code, then the metric will not serialize anything. +// +// Example Usage: +// Metric cdm_version("drm/cdm/version") +// .Record("a.b.c.d"); +// +// MyMetricSerializerImpl serialzer; +// cdm_version.Serialize(&serializer); +// +// Example Error Usage: +// +// Metric cdm_version("drm/cdm/version") +// .SetError(error_code); +// +// Note that serialization is the same. But the ValueMetric will serialize +// the error code to /error instead of just . +template +class ValueMetric : public MetricSerializable { + public: + // Constructs a metric with the given metric name. + explicit ValueMetric(const std::string& metric_name) + : metric_name_(metric_name), error_code_(0), + has_error_(false), has_value_(false) {} + + // Serialize the metric name and value using the given serializer. + // Caller owns |serializer| which cannot be null. + virtual void Serialize(MetricSerializer* serializer) { + if (has_value_) { + impl::Serialize(serializer, metric_name_, value_); + } else if (has_error_) { + impl::SerializeError(serializer, metric_name_, error_code_); + } else { + // Do nothing if there is no value and no error. + } + } + + // Record the value of the metric. + void Record(const T& value) { + value_ = value; + has_value_ = true; + has_error_ = false; + } + + // Set the error code if an error was encountered. + void SetError(int error_code) { + error_code_ = error_code; + has_value_ = false; + has_error_ = true; + } + + // Get the current value of the metric. + const T& GetValue() { return value_; } + + private: + std::string metric_name_; + T value_; + int error_code_; + bool has_error_; + bool has_value_; +}; + +} // namespace metrics +} // namespace wvcdm + +#endif // WVCDM_METRICS_VALUE_METRIC_H_ diff --git a/libwvdrmengine/cdm/metrics/src/counter_metric.cpp b/libwvdrmengine/cdm/metrics/src/counter_metric.cpp new file mode 100644 index 00000000..5f561a74 --- /dev/null +++ b/libwvdrmengine/cdm/metrics/src/counter_metric.cpp @@ -0,0 +1,33 @@ +// Copyright 2017 Google Inc. All Rights Reserved. +// +// This file contains implementations for the BaseCounterMetric, the base class +// for CounterMetric. + +#include "counter_metric.h" + +namespace wvcdm { +namespace metrics { + +void BaseCounterMetric::Increment(const std::string& field_names_values, + int64_t value) { + AutoLock lock(internal_lock_); + + if (value_map_.find(field_names_values) == value_map_.end()) { + value_map_[field_names_values] = value; + } else { + value_map_[field_names_values] = value_map_[field_names_values] + value; + } +} + +void BaseCounterMetric::Serialize(MetricSerializer* serializer) { + AutoLock lock(internal_lock_); + + for (std::map::iterator it + = value_map_.begin(); it != value_map_.end(); it++) { + serializer->SetInt64(metric_name_ + "/count" + it->first, it->second); + } +} + +} // namespace metrics +} // namespace wvcdm + diff --git a/libwvdrmengine/cdm/metrics/src/value_metric.cpp b/libwvdrmengine/cdm/metrics/src/value_metric.cpp new file mode 100644 index 00000000..78e4f644 --- /dev/null +++ b/libwvdrmengine/cdm/metrics/src/value_metric.cpp @@ -0,0 +1,115 @@ +// Copyright 2017 Google Inc. All Rights Reserved. +// +// This file contains the specializations for helper methods for the +// ValueMetric class. + +#include +#include + +#include "value_metric.h" + +#include "metrics_collections.h" +#include "OEMCryptoCENC.h" +#include "wv_cdm_types.h" + +namespace wvcdm { +namespace metrics { + +// Private namespace for some helper implementation functions. +namespace impl { + +template<> +void Serialize(MetricSerializer* serializer, + const std::string& metric_name, + const int32_t& value) { + serializer->SetInt32(metric_name, value); +} + +template<> +void Serialize(MetricSerializer* serializer, + const std::string& metric_name, + const int64_t& value) { + serializer->SetInt64(metric_name, value); +} + +// This specialization forces the uint32_t to an int32_t. +template<> +void Serialize(MetricSerializer* serializer, + const std::string& metric_name, + const uint32_t& value) { + serializer->SetInt32(metric_name, value); +} + +// This specialization forces the uint32_t to an int64_t. +template<> +void Serialize(MetricSerializer* serializer, + const std::string& metric_name, + const uint64_t& value) { + serializer->SetInt64(metric_name, value); +} + +// This specialization forces a bool to an int32_t. +template<> +void Serialize(MetricSerializer* serializer, + const std::string& metric_name, + const bool& value) { + serializer->SetInt32(metric_name, value); +} + +// This specialization forces an unsigned short to an int32_t. +template<> +void Serialize(MetricSerializer* serializer, + const std::string& metric_name, + const unsigned short& value) { + serializer->SetInt32(metric_name, value); +} + +template<> +void Serialize(MetricSerializer* serializer, + const std::string& metric_name, + const std::string& value) { + serializer->SetString(metric_name, value); +} + +template<> +void Serialize(MetricSerializer* serializer, + const std::string& metric_name, + const double& value) { + serializer->SetDouble(metric_name, value); +} + +// These specializations force CDM-specific types to int32_t +template<> +void Serialize(MetricSerializer* serializer, + const std::string& metric_name, + const CdmSecurityLevel& value) { + serializer->SetInt32(metric_name, value); +} + +template<> +void Serialize( + MetricSerializer* serializer, + const std::string& metric_name, + const OEMCrypto_HDCP_Capability& value) { + serializer->SetInt32(metric_name, value); +} + +template<> +void Serialize( + MetricSerializer* serializer, + const std::string& metric_name, + const OEMCrypto_ProvisioningMethod& value) { + serializer->SetInt32(metric_name, value); +} + +template<> +void Serialize( + MetricSerializer* serializer, + const std::string& metric_name, + const OEMCryptoInitializationMode& value) { + serializer->SetInt32(metric_name, value); +} + +} // namespace impl +} // namespace metrics +} // namespace wvcdm diff --git a/libwvdrmengine/cdm/metrics/test/counter_metric_unittest.cpp b/libwvdrmengine/cdm/metrics/test/counter_metric_unittest.cpp new file mode 100644 index 00000000..c3e26036 --- /dev/null +++ b/libwvdrmengine/cdm/metrics/test/counter_metric_unittest.cpp @@ -0,0 +1,177 @@ +// Copyright 2017 Google Inc. All Rights Reserved. +// +// Unit tests for CounterMetric + +#include "counter_metric.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "metric_serialization.h" +#include "scoped_ptr.h" + +using testing::IsNull; +using testing::NotNull; + +namespace wvcdm { +namespace metrics { + +class MockCounterMetricSerializer : public MetricSerializer { + public: + MOCK_METHOD2(SetString, void(const std::string& metric_id, + const std::string& value)); + MOCK_METHOD2(SetInt32, void(const std::string& metric_id, + int32_t value)); + MOCK_METHOD2(SetInt64, void(const std::string& metric_id, + int64_t value)); + MOCK_METHOD2(SetDouble, void(const std::string& metric_id, + double value)); +}; + +class CounterMetricTest : public ::testing::Test { + public: + void SetUp() { + mock_serializer_.reset(new MockCounterMetricSerializer()); + } + protected: + template + const std::map + GetValueMap( + const wvcdm::metrics::CounterMetric& + metric) { + return metric.value_map_; + } + + scoped_ptr mock_serializer_; +}; + +TEST_F(CounterMetricTest, NoFieldsSuccessNullCallback) { + wvcdm::metrics::CounterMetric<> metric("no/fields/metric"); + metric.Increment(); + metric.Increment(10); + + std::map value_map = GetValueMap(metric); + ASSERT_EQ(1u, GetValueMap(metric).size()); + EXPECT_EQ(11, value_map.begin()->second); + EXPECT_EQ("", value_map.begin()->first); +} + +TEST_F(CounterMetricTest, NoFieldsSuccessWithCallback) { + wvcdm::metrics::CounterMetric<> metric("no/fields/metric"); + EXPECT_CALL(*mock_serializer_, + SetInt64("no/fields/metric/count", 11)); + + metric.Increment(); + metric.Increment(10); + metric.Serialize(mock_serializer_.get()); + + std::map value_map = GetValueMap(metric); + ASSERT_EQ(1u, GetValueMap(metric).size()); + EXPECT_EQ(11, value_map.begin()->second); + EXPECT_EQ("", value_map.begin()->first); +} + +TEST_F(CounterMetricTest, NoFieldsSuccessSingleIncrementWithCallback) { + wvcdm::metrics::CounterMetric<> metric("no/fields/metric"); + EXPECT_CALL(*mock_serializer_, + SetInt64("no/fields/metric/count", 1)); + + metric.Increment(); + metric.Serialize(mock_serializer_.get()); + + std::map value_map = GetValueMap(metric); + ASSERT_EQ(1u, GetValueMap(metric).size()); + EXPECT_EQ(1, value_map.begin()->second); + EXPECT_EQ("", value_map.begin()->first); +} + +TEST_F(CounterMetricTest, OneFieldSuccessNoCallback) { + wvcdm::metrics::CounterMetric metric( + "single/fields/metric", + "error_code"); + metric.Increment(7); + metric.Increment(10, 7); + metric.Increment(13); + metric.Increment(20, 13); + std::map value_map = GetValueMap(metric); + ASSERT_EQ(2u, GetValueMap(metric).size()); + + // Verify both instances. + EXPECT_EQ(11, value_map["{error_code:7}"]); + EXPECT_EQ(21, value_map["{error_code:13}"]); +} + +TEST_F(CounterMetricTest, TwoFieldsSuccess) { + wvcdm::metrics::CounterMetric metric( + "two/fields/metric", + "error_code", + "size"); + metric.Increment(7, 23); + metric.Increment(2, 7, 29); + metric.Increment(3, 11, 23); + metric.Increment(4, 11, 29); + metric.Increment(5, 7, 23); + metric.Increment(-5, 7, 29); + + std::map value_map = GetValueMap(metric); + ASSERT_EQ(4u, GetValueMap(metric).size()); + + // Verify all instances. + EXPECT_EQ(6, value_map["{error_code:7&size:23}"]); + EXPECT_EQ(-3, value_map["{error_code:7&size:29}"]); + EXPECT_EQ(3, value_map["{error_code:11&size:23}"]); + EXPECT_EQ(4, value_map["{error_code:11&size:29}"]); + + // Verify that a non-existent distribution returns default 0 + EXPECT_EQ(0, value_map["error_code:1,size:1"]); +} + +TEST_F(CounterMetricTest, TwoFieldsSuccessWithCallback) { + wvcdm::metrics::CounterMetric metric("two/fields/metric", + "error_code", + "stringval"); + + EXPECT_CALL( + *mock_serializer_, + SetInt64("two/fields/metric/count{error_code:11&stringval:foo}", 5)); + metric.Increment(11, "foo"); + metric.Increment(4, 11, "foo"); + metric.Serialize(mock_serializer_.get()); +} + +TEST_F(CounterMetricTest, ThreeFieldsSuccess) { + wvcdm::metrics::CounterMetric metric( + "three/fields/metric", + "error_code", + "size", + "woke up happy"); + metric.Increment(7, 13, false); + + std::map value_map = GetValueMap(metric); + ASSERT_EQ(1u, GetValueMap(metric).size()); + EXPECT_EQ("{error_code:7&size:13&woke up happy:0}", + value_map.begin()->first); + EXPECT_EQ(1, value_map.begin()->second); +} + +TEST_F(CounterMetricTest, FourFieldsSuccess) { + wvcdm::metrics::CounterMetric metric( + "Four/fields/metric", + "error_code", + "size", + "woke up happy", + "horoscope"); + metric.Increment(10LL, 7, 13, true, "find your true love"); + + std::map value_map = GetValueMap(metric); + ASSERT_EQ(1u, GetValueMap(metric).size()); + EXPECT_EQ( + "{error_code:7&size:13&woke up happy:1&horoscope:find your true love}", + value_map.begin()->first); + EXPECT_EQ(10, value_map.begin()->second); +} + +} // namespace metrics +} // namespace wvcdm diff --git a/libwvdrmengine/cdm/metrics/test/distribution_test.cpp b/libwvdrmengine/cdm/metrics/test/distribution_unittest.cpp similarity index 100% rename from libwvdrmengine/cdm/metrics/test/distribution_test.cpp rename to libwvdrmengine/cdm/metrics/test/distribution_unittest.cpp diff --git a/libwvdrmengine/cdm/metrics/test/event_metric_test.cpp b/libwvdrmengine/cdm/metrics/test/event_metric_unittest.cpp similarity index 100% rename from libwvdrmengine/cdm/metrics/test/event_metric_test.cpp rename to libwvdrmengine/cdm/metrics/test/event_metric_unittest.cpp diff --git a/libwvdrmengine/cdm/metrics/test/metrics_collections_test.cpp b/libwvdrmengine/cdm/metrics/test/metrics_collections_unittest.cpp similarity index 99% rename from libwvdrmengine/cdm/metrics/test/metrics_collections_test.cpp rename to libwvdrmengine/cdm/metrics/test/metrics_collections_unittest.cpp index 56672149..ee644414 100644 --- a/libwvdrmengine/cdm/metrics/test/metrics_collections_test.cpp +++ b/libwvdrmengine/cdm/metrics/test/metrics_collections_unittest.cpp @@ -1,7 +1,7 @@ // Copyright 2017 Google Inc. All Rights Reserved. // // Unit tests for the metrics collections, -// EngineMetrics, SessionMetrics and CrytpoMetrics. +// EngineMetrics, SessionMetrics and CryptoMetrics. #include "metrics_collections.h" diff --git a/libwvdrmengine/cdm/metrics/test/value_metric_unittest.cpp b/libwvdrmengine/cdm/metrics/test/value_metric_unittest.cpp new file mode 100644 index 00000000..77f5570a --- /dev/null +++ b/libwvdrmengine/cdm/metrics/test/value_metric_unittest.cpp @@ -0,0 +1,77 @@ +// Copyright 2017 Google Inc. All Rights Reserved. +// +// Unit tests for ValueMetric. + +#include + +#include "value_metric.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "metric_serialization.h" +#include "scoped_ptr.h" + +namespace wvcdm { +namespace metrics { + +class MockMetricSerializer : public MetricSerializer { + public: + MOCK_METHOD2(SetString, void(const std::string& metric_id, + const std::string& value)); + MOCK_METHOD2(SetInt32, void(const std::string& metric_id, + int32_t value)); + MOCK_METHOD2(SetInt64, void(const std::string& metric_id, + int64_t value)); + MOCK_METHOD2(SetDouble, void(const std::string& metric_id, + double value)); +}; + +class ValueMetricTest : public ::testing::Test { + public: + void SetUp() { + mock_serializer_.reset(new MockMetricSerializer()); + } + + protected: + scoped_ptr mock_serializer_; +}; + +TEST_F(ValueMetricTest, StringValue) { + ValueMetric value_metric("string/metric"); + value_metric.Record("foo"); + + EXPECT_CALL(*mock_serializer_, + SetString("string/metric", "foo")); + value_metric.Serialize(mock_serializer_.get()); +} + +TEST_F(ValueMetricTest, DoubleValue) { + ValueMetric value_metric("double/metric"); + value_metric.Record(42.0); + + EXPECT_CALL(*mock_serializer_, + SetDouble("double/metric", 42.0)); + value_metric.Serialize(mock_serializer_.get()); +} + +TEST_F(ValueMetricTest, Int32Value) { + ValueMetric value_metric("int32/metric"); + value_metric.Record(42); + + EXPECT_CALL(*mock_serializer_, + SetInt32("int32/metric", 42)); + value_metric.Serialize(mock_serializer_.get()); +} + +TEST_F(ValueMetricTest, Int64Value) { + ValueMetric value_metric("int64/metric"); + value_metric.Record(42); + + EXPECT_CALL(*mock_serializer_, + SetInt64("int64/metric", 42)); + value_metric.Serialize(mock_serializer_.get()); +} + +} // namespace metrics +} // namespace wvcdm + diff --git a/libwvdrmengine/cdm/test/Android.mk b/libwvdrmengine/cdm/test/Android.mk index 3d536776..b42b99af 100644 --- a/libwvdrmengine/cdm/test/Android.mk +++ b/libwvdrmengine/cdm/test/Android.mk @@ -27,6 +27,10 @@ test_name := cdm_session_unittest test_src_dir := ../core/test include $(LOCAL_PATH)/unit-test.mk +test_name := counter_metric_unittest +test_src_dir := ../metrics/test +include $(LOCAL_PATH)/unit-test.mk + test_name := crypto_session_unittest test_src_dir := ../core/test include $(LOCAL_PATH)/unit-test.mk @@ -35,6 +39,14 @@ test_name := device_files_unittest test_src_dir := ../core/test include $(LOCAL_PATH)/unit-test.mk +test_name := distribution_unittest +test_src_dir := ../metrics/test +include $(LOCAL_PATH)/unit-test.mk + +test_name := event_metric_unittest +test_src_dir := ../metrics/test +include $(LOCAL_PATH)/unit-test.mk + test_name := file_store_unittest test_src_dir := ../core/test include $(LOCAL_PATH)/unit-test.mk @@ -83,17 +95,13 @@ test_name := usage_table_header_unittest test_src_dir := ../core/test include $(LOCAL_PATH)/unit-test.mk +test_name := value_metric_unittest +test_src_dir := ../metrics/test +include $(LOCAL_PATH)/unit-test.mk + test_name := wv_cdm_metrics_test test_src_dir := . include $(LOCAL_PATH)/unit-test.mk -test_name := distribution_test -test_src_dir := ../metrics/test -include $(LOCAL_PATH)/unit-test.mk - -test_name := event_metric_test -test_src_dir := ../metrics/test -include $(LOCAL_PATH)/unit-test.mk - test_name := test_src_dir := diff --git a/libwvdrmengine/run_all_unit_tests.sh b/libwvdrmengine/run_all_unit_tests.sh index e084adb1..2b022643 100755 --- a/libwvdrmengine/run_all_unit_tests.sh +++ b/libwvdrmengine/run_all_unit_tests.sh @@ -83,10 +83,11 @@ adb_shell_run base64_test adb_shell_run buffer_reader_test adb_shell_run cdm_engine_test adb_shell_run cdm_session_unittest +adb_shell_run counter_metric_unittest adb_shell_run crypto_session_unittest adb_shell_run device_files_unittest -adb_shell_run distribution_test -adb_shell_run event_metric_test +adb_shell_run distribution_unittest +adb_shell_run event_metric_unittest adb_shell_run file_store_unittest adb_shell_run file_utils_unittest adb_shell_run http_socket_test @@ -102,6 +103,7 @@ adb_shell_run policy_engine_unittest adb_shell_run service_certificate_unittest adb_shell_run timer_unittest adb_shell_run usage_table_header_unittest +adb_shell_run value_metric_unittest adb_shell_run wv_cdm_metrics_test # Run the non-Treble test on non-Treble devices diff --git a/tests/Android.mk b/tests/Android.mk index 1a78d99d..b3206cfc 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -12,10 +12,11 @@ WIDEVINE_TEST_MAKE_TARGETS := \ cdm_engine_test \ cdm_extended_duration_test \ cdm_session_unittest \ + counter_metric_unittest \ crypto_session_unittest \ device_files_unittest \ - distribution_test \ - event_metric_test \ + distribution_unittest \ + event_metric_unittest \ file_store_unittest \ file_utils_unittest \ http_socket_test \ @@ -35,6 +36,7 @@ WIDEVINE_TEST_MAKE_TARGETS := \ service_certificate_unittest \ timer_unittest \ usage_table_header_unittest \ + value_metric_unittest \ CastSignAPITest \ MediaDrmAPITest \ From 890f535b06d04f08ae2425ac3514ef7c7ab5a188 Mon Sep 17 00:00:00 2001 From: "John W. Bruce" Date: Thu, 24 Aug 2017 16:04:38 -0700 Subject: [PATCH 2/2] Silence Version Number Canary (This is a merge of http://go/wvgerrit/32280) At the same time as the last version number canary change went in, the version number for O-MR1 was updated to its final value. Thus, the test never stopped failing on the dashboard. This patch silences the canary again. No Widevine version number update is needed, since we already updated it for O-MR1 in the previous patch. Bug: 64951985 Test: request_license_test Change-Id: Idf3c4b96e3f200d08a089bd08afdee8b0fd9dd76 --- libwvdrmengine/cdm/test/request_license_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libwvdrmengine/cdm/test/request_license_test.cpp b/libwvdrmengine/cdm/test/request_license_test.cpp index 3f4ebbe6..d90eeb5e 100644 --- a/libwvdrmengine/cdm/test/request_license_test.cpp +++ b/libwvdrmengine/cdm/test/request_license_test.cpp @@ -3726,7 +3726,7 @@ TEST(VersionNumberTest, VersionNumberChangeCanary) { char release_number[PROPERTY_VALUE_MAX]; ASSERT_GT(property_get("ro.build.version.release", release_number, "Unknown"), 0); - EXPECT_STREQ("OMR1", release_number) + EXPECT_STREQ("8.1.0", release_number) << "The Android version number has changed. You need to update this test " "and also possibly update the Widevine version number in " "properties_android.cpp.";