Add request_id field to Activity - #591
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Normal TurnContext sends and agent-to-agent posts still drop request_id, leaving propagation incomplete.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds internal request_id correlation across activities, conversation references, and HTTP ingress.
Changes:
- Adds and propagates
request_idfields. - Generates UUIDs for incoming HTTP activities.
- Adds propagation tests.
File summaries
| File | Description |
|---|---|
activity.py |
Adds Activity request ID support. |
conversation_reference.py |
Adds continuation propagation. |
_http_adapter_base.py |
Generates request IDs. |
test_activity.py |
Tests Activity propagation. |
test_conversation_reference.py |
Tests continuation propagation. |
test_http_adapter_telemetry.py |
Updates telemetry test imports. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
Normal TurnContext reply paths still drop request_id, and generation lacks a focused test.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
tests/hosting_core/telemetry/test_http_adapter_telemetry.py:17
- Neither
uuidnorActivityis referenced anywhere in this test module, so these new imports trigger unused-import lint failures and should be removed unless the missing request-ID assertion is added.
libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py:310
- This updates only
Activity.apply_conversation_reference, but normal replies and updates go through the separateTurnContext.apply_conversation_referenceimplementation (turn_context.py:346-374), which does not copyreference.request_id. As a result,context.send_activity()produces an outgoing activity with a missing request ID, so this propagation is bypassed on the primary send path. Update the TurnContext helper as well (or delegate to the Activity implementation) and cover that path with a test.
self.request_id = reference.request_id
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_http_adapter_base.py:120
- The new UUID assignment is not covered by the added telemetry tests: none asserts that the activity passed to
process_activityreceives a valid per-request ID. The newly addeduuidandActivityimports intest_http_adapter_telemetry.pyare also unused, which suggests the intended assertion is missing; add a focused test that captures the processed activity and validates itsrequest_id.
activity.request_id = str(uuid4())
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
Derived activity helpers drop request_id, and HTTP request-ID generation lacks regression coverage.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py:202
- Adding the field here does not propagate it through
Activity.create_reply()andActivity.create_trace(): both helpers construct a newActivityfromselfbut never passself.request_id. A caller that uses either helper directly (or beforeTurnContext.apply_conversation_reference) loses the correlation ID, so the new field is not preserved across these Activity transformations. Pass the ID into both derived activities and cover it.
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_http_adapter_base.py:120
- The new UUID assignment has no regression test. The updated HTTP adapter telemetry module only adds unused
uuidandActivityimports, so the suite would still pass ifrequest_idwere never assigned or were reused. Capture the activity passed toprocess_activityand assert a valid, distinct UUID for separate requests.
activity.request_id = str(uuid4())
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate findings remain around request-ID generation, serialization, and HTTP-channel propagation coverage.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (5)
libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py:202
Activity's public fields are documented in the class docstring, but the newrequest_idfield has no corresponding:param:/:type:entry. Document that it is an internal request identifier and explain its wire-serialization behavior so the generated API documentation does not omit the new field's contract.
request_id: str | None = Field(None, exclude=True)
libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py:202
- The
exclude=Truechoice is an important wire-format contract, but no updated serialization test setsrequest_idand verifies that top-levelrequestIdis omitted fromActivity.model_dump(...). Add that assertion so a future model/serializer change cannot accidentally expose every generated request ID in channel payloads.
request_id: str | None = Field(None, exclude=True)
libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py:56
ConversationReference's class docstring documents the existing fields throughservice_url, but it does not document the newrequest_idfield. Add its parameter and type documentation so consumers of this public model understand how the propagated identifier is intended to be used.
request_id: str | None = None
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_http_adapter_base.py:120
- The new UUID generation is not covered by the updated telemetry tests: they only assert spans/metrics, and the
Activityreceived byprocess_activityis never inspected. Add a test that captures that activity and verifiesrequest_idis a UUID (and that separate requests get distinct IDs), otherwise this tracing contract can regress unnoticed.
activity.request_id = str(uuid4())
tests/hosting_core/telemetry/test_http_adapter_telemetry.py:10
- The newly added
uuidandActivityimports are unused anywhere in this test module. Please either use them in the request-ID test or remove them; as written they add lint noise and indicate the new HTTP-path behavior is not covered.
import uuid
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
This pull request introduces support for propagating a
request_idfield throughout the activity and conversation reference lifecycle, ensuring that requests can be traced end-to-end. Therequest_idis now generated for each incoming HTTP request, attached toActivityobjects, and properly propagated throughConversationReferenceand related methods. Comprehensive tests have been added to verify the correct handling and propagation ofrequest_id.Key changes include:
Request ID Propagation:
request_idfield to both theActivityandConversationReferencemodels, allowing the request ID to be stored and transferred between these objects. [1] [2]apply_conversation_referenceandget_conversation_referencemethods inActivityto propagate therequest_idfield when converting betweenActivityandConversationReference. [1] [2]get_continuation_activityinConversationReferencealso propagates therequest_idto the newActivityinstance.Request ID Generation:
request_idis generated usinguuid4()for each incoming request and assigned to the correspondingActivity. [1] [2]Testing Enhancements:
request_idis correctly propagated throughActivityandConversationReferencemethods, and that it defaults toNonewhen not provided. [1] [2] [3] [4] [5]request_idhandling.