Bug 1188888 - Part 5 - Restrict label values. r=chutten
authorGeorg Fritzsche <georg.fritzsche@googlemail.com>
Wed, 20 Jul 2016 17:10:24 +0200
changeset 345898 9f2ef29dee012f0359a70188ad0f00708f04605e
parent 345897 a662e130c9ded8a98cf4b28ed10acd4fd7d1d78d
child 345899 13b22aa53d11db1cc9d54566ea8673a74f80c435
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschutten
bugs1188888
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1188888 - Part 5 - Restrict label values. r=chutten
toolkit/components/telemetry/histogram_tools.py
--- a/toolkit/components/telemetry/histogram_tools.py
+++ b/toolkit/components/telemetry/histogram_tools.py
@@ -4,16 +4,20 @@
 
 import collections
 import json
 import math
 import os
 import re
 import sys
 
+# Constants.
+MAX_LABEL_LENGTH = 20
+MAX_LABEL_COUNT = 100
+
 # histogram_tools.py is used by scripts from a mozilla-central build tree
 # and also by outside consumers, such as the telemetry server.  We need
 # to ensure that importing things works in both contexts.  Therefore,
 # unconditionally importing things that are local to the build tree, such
 # as buildconfig, is a no-no.
 try:
     import buildconfig
 
@@ -227,18 +231,19 @@ associated with the histogram.  Returns 
         if not self._strict_type_checks:
             table['exponential'].append('extended_statistics_ok')
 
         table_dispatch(definition['kind'], table,
                        lambda allowed_keys: Histogram.check_keys(name, definition, allowed_keys))
 
         self.check_name(name)
         self.check_field_types(name, definition)
+        self.check_whitelistable_fields(name, definition)
         self.check_expiration(name, definition)
-        self.check_whitelistable_fields(name, definition)
+        self.check_label_values(name, definition)
 
     def check_name(self, name):
         if '#' in name:
             raise ValueError, '"#" not permitted for %s' % (name)
 
     def check_expiration(self, name, definition):
         expiration = definition.get('expires_in_version')
 
@@ -247,16 +252,38 @@ associated with the histogram.  Returns 
 
         if re.match(r'^[1-9][0-9]*$', expiration):
             expiration = expiration + ".0a1"
         elif re.match(r'^[1-9][0-9]*\.0$', expiration):
             expiration = expiration + "a1"
 
         definition['expires_in_version'] = expiration
 
+    def check_label_values(self, name, definition):
+        labels = definition.get('labels')
+        if not labels:
+            return
+
+        invalid = filter(lambda l: len(l) > MAX_LABEL_LENGTH, labels)
+        if len(invalid) > 0:
+            raise ValueError, 'Label values for %s exceed length limit of %d: %s' % \
+                              (name, MAX_LABEL_LENGTH, ', '.join(invalid))
+
+        if len(labels) > MAX_LABEL_COUNT:
+            raise ValueError, 'Label count for %s exceeds limit of %d' % \
+                              (name, MAX_LABEL_COUNT)
+
+        # To make it easier to generate C++ identifiers from this etc., we restrict
+        # the label values to a strict pattern.
+        pattern = '^[a-z][a-z0-9_]+[a-z0-9]$'
+        invalid = filter(lambda l: not re.match(pattern, l, re.IGNORECASE), labels)
+        if len(invalid) > 0:
+            raise ValueError, 'Label values for %s are not matching pattern "%s": %s' % \
+                              (name, pattern, ', '.join(invalid))
+
     # Check for the presence of fields that old histograms are whitelisted for.
     def check_whitelistable_fields(self, name, definition):
         # Use counters don't have any mechanism to add the fields checked here,
         # so skip the check for them.
         if self._is_use_counter:
             return
 
         # In the pipeline we don't have whitelists available.