Add opt-in AgentPond tracing for simulation LLM calls - #17
Add opt-in AgentPond tracing for simulation LLM calls#17marcusschiesser wants to merge 2 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1e44f0f0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } catch (error) { | ||
| const exception = | ||
| error instanceof Error ? error : new Error(String(error)); | ||
| span.recordException(exception); |
There was a problem hiding this comment.
When an LLM provider returns a non-2xx response, sendRequest includes its entire response body in the thrown error, and recordException(exception) persists that message and stack in AgentPond. Providers may echo prompt fragments or generated content in error bodies, so this violates the documented guarantee that tracing does not record prompts or responses; export only a sanitized error category/status instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0062ca6. Error spans now export only the fixed category LLMOperationError and status message LLM operation failed; the original provider error still propagates to application code but is never recorded on the span. The E2E fixture returns an error body containing a private prompt and asserts that neither the prompt nor echoed body exists in the raw Files SDK object. AgentPond read back sanitized error trace fe6ce0e9b0df0f9b1289deb82399e503.
| process.once("beforeExit", () => { | ||
| void shutdownAgentPond(); | ||
| }); |
There was a problem hiding this comment.
For long-running deployments stopped with SIGTERM or SIGINT (such as containers and service managers), Node exits without emitting beforeExit, so shutdownAgentPond() is never called and any spans still queued by BatchSpanProcessor are lost. Register shutdown handling at the server's actual signal lifecycle boundary and await it before exiting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0062ca6. AgentPond now registers one-shot SIGINT/SIGTERM handlers, awaits SDK shutdown, logs any flush failure, and then re-sends the original signal so the normal signal exit status is preserved. A production-module signal check exited with SIGTERM status 143 and wrote the queued trace object before termination.
| assert.equal(requests.length, 1); | ||
| assert.equal(requests[0].url, "/v1/chat/completions"); | ||
| assert.equal(requests[0].authorization, "Bearer test-key"); | ||
| assert.equal(requests[0].body.model, "worldx-e2e-model"); |
There was a problem hiding this comment.
This test only flushes the processor and then checks the mock LLM request; it never reads the configured filesystem store or asserts that a span was exported. If exporter initialization or asynchronous export fails, tracing falls back or the batch processor swallows the export failure and every current assertion still passes, so the test does not verify the behavior named by the test.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0062ca6. The E2E test now recursively reads the configured filesystem store (excluding adapter metadata), parses every exported batch, and asserts the two expected spans, trace IDs, model/token attributes, sanitized error status, and absence of prompts/provider bodies. The current AgentPond CLI then synced 1 object / 4 events and read back success trace 0e0258c91dab0a6aa1c17964dada6ccd plus sanitized error trace fe6ce0e9b0df0f9b1289deb82399e503.
|
Closing this because I submitted the AgentPond integration before confirming it was wanted. I’m switching to a permission-first approach and don’t want to leave unsolicited review work in your queue. Sorry for the noise; I’ll only revisit if a maintainer explicitly invites it. |
1 participant
What changed
LLMClient.call()pathnpx agentpond initThis fits WorldX's OpenAI-compatible simulation client without changing its request, retry, structured-output, or cost-tracking behavior.
Configuration and privacy
Tracing is disabled unless
AGENTPOND_ENABLED=true. Storage is selected entirely through Files SDK environment variables, so no credentials or backend details are committed. Spans contain the OpenInference kind, model, WorldX task type, status/errors, and token counts. They intentionally omit prompts, responses, API keys, character identifiers, and tool payloads.The filesystem adapter documented in the README is for local verification; production deployments should choose a persistent Files SDK provider.
Validation
Installed all declared workspaces with Node 22.22.1:
Checks:
All passed. The E2E test ran the production
LLMClient.call()against a local OpenAI-compatible HTTP server, verified the/v1/chat/completionsrequest and parsed application result, then flushed the real AgentPond exporter.Trace read-back:
The trace contained one
generation-createobservation namedworldx.llm.call, modelworldx-e2e-model, taskagentpond-e2e, and token counts 9 prompt / 4 completion / 13 total. Its input and output fields were both null as intended.Existing npm audit findings were left unchanged; this PR does not weaken or skip any test.