Repository navigation
fix(worker): a returned 5xx marks the worker's server span as an error - #1406
Conversation
The worker's server span (_traced) was marked ERROR only when a handler raised. The canvas routes return their failures as a 500 rather than raising, so a failed canvas start, approval or stop left the span UNSET with only http.status_code, and Tempo's error filters missed it. After the app answers, a 5xx now sets Status(ERROR, "HTTP <status>") and error.type to the status code, as OpenTelemetry's HTTP conventions mark a server span. No message text goes on the span, so #1107's type-only rule holds. A 4xx is the caller's and stays UNSET. Closes #1310 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dave Arthur <davidarthur@hotmail.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.
Once extra usage is available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Record and test response status services/workspace-worker/worker_otel.py, orchestrator/tests/test_prd256_o2b_worker_span.py |
_answered records the response status and marks 5xx spans as errors with a status-based description and error.type. The tracing middleware uses this helper. Tests check 5xx and 4xx span behavior and verify that secret response text is not recorded. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related PRs
- AutomatosAI/automatos-ai#1220: This PR continues the worker tracing implementation and tests introduced in PR
#1220.
Suggested reviewers: automatosai
Merge Risk: 
08bcd
-
Returned 5xx responses are intended to be marked as errors without exposing response text. No merge-blocking risk was identified.
Pre-merge checks: 
-
Check name Status Explanation Title check The title names the main change: a returned 5xx response now marks the workspace worker's server span as an error. It matches the raw summary (_answered marks status codes of 500 or higher as ERROR) a… Linked Issues check Issue [ #1310] requires returned 5xx responses to mark the worker server span as ERROR, seterror.typeto the status code, and avoid message text. It also requires returned 4xx responses to remain UN…Out of Scope Changes check The changed files are limited to the worker tracing implementation and its focused span test. The documentation update explains the changed middleware behavior. The test additions support issue [ #1310…Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Description check The description follows the repository template closely: it has a 'What and why' section that links issue #1310and explains the problem and fix, and a 'How it was verified' section with a named test …Full details: Title check
-
Explanation
The title names the main change: a returned 5xx response now marks the workspace worker's server span as an error. It matches the raw summary (_answered marks status codes of 500 or higher as ERROR) and the linked issue
#1310. The 'fix(worker)' prefix is a conventional-commit form, which is acceptable here.
Full details: Linked Issues check
-
Explanation
Issue [
#1310] requires returned 5xx responses to mark the worker server span as ERROR, seterror.typeto the status code, and avoid message text. It also requires returned 4xx responses to remain UNSET with test coverage.worker_otel.pyadds post-response handling for status codes at or above 500. The reported tests verify 500 and 503 ERROR spans, 404 and 409 UNSET spans, status-basederror.type, and no response-body text. Raised exceptions remain handled by the existing path.
Full details: Out of Scope Changes check
-
Explanation
The changed files are limited to the worker tracing implementation and its focused span test. The documentation update explains the changed middleware behavior. The test additions support issue [
#1310]. No unrelated product behavior or unrelated files are identified.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 2 files.
Full details: Description check
-
Explanation
The description follows the repository template closely: it has a 'What and why' section that links issue
#1310and explains the problem and fix, and a 'How it was verified' section with a named test file, a fail-without-fix check, and local test, ruff, and shape-check results. The AI assistance section is filled in and names the tool. Most checklist items are ticked. The gaps are that CI is explicitly not green and the 'Tests added or updated' box is ticked, but the author states the test was run locally and CI status is unconfirmed. The description is substantive and specific, so the check passes.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
The API's export rule (_failed_type in core/observability/otel.py) keeps an ERROR description only when it equals the span's error.type, and the worker's own _failed sets the description to the error's type. "HTTP 500" would be dropped there; "500" is kept. Found in review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dave Arthur <davidarthur@hotmail.com>
What and why
Closes #1310.
The problem. The workspace worker's server span (
_tracedinservices/workspace-worker/worker_otel.py) was markedERRORonly when a handler raised. The canvas routes (worker_routes_canvas.py) return their failures as a 500 instead of raising. So a failed canvas start, approval or stop left its spanUNSET, with onlyhttp.status_codeset. Tempo's error-rate queries and "traces with errors" filters missed those failures.The fix. A new
_answeredruns after the app answers:error.type= the status code andStatus(ERROR, "<status>"). That's how OpenTelemetry's HTTP semantic conventions mark a failed server span. The description equals theerror.type, as the worker's_faileddoes. The API's export rule (_failed_typeincore/observability/otel.py) keeps a description only when it equals theerror.type.UNSET, as before._failedrecords its type and re-raises.How it was verified
New test in
orchestrator/tests/test_prd256_o2b_worker_span.py. Routes that return 500 and 503 give anERRORspan witherror.typeset to the code. Routes that return 404 and 409 stayUNSETwith noerror.type. A path in the response body never reaches the span.It fails without the fix: with
worker_otel.pystashed,StatusCode.UNSET != StatusCode.ERROR.Review: a subagent review found no bugs. It flagged one nit, the description format, now matched to the API's rule.
Local runs (Python 3.11 venv from
orchestrator/requirements.lock+requirements-dev.lock):pytest tests/test_prd256_o2b_worker_span.py tests/test_prd256_o5_logs_metrics.py: 19 passed.ruff checkon both files: clean.scripts/ci/check_changed_code_shape.py: 0 errors.Tests added or updated, and they fail without this change
CI green (a lane marked non-required counts too)
Both editions considered: local (
AUTH_EDITION=local) and hosted. It's the worker's tracing, the same code in both.UI change: screenshots below (none)
AI assistance
Checklist (AGENTS.md)
git commit -s)orchestrator/config.py; no hardcoded values (none added)V2copies)orchestrator/reports/route-manifest.json(none)🤖 Generated with Claude Code