Skip to content
2 changes: 2 additions & 0 deletions ddtrace/llmobs/_constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,8 @@ class LLMOBS_STRUCT:
EXPECTED_OUTPUT: Final = "expected_output"
VALUE: Final = "value"
MESSAGES: Final = "messages"
AUDIO_PARTS: Final = "audio_parts"
IMAGE_PARTS: Final = "image_parts"
DOCUMENTS: Final = "documents"
AGENT_MANIFEST: Final = "agent_manifest"
SPAN: Final = "span"
Expand Down
29 changes: 27 additions & 2 deletions ddtrace/llmobs/_llmobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@
from ddtrace.llmobs._utils import _trace_id_to_wire
from ddtrace.llmobs._utils import _validate_prompt
from ddtrace.llmobs._utils import add_span_link
from ddtrace.llmobs._utils import collapse_messages_to_value
from ddtrace.llmobs._utils import enforce_message_role
from ddtrace.llmobs._utils import get_asyncio
from ddtrace.llmobs._utils import get_llmobs_ml_app
Expand All @@ -174,6 +175,7 @@
from ddtrace.llmobs._utils import resolve_llmobs_git_metadata
from ddtrace.llmobs._utils import resolve_ml_app
from ddtrace.llmobs._utils import safe_json
from ddtrace.llmobs._utils import span_kind_keeps_messages
from ddtrace.llmobs._writer import LLMObsAPIClient
from ddtrace.llmobs._writer import LLMObsEvalMetricWriter
from ddtrace.llmobs._writer import LLMObsExperimentsClient
Expand Down Expand Up @@ -424,7 +426,7 @@ def _build_llmobs_span(
llmobs_span.input = [Message(content=safe_json(input_value, ensure_ascii=False) or "", role="")]

input_messages = llmobs_input.get(LLMOBS_STRUCT.MESSAGES)
if span_kind == "llm" and input_messages is not None:
if input_messages is not None and span_kind_keeps_messages(span_kind, input_messages):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A later annotate() no longer overrides an earlier media annotation.

annotate(input_data=[media msg]) followed by annotate(input_data="corrected") emits the first one. This line flips input_type back to "messages" from the stale messages, and line 550 then regenerates value from them. Neither writer clears its sibling key (there is no pop of MESSAGES or VALUE anywhere in ddtrace/llmobs/), and meta.input is a read-modify-write struct that persists across annotate calls, so the two representations coexist. This breaks the override contract documented at line 2929, and a caller re-annotating specifically to redact leaves the original media payload in place.

Fix: in _annotate_llmobs_span_data, have the value path pop(LLMOBS_STRUCT.MESSAGES) and the messages path pop(LLMOBS_STRUCT.VALUE), so only one representation is ever live. Worth a test for the two-call override sequence in both orders, since the reverse order currently only works by accident.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed as suggested. _annotate_llmobs_span_data now pops the sibling key (MESSAGES or VALUE) on every write so only one representation remains live.

Added tests covering both re-annotation orders across all four span kinds.

input_type = "messages"
llmobs_span.input = enforce_message_role(input_messages)

Expand All @@ -439,7 +441,7 @@ def _build_llmobs_span(
llmobs_span.output = [Message(content=safe_json(output_value, ensure_ascii=False) or "", role="")]

output_messages = llmobs_output.get(LLMOBS_STRUCT.MESSAGES)
if span_kind == "llm" and output_messages is not None:
if output_messages is not None and span_kind_keeps_messages(span_kind, output_messages):
output_type = "messages"
llmobs_span.output = enforce_message_role(output_messages)

Expand Down Expand Up @@ -541,6 +543,11 @@ def _normalize_llmobs_meta(

if input_type == "messages":
meta_input[LLMOBS_STRUCT.MESSAGES] = llmobs_span.input
if span_kind != "llm":
# Non-LLM spans carry both: value for readers that only understand value, and
# messages for the typed media parts value cannot represent. Derived here, after
# the user span processor, so processor edits reach both fields.
meta_input[LLMOBS_STRUCT.VALUE] = collapse_messages_to_value(span_kind, llmobs_span.input)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicating the text halves the size headroom, and truncation then drops the media too.

messages and the derived value both carry the text, with no size guard. Measured on an agent span with a 4 MiB image part (the existing LLMOBS_IMAGE_INLINE_MAX_BYTES cap) against the 5,000,000 byte default:

text=390KB   before 4,594,247 ok          after 4,993,895 ok
text=400KB   before 4,604,487 ok          after 5,014,375 truncated
text=780KB   before 4,993,607 ok          after 5,792,615 truncated

Past the limit _truncate_span_event (_writer.py:1161) replaces meta.input and meta.output wholesale, so the field added to carry media is exactly what gets dropped. The budgets in _integrations/utils.py:401 do not cover this: they guard only integration capture paths, which are all kind="llm" and never duplicate.

Fix: skip the derived value (or emit a short marker in its place) when it would push the projected event size past config._llmobs_event_size_limit.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. collapse_messages_to_value now checks projected event sizes against _llmobs_event_size_limit and emits [value omitted: event size limit] if exceeded, preserving the typed media.

Added tests to verify the omission trigger and prevent false positives on standard payloads.

elif input_type == "value" and llmobs_span.input:
meta_input[LLMOBS_STRUCT.VALUE] = llmobs_span.input[0].get("content", "")
elif input_type == "documents":
Expand All @@ -552,6 +559,8 @@ def _normalize_llmobs_meta(

if output_type == "messages":
meta_output[LLMOBS_STRUCT.MESSAGES] = llmobs_span.output
if span_kind != "llm":
meta_output[LLMOBS_STRUCT.VALUE] = collapse_messages_to_value(span_kind, llmobs_span.output)
elif output_type == "value" and llmobs_span.output:
meta_output[LLMOBS_STRUCT.VALUE] = llmobs_span.output[0].get("content", "")
elif output_type == "documents":
Expand Down Expand Up @@ -3077,6 +3086,22 @@ def annotate(
)
elif span_kind == "experiment":
cls._tag_freeform_io(span, input_value=input_data, output_value=output_data)
elif span_kind_keeps_messages(span_kind, input_data) or span_kind_keeps_messages(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: this branch can raise into user application code.

messages_carry_media only checks that the media key holds a non-empty list, never that its entries are dicts. So image_parts: ["/tmp/a.png"], audio_parts: ["not-a-dict"], and a valid part sitting next to a non-string content all get diverted here, Messages() raises TypeError, and line 3117 converts it to LLMObsAnnotateSpanError. Pre-PR every one of those inputs went to _tag_text_io and could not raise.

The blast radius is decorators.py. _llmobs_decorator calls LLMObs.annotate at lines 237, 258, 261, 286, 316 and 319 with no try/except, and _automatic_io_annotation defaults to True. On the input side the annotate call precedes resp = func(*args, **kwargs), so the user's function body never runs.

Two fixes, both worth doing:

  1. In messages_carry_media, require the media list's elements to be dicts, so malformed payloads stay on the pre-PR _tag_text_io path. That matches the docstring's stated intent of leaving malformed input on whatever path it takes today.
  2. Wrap the six _llmobs_decorator annotate calls in try/except LLMObsAnnotateSpanError: log.debug(...), mirroring the guard _model_decorator already has at decorators.py:119.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. messages_carry_media now requires media elements to be dicts so non-dict payloads stay safely on the _tag_text_io path.

For the decorator calls, I added a _annotate_or_log helper to keep all six guards clean without repeating try/except blocks, and added test coverage.

span_kind, output_data
):
Comment thread
joizddog marked this conversation as resolved.
# Media-bearing sides route through the message tagger so the typed parts
# survive. Each side is decided on its own: a plain-string side keeps the
# value tagging it has today rather than being reshaped into a message.
media_input = input_data if span_kind_keeps_messages(span_kind, input_data) else None
media_output = output_data if span_kind_keeps_messages(span_kind, output_data) else None
annotation_error_message, error = cls._tag_llm_io(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding one image part silently erases the rest of a freeform payload.

Messages.__init__ rebuilds each message from a strict whitelist (content, role, tool_calls, tool_results, audio_parts, image_parts) and drops everything else with no log or error. Because collapse_messages_to_value derives value from those already-stripped messages, the dropped keys are gone from both fields:

input_data = {"user_id": 42, "query": "find me a hat", "image_parts": [<valid part>]}

before: value = '{"user_id": 42, "query": "find me a hat", "image_parts": [...]}'
after:  messages = [{"content": "", "role": "", "image_parts": [...]}]
        value    = ''

image_parts is a plausible key in arbitrary user data, so a caller who never intended media semantics gets a blank span, and telemetry.record_llmobs_annotate still records error=None.

Fix: requiring dict entries in messages_carry_media handles the malformed cases. For the valid-media case above, either carry unrecognized keys through Messages or log.warning when the strict path drops any, so the loss is not silent.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Malformed cases are safely routed to text by the dict-entry check, while valid media payloads now trigger a log.warning when unrecognized keys are dropped.

I also added an upgrade entry to the release note detailing this behavior change.

span, input_messages=media_input, output_messages=media_output
)
cls._tag_text_io(
span,
input_value=input_data if media_input is None else None,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A failed media parse now drops the whole side, and the new decorator guard makes it silent.

On the failure path media_input is not None, so input_value is passed as None here and nothing is recorded for that side. _tag_llm_io also returns early on input failure, so a valid output side is never processed either.

Verified on an agent span, input_data={"image_parts": [{"url": "u"}], "user_id": 5} with output_data="the answer":

meta = {'input': {}, 'output': {'value': '"the answer"'}}

# media on both sides, input malformed:
meta = {'input': {}, 'output': {}}

The emitted event has no meta.input at all. Under a decorator _annotate_or_log now swallows the exception, so this becomes total silent loss of span I/O where the payload was previously recorded as a value string.

Fix: on parse failure, fall back to the value path for that side instead of dropping it, and let _tag_llm_io still process the output side when the input side failed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Input and output sides are now tagged independently, and any side failing media parsing safely falls back to the value path instead of being silently dropped.

Verified that malformed inputs no longer suppress valid outputs, and added tests covering both single-side and double-side fallbacks.

output_value=output_data if media_output is None else None,
)
else:
cls._tag_text_io(span, input_value=input_data, output_value=output_data)
if _linked_spans and isinstance(_linked_spans, list):
Expand Down
90 changes: 86 additions & 4 deletions ddtrace/llmobs/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -795,10 +795,11 @@ def _annotate_llmobs_span_data(
llmobs_span_data[LLMOBS_STRUCT.SPAN_LINKS] = span_links
if config is not None:
llmobs_span_data[LLMOBS_STRUCT.CONFIG] = config
# Add I/O messages to messages field only for LLM spans, otherwise add to value field
is_llm = meta[LLMOBS_STRUCT.SPAN].get(LLMOBS_STRUCT.KIND) == "llm"
# Add I/O messages to the messages field for LLM spans, and for the non-LLM kinds that
# keep messages when a message carries media; every other case collapses to value.
annotated_span_kind = meta[LLMOBS_STRUCT.SPAN].get(LLMOBS_STRUCT.KIND)
if input_messages is not None:
if is_llm:
if span_kind_keeps_messages(annotated_span_kind, input_messages):
meta[LLMOBS_STRUCT.INPUT][LLMOBS_STRUCT.MESSAGES] = input_messages
else:
meta[LLMOBS_STRUCT.INPUT][LLMOBS_STRUCT.VALUE] = safe_json(input_messages, ensure_ascii=False) or ""
Expand All @@ -811,7 +812,7 @@ def _annotate_llmobs_span_data(
existing_prompt.update(cast(Prompt, prompt))
span._set_ctx_item(INPUT_PROMPT, existing_prompt)
if output_messages is not None:
if is_llm:
if span_kind_keeps_messages(annotated_span_kind, output_messages):
meta[LLMOBS_STRUCT.OUTPUT][LLMOBS_STRUCT.MESSAGES] = output_messages
else:
meta[LLMOBS_STRUCT.OUTPUT][LLMOBS_STRUCT.VALUE] = safe_json(output_messages, ensure_ascii=False) or ""
Expand Down Expand Up @@ -851,6 +852,87 @@ def enforce_message_role(messages: list[Message]) -> list[Message]:
return messages


# Non-LLM span kinds allowed to keep typed messages alongside the collapsed value string.
# Adding a kind here requires the serving API to populate messages for it first, otherwise the
# messages are dropped on read and the inline media has spent the per-event size budget for
# something that can never render. The serving API now builds messages for every kind below:
# agent has its own builder, and workflow / task / step / tool route through defaultSpanFromEvent,
# which populates them as of the non-LLM span builder change.
MEDIA_MESSAGE_SPAN_KINDS: frozenset = frozenset(("agent", "workflow", "task", "step", "tool"))

_SCALAR_VALUE_SPAN_KINDS: frozenset = frozenset(("agent", "workflow", "task", "step"))
_MEDIA_PART_KEYS = (LLMOBS_STRUCT.AUDIO_PARTS, LLMOBS_STRUCT.IMAGE_PARTS)


def messages_carry_media(messages: Any) -> bool:
"""True when any message carries a non-empty audio_parts or image_parts list.

Runs on the annotate path against unvalidated user input, so it tolerates any shape
rather than raising. A media key holding something other than a non-empty list does
not count, which leaves malformed input on whatever path it takes today.
"""
if isinstance(messages, dict):
messages = [messages]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A parameter named image_parts makes the decorator discard every other argument.

_get_span_inputs yields {argname: value}, and this predicate treats a bare dict as a single message, so the parameter name alone routes the whole argument map through Messages.

@agent
def render(image_parts, user_id, prompt): ...

render(image_parts=[{"mime_type": "image/png", "content": "AAAA"}], user_id=5, prompt="go")

Result: meta.input.messages keeps only the media, value collapses to "", and user_id and prompt are dropped. The span reads as having no input at all. Before this PR value held all three arguments.

Fix: force decorator-generated argument maps down the value path. They are a {param: value} mapping and never a message payload, so passing a flag through _annotate_or_log to skip the media routing would do it. Inferring message semantics from a key name is the underlying fragility here, and this is the case where it misfires on completely ordinary code.

Also flagged by the codex bot and by a third reviewer.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b3fc5a99. messages_carry_media now requires a dictionary to explicitly carry content or role before counting as media.

This keeps parameter maps like {image_parts: ..., user_id: ...} safely on the value path without discarding extra arguments.

if not isinstance(messages, list):
return False
for message in messages:
if not isinstance(message, dict):
continue
for key in _MEDIA_PART_KEYS:
parts = message.get(key)
if isinstance(parts, list) and parts:
return True
Comment thread
joizddog marked this conversation as resolved.
return False


def span_kind_keeps_messages(span_kind: Optional[str], messages: Any) -> bool:
"""True when message-shaped I/O should be stored as typed messages for this span kind.

LLM spans always keep messages. The other kinds keep them only when a message actually
carries media, mirroring the trace indexer, which retains messages on a non-LLM span
only for the typed parts the collapsed value string cannot represent.
"""
if span_kind == "llm":
return True
return span_kind in MEDIA_MESSAGE_SPAN_KINDS and messages_carry_media(messages)


def _strip_media_parts(messages: list[Message]) -> list[dict]:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Media parts bypass redaction span processors.

Stripping media here means value loses it while messages keeps it, and post-PR the processor receives the real typed message list instead of the old single collapsed Message. A processor that scrubs content therefore never touches the media. This repo's own documented idiom shows the effect: tests/llmobs/test_llmobs.py:177 does for message in span.input + span.output: message["content"] = "", which now ships messages: [{"content": "", "image_parts": [{...base64...}]}] while value reads "". The span looks scrubbed but is not.

LLMObsSpan.input is publicly typed list[Message] and Message declares image_parts, so a strictly correct processor should already handle this, but no existing processor expects media on an agent span. Suggest calling it out in the release note as a behavior change for processor authors, and adding one test that registers a processor against a media-bearing non-LLM span.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Added test_content_scrub_leaves_media_on_non_llm_span to assert how scrubbed spans behave when media is present.

Updated the release note with an upgrade entry advising processor authors to explicitly pop image_parts and audio_parts keys to scrub media.

"""Drop the media part lists so base64 payloads never reach the value string."""
return [{k: v for k, v in message.items() if k not in _MEDIA_PART_KEYS} for message in messages]


def _messages_have_tool_structure(messages: list[dict]) -> bool:
"""True when any message carries tool_calls or tool_results.

Plain-text rendering cannot represent tool structure without dropping it, so its
presence forces the JSON form of the value string.
"""
return any("tool_calls" in message or "tool_results" in message for message in messages)


def collapse_messages_to_value(span_kind: str, messages: list[Message]) -> str:
"""Render the scalar value string for a non-LLM span that also carries typed messages.

Mirrors nonLLMValueString in the trace indexer so a natively instrumented span and an
OTel one read back the same way. Agent, workflow, task and step render a lone
plain-text, non-system message as readable text; anything else keeps the JSON form,
which preserves roles, turns and tool structure. Media parts are stripped either way,
so the value stays small and the payload lives only on messages.
"""
if not messages:
return ""
stripped = _strip_media_parts(messages)
if (
span_kind in _SCALAR_VALUE_SPAN_KINDS
and len(stripped) == 1
and not _messages_have_tool_structure(stripped)
and stripped[0].get("role") != "system"
):
return stripped[0].get("content", "") or ""
return safe_json(stripped, ensure_ascii=False) or ""


def validate_tags_list(tags: list[str]) -> None:
if not isinstance(tags, list):
raise TypeError("Tags must be a list of strings")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
fixes:
- |
LLM Observability: Fixes an issue where images and audio annotated on non-LLM spans were lost.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two behavior changes are missing from this note, and the annotate docstring is now stale.

"Spans without media, and non-message input, are unchanged" reads as a no-behavior-change guarantee, but for agent/workflow/task/step/tool this PR also (a) drops non-whitelisted keys from message-shaped payloads and (b) makes LLMObs.annotate able to raise. Both are worth stating, along with the span-processor shape change.

Separately, per AGENTS.md rule 11 ("Update docs when changing internal or public APIs"), LLMObs.annotate's docstring still reads - other: any JSON serializable type. for input_data (_llmobs.py:2962) and output_data (:2975), and scopes all image_parts/audio_parts documentation to llm spans. That line is now false for the five widened kinds.

Minor: the PR title is feat(...) but the note is filed under fixes:.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed all three. Updated the release note with features: and upgrade: sections, removed the outdated guarantee sentence, and corrected the annotate docstrings for non-LLM spans.

Messages annotated via ``LLMObs.annotate`` on an ``agent``, ``workflow``, ``task``, ``tool`` or
``step`` span that carry ``image_parts`` or ``audio_parts`` are now recorded as structured
messages alongside the span's input and output values, so the media is collected and rendered
instead of being flattened into text. Spans without media, and non-message input, are unchanged.
Loading
Loading