[TimeZonePolicy] Add OTHER_TIME_ZONE time zone policy
Adds a new option for a metric's TimeZonePolicy, in
addition to UTC or device-local time. If the new
option is selected, a valid IANA time zone identifier
(https://www.iana.org/time-zones) should be specified
using the new `other_time_zone` field of
MetricDefinition.
The new option will be implemented in follow-ups.
Bug: 92131
Change-Id: Ieef6ca6c795ff2fbcedad433aa6ed1362d95b8d9
Reviewed-on: https://fuchsia-review.googlesource.com/c/cobalt/+/611001
Reviewed-by: Zach Bush <zmbush@google.com>
Reviewed-by: Cameron Dale <camrdale@google.com>
Commit-Queue: Laura Peskin <pesk@google.com>
diff --git a/src/bin/config_parser/src/config_validator/metric_definitions.go b/src/bin/config_parser/src/config_validator/metric_definitions.go
index dfabe88..f6e9838 100644
--- a/src/bin/config_parser/src/config_validator/metric_definitions.go
+++ b/src/bin/config_parser/src/config_validator/metric_definitions.go
@@ -100,6 +100,10 @@
validationErrors.addError("metric_units", err)
}
+ if err := validateMetricTimeZone(m); err != nil {
+ validationErrors.addError("time_zone_policy", err)
+ }
+
if m.IntBuckets != nil && m.MetricType != config.MetricDefinition_INT_HISTOGRAM && m.MetricType != config.MetricDefinition_INTEGER_HISTOGRAM {
validationErrors.addError("int_buckets", fmt.Errorf("int_buckets can only be set for metrics for metric type INT_HISTOGRAM and INTEGER_HISTOGRAM"))
}
@@ -340,6 +344,23 @@
return nil
}
+func validateMetricTimeZone(m config.MetricDefinition) error {
+ if m.TimeZonePolicy == config.MetricDefinition_OTHER_TIME_ZONE {
+ if m.OtherTimeZone == "" {
+ return fmt.Errorf("metrics with TimeZonePolicy OTHER_TIME_ZONE must specify other_time_zone")
+ }
+ _, err := time.LoadLocation(m.OtherTimeZone)
+ if err != nil {
+ return fmt.Errorf("other_time_zone is not a valid IANA time zone identifier (see https://iana.org/time-zones)")
+ }
+ } else {
+ if m.OtherTimeZone != "" {
+ return fmt.Errorf("only metrics with TimeZonePolicy OTHER_TIME_ZONE should specify other_time_zone")
+ }
+ }
+ return nil
+}
+
///////////////////////////////////////////////////////////////
// Validation for specific metric types:
///////////////////////////////////////////////////////////////
diff --git a/src/bin/config_parser/src/config_validator/metric_definitions_test.go b/src/bin/config_parser/src/config_validator/metric_definitions_test.go
index d3266d9..2ffcd3f 100644
--- a/src/bin/config_parser/src/config_validator/metric_definitions_test.go
+++ b/src/bin/config_parser/src/config_validator/metric_definitions_test.go
@@ -418,6 +418,50 @@
}
////////////////////////////////////////////////////////////////////////////////
+// Tests concerning metric time zone policy.
+////////////////////////////////////////////////////////////////////////////////
+
+func TestValidateTimeZonePolicyOtherTimeZoneNotRequired(t *testing.T) {
+ m := makeValidMetric(config.MetricDefinition_OCCURRENCE)
+ m.TimeZonePolicy = config.MetricDefinition_UTC
+ if err := validateMetricDefinition(m); err != nil {
+ t.Error("Rejected metric with valid UTC TimeZonePolicy")
+ }
+ m.OtherTimeZone = "America/Los_Angeles"
+ if err := validateMetricDefinition(m); err == nil {
+ t.Error("Accepted metric with UTC TimeZonePolicy and with other_time_zone set")
+ }
+
+ m.TimeZonePolicy = config.MetricDefinition_LOCAL
+ m.OtherTimeZone = ""
+ if err := validateMetricDefinition(m); err != nil {
+ t.Error("Rejected metric with valid LOCAL TimeZonePolicy")
+ }
+ m.OtherTimeZone = "America/Los_Angeles"
+ if err := validateMetricDefinition(m); err == nil {
+ t.Error("Accepted metric with LOCAL TimeZonePolicy and with other_time_zone set")
+ }
+}
+
+func TestValidateTimeZonePolicyOtherTimeZoneRequired(t *testing.T) {
+ m := makeValidMetric(config.MetricDefinition_OCCURRENCE)
+ m.TimeZonePolicy = config.MetricDefinition_OTHER_TIME_ZONE
+ if err := validateMetricDefinition(m); err == nil {
+ t.Error("Accepted metric with OTHER_TIME_ZONE TimeZonePolicy but no other_time_zone set")
+ }
+
+ m.OtherTimeZone = "PST"
+ if err := validateMetricDefinition(m); err == nil {
+ t.Error("Accepted metric with OTHER_TIME_ZONE TimeZone and invalid time zone identifier as other_time_zone")
+ }
+
+ m.OtherTimeZone = "America/Los_Angeles"
+ if err := validateMetricDefinition(m); err != nil {
+ t.Error("Rejected metric with OTHER_TIME_ZONE TimeZonePolicy and valid other_time_zone time zone identifier")
+ }
+}
+
+////////////////////////////////////////////////////////////////////////////////
// Tests concerning INT_HISTOGRAM, INTEGER and INTEGER_HISTOGRAM metrics.
////////////////////////////////////////////////////////////////////////////////
diff --git a/src/registry/metric_definition.proto b/src/registry/metric_definition.proto
index 890d430..458a724 100644
--- a/src/registry/metric_definition.proto
+++ b/src/registry/metric_definition.proto
@@ -30,7 +30,7 @@
//
// A MetricDefinition is used to define a Metric.
//
-// Next ID: 25
+// Next ID: 26
message MetricDefinition {
reserved 6, 7, 9, 17;
reserved "event_codes", "max_event_code", "parts", "event_code_buffer_max";
@@ -391,19 +391,27 @@
/////////// The rest of the fields are used with all Metric types ///////////
- // A TimeZonePolicy specifies how the day_index of an Event should
- // be computed based on the actual time of logging.
+ // A TimeZonePolicy specifies how the day index of an Event should be computed
+ // based on the system time when the Event is logged.
enum TimeZonePolicy {
- // Use the date in UTC at logging time to compute the day_index.
+ // Use the date in UTC at logging time to compute the day index.
UTC = 0;
- // Use the local date at logging time to compute the day_index.
+ // Use the device-local date at logging time to compute the day index.
LOCAL = 1;
+
+ // Use the time zone specified in the `other_time_zone` field to compute the
+ // day index.
+ OTHER_TIME_ZONE = 2;
}
// The TimeZonePolicy for this Metric (Optional. Defaults to UTC)
TimeZonePolicy time_zone_policy = 10;
+ // An IANA time zone identifier (https://iana.org/time-zones). Should be set if
+ // and only if the metric's `time_zone_policy` is OTHER_TIME_ZONE.
+ string other_time_zone = 25;
+
message Metadata {
// The date after which this metric is no longer valid. If this field is not
// supplied, the metric is considered currently expired, and is not
