-
Notifications
You must be signed in to change notification settings - Fork 542
feat(llmobs): carry typed messages with media on non-LLM span kinds #19805
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
0c112c1
a83d226
7208a1c
63d95a7
dcc30aa
b3fc5a9
dcfcdc4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
|
@@ -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 | ||
|
|
@@ -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): | ||
| input_type = "messages" | ||
| llmobs_span.input = enforce_message_role(input_messages) | ||
|
|
||
|
|
@@ -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) | ||
|
|
||
|
|
@@ -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) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Past the limit Fix: skip the derived
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. 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": | ||
|
|
@@ -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": | ||
|
|
@@ -2950,6 +2959,10 @@ def annotate( | |
| "mime_type" and one of "content" (base64-encoded image) or "attachment_key". | ||
| - embedding spans: accepts a string, list of strings, or a dictionary of form | ||
| {"text": "...", ...} or a list of dictionaries with the same signature. | ||
| - agent, workflow, task, step and tool spans: any JSON serializable type. A message-shaped | ||
| payload whose messages carry "image_parts" or "audio_parts" is recorded | ||
| as typed messages alongside the collapsed value string, and keys outside | ||
| the message schema described above are dropped. | ||
| - other: any JSON serializable type. | ||
| :param output_data: A single output string, dictionary, or a list of dictionaries based on the span kind: | ||
| - llm spans: accepts a string, or a dictionary of form {"content": "...", "role": "...", | ||
|
|
@@ -2963,6 +2976,10 @@ def annotate( | |
| - retrieval spans: a dictionary containing any of the key value pairs | ||
| {"name": str, "id": str, "text": str, "score": float}, | ||
| or a list of dictionaries with the same signature. | ||
| - agent, workflow, task, step and tool spans: any JSON serializable type. A message-shaped | ||
| payload whose messages carry "image_parts" or "audio_parts" is recorded | ||
| as typed messages alongside the collapsed value string, and keys outside | ||
| the message schema described above are dropped. | ||
| - other: any JSON serializable type. | ||
| :param metadata: Dictionary of JSON serializable key-value metadata pairs relevant to the input/output operation | ||
| described by the LLMObs span. | ||
|
|
@@ -3077,6 +3094,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( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking: this branch can raise into user application code.
The blast radius is Two fixes, both worth doing:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. For the decorator calls, I added a |
||
| span_kind, output_data | ||
| ): | ||
|
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( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding one image part silently erases the rest of a freeform payload.
Fix: requiring dict entries in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Verified on an agent span, The emitted event has no Fix: on parse failure, fall back to the value path for that side instead of dropping it, and let
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -795,14 +795,21 @@ 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.input persists across annotate calls, and the emit path prefers messages | ||
| # over value. Clearing the sibling keeps a later annotate authoritative rather | ||
| # than leaving a stale representation to win. | ||
| meta[LLMOBS_STRUCT.INPUT].pop(LLMOBS_STRUCT.VALUE, None) | ||
| meta[LLMOBS_STRUCT.INPUT][LLMOBS_STRUCT.MESSAGES] = input_messages | ||
| else: | ||
| meta[LLMOBS_STRUCT.INPUT].pop(LLMOBS_STRUCT.MESSAGES, None) | ||
| meta[LLMOBS_STRUCT.INPUT][LLMOBS_STRUCT.VALUE] = safe_json(input_messages, ensure_ascii=False) or "" | ||
| if input_value is not None: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Scope this pop to non-LLM span kinds. The pop is unconditional on span kind, which reverses the messages-wins precedence The reachable case is an integration writing The same widening applies on llm spans (verified), though I found no in-tree caller that hits it today, so that half is latent rather than active. Fix: gate the pop on
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. Sibling key pops are now strictly gated on non-LLM span kinds so standard LLM spans retain their message-precedence rules. Added test coverage ensuring user-annotated media on LLM spans survives subsequent integration value writes. |
||
| meta[LLMOBS_STRUCT.INPUT].pop(LLMOBS_STRUCT.MESSAGES, None) | ||
| meta[LLMOBS_STRUCT.INPUT][LLMOBS_STRUCT.VALUE] = safe_json(input_value, ensure_ascii=False) or "" | ||
| if input_documents is not None: | ||
| meta[LLMOBS_STRUCT.INPUT][LLMOBS_STRUCT.DOCUMENTS] = input_documents | ||
|
|
@@ -811,11 +818,14 @@ 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].pop(LLMOBS_STRUCT.VALUE, None) | ||
| meta[LLMOBS_STRUCT.OUTPUT][LLMOBS_STRUCT.MESSAGES] = output_messages | ||
| else: | ||
| meta[LLMOBS_STRUCT.OUTPUT].pop(LLMOBS_STRUCT.MESSAGES, None) | ||
| meta[LLMOBS_STRUCT.OUTPUT][LLMOBS_STRUCT.VALUE] = safe_json(output_messages, ensure_ascii=False) or "" | ||
| if output_value is not None: | ||
| meta[LLMOBS_STRUCT.OUTPUT].pop(LLMOBS_STRUCT.MESSAGES, None) | ||
| meta[LLMOBS_STRUCT.OUTPUT][LLMOBS_STRUCT.VALUE] = safe_json(output_value, ensure_ascii=False) or "" | ||
| if output_documents is not None: | ||
| meta[LLMOBS_STRUCT.OUTPUT][LLMOBS_STRUCT.DOCUMENTS] = output_documents | ||
|
|
@@ -851,6 +861,116 @@ 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 anything other than a non-empty list of dicts | ||
| does not count, which leaves malformed input on whatever path it takes today: routing it | ||
| here instead would hand it to Messages(), which raises TypeError on a non-dict part, and | ||
| annotate turns that into an LLMObsAnnotateSpanError in the caller's own code. | ||
| """ | ||
| if isinstance(messages, dict): | ||
| messages = [messages] | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A parameter named
@agent
def render(image_parts, user_id, prompt): ...
render(image_parts=[{"mime_type": "image/png", "content": "AAAA"}], user_id=5, prompt="go")Result: Fix: force decorator-generated argument maps down the value path. They are a Also flagged by the codex bot and by a third reviewer.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in This keeps parameter maps like |
||
| 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 and all(isinstance(part, dict) for part in parts): | ||
| return True | ||
|
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]: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Media parts bypass redaction span processors. Stripping media here means
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. Added Updated the release note with an upgrade entry advising processor authors to explicitly pop |
||
| """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] | ||
|
|
||
|
|
||
| _VALUE_OMITTED_MARKER = "[value omitted: event size limit]" | ||
|
|
||
|
|
||
| def _media_payload_chars(messages: list[Message]) -> int: | ||
| """Characters the inline media parts contribute, without serializing the whole list.""" | ||
| total = 0 | ||
| for message in messages: | ||
| if not isinstance(message, dict): | ||
| continue | ||
| for key in _MEDIA_PART_KEYS: | ||
| parts = message.get(key) | ||
| if not isinstance(parts, list): | ||
| continue | ||
| for part in parts: | ||
| if isinstance(part, dict): | ||
| total += len(part.get("content", "") or "") | ||
| return total | ||
|
|
||
|
|
||
| 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" | ||
| ): | ||
| value = stripped[0].get("content", "") or "" | ||
| else: | ||
| value = safe_json(stripped, ensure_ascii=False) or "" | ||
| # The messages already carry this text, so the derived value duplicates it and halves the | ||
| # headroom the media needs. Past the limit the writer replaces input and output wholesale | ||
| # (_writer._truncate_span_event), discarding the very media this field exists to accompany, | ||
| # so drop the duplicate instead and keep the payload. | ||
| if _media_payload_chars(messages) + 2 * len(value) > config._llmobs_event_size_limit: | ||
| return _VALUE_OMITTED_MARKER | ||
| return value | ||
|
|
||
|
|
||
| def validate_tags_list(tags: list[str]) -> None: | ||
| if not isinstance(tags, list): | ||
| raise TypeError("Tags must be a list of strings") | ||
|
|
||
There was a problem hiding this comment.
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 byannotate(input_data="corrected")emits the first one. This line flipsinput_typeback to"messages"from the stale messages, and line 550 then regeneratesvaluefrom them. Neither writer clears its sibling key (there is nopopofMESSAGESorVALUEanywhere inddtrace/llmobs/), andmeta.inputis 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 pathpop(LLMOBS_STRUCT.MESSAGES)and the messages pathpop(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.There was a problem hiding this comment.
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_datanow pops the sibling key (MESSAGESorVALUE) on every write so only one representation remains live.Added tests covering both re-annotation orders across all four span kinds.