[opentelemetry-instrumentation-genai-langchain] Divert SystemMessage to gen_ai.system_instructions - #511
Conversation
`on_chat_model_start` used to fold every ``SystemMessage`` into ``gen_ai.input.messages`` as ``role="system"`` while leaving ``gen_ai.system_instructions`` empty, so the semconv system-instruction attribute never appeared on chat spans. Split the flattened message list once — system messages populate ``InferenceInvocation.system_instruction`` via a new ``split_system_and_input_messages`` helper, non-system messages pass through ``to_input_messages`` unchanged. ``SystemMessageChunk`` is handled via ``isinstance``.
There was a problem hiding this comment.
Pull request overview
This PR updates the LangChain GenAI instrumentation to route LangChain SystemMessage (and streaming SystemMessageChunk) content into the GenAI semconv gen_ai.system_instructions attribute instead of representing it as "role": "system" inside gen_ai.input.messages, aligning chat spans/logs with the intended semantic convention mapping.
Changes:
- Add
to_system_instruction()andsplit_system_and_input_messages()utilities to extract and partition system instructions from chat inputs. - Update
on_chat_model_startto populateInferenceInvocation.system_instructionseparately frominput_messages. - Extend/adjust unit + cassette-backed assertions to ensure system instructions are emitted via
gen_ai.system_instructionsand not as an input message role.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| instrumentation/opentelemetry-instrumentation-genai-langchain/src/opentelemetry/instrumentation/genai/langchain/utils.py | Adds system-instruction extraction + message splitting helper used by callbacks. |
| instrumentation/opentelemetry-instrumentation-genai-langchain/src/opentelemetry/instrumentation/genai/langchain/callback_handler.py | Switches chat-start handling to split system vs non-system messages and set system_instruction. |
| instrumentation/opentelemetry-instrumentation-genai-langchain/tests/test_llm_call.py | Adds/updates tests and assertions to validate system instruction diversion for spans and logs. |
| instrumentation/opentelemetry-instrumentation-genai-langchain/.changelog/511.fixed | Records the user-visible behavior fix in the package changelog fragments. |
Suppressed comments (1)
instrumentation/opentelemetry-instrumentation-genai-langchain/src/opentelemetry/instrumentation/genai/langchain/utils.py:247
split_system_and_input_messages()has the same iterator-consumption issue asto_system_instruction(): ifmessagesis a generator andconvert_to_messages(...)raises, the fallback list comprehension iterates an already-consumed iterable and drops everything.
try:
normalized: Iterable[BaseMessage] = convert_to_messages(list(messages))
except Exception: # pylint: disable=broad-except
normalized = [m for m in messages if isinstance(m, BaseMessage)]
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Pull request dashboard statusWaiting on the author · refreshed 2026-09-01 01:38 UTC Resolve merge conflicts. Respond to 3 review items (e.g. link a commit, explain why not, ask a follow-up): Status above doesn't look right?
|
Move ``list(messages)`` outside the try so the fallback in ``except`` still sees the input if ``convert_to_messages`` raises. Without this, a one-shot iterator would be consumed by ``list(messages)`` inside the try and the except branch would silently drop all content.
lmolkova
left a comment
There was a problem hiding this comment.
In addition to the inline suggestion:
In to_input_messages, materialized = list(messages) is still missing before the try block. If messages is a one-shot iterator and convert_to_messages raises, the except fallback will iterate over an already-exhausted iterable.
| def split_system_and_input_messages( | ||
| messages: Iterable[Any], | ||
| ) -> tuple[list[MessagePart], list[InputMessage]]: | ||
| """Split ``messages`` into ``system_instruction`` parts and ``InputMessage`` s.""" | ||
| materialized = list(messages) | ||
| try: | ||
| normalized: Iterable[BaseMessage] = convert_to_messages(materialized) | ||
| except Exception: # pylint: disable=broad-except | ||
| normalized = [m for m in materialized if isinstance(m, BaseMessage)] | ||
| system: list[BaseMessage] = [] | ||
| non_system: list[BaseMessage] = [] | ||
| for message in normalized: | ||
| if isinstance(message, SystemMessage): | ||
| system.append(message) | ||
| else: | ||
| non_system.append(message) | ||
| return to_system_instruction(system), to_input_messages(non_system) | ||
|
|
||
|
|
||
| def to_output_messages( | ||
| messages: Iterable[BaseMessage], |
There was a problem hiding this comment.
| def split_system_and_input_messages( | |
| messages: Iterable[Any], | |
| ) -> tuple[list[MessagePart], list[InputMessage]]: | |
| """Split ``messages`` into ``system_instruction`` parts and ``InputMessage`` s.""" | |
| materialized = list(messages) | |
| try: | |
| normalized: Iterable[BaseMessage] = convert_to_messages(materialized) | |
| except Exception: # pylint: disable=broad-except | |
| normalized = [m for m in materialized if isinstance(m, BaseMessage)] | |
| system: list[BaseMessage] = [] | |
| non_system: list[BaseMessage] = [] | |
| for message in normalized: | |
| if isinstance(message, SystemMessage): | |
| system.append(message) | |
| else: | |
| non_system.append(message) | |
| return to_system_instruction(system), to_input_messages(non_system) | |
| def to_output_messages( | |
| messages: Iterable[BaseMessage], | |
| def split_system_and_input_messages( | |
| messages: Iterable[Any], | |
| ) -> tuple[list[MessagePart], list[InputMessage]]: | |
| """Split ``messages`` into ``system_instruction`` parts and ``InputMessage`` s.""" | |
| materialized = list(messages) | |
| try: | |
| normalized: Iterable[BaseMessage] = convert_to_messages(materialized) | |
| except Exception: # pylint: disable=broad-except | |
| normalized = [m for m in materialized if isinstance(m, BaseMessage)] | |
| system_parts: list[MessagePart] = [] | |
| input_messages: list[InputMessage] = [] | |
| for message in normalized: | |
| if isinstance(message, SystemMessage): | |
| system_parts.extend(_content_to_parts(message.content)) | |
| else: | |
| parts = _message_parts(message) | |
| if parts: | |
| input_messages.append( | |
| InputMessage(role=_normalize_role(message), parts=parts) | |
| ) | |
| return system_parts, input_messages |
This processes the normalized messages in a single pass instead of splitting them and calling to_system_instruction and to_input_messages, which each re-run convert_to_messages and iterate the list again.
| def test_split_system_and_input_messages_diverts_system_instructions(): | ||
| system, inputs = split_system_and_input_messages( | ||
| [ | ||
| SystemMessage(content="You are helpful."), |
There was a problem hiding this comment.
could you please add system instructions to conformance tests? this is where we validate conformance to semantic conventions including value types
Description
on_chat_model_startused to fold everySystemMessageintogen_ai.input.messagesasrole="system"while leavinggen_ai.system_instructionsempty, so the semconv system-instruction attribute never appeared on chat spans. This adds asplit_system_and_input_messageshelper (plus ato_system_instructioncompanion toto_input_messages) that routesSystemMessage— includingSystemMessageChunkfor streaming — toInferenceInvocation.system_instruction, and lets everything else fall throughto_input_messagesunchanged.to_input_messagesitself is untouched.Type of change
How has this been tested?
New unit tests in
tests/test_llm_call.pycover the split helper directly (diversion, position-independent handling of interleaved system messages,SystemMessageChunk, short-hand tuple / dict inputs, no-system case) andto_system_instruction(non-system entries ignored, short-hand normalization). Existing OpenAI VCR-cassette tests were updated to assertrole="system"no longer appears ingen_ai.input.messagesand thatgen_ai.system_instructionscarries the content.langchainsuite: 25 passedChecklist