From 7f3d38ae2eda1c3c31b7e812a96ca452fd47335c Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 8 May 2023 10:29:53 +0200 Subject: [PATCH 1/6] Using epoch in Celery task check-ins --- sentry_sdk/integrations/celery.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/integrations/celery.py b/sentry_sdk/integrations/celery.py index 3975990d8d..db36d35e46 100644 --- a/sentry_sdk/integrations/celery.py +++ b/sentry_sdk/integrations/celery.py @@ -1,6 +1,7 @@ from __future__ import absolute_import import sys +import time from sentry_sdk.consts import OP from sentry_sdk._compat import reraise @@ -15,7 +16,6 @@ capture_internal_exceptions, event_from_exception, logger, - now, ) if TYPE_CHECKING: @@ -114,6 +114,13 @@ def sentry_build_tracer(name, task, *args, **kwargs): ignore_logger("celery.redirected") +def _now_seconds_since_epoch(): + # We can not use a perf_counter here, because the start of a Celery + # tasks and the finish are recorded in differenct processes + # Start happens in the Celery Beat process, the end in the Celery Worker process + return time.time() + + def _wrap_apply_async(f): # type: (F) -> F @wraps(f) @@ -130,7 +137,8 @@ def apply_async(*args, **kwargs): if integration.monitor_beat_tasks: headers.update( { - "sentry-monitor-start-timestamp-s": "%.9f" % now(), + "sentry-monitor-start-timestamp-s": "%.9f" + % _now_seconds_since_epoch(), } ) @@ -449,7 +457,7 @@ def crons_task_success(sender, **kwargs): monitor_slug=headers["sentry-monitor-slug"], monitor_config=monitor_config, check_in_id=headers["sentry-monitor-check-in-id"], - duration=now() - start_timestamp_s, + duration=_now_seconds_since_epoch() - start_timestamp_s, status=MonitorStatus.OK, ) @@ -470,7 +478,7 @@ def crons_task_failure(sender, **kwargs): monitor_slug=headers["sentry-monitor-slug"], monitor_config=monitor_config, check_in_id=headers["sentry-monitor-check-in-id"], - duration=now() - start_timestamp_s, + duration=_now_seconds_since_epoch() - start_timestamp_s, status=MonitorStatus.ERROR, ) @@ -491,6 +499,6 @@ def crons_task_retry(sender, **kwargs): monitor_slug=headers["sentry-monitor-slug"], monitor_config=monitor_config, check_in_id=headers["sentry-monitor-check-in-id"], - duration=now() - start_timestamp_s, + duration=_now_seconds_since_epoch() - start_timestamp_s, status=MonitorStatus.ERROR, ) From 753e46cbfbee22644671c76da2c47f4bad6f2111 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 8 May 2023 10:33:00 +0200 Subject: [PATCH 2/6] Wording --- sentry_sdk/integrations/celery.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/celery.py b/sentry_sdk/integrations/celery.py index db36d35e46..55d54fc94b 100644 --- a/sentry_sdk/integrations/celery.py +++ b/sentry_sdk/integrations/celery.py @@ -115,9 +115,11 @@ def sentry_build_tracer(name, task, *args, **kwargs): def _now_seconds_since_epoch(): - # We can not use a perf_counter here, because the start of a Celery - # tasks and the finish are recorded in differenct processes - # Start happens in the Celery Beat process, the end in the Celery Worker process + # We can not use a perf_counter when dealing with durations + # of Celery tasks, because the start of a Celery tasks and + # the finish are recorded in differenct processes. + # Start happens in the Celery Beat process, + # the end in the Celery Worker process. return time.time() From 29ffcd415e58e6331bae7444f2b0077a9df9496a Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 8 May 2023 10:36:23 +0200 Subject: [PATCH 3/6] Wording --- sentry_sdk/integrations/celery.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/integrations/celery.py b/sentry_sdk/integrations/celery.py index 55d54fc94b..39163949c5 100644 --- a/sentry_sdk/integrations/celery.py +++ b/sentry_sdk/integrations/celery.py @@ -115,11 +115,11 @@ def sentry_build_tracer(name, task, *args, **kwargs): def _now_seconds_since_epoch(): - # We can not use a perf_counter when dealing with durations - # of Celery tasks, because the start of a Celery tasks and - # the finish are recorded in differenct processes. + # We can not use `time.perf_counter()` when dealing with the duration + # of a Celery tasks, because the start of a Celery tasks and + # the end are recorded in differenct processes. # Start happens in the Celery Beat process, - # the end in the Celery Worker process. + # the end in a Celery Worker process. return time.time() From 8df7312cc86edc7945a9ddaff6a2e3d0c70e9006 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 8 May 2023 10:42:46 +0200 Subject: [PATCH 4/6] Updated tests --- .../integrations/celery/test_celery_beat_crons.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/integrations/celery/test_celery_beat_crons.py b/tests/integrations/celery/test_celery_beat_crons.py index d521c4e037..431e32642d 100644 --- a/tests/integrations/celery/test_celery_beat_crons.py +++ b/tests/integrations/celery/test_celery_beat_crons.py @@ -89,7 +89,10 @@ def test_crons_task_success(): with mock.patch( "sentry_sdk.integrations.celery.capture_checkin" ) as mock_capture_checkin: - with mock.patch("sentry_sdk.integrations.celery.now", return_value=500.5): + with mock.patch( + "sentry_sdk.integrations.celery._now_seconds_since_epoch", + return_value=500.5, + ): crons_task_success(fake_task) mock_capture_checkin.assert_called_once_with( @@ -130,7 +133,10 @@ def test_crons_task_failure(): with mock.patch( "sentry_sdk.integrations.celery.capture_checkin" ) as mock_capture_checkin: - with mock.patch("sentry_sdk.integrations.celery.now", return_value=500.5): + with mock.patch( + "sentry_sdk.integrations.celery._now_seconds_since_epoch", + return_value=500.5, + ): crons_task_failure(fake_task) mock_capture_checkin.assert_called_once_with( @@ -171,7 +177,10 @@ def test_crons_task_retry(): with mock.patch( "sentry_sdk.integrations.celery.capture_checkin" ) as mock_capture_checkin: - with mock.patch("sentry_sdk.integrations.celery.now", return_value=500.5): + with mock.patch( + "sentry_sdk.integrations.celery._now_seconds_since_epoch", + return_value=500.5, + ): crons_task_retry(fake_task) mock_capture_checkin.assert_called_once_with( From ff3ee1542bd6360d2b168ebfeda861ea1e4836db Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 8 May 2023 10:51:08 +0200 Subject: [PATCH 5/6] Added typing --- sentry_sdk/integrations/celery.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sentry_sdk/integrations/celery.py b/sentry_sdk/integrations/celery.py index 39163949c5..9e7fee29e9 100644 --- a/sentry_sdk/integrations/celery.py +++ b/sentry_sdk/integrations/celery.py @@ -115,6 +115,7 @@ def sentry_build_tracer(name, task, *args, **kwargs): def _now_seconds_since_epoch(): + # type: () -> float # We can not use `time.perf_counter()` when dealing with the duration # of a Celery tasks, because the start of a Celery tasks and # the end are recorded in differenct processes. From 0c9588f2b1089e3aae848403c94c1d654c5c8f15 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 8 May 2023 12:05:04 +0200 Subject: [PATCH 6/6] Update sentry_sdk/integrations/celery.py Co-authored-by: Ivana Kellyerova <131587164+sentrivana@users.noreply.github.com> --- sentry_sdk/integrations/celery.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/celery.py b/sentry_sdk/integrations/celery.py index 9e7fee29e9..8c9484e2f0 100644 --- a/sentry_sdk/integrations/celery.py +++ b/sentry_sdk/integrations/celery.py @@ -116,9 +116,9 @@ def sentry_build_tracer(name, task, *args, **kwargs): def _now_seconds_since_epoch(): # type: () -> float - # We can not use `time.perf_counter()` when dealing with the duration - # of a Celery tasks, because the start of a Celery tasks and - # the end are recorded in differenct processes. + # We cannot use `time.perf_counter()` when dealing with the duration + # of a Celery task, because the start of a Celery task and + # the end are recorded in different processes. # Start happens in the Celery Beat process, # the end in a Celery Worker process. return time.time()