feat(llmobs): add annotateAgentManifest manual API to LLMObs SDK - #12318
feat(llmobs): add annotateAgentManifest manual API to LLMObs SDK#12318yahya-mouman wants to merge 7 commits into
Conversation
- Add AgentManifest immutable value class with Builder pattern in LLMObs.java - Add AgentTool immutable value class in LLMObs.java - Add AGENT_MANIFEST constant to LLMObsTags - Add annotateAgentManifest() default method to LLMObsSpan interface - Add comprehensive builder tests for AgentManifest and AgentTool Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Stores agent manifest fields (name, instructions, model, model_settings, tools) as an internal tag `_ml_obs_tag.agent_manifest`. Only applies to agent spans; warns and no-ops on other span kinds. Null manifest is silently ignored. Tools with null/empty names are skipped with a warning. A second call overwrites the previous manifest. Framework field "AgentObs SDK" is always added when any manifest fields are present. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add AGENT_MANIFEST_KEY byte constant, include the tag in TAGS_FOR_REMAPPING, and handle it in the meta serialization loop as a msgpack map. Add two tests: one verifying all manifest fields appear in meta, one verifying the tag does not leak into the tags list. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5f56f11d6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
ncybul
left a comment
There was a problem hiding this comment.
A couple minor suggestions but overall looks good to me! Would be nice to see a manual verification of setting the manifest via the SDK.
| * | ||
| * <p>Build via {@link AgentManifest#builder()} and pass to {@link | ||
| * LLMObsSpan#annotateAgentManifest(AgentManifest)}. Only applied on agent spans; ignored on other | ||
| * span kinds. A subsequent call on the same span overwrites the previous manifest. |
There was a problem hiding this comment.
This is different from the Python implementation, right? I think in Python, we were merging the two manifests together whenever possible. It's probably best that we align the implementations and choose one approach.
Personally, I would lean towards merging the fields whenever possible.
There was a problem hiding this comment.
Yes we can do that. It'll help the async distributed trace cases too,
There was a problem hiding this comment.
Switched to merge semantics in 71284a3. The second call now overlays non-null/non-empty fields on top of the existing manifest, name/instructions/model use new-value-wins with fallback, model_settings shallow-merges (new keys win on conflicts), and tools replace only when the new call provides a non-empty list.
There was a problem hiding this comment.
i think the docstring still needs to be updated here from overwrites → merges
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
There was a problem hiding this comment.
More details
The default API method keeps compatibility. The implementation accepts manifests only for agent spans and writes them to meta.agent_manifest.
🤖 Datadog Autotest · Commit b5f56f1 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
…test hardening - Change annotateAgentManifest to merge successive calls instead of overwriting: name/instructions/model use new-value-wins logic with fallbacks; model_settings shallow-merges; tools replace only when caller provides a non-empty list. - Change MANUAL_FRAMEWORK constant from "AgentObs SDK" to "manual". - Update all test assertions that expected "AgentObs SDK" to "manual". - Replace overwrite test with merge-semantics test; add model_settings merge test. - Tests for post-finish no-op and empty-tools-list were already present. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sabrenner
left a comment
There was a problem hiding this comment.
one main comment a couple nits otherwise lgtm!
| } | ||
| } | ||
| // instructions | ||
| if (manifest.getInstructions() != null && !manifest.getInstructions().isEmpty()) { |
There was a problem hiding this comment.
nit - can we follow the same pattern above of getting
String manifestInstructions = manifet.getInstructions();so that we only do access once? same thing with the other gets below
| } | ||
|
|
||
| /** A tool declared in an agent manifest. */ | ||
| public static final class AgentTool { |
There was a problem hiding this comment.
can we re-use the existing ToolDefinition class? I know it has schema vs description, etc., but wondering if we would be able to do that to consolidate types so the API wasn't as packed
There was a problem hiding this comment.
It would mean we would need to add code in the processor to accept this. As I understand the processor expects that tool format in the manifest. Is it needed enough to require us to propagate the change ?
| base.put("model", manifest.getModel()); | ||
| } | ||
| // model_settings: shallow merge | ||
| if (manifest.getModelSettings() != null && !manifest.getModelSettings().isEmpty()) { |
There was a problem hiding this comment.
should we have an allowlist for model settings here? could we potentially be adding sensitive information?
There was a problem hiding this comment.
I thought about adding an allowlist however I don't want to block users from adding any non supported configs. like agent related settings example : retry_number or framework specific agentic settings
…domain/DDLLMObsSpan.java Co-authored-by: Sam Brenner <106700075+sabrenner@users.noreply.github.com>
…nce; update docstring Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Adds
annotateAgentManifest(LLMObs.AgentManifest)to the Java LLMObs SDK, allowing users to manually declare an agent span's configuration (name, instructions, model, model_settings, tools). Mirrors the Python implementation (DataDog/dd-trace-py#19771) and follows theannotatePromptpattern from #12161.Changes
LLMObsTags.java— addsAGENT_MANIFEST = "agent_manifest"constantLLMObs.java— adds two public immutable builder classes:LLMObs.AgentTool— represents a single tool (name, optionaldescription, optionalparameters)LLMObs.AgentManifest— builder withname,instructions,model,model_settings,toolsLLMObsSpan.java— addsdefault void annotateAgentManifest(LLMObs.AgentManifest)(no-op default for backwards compat)NoOpLLMObsSpan.java— explicit@Overrideno-opDDLLMObsSpan.java— real implementation: validates span kind (agent only), builds manifest map, stores as_ml_obs_tag.agent_manifestLLMObsSpanMapper.java— addsagent_manifesttoTAGS_FOR_REMAPPING; serializes tometa.agent_manifestas a msgpack mapBehaviour
agentspan kind; other span kinds emit a log warning and no-opframeworkis set to"AgentObs SDK"automatically by the SDKnamedefaults to the span name if not providedmodel_settingskeys are forwarded as-is (no allowlist in this initial PR)Test plan
./gradlew :dd-trace-api:test --tests "datadog.trace.api.llmobs.LLMObsTest"— 27 tests pass (4 new)./gradlew :dd-java-agent:agent-llmobs:test --tests "datadog.trace.llmobs.domain.DDLLMObsSpanTest"— 39 tests pass (7 new)./gradlew :dd-trace-core:test --tests "datadog.trace.llmobs.writer.ddintake.LLMObsSpanMapperTest"— 18 tests pass (2 new)🤖 Generated with Claude Code