feat: add lock lifecycle breadcrumbs and task tracking to coverage tasks#718
feat: add lock lifecycle breadcrumbs and task tracking to coverage tasks#718thomasrockhu-codecov wants to merge 1 commit intomainfrom
Conversation
Add lock lifecycle breadcrumbs (LOCK_ACQUIRING, LOCK_ACQUIRED, LOCK_RELEASED) around all lock acquisitions in upload_finisher and notify tasks. Add task_name and parent_task_id to all existing breadcrumb calls across upload_processor, upload_finisher, and notify tasks for pipeline traceability. upload_processor: - Add task_name/parent_task_id to all existing breadcrumb calls (no locks in this task) upload_finisher: - Lock lifecycle breadcrumbs around UPLOAD_PROCESSING lock - Lock lifecycle breadcrumbs around UPLOAD_FINISHER lock - task_name/parent_task_id on all breadcrumb calls notify: - Lock lifecycle breadcrumbs around NOTIFICATION lock - task_name/parent_task_id on all breadcrumb calls Co-authored-by: Cursor <cursoragent@cursor.com>
| bc_kwargs = { | ||
| "commit_sha": commitid, | ||
| "repo_id": repoid, | ||
| "upload_ids": [arguments["upload_id"]], | ||
| "task_name": self.name, | ||
| "parent_task_id": self.request.parent_id, |
There was a problem hiding this comment.
Bug: The code passes unexpected arguments task_name and parent_task_id to _call_upload_breadcrumb_task and references undefined Milestones enum values like LOCK_ACQUIRING, causing runtime errors.
Severity: CRITICAL
Suggested Fix
Merge the dependency PR that adds the missing values (LOCK_ACQUIRING, etc.) to the Milestones enum. Also, update the _call_upload_breadcrumb_task function signature or the underlying BreadcrumbData model to accept task_name and parent_task_id arguments, as intended by the dependency.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: apps/worker/tasks/upload_processor.py#L66-L71
Potential issue: The function `_call_upload_breadcrumb_task` is called with `task_name`
and `parent_task_id` arguments, but its signature does not include these parameters,
which will result in a `TypeError`. Additionally, the code references `Milestones` enum
members such as `Milestones.LOCK_ACQUIRING`, `Milestones.LOCK_ACQUIRED`, and
`Milestones.LOCK_RELEASED`, which are not defined in the `Milestones` enum class. This
will cause an `AttributeError` at runtime. Both issues appear to stem from a dependency
that has not been merged, which was intended to introduce these changes.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| repo_id=repoid, | ||
| milestone=Milestones.PROCESSING_UPLOAD, | ||
| upload_ids=[arguments["upload_id"]], | ||
| error=Errors.INTERNAL_RETRYING, |
There was a problem hiding this comment.
Missing milestone parameter in breadcrumb calls after refactoring
Medium Severity
The refactoring to use bc_kwargs accidentally dropped the milestone parameter from nine _call_upload_breadcrumb_task calls across upload_processor.py (3 calls) and upload_finisher.py (6 calls in _process_reports_with_lock and _handle_finisher_lock). The original code always passed milestone (e.g., Milestones.PROCESSING_UPLOAD or the local milestone variable), but it was neither added to bc_kwargs nor passed explicitly in the error-handling paths. Since milestone defaults to None, these breadcrumbs will be recorded without any milestone value, losing important pipeline stage tracking for all error and retry scenarios.


Summary
upload_finisher(both UPLOAD_PROCESSING and UPLOAD_FINISHER locks) andnotify(NOTIFICATION lock)task_nameandparent_task_idto all existing_call_upload_breadcrumb_taskcalls acrossupload_processor,upload_finisher, andnotifyfor pipeline traceabilityMilestonesvalues andBreadcrumbDatafieldsTest plan
Made with Cursor
Note
Low Risk
Observability-only changes that add breadcrumb calls/metadata around existing locking and error paths, with minimal impact on business logic aside from potential noise/perf from extra breadcrumb writes.
Overview
Adds lock lifecycle breadcrumbing to coverage worker tasks by emitting
LOCK_ACQUIRING/LOCK_ACQUIRED/LOCK_RELEASEDmilestones around lock usage innotify(notification lock) andupload_finisher(both processing and finisher locks).Enriches all upload-breadcrumb emissions in
upload_processor,upload_finisher, andnotifywith consistent task metadata (task_name,parent_task_id, and relevantupload_ids), primarily via sharedbc_kwargspassed into_call_upload_breadcrumb_taskfor improved pipeline traceability and debugging.Written by Cursor Bugbot for commit e7ff3d9. This will update automatically on new commits. Configure here.