bug-1889200: specify release in sentry-wrap.py (#2917)

Because:
* We want to be able to mark an error hit in `sentry_wrap.py` as "Resolved in the next release" in Sentry, but we weren't passing in the `release` in this context (see this [reference error's events](https://mozilla.sentry.io/issues/4723701783/events/?project=6700123)).

This commit:
* Passes `release` when calling `sentry_sdk.init` in `sentry_wrap.py`.

bin/run_migrations.sh

@@ -18,7 +18,7 @@ PRECMD=""
 # send errors to sentry.
 if [ -n "${SENTRY_DSN:-}" ]; then
     echo "SENTRY_DSN defined--enabling sentry."
-    PRECMD="python bin/sentry-wrap.py wrap-process --timeout=600 --"
+    PRECMD="python bin/sentry_wrap.py wrap-process --timeout=600 --"
 else
     echo "SENTRY_DSN not defined--not enabling sentry."
 fi

bin/run_web_disk_manager.sh

@@ -20,10 +20,10 @@ SLEEP_SECONDS=60
 PROCESS_TIMEOUT_SECONDS=120
 
 # Run disk manager in a loop sleeping SLEEP_SECONDS between rounds. Wrap in
-# sentry-wrap so it sends errors to Sentry.
+# sentry_wrap so it sends errors to Sentry.
 while true
 do
-    python /app/bin/sentry-wrap.py wrap-process --timeout="${PROCESS_TIMEOUT_SECONDS}" -- \
+    python /app/bin/sentry_wrap.py wrap-process --timeout="${PROCESS_TIMEOUT_SECONDS}" -- \
         python /app/manage.py remove_orphaned_files --skip-checks
     sleep "${SLEEP_SECONDS}"
 done

bin/sentry-wrap.py → bin/sentry_wrap.py

@@ -7,17 +7,18 @@
 # Wraps a command such that if it fails, an error report is sent to the Sentry service
 # specified by SENTRY_DSN in the environment.
 #
-# Usage: python bin/sentry-wrap.py wrap-process -- [CMD]
+# Usage: python bin/sentry_wrap.py wrap-process -- [CMD]
 #    Wraps a process in error-reporting Sentry goodness.
 #
-# Usage: python bin/sentry-wrap.py test-sentry
+# Usage: python bin/sentry_wrap.py test-sentry
 #    Tests Sentry configuration and connection.
 
 
+import json
 import os
+from pathlib import Path
 import shlex
 import subprocess
-import sys
 import time
 import traceback
 
@@ -26,6 +27,48 @@
 from sentry_sdk import capture_exception, capture_message
 
 
+# get_version_info and get_release_name are copied from
+# tecken/libdockerflow.py. We can't use them directly,
+# because libdockerflow.py loads modules from tecken, and
+# sentry_wrap needs to be independent from tecken (i.e.
+# any Django app context) to ensure it continues to work
+# outside of the app context (e.g. to run cron jobs).
+def get_version_info(basedir):
+    """Returns version.json data from deploys"""
+    path = Path(basedir) / "version.json"
+    if not path.exists():
+        return {}
+
+    try:
+        data = path.read_text()
+        return json.loads(data)
+    except (OSError, json.JSONDecodeError):
+        return {}
+
+
+def get_release_name(basedir):
+    """Return a friendly name for the release that is running
+
+    This pulls version data and then returns the best version-y thing available: the
+    version, the commit, or "unknown" if there's no version data.
+
+    :returns: string
+
+    """
+    version_info = get_version_info(basedir)
+    version = version_info.get("version", "none")
+    commit = version_info.get("commit")
+    commit = commit[:8] if commit else "unknown"
+    return f"{version}:{commit}"
+
+
+def set_up_sentry(sentry_dsn):
+    current_dir = os.path.dirname(os.path.abspath(__file__))
+    base_dir = os.path.dirname(current_dir)
+    release = get_release_name(base_dir)
+    sentry_sdk.init(dsn=sentry_dsn, release=release)
+
+
 @click.group()
 def cli_main():
     pass
@@ -37,10 +80,11 @@ def test_sentry(ctx):
     sentry_dsn = os.environ.get("SENTRY_DSN")
 
     if not sentry_dsn:
-        click.echo("SENTRY_DSN is not defined. Exiting.")
-        sys.exit(1)
+        click.echo("SENTRY_DSN is not defined. Exiting.", err=True)
+        ctx.exit(1)
+
+    set_up_sentry(sentry_dsn)
 
-    sentry_sdk.init(sentry_dsn)
     capture_message("Sentry test")
     click.echo("Success. Check Sentry.")
 
@@ -59,18 +103,18 @@ def test_sentry(ctx):
 @click.argument("cmd", nargs=-1)
 @click.pass_context
 def wrap_process(ctx, timeout, verbose, cmd):
+    if not cmd:
+        raise click.UsageError("CMD required")
+
     sentry_dsn = os.environ.get("SENTRY_DSN")
 
     if not sentry_dsn:
         click.echo("SENTRY_DSN is not defined. Exiting.", err=True)
-        sys.exit(1)
-
-    if not cmd:
-        raise click.UsageError("CMD required")
+        ctx.exit(1)
 
     start_time = time.time()
 
-    sentry_sdk.init(sentry_dsn)
+    set_up_sentry(sentry_dsn)
 
     cmd = " ".join(cmd)
     cmd_args = shlex.split(cmd)

tecken/libdockerflow.py

@@ -45,6 +45,7 @@ def check_url(url, setting_key):
     return errors
 
 
+# Please update bin/sentry_wrap.py when updating this function.
 def get_version_info(basedir):
     """Returns version.json data from deploys"""
     path = Path(basedir) / "version.json"
@@ -58,6 +59,7 @@ def get_version_info(basedir):
         return {}
 
 
+# Please update bin/sentry_wrap.py when updating this function.
 def get_release_name(basedir):
     """Return a friendly name for the release that is running
 

tecken/tests/test_sentry.py

@@ -2,13 +2,18 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
+import os
+import shlex
+import subprocess
 from unittest.mock import ANY
 
 from fillmore.test import diff_event
+import requests
 from werkzeug.test import Client
 
 from django.contrib.auth.models import User
 
+from bin.sentry_wrap import get_release_name
 from tecken.apps import count_sentry_scrub_error
 from tecken.tokens.models import Token
 from tecken.wsgi import application
@@ -226,3 +231,34 @@ def test_count_sentry_scrub_error(metricsmock):
     metricsmock.assert_incr(
         "tecken.sentry_scrub_error", value=1, tags=["host:testnode"]
     )
+
+
+def test_sentry_wrap_non_app_error_has_release():
+    port = os.environ.get("EXPOSE_SENTRY_PORT", 8090)
+
+    # Flush fakesentry to ensure we fetch only the desired error downstream
+    requests.post(f"http://fakesentry:{port}/api/flush/")
+
+    current_dir = os.path.dirname(os.path.abspath(__file__))
+    base_dir = os.path.join(os.path.dirname(current_dir), "..")
+    release = get_release_name(base_dir)
+
+    expected_event = {"release": release}
+
+    # Pass a non-Django command that will error to sentry_wrap
+    non_app_command = "ls -2"
+    sentry_wrap_command = f"python bin/sentry_wrap.py wrap-process -- {non_app_command}"
+    cmd_args = shlex.split(sentry_wrap_command)
+    subprocess.run(cmd_args, timeout=10)
+
+    # We don't have to worry about a race condition here, because when the
+    # subprocess exits, we know the sentry_sdk sent the event, and it has
+    # been processed successfully by fakesentry.
+    errors_resp = requests.get(f"http://fakesentry:{port}/api/errorlist/")
+    errors_resp.raise_for_status()
+    error_id = errors_resp.json()["errors"][0]
+    error_resp = requests.get(f"http://fakesentry:{port}/api/error/{error_id}")
+    error_resp.raise_for_status()
+    actual_event = error_resp.json()["payload"]
+
+    assert actual_event["release"] == expected_event["release"]