Skip to content

Commit 73d2bcf

Browse files
fix(obs): wire the Temporal worker, unblock the bridge, bound the drain
Addresses all three blocking review comments on #518. Each was real and each was a silent failure — the wiring reported success while producing nothing. 1. The Temporal worker was never wired (comment 1) init_sgp_obs() was called only from BaseACPServer.__init__, and a Temporal agent runs its model calls in a separate AgentexWorker process that never constructs one. So an agent that installed sgp-obs and set the whole documented environment still got no metrics, traces or structured logs from the process doing the interesting work. That is the shape compass-sleep-agent has, and it is the one the pilot never covered. AgentexWorker.run() now inits at entry and drains in a finally. No `app=`: there is no ASGI application in that process — the health-check server is aiohttp, which sgp-obs' ASGI middleware does not apply to — so the worker contributes model and egress telemetry but no http.server.*, which is correct since nothing there serves agent traffic. 2. The bridge was installed into a tracing system that was switched off (comment 2) The openai-agents scaffolds call set_tracing_disabled(True). That stops spans being produced at all, so the bridge from the previous commit registered itself and sat idle forever, returning True. Measured against a spy processor: set_tracing_disabled(True) -> processors ['BatchTraceProcessor', 'Spy'], spy saw 0 set_trace_processors([]) -> processors ['Spy'], spy saw 1 Note the first row. Disabling tracing leaves the OpenAI exporter REGISTERED and merely never feeds it; clearing the list actually removes it. So the replacement is strictly better at what the original comment said it wanted — keeping traces away from api.openai.com with what may be a proxy key — while also letting the bridge work. Changed in the 4 openai-agents scaffolds. Existing agents already carry the old line, and a scaffold change cannot reach them, so init_sgp_obs also warns when the bridge is installed while tracing is disabled, naming the one-line fix. Reads a private attribute (`_disabled`), fully guarded: if upstream renames it we stop warning rather than break. 3. The sync-processor drain could eat the whole grace period (comment 3) SGPSyncTracingProcessor.shutdown() calls flush_queue(), a blocking HTTP flush with retries, and the drain awaited it inline with no limit. A slow or unreachable collector would stall the lifespan loop and stop shutdown_sgp_obs() ever running — trading a few business spans for all of the OTel ones. Now each processor runs in a worker thread under a deadline SHARED across all of them, so N stalled processors cost one budget, not N. Tested: three processors each blocking 2s complete the drain in under 1s against a 0.25s budget, and a heartbeat task keeps ticking throughout, proving the loop is not blocked. A timed-out flush leaks its thread until the process exits; that is accepted and documented, since this runs only during shutdown and the alternative is blocking on it. The drain moved from base_acp_server to tracing_processor_manager, next to the processors it drains, because both entry points now need it. 117 tests, ruff clean, pyright 0 errors, all with sgp-obs absent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 45e4ecc commit 73d2bcf

11 files changed

Lines changed: 336 additions & 68 deletions

File tree

‎src/agentex/lib/cli/templates/default-openai-agents/project/acp.py.j2‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ from dotenv import load_dotenv
2020

2121
load_dotenv()
2222

23-
from agents import Agent, Runner, function_tool, set_tracing_disabled
23+
from agents import Agent, Runner, function_tool, set_trace_processors
2424

2525
from agentex.lib import adk
2626
from agentex.lib.types.acp import SendEventParams, CancelTaskParams, CreateTaskParams
@@ -34,10 +34,17 @@ from agentex.lib.core.harness.emitter import UnifiedEmitter
3434
from agentex.lib.adk import OpenAITurn
3535
from agentex.lib.core.tracing.tracing_processor_manager import add_tracing_processor_config
3636

