Bug 1370599 - Only issue alerts on installer size changes to builds we ship; r?mshal draft
authorGregory Szorc <gps@mozilla.com>
Wed, 07 Jun 2017 12:23:28 -0700
changeset 590392 d1358e93932e77418b053ca6b53ac0f8bb2cdaed
parent 590317 a49112c7a5765802096b3fc298069b9495436107
child 632226 23979b63a5541f881c5af49d1cff282edb1deb83
push id62744
push usergszorc@mozilla.com
push dateWed, 07 Jun 2017 19:23:38 +0000
reviewersmshal
bugs1370599
milestone55.0a1
Bug 1370599 - Only issue alerts on installer size changes to builds we ship; r?mshal Currently, every metric we send to Perfherder can alert. Some alerts aren't relevant so we shouldn't cause noise by producing them. The installer size alert is only relevant to builds we ship. So, this commit adds a method for determining whether the build configuration is shipped and then conditionally changes the Perfherder data to disable alerting if we don't ship. Determining whether we ship a build configuration is a bit fragile. I'm not 100% confident the implementation is correct. There's also no good way to ensure we don't break this going forward by e.g. introducing a build configuration we ship but isn't detected as such. I'd be in favor of a more conservative implementation, but I'm not sure exactly what that would look like. MozReview-Commit-ID: CUxzP1TGcI7
testing/mozharness/mozharness/mozilla/building/buildbase.py
--- a/testing/mozharness/mozharness/mozilla/building/buildbase.py
+++ b/testing/mozharness/mozharness/mozilla/building/buildbase.py
@@ -1844,16 +1844,48 @@ or run without that action (ie: --no-{ac
         if return_code:
             self.return_code = self.worst_level(
                 return_code,  self.return_code,
                 AUTOMATION_EXIT_CODES[::-1]
             )
             self.error("'mach build check' did not run successfully. Please "
                        "check log for errors.")
 
+    def _is_configuration_shipped(self):
+        """Determine if the current build configuration is shipped to users.
+
+        This is used to drive alerting so we don't see alerts for build
+        configurations we care less about.
+        """
+        # Ideally this would be driven by a config option. However, our
+        # current inheritance mechanism of using a base config and then
+        # one-off configs for variants isn't conducive to this since derived
+        # configs we need to be reset and we don't like requiring boilerplate
+        # in derived configs.
+
+        # All PGO builds are shipped. This takes care of Linux and Windows.
+        if self.config.get('pgo_build'):
+            return True
+
+        # Debug builds are never shipped.
+        if self.config.get('debug_build'):
+            return False
+
+        # OS X opt builds without a variant are shipped.
+        if self.config.get('platform') == 'macosx64':
+            if not self.config.get('build_variant'):
+                return True
+
+        # Android opt builds without a variant are shipped.
+        if self.config.get('platform') == 'android':
+            if not self.config.get('build_variant'):
+                return True
+
+        return False
+
     def _load_build_resources(self):
         p = self.config.get('build_resources_path') % self.query_abs_dirs()
         if not os.path.exists(p):
             self.info('%s does not exist; not loading build resources' % p)
             return None
 
         with open(p, 'rb') as fh:
             resources = json.load(fh)
@@ -1982,31 +2014,40 @@ or run without that action (ie: --no-{ac
                             {'name': name, 'value': subtests[name]})
             except:
                 self.info('Unable to search %s for component sizes.' % installer)
                 size_measurements = []
 
         if not installer_size and not size_measurements:
             return
 
+        # We want to always collect metrics. But alerts for installer size are
+        # only use for builds with ship. So nix the alerts for builds we don't
+        # ship.
+        def filter_alert(alert):
+            if not self._is_configuration_shipped():
+                alert['shouldAlert'] = False
+
+            return alert
+
         if installer.endswith('.apk'): # Android
-            yield {
+            yield filter_alert({
                 "name": "installer size",
                 "value": installer_size,
                 "alertChangeType": "absolute",
                 "alertThreshold": (200 * 1024),
                 "subtests": size_measurements
-            }
+            })
         else:
-            yield {
+            yield filter_alert({
                 "name": "installer size",
                 "value": installer_size,
                 "alertThreshold": 1.0,
                 "subtests": size_measurements
-            }
+            })
 
     def _generate_build_stats(self):
         """grab build stats following a compile.
 
         This action handles all statistics from a build: 'count_ctors'
         and then posts to graph server the results.
         We only post to graph server for non nightly build
         """