Skip to content

Commit 66ee86f

Browse files
authored
fix(services): Improve status code consistency (#1090)
1 parent 56038b8 commit 66ee86f

54 files changed

Lines changed: 1349 additions & 633 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.claude/conventions/celery.md‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ worker pool so a fast-eval burst can't occupy `default`'s worker slots and delay
88
LLM jobs; priority only reorders queued-not-yet-claimed messages, it can't preempt a task a
99
worker already started). Within each queue, tasks are ordered by a per-task `priority` (higher
1010
drains first, FIFO within a band). Read `app/celery/tasks/job_execution.py` before writing — it
11-
shows the full pattern (decorator + timeout + OTel propagation + delegation to a service).
11+
shows the full pattern (decorator + timeout + delegation to a service).
1212

1313
## Canonical decorator stack
1414

@@ -19,20 +19,18 @@ def run_my_job(self, project_id: int, job_id: str, trace_id: str, **kwargs):
1919
from app.services.my_domain.jobs import do_the_work # late import to avoid cycles
2020

2121
_set_trace(trace_id)
22-
return _run_with_otel_parent(
23-
self,
24-
lambda: do_the_work(
25-
project_id=project_id,
26-
job_id=job_id,
27-
task_id=current_task.request.id,
28-
task_instance=self,
29-
**kwargs,
30-
),
22+
return do_the_work(
23+
project_id=project_id,
24+
job_id=job_id,
25+
task_id=current_task.request.id,
26+
task_instance=self,
27+
**kwargs,
3128
)
3229
```
3330

34-
`_set_trace`, `_run_with_otel_parent`, and `gevent_timeout` already exist in this module /
35-
`app/celery/utils.py` — reuse them, don't reinvent.
31+
`_set_trace` and `gevent_timeout` already exist in this module / `app/celery/utils.py` — reuse
32+
them, don't reinvent. Enqueue through a `start_*` helper in `app/celery/utils.py`, which calls
33+
plain `task.apply_async(kwargs={...})`.
3634

3735
## Queue and priority choice — be explicit
3836

@@ -61,7 +59,7 @@ inherits its priority unless you override it.
6159
- **`bind=True`** so you have `self` (the task instance) for retries, IDs, etc.
6260
- **Always pass an explicit `queue` (`"default"` or `"evaluations"`) and `priority`.** Only fast-eval chunk/aggregate tasks use `"evaluations"`; everything else uses `"default"`.
6361
- **Pass `trace_id` explicitly** as a parameter and call `_set_trace(trace_id)` first thing. This wires `asgi_correlation_id` so logs from inside the task match the originating request.
64-
- **Wrap the work in `_run_with_otel_parent(self, lambda: ...)`** so OpenTelemetry parent context propagates from the enqueueing process.
62+
- **Don't propagate trace context by hand.** OTel's `CeleryInstrumentor` already parents the worker's `run/...` span to the enqueue span. Injecting headers on enqueue or attaching an extracted context in the task reparents the task's spans to the API request, and Sentry splits them into separate transactions.
6563
- **Delegate to a service.** The task body should be a thin shim over `app/services/<domain>/`. No DB queries, no external HTTP, no business logic inside the task itself.
6664
- **Late-import the service inside the function body** (as the canonical pattern does). Celery workers boot faster and you avoid model-import cycles.
6765
- **Idempotency.** Celery will redeliver. Either:

‎.gitignore‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,6 @@ ENV/
2121

2222
#
2323
/backend/app/logs
24+
25+
# local assessment scratch data
26+
z_assessment_test/

‎backend/app/api/deps.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ def _set_tenant_span_attributes(auth_context: AuthContext) -> None:
5050
if span.is_recording():
5151
span.set_attribute("user.id", str(auth_context.user.id))
5252
if auth_context.organization:
53-
span.set_attribute("tenant.org_id", auth_context.organization.id)
53+
span.set_attribute("tenant.org_id", str(auth_context.organization.id))
5454
if auth_context.project:
55-
span.set_attribute("tenant.project_id", auth_context.project.id)
55+
span.set_attribute("tenant.project_id", str(auth_context.project.id))
5656

5757
org_id = auth_context.organization.id if auth_context.organization else None
5858
project_id = auth_context.project.id if auth_context.project else None

‎backend/app/api/routes/collections.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from fastapi import Path as FastPath
77

88
from app.api.deps import SessionDep, AuthContextDep
9+
from app.crud.collection.collection import CollectionNameConflictError
910
from app.api.permissions import Permission, require_permission
1011
from app.core.telemetry import log_context
1112
from app.core.rate_monitor import monitor_rate
@@ -14,7 +15,6 @@
1415
CollectionJobCrud,
1516
DocumentCollectionCrud,
1617
)
17-
from app.crud.collection.collection import CollectionNameConflictError
1818
from app.core.cloud import get_cloud_storage
1919
from app.models import (
2020
CollectionJobStatus,

‎backend/app/celery/celery_app.py‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,20 @@ def _initialize_worker_observability() -> None:
4646
)
4747

4848
@task_prerun.connect(weak=False)
49-
def _tag_sentry_task(task: "celery.Task", **_: object) -> None: # type: ignore[name-defined]
50-
"""Tag the task's Sentry scope with its name so Issues can be filtered by task."""
49+
def _tag_sentry_task(
50+
task: "celery.Task", # type: ignore[name-defined]
51+
kwargs: dict[str, object] | None = None,
52+
**_: object,
53+
) -> None:
54+
"""Tag the task's Sentry scope with its name and tenant so Issues can be filtered by both."""
5155
sentry_sdk.set_tag("celery.task_name", task.name)
56+
task_kwargs = kwargs or {}
57+
for tag, key in (
58+
("tenant.project_id", "project_id"),
59+
("tenant.org_id", "organization_id"),
60+
):
61+
if task_kwargs.get(key) is not None:
62+
sentry_sdk.set_tag(tag, str(task_kwargs[key]))
5263

5364
_sentry_initialized = True
5465

0 commit comments

Comments
 (0)