From 405f67741f87e0703ebce006bd9ba9f71dcdfaab Mon Sep 17 00:00:00 2001 From: Matt Lewellyn Date: Mon, 15 Jan 2024 16:22:00 -0500 Subject: [PATCH] use sentry SDK for cron monitor setup and check-ins --- keg_elements/sentry.py | 110 +++++++------- keg_elements/tests/test_sentry.py | 240 +++++++++++------------------- 2 files changed, 141 insertions(+), 209 deletions(-) diff --git a/keg_elements/sentry.py b/keg_elements/sentry.py index d2e87f0..62df276 100644 --- a/keg_elements/sentry.py +++ b/keg_elements/sentry.py @@ -1,6 +1,7 @@ import base64 import contextlib import re +import sys import urllib.parse import flask @@ -261,7 +262,7 @@ class SentryMonitor: """Sentry allows monitoring scheduled job execution, and exposes this feature in its API. SentryMonitor wraps usage. - Init the class with the Sentry org name, "base" monitor key, and the environment to + Init the class with the "base" monitor key, and the environment to tie the monitor to in Sentry. Note: environment is not yet well-supported. At the time of this writing, there is no @@ -273,28 +274,32 @@ class SentryMonitor: jobs: my-cron-job: checkin_margin: 1 # in minutes - schedule_type: crontab - schedule: '30 2 * * *' + schedule: + type: crontab + value: '30 2 * * *' max_runtime: 1 # in minutes + failure_issue_threshold: 2 # optional timezone: Etc/UTC other-stats-run: checkin_margin: 1 - schedule_type: interval - schedule: [6, minute] + schedule: + type: interval + value: 6 + unit: minute max_runtime: 1 timezone: Etc/UTC """ - def __init__(self, org: str, key: str, env: str): + def __init__(self, key: str, env: str): if requests is None: raise Exception('requests library required for Sentry monitoring') - self.org = org self.key = key self.env = env self.dsn = flask.current_app.config.get('SENTRY_DSN') self.checkin_id = None + self.start_timestamp = None self.status = None @classmethod @@ -306,22 +311,17 @@ def read_config(cls, path): return yaml.safe_load(f) @classmethod - def apply_config(cls, org: str, config_data: dict): + def apply_config(cls, config_data: dict): """Runs monitor config on Sentry API for each job in config data.""" env = flask.current_app.config.get('SENTRY_ENVIRONMENT') return_data = {} error_data = {} for key, monitor_config in config_data.get('jobs', {}).items(): # monitor gets created via the check-in, which requires status - config_data = { - 'monitor_config': monitor_config, - 'status': 'ok', - 'environment': env, - } - monitor = SentryMonitor(org, key, env) + monitor = SentryMonitor(key, env) try: - return_data[key] = monitor.make_request(config_data) - if not return_data[key].get('id'): + return_data[key] = monitor.configure(monitor_config) + if not return_data[key]: error_data[key] = return_data[key] except SentryMonitorError as exc: error_data[key] = str(exc) @@ -335,49 +335,50 @@ def apply_config(cls, org: str, config_data: dict): def monitor_key(self): return f'{self.key}-{self.env}'.lower() - @property - def url(self): - mkey = self.monitor_key - base = f'https://sentry.io/api/0/organizations/{self.org}/monitors/{mkey}/checkins/' - if self.checkin_id: - return f'{base}{self.checkin_id}/' - return base - - def make_request(self, payload): - method = requests.put if self.checkin_id else requests.post - resp = method( - self.url, - json=payload, - headers={ - 'Authorization': f'DSN {self.dsn}', - }, - timeout=10, + def configure(self, payload): + return sentry_sdk.crons.api.capture_checkin( + monitor_slug=self.monitor_key, + status=sentry_sdk.crons.consts.MonitorStatus.OK, + monitor_config=payload, ) - if resp.status_code >= 400: - raise SentryMonitorError(f'{resp.status_code}: {resp.content}') - return resp.json() - def ping_status(self, status): + def get_duration(self): + duration = None + if self.start_timestamp: + duration = sentry_sdk.utils.now() - self.start_timestamp + return duration + + def ping_status(self, status, duration=None): self.status = status - data = self.make_request({'status': status, 'environment': self.env}) - if 'id' in data: - # Store the ID so further status updates in this "session" are tied to the same - # checkin. Prevents issues with overlap of job runs. - self.checkin_id = data['id'] - return data + kwargs = {} + if duration is not None: + kwargs['duration'] = duration + if self.checkin_id is not None: + kwargs['check_in_id'] = self.checkin_id + self.checkin_id = sentry_sdk.crons.api.capture_checkin( + monitor_slug=self.monitor_key, status=status, **kwargs + ) + return self.checkin_id def ping_ok(self): - return self.ping_status('ok') + return self.ping_status( + sentry_sdk.crons.consts.MonitorStatus.OK, + duration=self.get_duration(), + ) def ping_error(self): - return self.ping_status('error') + return self.ping_status( + sentry_sdk.crons.consts.MonitorStatus.ERROR, + duration=self.get_duration(), + ) def ping_in_progress(self): - return self.ping_status('in_progress') + self.start_timestamp = sentry_sdk.utils.now() + return self.ping_status(sentry_sdk.crons.consts.MonitorStatus.IN_PROGRESS) @contextlib.contextmanager -def sentry_monitor_job(org: str, key: str, env: str = None, do_ping: bool = False): +def sentry_monitor_job(key: str, env: str = None, do_ping: bool = False): """Context manager for running in_progress, then ok status pings on a Sentry monitor. Sentry will not be pinged unless the ``do_ping`` kwarg is True (default is False). @@ -391,13 +392,18 @@ def sentry_monitor_job(org: str, key: str, env: str = None, do_ping: bool = Fals if not do_ping: yield else: - monitor = SentryMonitor(org, key, env) - monitor.ping_in_progress() + monitor = SentryMonitor(key, env) + try: + monitor.ping_in_progress() + yield monitor - if monitor.status == 'in_progress': - # Code wrapped in context may have manually pinged another status + + if monitor.status == sentry_sdk.crons.consts.MonitorStatus.IN_PROGRESS: + # Code wrapped in context likely has not manually pinged another status. + # This block allows us to manually set an error based on app logic. monitor.ping_ok() except Exception: monitor.ping_error() - raise + exc_info = sys.exc_info() + sentry_sdk._compat.reraise(*exc_info) diff --git a/keg_elements/tests/test_sentry.py b/keg_elements/tests/test_sentry.py index a1fba81..cf5b37e 100644 --- a/keg_elements/tests/test_sentry.py +++ b/keg_elements/tests/test_sentry.py @@ -2,8 +2,6 @@ import flask import pytest -import responses -from responses import matchers from keg_elements import sentry @@ -376,173 +374,109 @@ class Filter(sentry.SentryEventFilter): class TestSentryMonitorUtils: - @responses.activate @mock.patch.dict('flask.current_app.config', { - 'SENTRY_DSN': 'mydsn', 'SENTRY_ENVIRONMENT': 'testenv' }) - def test_job_normal(self): - resp_in_progress = responses.add( - responses.POST, - 'https://sentry.io/api/0/organizations/myorg/monitors/monitor-key-testenv/checkins/', - json={'id': 'test-checkin'}, - status=200, - match=[ - matchers.header_matcher({'Authorization': 'DSN mydsn'}), - matchers.json_params_matcher({'status': 'in_progress', 'environment': 'testenv'}), - ], - ) - resp_ok = responses.add( - responses.PUT, - 'https://sentry.io/api/0/organizations/myorg/monitors/' - 'monitor-key-testenv/checkins/test-checkin/', - json={'id': 'test-checkin'}, - status=200, - match=[ - matchers.header_matcher({'Authorization': 'DSN mydsn'}), - matchers.json_params_matcher({'status': 'ok', 'environment': 'testenv'}), - ], - ) - with sentry.sentry_monitor_job('myorg', 'monitor-key', do_ping=True): + @mock.patch( + 'keg_elements.sentry.sentry_sdk.crons.api.capture_checkin', autospec=True, spec_set=True + ) + def test_job_normal(self, m_sentry_checkin): + m_sentry_checkin.return_value = 'foo123' + + with sentry.sentry_monitor_job('monitor-key', do_ping=True) as monitor: pass - assert resp_in_progress.call_count == 1 - assert resp_ok.call_count == 1 + assert m_sentry_checkin.call_count == 2 + first_call = m_sentry_checkin.call_args_list[0].kwargs + assert first_call == dict(monitor_slug='monitor-key-testenv', status='in_progress') + second_call = m_sentry_checkin.call_args_list[1].kwargs + assert second_call['monitor_slug'] == 'monitor-key-testenv' + assert second_call['status'] == 'ok' + assert second_call['check_in_id'] == 'foo123' + assert second_call['duration'] + assert monitor.checkin_id == 'foo123' - @responses.activate @mock.patch.dict('flask.current_app.config', { - 'SENTRY_DSN': 'mydsn', 'SENTRY_ENVIRONMENT': 'testenv' }) - def test_job_network_error(self): - resp_in_progress = responses.add( - responses.POST, - 'https://sentry.io/api/0/organizations/myorg/monitors/monitor-key-testenv/checkins/', - body='something wrong', - status=500, - match=[ - matchers.header_matcher({'Authorization': 'DSN mydsn'}), - matchers.json_params_matcher({'status': 'in_progress', 'environment': 'testenv'}), - ], - ) - with pytest.raises(sentry.SentryMonitorError, match='something wrong'): - with sentry.sentry_monitor_job('myorg', 'monitor-key', do_ping=True): + @mock.patch( + 'keg_elements.sentry.sentry_sdk.crons.api.capture_checkin', autospec=True, spec_set=True + ) + def test_job_sentry_exception(self, m_sentry_checkin): + m_sentry_checkin.side_effect = Exception('something wrong'), 'foo123' + with pytest.raises(Exception, match='something wrong'): + with sentry.sentry_monitor_job('monitor-key', do_ping=True): pass - assert resp_in_progress.call_count == 1 + assert m_sentry_checkin.call_count == 2 + first_call = m_sentry_checkin.call_args_list[0].kwargs + assert first_call == dict(monitor_slug='monitor-key-testenv', status='in_progress') + second_call = m_sentry_checkin.call_args_list[1].kwargs + assert second_call['monitor_slug'] == 'monitor-key-testenv' + assert second_call['status'] == 'error' - @responses.activate @mock.patch.dict('flask.current_app.config', { - 'SENTRY_DSN': 'mydsn', 'SENTRY_ENVIRONMENT': 'testenv' }) - def test_job_override_final_ping(self): - resp_in_progress = responses.add( - responses.POST, - 'https://sentry.io/api/0/organizations/myorg/monitors/monitor-key-testenv/checkins/', - json={'id': 'test-checkin'}, - status=200, - match=[ - matchers.header_matcher({'Authorization': 'DSN mydsn'}), - matchers.json_params_matcher({'status': 'in_progress', 'environment': 'testenv'}), - ], - ) - resp_ok = responses.add( - responses.PUT, - 'https://sentry.io/api/0/organizations/myorg/monitors/' - 'monitor-key-testenv/checkins/test-checkin/', - json={'id': 'test-checkin'}, - status=200, - match=[ - matchers.header_matcher({'Authorization': 'DSN mydsn'}), - matchers.json_params_matcher({'status': 'error', 'environment': 'testenv'}), - ], - ) - with sentry.sentry_monitor_job('myorg', 'monitor-key', do_ping=True) as monitor: + @mock.patch( + 'keg_elements.sentry.sentry_sdk.crons.api.capture_checkin', autospec=True, spec_set=True + ) + def test_job_override_final_ping(self, m_sentry_checkin): + m_sentry_checkin.return_value = 'foo123' + + with sentry.sentry_monitor_job('monitor-key', do_ping=True) as monitor: monitor.ping_error() - assert resp_in_progress.call_count == 1 - assert resp_ok.call_count == 1 + assert m_sentry_checkin.call_count == 2 + first_call = m_sentry_checkin.call_args_list[0].kwargs + assert first_call == dict(monitor_slug='monitor-key-testenv', status='in_progress') + second_call = m_sentry_checkin.call_args_list[1].kwargs + assert second_call['monitor_slug'] == 'monitor-key-testenv' + assert second_call['status'] == 'error' + assert second_call['check_in_id'] == 'foo123' + assert second_call['duration'] - @responses.activate @mock.patch.dict('flask.current_app.config', { - 'SENTRY_DSN': 'mydsn', 'SENTRY_ENVIRONMENT': 'testenv' }) - def test_job_no_ping(self): - resp_in_progress = responses.add( - responses.POST, - 'https://sentry.io/api/0/organizations/myorg/monitors/monitor-key-testenv/checkins/', - json={'id': 'test-checkin'}, - status=200, - ) - resp_ok = responses.add( - responses.PUT, - 'https://sentry.io/api/0/organizations/myorg/monitors/' - 'monitor-key-testenv/checkins/test-checkin/', - json={'id': 'test-checkin'}, - status=200, - ) - with sentry.sentry_monitor_job('myorg', 'monitor-key', do_ping=False): + @mock.patch( + 'keg_elements.sentry.sentry_sdk.crons.api.capture_checkin', autospec=True, spec_set=True + ) + def test_job_no_ping(self, m_sentry_checkin): + with sentry.sentry_monitor_job('monitor-key', do_ping=False): pass - assert resp_in_progress.call_count == 0 - assert resp_ok.call_count == 0 + assert m_sentry_checkin.call_count == 0 - @responses.activate @mock.patch.dict('flask.current_app.config', { - 'SENTRY_DSN': 'mydsn', 'SENTRY_ENVIRONMENT': 'testenv' }) - def test_job_exception(self): - resp_in_progress = responses.add( - responses.POST, - 'https://sentry.io/api/0/organizations/myorg/monitors/monitor-key-testenv/checkins/', - json={'id': 'test-checkin'}, - status=200, - match=[ - matchers.header_matcher({'Authorization': 'DSN mydsn'}), - matchers.json_params_matcher({'status': 'in_progress', 'environment': 'testenv'}), - ], - ) - resp_ok = responses.add( - responses.PUT, - 'https://sentry.io/api/0/organizations/myorg/monitors/' - 'monitor-key-testenv/checkins/test-checkin/', - json={'id': 'test-checkin'}, - status=200, - match=[ - matchers.header_matcher({'Authorization': 'DSN mydsn'}), - matchers.json_params_matcher({'status': 'error', 'environment': 'testenv'}), - ], - ) + @mock.patch( + 'keg_elements.sentry.sentry_sdk.crons.api.capture_checkin', autospec=True, spec_set=True + ) + def test_job_exception(self, m_sentry_checkin): + m_sentry_checkin.return_value = 'foo123' + with pytest.raises(Exception): - with sentry.sentry_monitor_job('myorg', 'monitor-key', do_ping=True): + with sentry.sentry_monitor_job('monitor-key', do_ping=True): raise ValueError('foo') - assert resp_in_progress.call_count == 1 - assert resp_ok.call_count == 1 + assert m_sentry_checkin.call_count == 2 + first_call = m_sentry_checkin.call_args_list[0].kwargs + assert first_call == dict(monitor_slug='monitor-key-testenv', status='in_progress') + second_call = m_sentry_checkin.call_args_list[1].kwargs + assert second_call['monitor_slug'] == 'monitor-key-testenv' + assert second_call['status'] == 'error' + assert second_call['check_in_id'] == 'foo123' + assert second_call['duration'] - @responses.activate @mock.patch.dict('flask.current_app.config', { - 'SENTRY_DSN': 'mydsn', 'SENTRY_ENVIRONMENT': 'testenv' }) - def test_config_applied(self): - resp_ok = responses.add( - responses.POST, - 'https://sentry.io/api/0/organizations/myorg/monitors/monitor-key-testenv/checkins/', - json={'id': 'test-checkin'}, - status=200, - match=[ - matchers.header_matcher({'Authorization': 'DSN mydsn'}), - matchers.json_params_matcher({ - 'monitor_config': {'key': 'value'}, - 'status': 'ok', - 'environment': 'testenv', - }), - ], - ) + @mock.patch( + 'keg_elements.sentry.sentry_sdk.crons.api.capture_checkin', autospec=True, spec_set=True + ) + def test_config_applied(self, m_sentry_checkin): config_data = { 'jobs': { 'monitor-key': { @@ -550,29 +484,22 @@ def test_config_applied(self): } } } - sentry.SentryMonitor.apply_config('myorg', config_data) - assert resp_ok.call_count == 1 + sentry.SentryMonitor.apply_config(config_data) + m_sentry_checkin.assert_called_once_with( + monitor_slug='monitor-key-testenv', + status='ok', + monitor_config={'key': 'value'}, + ) - @responses.activate @mock.patch.dict('flask.current_app.config', { - 'SENTRY_DSN': 'mydsn', 'SENTRY_ENVIRONMENT': 'testenv' }) - def test_config_error(self): - resp_ok = responses.add( - responses.POST, - 'https://sentry.io/api/0/organizations/myorg/monitors/monitor-key-testenv/checkins/', - json={'error': 'something wrong'}, - status=200, - match=[ - matchers.header_matcher({'Authorization': 'DSN mydsn'}), - matchers.json_params_matcher({ - 'monitor_config': {'key': 'value'}, - 'status': 'ok', - 'environment': 'testenv', - }), - ], - ) + @mock.patch( + 'keg_elements.sentry.sentry_sdk.crons.api.capture_checkin', autospec=True, spec_set=True + ) + def test_config_error(self, m_sentry_checkin): + m_sentry_checkin.return_value = None + config_data = { 'jobs': { 'monitor-key': { @@ -580,6 +507,5 @@ def test_config_error(self): } } } - with pytest.raises(sentry.SentryMonitorConfigError, match='something wrong'): - sentry.SentryMonitor.apply_config('myorg', config_data) - assert resp_ok.call_count == 1 + with pytest.raises(sentry.SentryMonitorConfigError): + sentry.SentryMonitor.apply_config(config_data)