Bug 1645423 - Sentry ignores errors caused by local changes r=mhentges

Here, "errors caused by local changes" means "errors whose stack traces contain a reference to a file that is in the set of files changed locally". This implementation is a trade-off:

1. This check will not catch issues caused transitively by changes to local files. For example, consider a function that has been updated and its return type changed in a backwards-incompatible way, whereas callers were not updated appropriately. This would likely manifest as a type error in the calling function after the callee has returned.

2. This check WILL catch issues that come from locally changed files where the cause of the error doesn't originate from those local changes. For example, consider a function that's been locally updated but is never called in the failing codepath; if an exception is thrown, it's not due to this local change, and we shouldn't filter it out.

There are conceivable improvements that we could apply to fix deficiency (1); for example, we could track imports recursively starting from the oldest frame in the stack trace and match on that set of imported files. Note this would not handle dynamic imports properly, and that this could exacerbate issue (2).

Issue (2) could conceivably be addressed by attempting to filter the actual local diffs down to changes that actually may be causing the error. This is difficult to do generally especially in light of Python's dynamism, but there mayb be conservative improvements that we could make in this space.

Overall, neither of the above caveats are deemed to be sufficiently concerning that this patch should be blocked as-is, and the current situation with our Sentry logs is unusable due to all the noise. This patch will probably have a substantial impact on that noise without incidentally filtering out too much signal.

Differential Revision: https://phabricator.services.mozilla.com/D95607

python/mach/mach/sentry.py

@@ -6,7 +6,11 @@ from __future__ import absolute_import
 
 import abc
 import re
-from os.path import expanduser
+from os.path import (
+    abspath,
+    expanduser,
+    join,
+)
 
 import sentry_sdk
 from mozboot.util import get_state_dir
@@ -14,6 +18,7 @@ from mach.telemetry import is_telemetry_enabled
 from mozversioncontrol import (
     get_repository_object,
     InvalidRepoPath,
+    MissingUpstreamRepo,
     MissingVCSTool,
 )
 from six import string_types
@@ -59,13 +64,11 @@ def register_sentry(argv, settings, topsrcdir=None):
         return NoopErrorReporter()
 
     if topsrcdir:
-        try:
-            repo = get_repository_object(topsrcdir)
+        repo = _get_repository_object(topsrcdir)
+        if repo is not None:
             email = repo.get_user_email()
             if email in _DEVELOPER_BLOCKLIST:
                 return NoopErrorReporter()
-        except (InvalidRepoPath, MissingVCSTool):
-            pass
 
     sentry_sdk.init(
         _SENTRY_DSN, before_send=lambda event, _: _process_event(event, topsrcdir)
@@ -75,6 +78,10 @@ def register_sentry(argv, settings, topsrcdir=None):
 
 
 def _process_event(sentry_event, topsrcdir):
+    if _any_modified_files_matching_event(sentry_event, topsrcdir):
+        # Returning None causes the event to be dropped:
+        # https://docs.sentry.io/platforms/python/configuration/filtering/#using-beforesend
+        return None
     for map_fn in (_settle_mach_module_id, _patch_absolute_paths, _delete_server_name):
         sentry_event = map_fn(sentry_event, topsrcdir)
     return sentry_event
@@ -144,3 +151,32 @@ def _patch_absolute_paths(sentry_event, topsrcdir):
 def _delete_server_name(sentry_event, _):
     sentry_event.pop("server_name")
     return sentry_event
+
+
+def _get_repository_object(topsrcdir):
+    try:
+        return get_repository_object(topsrcdir)
+    except (InvalidRepoPath, MissingVCSTool):
+        return None
+
+
+def _any_modified_files_matching_event(sentry_event, topsrcdir):
+    repo = _get_repository_object(topsrcdir)
+    if repo is None:
+        return False  # Conservatively assume the tree is clean.
+
+    try:
+        files = set(repo.get_outgoing_files()) | set(repo.get_changed_files())
+    except MissingUpstreamRepo:
+        return False
+
+    files = set(abspath(join(topsrcdir, s)) for s in files)
+
+    # Return True iff the abs_path in any of the stack traces match the set of
+    # changed files locally. Be careful not to crash if something's missing from
+    # the dictionary.
+    for exception in sentry_event.get("exception", {}).get("values", []):
+        for frame in exception.get("stacktrace", {}).get("frames", []):
+            if frame.get("abs_path", None) in files:
+                return True
+    return False