feat(telemetry): emit system prompt on chat spans per GenAI semconv#1818
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
Assessment: Approve (pending minor fix) This PR properly implements system prompt emission on chat spans per OTel GenAI semantic conventions, addressing #822. The implementation correctly handles both legacy and experimental convention modes while maintaining backward compatibility. Review Summary
The falsy check issue lizradway noted on line 839 should be addressed before merge. Otherwise, this looks ready to go! 🎉 |
957d83e to
e5ffedf
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Follow-up Review: Tests are passing ✅ I verified the latest commit ( The concern about This PR is ready to merge from my perspective. |
Description
System prompts provided separately from chat history are currently missing from
chatspans, which means downstream OpenTelemetry consumers cannot reconstruct the full prompt context for individual model calls.This PR is the model/chat-span follow-up to the earlier agent-span discussion in #1452 and PR #1455. The direct motivation here is the unmet chat-span need called out in #822.
This change updates
start_model_invoke_spanto accept optionalsystem_promptandsystem_prompt_contentarguments and emits semconv-compliant system prompt data for both supported GenAI convention modes:gen_ai.system.messagegen_ai_latest_experimental:gen_ai.system_instructionsongen_ai.client.inference.operation.detailsThis is backward compatible. Callers that omit the new arguments keep the existing behavior.
Related Issues
Documentation PR
N/A
Type of Change
New feature
Testing
How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli
hatch run preparehatch test tests/strands/telemetry/test_tracer.py tests/strands/event_loop/test_event_loop.pyAWS_DEFAULT_REGION=us-west-2 hatch run test-integlocally; the suite is broadly red in this environment across unrelated areas, so I did not use it as the primary signal for this patchChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.