37-
# Disable the openai-agents SDK's native tracer so it doesn't ship traces to
38-
# api.openai.com using OPENAI_API_KEY (which may be a LiteLLM proxy key).
39-
# SGP tracing below still runs via the Agentex tracing manager.
40-
set_tracing_disabled(True)
37+
# Drop the openai-agents SDK's own exporter, so it can't ship traces to
38+
# api.openai.com using OPENAI_API_KEY (which may be a gateway/proxy key and would 401).
39+
#
40+
# Clearing the processor list rather than disabling tracing outright: disabling stops
41+
# spans being produced AT ALL, which silently starves any processor added later —
42+
# including the sgp-obs bridge the SDK installs when observability is on, so a Runner
43+
# turn would contribute no model spans. Clearing instead removes the OpenAI exporter
44+
# (which otherwise stays registered and is merely never fed) while leaving the
45+
# machinery alive for the bridge to attach to.
46+
# Agentex/SGP tracing still runs via the tracing manager.
47+
set_trace_processors([])
4148

4249
logger = make_logger(__name__)
4350

‎src/agentex/lib/cli/templates/sync-openai-agents-local-sandbox/project/agent.py.j2‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ from __future__ import annotations
1515

1616
from datetime import datetime
1717

18-
from agents import Runner, set_tracing_disabled
18+
from agents import Runner, set_trace_processors
1919
from agents.sandbox import SandboxAgent, SandboxRunConfig
2020
from agents.run_config import RunConfig
2121
from agents.sandbox.sandboxes.unix_local import (
@@ -25,10 +25,17 @@ from agents.sandbox.sandboxes.unix_local import (
2525

2626
from project.tools import get_capabilities
2727

28-
# Disable the openai-agents SDK's native tracer so it doesn't ship traces to
29-
# api.openai.com using OPENAI_API_KEY (which may be a gateway/proxy key and would
30-
# 401). Agentex tracing still runs via the tracing manager configured in acp.py.
31-
set_tracing_disabled(True)
28+
# Drop the openai-agents SDK's own exporter, so it can't ship traces to
29+
# api.openai.com using OPENAI_API_KEY (which may be a gateway/proxy key and would 401).
30+
#
31+
# Clearing the processor list rather than disabling tracing outright: disabling stops
32+
# spans being produced AT ALL, which silently starves any processor added later —
33+
# including the sgp-obs bridge the SDK installs when observability is on, so a Runner
34+
# turn would contribute no model spans. Clearing instead removes the OpenAI exporter
35+
# (which otherwise stays registered and is merely never fed) while leaving the
36+
# machinery alive for the bridge to attach to.
37+
# Agentex/SGP tracing still runs via the tracing manager.
38+
set_trace_processors([])
3239

3340
MODEL_NAME = "gpt-4o-mini"
3441
INSTRUCTIONS = """You are a local sandbox assistant.

‎src/agentex/lib/cli/templates/sync-openai-agents/project/acp.py.j2‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,19 @@ from agentex.types.task_message_update import TaskMessageUpdate, StreamTaskMessa
1313
from agentex.types.task_message_content import TaskMessageContent
1414
from agentex.types.text_content import TextContent
1515
from agentex.lib.utils.logging import make_logger
16-
from agents import Agent, Runner, RunConfig, function_tool, set_tracing_disabled
17-
18-
# Disable the openai-agents SDK's native tracer so it doesn't ship traces to
19-
# api.openai.com using OPENAI_API_KEY (which may be a LiteLLM proxy key).
20-
# SGP tracing below still runs via the Agentex tracing manager.
21-
set_tracing_disabled(True)
16+
from agents import Agent, Runner, RunConfig, function_tool, set_trace_processors
17+
18+
# Drop the openai-agents SDK's own exporter, so it can't ship traces to
19+
# api.openai.com using OPENAI_API_KEY (which may be a gateway/proxy key and would 401).
20+
#
21+
# Clearing the processor list rather than disabling tracing outright: disabling stops
22+
# spans being produced AT ALL, which silently starves any processor added later —
23+
# including the sgp-obs bridge the SDK installs when observability is on, so a Runner
24+
# turn would contribute no model spans. Clearing instead removes the OpenAI exporter
25+
# (which otherwise stays registered and is merely never fed) while leaving the
26+
# machinery alive for the bridge to attach to.
27+
# Agentex/SGP tracing still runs via the tracing manager.
28+
set_trace_processors([])
2229

2330

2431
logger = make_logger(__name__)

‎src/agentex/lib/cli/templates/temporal-openai-agents/project/workflow.py.j2‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,19 @@ from agentex.lib.core.temporal.types.workflow import SignalName
1010
from agentex.lib.utils.logging import make_logger
1111
from agentex.types.text_content import TextContent
1212
from agentex.lib.environment_variables import EnvironmentVariables
13-
from agents import Agent, Runner, set_tracing_disabled
14-
15-
# Disable the openai-agents SDK's native tracer so it doesn't ship traces to
16-
# api.openai.com using OPENAI_API_KEY (which may be a LiteLLM proxy key).
17-
# SGP tracing below still runs via the Agentex tracing manager.
18-
set_tracing_disabled(True)
13+
from agents import Agent, Runner, set_trace_processors
14+
15+
# Drop the openai-agents SDK's own exporter, so it can't ship traces to
16+
# api.openai.com using OPENAI_API_KEY (which may be a gateway/proxy key and would 401).
17+
#
18+
# Clearing the processor list rather than disabling tracing outright: disabling stops
19+
# spans being produced AT ALL, which silently starves any processor added later —
20+
# including the sgp-obs bridge the SDK installs when observability is on, so a Runner
21+
# turn would contribute no model spans. Clearing instead removes the OpenAI exporter
22+
# (which otherwise stays registered and is merely never fed) while leaving the
23+
# machinery alive for the bridge to attach to.
24+
# Agentex/SGP tracing still runs via the tracing manager.
25+
set_trace_processors([])
1926

2027
from agentex.lib.core.temporal.plugins.openai_agents.hooks.hooks import TemporalStreamingHooks
2128
from pydantic import BaseModel

‎src/agentex/lib/cli/tests/__init__.py‎

Whitespace-only changes.
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
"""The openai-agents scaffolds must not disable tracing outright.
2+
3+
`set_tracing_disabled(True)` stops openai-agents producing spans AT ALL, which
4+
silently starves any processor registered later — including the sgp-obs bridge the SDK
5+
installs when observability is on. The bridge still reports itself installed, so a
6+
Runner turn contributes no model spans and nothing says why.
7+
8+
Measured against a spy processor:
9+
10+
set_tracing_disabled(True) -> processors ['BatchTraceProcessor', 'Spy'], spy saw 0
11+
set_trace_processors([]) -> processors ['Spy'], spy saw 1
12+
13+
Note the first row: disabling tracing leaves the OpenAI exporter REGISTERED, merely
14+
never fed. Clearing the list actually removes it, so the replacement is strictly better
15+
at the thing the original was trying to do — keep traces away from api.openai.com.
16+
"""
17+
18+
from __future__ import annotations
19+
20+
from pathlib import Path
21+
22+
import pytest
23+
24+
TEMPLATES = Path(__file__).resolve().parents[1] / "templates"
25+
26+
27+
def _templates_using_agents_tracing() -> list[Path]:
28+
return sorted(
29+
p for p in TEMPLATES.rglob("*.j2") if "set_trace_processors" in p.read_text()
30+
)
31+
32+
33+
def test_some_templates_were_found():
34+
"""Guards the glob itself: if the templates move, the assertions below would
35+
vacuously pass on an empty list."""
36+
assert _templates_using_agents_tracing(), f"no templates found under {TEMPLATES}"
37+
38+
39+
@pytest.mark.parametrize(
40+
"template", _templates_using_agents_tracing(), ids=lambda p: p.parent.parent.name
41+
)
42+
class TestOpenAIAgentsScaffolds:
43+
def test_does_not_disable_tracing(self, template: Path):
44+
text = template.read_text()
45+
assert "set_tracing_disabled(" not in text, (
46+
f"{template} disables openai-agents tracing, which starves the sgp-obs bridge"
47+
)
48+
49+
def test_clears_the_processor_list_instead(self, template: Path):
50+
assert "set_trace_processors([])" in template.read_text()
51+
52+
def test_imports_what_it_calls(self, template: Path):
53+
text = template.read_text()
54+
assert "set_trace_processors" in text.split("\n\n")[0] or any(
55+
"import" in line and "set_trace_processors" in line
56+
for line in text.splitlines()
57+
), f"{template} calls set_trace_processors without importing it"

‎src/agentex/lib/core/observability/sgp_obs_setup.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ def _install_openai_agents_bridge() -> bool:
176176
installed = bool(install_openai_agents_bridge())
177177
if installed:
178178
logger.debug("sgp-obs openai-agents bridge installed")
179+
_warn_if_openai_agents_tracing_disabled()
179180
else:
180181
# Only reachable if `agents` is not importable, which should not happen
181182
# while openai-agents is a hard dependency — so say so rather than shrug.
@@ -189,6 +190,37 @@ def _install_openai_agents_bridge() -> bool:
189190
return False
190191

191192

193+
def _warn_if_openai_agents_tracing_disabled() -> None:
194+
"""Warn when the bridge is installed but openai-agents tracing is switched off.
195+
196+
``install_openai_agents_bridge()`` returns True as soon as it registers itself as a
197+
trace processor — it cannot tell whether the provider will ever feed it. If the
198+
agent called ``set_tracing_disabled(True)``, no spans are produced at all, so the
199+
bridge is registered and permanently idle, and nothing says so.
200+
201+
That is not hypothetical: it is what the openai-agents scaffolds used to do, so
202+
agents generated before this change carry it. Those scaffolds now clear the
203+
processor list instead, which removes the OpenAI exporter (the thing they were
204+
actually trying to avoid) while leaving spans flowing to the bridge.
205+
206+
Reads a private attribute, so it is fully guarded: a diagnostic must never be the
207+
reason startup fails, and if upstream renames it we simply stop warning.
208+
"""
209+
try:
210+
from agents.tracing import get_trace_provider
211+
212+
if getattr(get_trace_provider(), "_disabled", False):
213+
logger.warning(
214+
"The sgp-obs openai-agents bridge is installed but openai-agents "
215+
"tracing is disabled, so Runner turns will produce no model spans. "
216+
"Replace set_tracing_disabled(True) with set_trace_processors([]): "
217+
"that still stops traces reaching api.openai.com, but keeps spans "
218+
"flowing to the bridge."
219+
)
220+
except Exception: # pragma: no cover - a diagnostic must never break startup
221+
logger.debug("could not determine openai-agents tracing state", exc_info=True)
222+
223+
192224
def _warn_if_correlation_backend_mismatched() -> None:
193225
"""Warn when sgp-obs is exporting OTel traces but the SDK's business-span
194226
correlation is still reading ddtrace.

‎src/agentex/lib/core/temporal/workers/worker.py‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
from agentex.lib.core.tracing.temporal import temporal_tracing_interceptors
3333
from agentex.lib.environment_variables import EnvironmentVariables
3434
from agentex.lib.core.compat.version_guard import assert_backend_compatible
35+
from agentex.lib.core.observability.sgp_obs_setup import init_sgp_obs, shutdown_sgp_obs
36+
from agentex.lib.core.tracing.tracing_processor_manager import shutdown_sync_tracing_processors
3537

3638
logger = make_logger(__name__)
3739

@@ -220,6 +222,18 @@ async def run(
220222
workflow: type | None = None,
221223
workflows: list[type] | None = None,
222224
):
225+
# A Temporal agent runs its model calls HERE, in a separate process from the
226+
# ACP server, and this process never constructs a BaseACPServer — so without
227+
# this call an agent that installed sgp-obs and set the documented environment
228+
# would still get no metrics, traces or structured logs from its worker, which
229+
# is where the interesting work happens.
230+
#
231+
# No `app=`: there is no ASGI application in this process. The health-check
232+
# server is aiohttp, which sgp-obs' ASGI middleware does not apply to, so the
233+
# worker contributes model and egress telemetry but no http.server.* — correct,
234+
# since nothing here serves agent traffic.
235+
init_sgp_obs()
236+
223237
await self.start_health_check_server()
224238
await self._register_agent()
225239

@@ -265,7 +279,14 @@ async def run(
265279
# Eagerly set the worker status to healthy
266280
self.healthy = True
267281
logger.info(f"Running workers for task queue: {self.task_queue}")
268-
await worker.run()
282+
try:
283+
await worker.run()
284+
finally:
285+
# Same drains as the ACP lifespan, for the same reason: whatever is still
286+
# queued when the pod stops is otherwise dropped. Both are bounded and
287+
# fail-open, so neither can stop the worker exiting.
288+
await shutdown_sync_tracing_processors()
289+
await shutdown_sgp_obs()
269290

270291
async def _health_check(self):
271292
return web.json_response(self.healthy)

‎src/agentex/lib/core/tracing/tracing_processor_manager.py‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
from __future__ import annotations
22

3+
import asyncio
4+
import logging
35
from typing import TYPE_CHECKING
46
from threading import Lock
57

@@ -78,3 +80,72 @@ def get_sync_tracing_processors():
7880

7981
def get_async_tracing_processors():
8082
return GLOBAL_TRACING_PROCESSOR_MANAGER.get_async_processors()
83+
84+
85+
_logger = logging.getLogger(__name__)
86+
87+
# Total wall-clock budget for draining every sync tracing processor. A pod's
88+
# terminationGracePeriodSeconds (30s by default) is shared with the OTel flush that
89+
# follows this, so the drain takes a small slice of it.
90+
SYNC_TRACING_SHUTDOWN_BUDGET_S = 5.0
91+
92+
93+
async def shutdown_sync_tracing_processors(
94+
budget_s: float = SYNC_TRACING_SHUTDOWN_BUDGET_S,
95+
) -> None:
96+
"""Drain the sync tracing processors' queues at shutdown. Never raises.
97+
98+
Nothing used to call this. The ACP lifespan drained ``shutdown_default_span_queue``,
99+
which is the ASYNC path only, so a sync agent dropped whatever business spans were
100+
still queued when the pod stopped. That matters beyond the lost spans: the business
101+
span is what an obs span's ``agentex.business_trace_id`` resolves to, so losing it
102+
breaks the pivot from Tempo back to the SGP store.
103+
104+
Off the event loop and on a deadline, both deliberately.
105+
``SGPSyncTracingProcessor.shutdown`` calls ``flush_queue()``, a BLOCKING HTTP flush
106+
with retries. Calling it inline would stall the caller's loop, so a slow or
107+
unreachable collector could burn the whole termination grace period and stop the
108+
OTel flush that runs after it — trading a few business spans for all of the OTel
109+
ones. Each processor runs in a worker thread, and the budget is shared across all
110+
of them so one stalled export cannot starve the rest.
111+
112+
A timed-out flush leaks its thread until the process exits. Accepted: this runs
113+
only during shutdown, and the alternative is blocking on it.
114+
115+
Lives here rather than in the ACP server because both entry points need it — the
116+
ACP server AND the Temporal worker, which runs in its own process.
117+
"""
118+
try:
119+
processors = get_sync_tracing_processors()
120+
except Exception: # pragma: no cover - nothing to drain
121+
_logger.debug("sync tracing processors unavailable at shutdown", exc_info=True)
122+
return
123+
124+
loop = asyncio.get_running_loop()
125+
deadline = loop.time() + budget_s
126+
127+
for processor in processors:
128+
remaining = deadline - loop.time()
129+
if remaining <= 0:
130+
_logger.warning(
131+
"sync tracing shutdown budget of %.1fs exhausted; %s and any after it "
132+
"were not flushed and their business spans are lost",
133+
budget_s,
134+
type(processor).__name__,
135+
)
136+
break
137+
try:
138+
await asyncio.wait_for(asyncio.to_thread(processor.shutdown), remaining)
139+
except (TimeoutError, asyncio.TimeoutError):
140+
_logger.warning(
141+
"%s did not flush within the remaining %.1fs; its business spans are "
142+
"lost, but shutdown continues",
143+
type(processor).__name__,
144+
remaining,
145+
)
146+
except Exception: # noqa: PERF203 - one bad processor must not block the rest
147+
_logger.warning(
148+
"a sync tracing processor failed to flush on shutdown; "
149+
"some business spans may be lost",
150+
exc_info=True,
151+
)

0 commit comments

Comments
 (0)