refactor(tracing)!: drop the Agentex span processor and own the Span model in lib - #517
mohammadatallah-scale wants to merge 7 commits into
Conversation
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…he Span model in lib The Agentex server is retiring its Postgres-backed /spans API, so the processor that wrote to it goes, along with the Trace helpers that read spans back through the client. The SGP processor is the only sink. The in-memory Span handed to processors was the generated client type for that API. It now lives in agentex.lib.types.tracing and is still exported from agentex.lib.core.tracing, so it survives the generated surface disappearing when the server spec drops the routes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
P2 |
…, drop the tracer's dead client Review follow-ups on the span-processor removal: - Span keeps unknown keys like the generated model it replaces, so a custom processor's own attributes and older worker payloads survive. - set_processor_configs re-enters add_processor_config under the manager lock, which deadlocked on a plain Lock. Reentrant now, with tests for the SGP happy path and the batch registration. - Trace and Tracer no longer use the client they receive, so it is optional and the ADK tracing module stops building an httpx client per event loop. - Changelog entry for the breaking removals, stale prose updated. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Landed in 62b97be: changelog entry under the Unreleased breaking changes, SGP happy-path and batch-registration tests, the stale prose, the optional client (the ADK module no longer builds an httpx client per event loop) and the blank lines. Left as documented rather than shimmed: the generated base's to_dict / to_json defaults and model_construct coercion, named in the changelog entry. |
xsfa
left a comment
There was a problem hiding this comment.
P2 (non-blocking, cross-PR): open #512 adds from agentex.lib.types.tracing import SGPTracingProcessorConfig, AgentexTracingProcessorConfig and add_tracing_processor_config(AgentexTracingProcessorConfig()) to all 16 agentex init templates, while this head deletes that name (src/agentex/lib/types/tracing.py now ends at TracingProcessorConfig = SGPTracingProcessorConfig, line 49) and add_processor_config raises ValueError for any non-sgp type (tracing_processor_manager.py:32-37). The two PRs share no files, so whichever merges second is textually clean and leaves every scaffolded agent failing at import, and neither suite catches it: tests/lib/cli/test_init_templates.py:61 only ast.parses the rendered templates and #512's new test only string-matches them. #512's motivation (developer-UI traces tab) is served by scale-agentex#430's SGP-backed reader, so close it as superseded or rebase it to SGP-only, and reference it from this PR body.
Reviewed exact head 62b97be on base 0db6037: full 31-file diff (git diff --find-renames 0db6037e...62b97be6) plus CLAUDE.md, CONTRIBUTING.md, SECURITY.md and .cursor/rules/* at both SHAs; git grep AgentexTracingProcessorConfig 62b97be6 -> CHANGELOG.md only; on a git archive 62b97be6 tree, pytest tests/lib/core/tracing/test_span_model.py (2 passed), test_tracing_processor_manager.py and the tracing/adk consumer set (55 passed under a scale-gp-beta>=0.5.0 error-class shim; 199 passed across tests/lib/core/tracing), red/green for both new tests against d7d8720 and against copies with extra="allow" / RLock reverted (extras dropped; set_processor_configs hung); temporalio.contrib.pydantic.pydantic_data_converter round trip of a generated-Span EndSpanParams payload into the lib Span and back (tz-aware datetimes, __error__ data and an unknown key preserved); micro-benchmark of generated vs lib Span at ~5 KB and 1 MB widths (1.00x construct/copy/dump/validate, identical JSON bytes).
- The PR body still carries the
[!IMPORTANT] Pending Mohammad's self-reviewcallout; the self-review landed in 62b97be. - Optional hardening: have
test_init_templatesresolve each renderedfrom agentex.lib... import Xviaimportlibso a template naming a removed public symbol fails CI instead of only parsing. - Companion scale-agentex#430 (drops the Postgres spans API and table) states it is blocked on this PR landing first; both bodies agree on that order.
Parsing the rendered templates lets one keep naming a symbol the SDK removed, so every scaffolded agent fails on its first start instead of in CI. Each `from agentex... import X` now has to resolve. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@xsfa Thanks. P2 #512: commented there that it registers the processor this PR removes and that #430's platform reader serves the traces tab, so it needs an SGP-only rebase or closing. P3 Landed the hardening in 4a3c3f0: every |
xsfa
left a comment
There was a problem hiding this comment.
Reviewed exact head 4a3c3f0 on base 0db6037: full 32-file diff (git diff --find-renames 0db6037e...4a3c3f08, +199/-691) plus CLAUDE.md, CONTRIBUTING.md, SECURITY.md and .cursor/rules/* at both SHAs (none changed by the PR); git grep -E 'AgentexTracingProcessorConfig|agentex\.types\.span|get_span\(|list_spans\(' 4a3c3f08 outside generated code -> CHANGELOG.md only; on git archive 4a3c3f08 trees: pytest tests/lib/core/tracing/test_span_model.py (2 passed; red with extra="allow" removed), tests/lib/core/tracing/test_tracing_processor_manager.py (3 passed; red with RLock -> Lock: set_processor_configs hung), tests/lib/cli/test_init_templates.py (45 passed, all 19 import-resolution cases; red on a template importing AgentexTracingProcessorConfig), the tracing/adk consumer set (119 passed under a scale-gp-beta>=0.5.0 error-class shim), generated-vs-lib Span field/JSON-schema parity and a pydantic_data_converter round trip of an EndSpanParams payload with an unknown key preserved; head+#512 merged tree: 16 of this head's import-resolution cases fail as intended while #512's own tests stay green. CI push run 34869509934 is green on this SHA.
examples/tutorials/10_async/10_temporal/020_state_machine/project/state_machines/deep_research.py:6now importsSpanfromagentex.lib.types.tracing, which no published SDK ships (the 0.26.0 release merged tomaintoday at a71fa67 predates this PR) while the tutorial depends on unpinned PyPIagentex-sdkand its Dockerfile runsuv pip install --system .[dev]; tutorial CI passes only becauserun_agent_test.sh --build-cliinjects the locally built wheel.from agentex.lib.core.tracing import Span(in__all__at base, head and every release) resolves everywhere; self-heals once this PR ships.tests/lib/cli/test_init_templates.py:72:hasattr(importlib.import_module(module), name)is False forfrom agentex.lib import adkin a fresh process (a subpackage becomes an attribute of its parent only once imported), so the guard currently passes through import order:test_all_templates_import_existing_agentex_symbols[default-openai-agents]fails when it is the first case in a fresh process on this machine, while the whole file passes. Falling back toimportlib.import_module(f"{module}.{name}")onImportErrormirrorsfrom pkg import namesemantics; false-red only, never false-green.- Template prose still describes the removed processor:
default-pydantic-ai/project/acp.py.j2:40-42,sync-pydantic-ai/project/acp.py.j2:34-36,temporal-langgraph/project/workflow.py.j2:47-48andtemporal-pydantic-ai/project/workflow.py.j2:38-39say spans also reach the AgentEx backend via a default Agentex processor, and sevenREADME.md.j2files list "Tracing integration to SGP / AgentEx"; after this PR a scaffold withoutSGP_API_KEY/SGP_ACCOUNT_IDregisters no processor and exports nothing.
55a0178 to
a71fa67
Compare
|
Closing. The Agentex team owns this removal and will sequence it against the customers still on the legacy tracing path. The branch stays as a working reference: routes and table retired, sidebar reading from the platform, span model owned by the library. |
Important
Pending Mohammad's self-review. This note is removed by a human, not automation.
The problem. The Agentex server is retiring its Postgres spans API (scaleapi/scale-agentex#430). The SDK still ships the processor that writes to it and two trace helpers that read it back. The in-memory span handed to processors is that API's generated model, so it vanishes when the spec drops the routes.
The fix. A breaking change, surfaces below.
agentex.types.span.Span(generated)agentex.lib.core.tracing.Span, lib-ownedAgentexTracingProcessorConfigand theagentexprocessorTrace.get_spanandTrace.list_spans, sync and asyncclient.spanscreate, list, retrieve, update (generated)Not in this PR. #512 adds the removed processor to every scaffold template, so it needs an SGP rebase or closing. The template import test here catches it.
Test plan
The PR appears safe to merge.
Summary
Diagram
sequenceDiagram participant Caller participant Trace participant SpanQueue participant SGP Caller->>Trace: start_span(name, data) Trace->>Trace: create library-owned Span alt sync tracing Trace->>SGP: on_span_start(Span) else async tracing Trace->>SpanQueue: enqueue START copy SpanQueue->>SGP: on_span_start(Span) end Caller->>Trace: end_span(Span) Trace->>Trace: set end_time and serialize fields alt sync tracing Trace->>SGP: on_span_end(Span) else async tracing Trace->>SpanQueue: enqueue END copy SpanQueue->>SGP: on_span_end(Span) endReviews (3) · Last reviewed commit: "test(cli): resolve every template import..."