ref(uptime): Rename detectors module to autodetect#101534
Conversation
| @instrumented_task( | ||
| name="sentry.uptime.detectors.tasks.schedule_detections", | ||
| namespace=uptime_tasks, | ||
| processing_deadline_duration=60, | ||
| ) | ||
| def schedule_detections(): | ||
| def schedule_autodetections(): |
There was a problem hiding this comment.
@markstory Just want to double triple check here.
I've renamed this module and I've renamed this function. However the @instrumented_task annotation has NOT changed. This IS safe correct?
There was a problem hiding this comment.
Yes those changes are both safe. The task name and namespace attributes are what is serialized into activations and need to be consistent across deploys.
| metrics.incr("uptime.detectors.scheduler.scheduled_bucket") | ||
| last_processed = last_processed + timedelta(minutes=1) | ||
| process_detection_bucket.delay(last_processed.isoformat()) | ||
| process_autodetection_bucket.delay(last_processed.isoformat()) |
There was a problem hiding this comment.
@markstory when it dispatches this task, it's using the name from the annotation to dispatch the queued task right
There was a problem hiding this comment.
when it dispatches this task, it's using the name from the annotation to dispatch the queued task right
Correct.
sentry/src/sentry/taskworker/task.py
Lines 223 to 234 in 3ff36a4
Is what is stored in kafka when a task is spawned.
| get_candidate_projects_for_org, | ||
| get_candidate_urls_for_project, | ||
| get_organization_bucket, | ||
| should_detect_for_organization, | ||
| should_detect_for_project, | ||
| should_autodetect_for_organization, | ||
| should_autodetect_for_project, | ||
| ) | ||
| from sentry.uptime.subscriptions.subscriptions import ( | ||
| create_uptime_detector, |
There was a problem hiding this comment.
Potential bug: The TASKWORKER_IMPORTS configuration contains a stale import path, "sentry.uptime.detectors.tasks", which will cause worker processes to crash on startup with a ModuleNotFoundError.
-
Description: The module
sentry.uptime.detectors.taskswas moved tosentry.uptime.autodetect.tasks, but theTASKWORKER_IMPORTStuple insrc/sentry/conf/server.pywas not updated. When a taskworker process starts, it callsapp.load_modules(), which iterates throughTASKWORKER_IMPORTSand imports each module. The attempt to import the now non-existent path"sentry.uptime.detectors.tasks"will raise aModuleNotFoundError, causing the worker process to crash on startup and preventing any background tasks from running. -
Suggested fix: Update the stale import path in
src/sentry/conf/server.py. Change"sentry.uptime.detectors.tasks"to"sentry.uptime.autodetect.tasks"within theTASKWORKER_IMPORTStuple.
severity: 0.95, confidence: 1.0
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
oh I thought I fixed this, let me double check
There was a problem hiding this comment.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #101534 +/- ##
========================================
Coverage 80.97% 80.97%
========================================
Files 8706 8706
Lines 386918 386918
Branches 24520 24520
========================================
Hits 313315 313315
Misses 73252 73252
Partials 351 351 |
3f2bd7e to
6427973
Compare
Renames the uptime.detectors module to uptime.autodetect to avoid confusion with the workflow engine's detector concept. Changes: - Renamed src/sentry/uptime/detectors/ → src/sentry/uptime/autodetect/ - Renamed tests/sentry/uptime/detectors/ → tests/sentry/uptime/autodetect/ - Renamed functions: detect_base_url_for_project → autodetect_base_url_for_project - Renamed functions: should_detect_for_* → should_autodetect_for_* - Updated all import statements and mock.patch references IMPORTANT - NOT CHANGED (to avoid breaking production): - Celery task names remain unchanged to prevent breaking in-flight tasks: * "sentry.uptime.detectors.tasks.schedule_detections" * "sentry.uptime.detectors.tasks.process_detection_bucket" * "sentry.uptime.detectors.tasks.process_organization_url_ranking" - Redis keys remain unchanged (11 keys) to prevent breaking lookups - Lock name "uptime.detection.schedule_detections" remains unchanged - Django setting SENTRY_UPTIME_DETECTOR_CLUSTER remains unchanged - Feature flags remain unchanged - Metrics remain unchanged to avoid breaking dashboards
6427973 to
fb485d3
Compare
| @instrumented_task( | ||
| name="sentry.uptime.detectors.tasks.schedule_detections", | ||
| namespace=uptime_tasks, | ||
| processing_deadline_duration=60, | ||
| ) | ||
| def schedule_detections(): | ||
| def schedule_autodetections(): |
There was a problem hiding this comment.
Yes those changes are both safe. The task name and namespace attributes are what is serialized into activations and need to be consistent across deploys.
| metrics.incr("uptime.detectors.scheduler.scheduled_bucket") | ||
| last_processed = last_processed + timedelta(minutes=1) | ||
| process_detection_bucket.delay(last_processed.isoformat()) | ||
| process_autodetection_bucket.delay(last_processed.isoformat()) |
There was a problem hiding this comment.
when it dispatches this task, it's using the name from the annotation to dispatch the queued task right
Correct.
sentry/src/sentry/taskworker/task.py
Lines 223 to 234 in 3ff36a4
Is what is stored in kafka when a task is spawned.
|
Going to wait until tomorrow to merge |
Renames the uptime.detectors module to uptime.autodetect to avoid
confusion with the workflow engine's detector concept.
Changes:
IMPORTANT - NOT CHANGED (to avoid breaking production):