mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: Fix: Add system_instructions to ChatClient LLM span tracing (#3164)
* Fix: Add system_instructions to ChatClient LLM span tracing - Add system_instructions parameter to _capture_messages() calls in _trace_get_response() and _trace_get_streaming_response() - Extract instructions from chat_options in kwargs - Add unit tests to verify system_instructions are captured correctly When using ChatClient with ChatOptions.instructions, the OpenTelemetry LLM span was missing system messages in gen_ai.input.messages and the gen_ai.system_instructions attribute was not being set. This fix aligns the ChatClient-level tracing with the Agent-level tracing which already correctly passes system_instructions. Fixes #3163 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add edge case tests for system_instructions - Add test for empty string instructions (should not set attribute) - Add test for list-type instructions (verify multiple items captured) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Simplify: use options.get('instructions') directly instead of kwargs.get('chat_options') Addresses reviewer feedback: - Removed unnecessary chat_options variable from kwargs - Directly access instructions from the options parameter - Updated tests to use dict syntax for options (TypedDict convention) --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
3ec881509c
commit
f8c84d4ee6
@@ -1096,7 +1096,12 @@ def _trace_get_response(
|
||||
)
|
||||
with _get_span(attributes=attributes, span_name_attribute=SpanAttributes.LLM_REQUEST_MODEL) as span:
|
||||
if OBSERVABILITY_SETTINGS.SENSITIVE_DATA_ENABLED and messages:
|
||||
_capture_messages(span=span, provider_name=provider_name, messages=messages)
|
||||
_capture_messages(
|
||||
span=span,
|
||||
provider_name=provider_name,
|
||||
messages=messages,
|
||||
system_instructions=options.get("instructions"),
|
||||
)
|
||||
start_time_stamp = perf_counter()
|
||||
end_time_stamp: float | None = None
|
||||
try:
|
||||
@@ -1189,6 +1194,7 @@ def _trace_get_streaming_response(
|
||||
span=span,
|
||||
provider_name=provider_name,
|
||||
messages=messages,
|
||||
system_instructions=options.get("instructions"),
|
||||
)
|
||||
start_time_stamp = perf_counter()
|
||||
end_time_stamp: float | None = None
|
||||
|
||||
@@ -279,6 +279,133 @@ async def test_chat_client_streaming_observability(
|
||||
assert span.attributes[OtelAttr.OUTPUT_MESSAGES] is not None
|
||||
|
||||
|
||||
@pytest.mark.parametrize("enable_sensitive_data", [True], indirect=True)
|
||||
async def test_chat_client_observability_with_instructions(
|
||||
mock_chat_client, span_exporter: InMemorySpanExporter, enable_sensitive_data
|
||||
):
|
||||
"""Test that system_instructions from options are captured in LLM span."""
|
||||
import json
|
||||
|
||||
client = use_instrumentation(mock_chat_client)()
|
||||
|
||||
messages = [ChatMessage(role=Role.USER, text="Test message")]
|
||||
options = {"model_id": "Test", "instructions": "You are a helpful assistant."}
|
||||
span_exporter.clear()
|
||||
response = await client.get_response(messages=messages, options=options)
|
||||
|
||||
assert response is not None
|
||||
spans = span_exporter.get_finished_spans()
|
||||
assert len(spans) == 1
|
||||
span = spans[0]
|
||||
|
||||
# Verify system_instructions attribute is set
|
||||
assert OtelAttr.SYSTEM_INSTRUCTIONS in span.attributes
|
||||
system_instructions = json.loads(span.attributes[OtelAttr.SYSTEM_INSTRUCTIONS])
|
||||
assert len(system_instructions) == 1
|
||||
assert system_instructions[0]["content"] == "You are a helpful assistant."
|
||||
|
||||
# Verify input_messages contains system message
|
||||
input_messages = json.loads(span.attributes[OtelAttr.INPUT_MESSAGES])
|
||||
assert any(msg.get("role") == "system" for msg in input_messages)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("enable_sensitive_data", [True], indirect=True)
|
||||
async def test_chat_client_streaming_observability_with_instructions(
|
||||
mock_chat_client, span_exporter: InMemorySpanExporter, enable_sensitive_data
|
||||
):
|
||||
"""Test streaming telemetry captures system_instructions from options."""
|
||||
import json
|
||||
|
||||
client = use_instrumentation(mock_chat_client)()
|
||||
messages = [ChatMessage(role=Role.USER, text="Test")]
|
||||
options = {"model_id": "Test", "instructions": "You are a helpful assistant."}
|
||||
span_exporter.clear()
|
||||
|
||||
updates = []
|
||||
async for update in client.get_streaming_response(messages=messages, options=options):
|
||||
updates.append(update)
|
||||
|
||||
assert len(updates) == 2
|
||||
spans = span_exporter.get_finished_spans()
|
||||
assert len(spans) == 1
|
||||
span = spans[0]
|
||||
|
||||
# Verify system_instructions attribute is set
|
||||
assert OtelAttr.SYSTEM_INSTRUCTIONS in span.attributes
|
||||
system_instructions = json.loads(span.attributes[OtelAttr.SYSTEM_INSTRUCTIONS])
|
||||
assert len(system_instructions) == 1
|
||||
assert system_instructions[0]["content"] == "You are a helpful assistant."
|
||||
|
||||
|
||||
@pytest.mark.parametrize("enable_sensitive_data", [True], indirect=True)
|
||||
async def test_chat_client_observability_without_instructions(
|
||||
mock_chat_client, span_exporter: InMemorySpanExporter, enable_sensitive_data
|
||||
):
|
||||
"""Test that system_instructions attribute is not set when instructions are not provided."""
|
||||
client = use_instrumentation(mock_chat_client)()
|
||||
|
||||
messages = [ChatMessage(role=Role.USER, text="Test message")]
|
||||
options = {"model_id": "Test"} # No instructions
|
||||
span_exporter.clear()
|
||||
response = await client.get_response(messages=messages, options=options)
|
||||
|
||||
assert response is not None
|
||||
spans = span_exporter.get_finished_spans()
|
||||
assert len(spans) == 1
|
||||
span = spans[0]
|
||||
|
||||
# Verify system_instructions attribute is NOT set
|
||||
assert OtelAttr.SYSTEM_INSTRUCTIONS not in span.attributes
|
||||
|
||||
|
||||
@pytest.mark.parametrize("enable_sensitive_data", [True], indirect=True)
|
||||
async def test_chat_client_observability_with_empty_instructions(
|
||||
mock_chat_client, span_exporter: InMemorySpanExporter, enable_sensitive_data
|
||||
):
|
||||
"""Test that system_instructions attribute is not set when instructions is an empty string."""
|
||||
client = use_instrumentation(mock_chat_client)()
|
||||
|
||||
messages = [ChatMessage(role=Role.USER, text="Test message")]
|
||||
options = {"model_id": "Test", "instructions": ""} # Empty string
|
||||
span_exporter.clear()
|
||||
response = await client.get_response(messages=messages, options=options)
|
||||
|
||||
assert response is not None
|
||||
spans = span_exporter.get_finished_spans()
|
||||
assert len(spans) == 1
|
||||
span = spans[0]
|
||||
|
||||
# Empty string should not set system_instructions
|
||||
assert OtelAttr.SYSTEM_INSTRUCTIONS not in span.attributes
|
||||
|
||||
|
||||
@pytest.mark.parametrize("enable_sensitive_data", [True], indirect=True)
|
||||
async def test_chat_client_observability_with_list_instructions(
|
||||
mock_chat_client, span_exporter: InMemorySpanExporter, enable_sensitive_data
|
||||
):
|
||||
"""Test that list-type instructions are correctly captured."""
|
||||
import json
|
||||
|
||||
client = use_instrumentation(mock_chat_client)()
|
||||
|
||||
messages = [ChatMessage(role=Role.USER, text="Test message")]
|
||||
options = {"model_id": "Test", "instructions": ["Instruction 1", "Instruction 2"]}
|
||||
span_exporter.clear()
|
||||
response = await client.get_response(messages=messages, options=options)
|
||||
|
||||
assert response is not None
|
||||
spans = span_exporter.get_finished_spans()
|
||||
assert len(spans) == 1
|
||||
span = spans[0]
|
||||
|
||||
# Verify system_instructions attribute contains both instructions
|
||||
assert OtelAttr.SYSTEM_INSTRUCTIONS in span.attributes
|
||||
system_instructions = json.loads(span.attributes[OtelAttr.SYSTEM_INSTRUCTIONS])
|
||||
assert len(system_instructions) == 2
|
||||
assert system_instructions[0]["content"] == "Instruction 1"
|
||||
assert system_instructions[1]["content"] == "Instruction 2"
|
||||
|
||||
|
||||
async def test_chat_client_without_model_id_observability(mock_chat_client, span_exporter: InMemorySpanExporter):
|
||||
"""Test telemetry shouldn't fail when the model_id is not provided for unknown reason."""
|
||||
client = use_instrumentation(mock_chat_client)()
|
||||
|
||||
Reference in New Issue
Block a user