WIP | Add support for input and output scopes #113
Conversation
nikhilNava
changed the title
There was a problem hiding this comment.
Pull request overview
This pull request introduces observability support for tracking input and output message flows in agent conversations. The changes add OpenTelemetry-based tracing capabilities through new scope classes and middleware that automatically capture and log message exchanges.
Changes:
- Added InputScope and OutputScope classes for OpenTelemetry tracing of message flows
- Created MessageLoggingMiddleware to intercept and log user and bot messages
- Introduced ObservabilityMiddlewareRegistrar for fluent middleware configuration
- Added Response model to encapsulate output message data
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
observability_middleware_registrar.py |
Provides a fluent API for registering observability middleware with CloudAdapter |
message_logging_middleware.py |
Implements middleware that creates input/output telemetry scopes and logs messages |
input_scope.py |
OpenTelemetry scope class for tracing input messages from users to agents |
output_scope.py |
OpenTelemetry scope class for tracing output messages from agents to users |
response.py |
Data model for agent response messages |
__init__.py (2 files) |
Package initialization files with copyright headers |
| if self.log_user_messages and turn_context.activity.text: | ||
| input_scope = self._create_input_scope(turn_context.activity) | ||
| input_scope.__enter__() | ||
| self.logger.info(f"📥 User input message: {turn_context.activity.text}") | ||
|
|
||
| try: | ||
| # Hook into outgoing messages | ||
| if self.log_bot_messages: | ||
| # Pass activity to handler so we can create agent/tenant details | ||
| turn_context.on_send_activities(self._create_send_handler(turn_context.activity)) | ||
|
|
||
| # Execute bot logic | ||
| await logic() | ||
| except Exception as exc: | ||
| # Clean up and propagate exception (let __exit__ handle error recording) | ||
| if input_scope: | ||
| input_scope.__exit__(type(exc), exc, exc.__traceback__) | ||
| input_scope = None # Prevent double cleanup | ||
| raise | ||
| finally: | ||
| # Clean up the input scope if not already done | ||
| if input_scope: | ||
| input_scope.__exit__(None, None, None) |
There was a problem hiding this comment.
The method manually calls enter and exit on the InputScope context manager, which is an unusual pattern and potentially error-prone. The standard pattern would be to use a 'with' statement. However, if manual control is required due to the async execution flow, this should be documented with a comment explaining why the standard context manager pattern cannot be used here.
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT License. |
There was a problem hiding this comment.
There's an inconsistency in copyright headers across the codebase. Most existing files use 'Copyright (c) Microsoft. All rights reserved.' but the new files use 'Copyright (c) Microsoft Corporation.\n# Licensed under the MIT License.' While both are valid, this inconsistency could cause confusion. Consider aligning with the existing codebase standard.
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT License. |
There was a problem hiding this comment.
There's an inconsistency in copyright headers across the codebase. Most existing files use 'Copyright (c) Microsoft. All rights reserved.' but the new files use 'Copyright (c) Microsoft Corporation.\n# Licensed under the MIT License.' While both are valid, this inconsistency could cause confusion. Consider aligning with the existing codebase standard.
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT License. |
There was a problem hiding this comment.
There's an inconsistency in copyright headers across the codebase. Most existing files use 'Copyright (c) Microsoft. All rights reserved.' but the new files use 'Copyright (c) Microsoft Corporation.\n# Licensed under the MIT License.' While both are valid, this inconsistency could cause confusion. Consider aligning with the existing codebase standard.
| def record_output_messages(self, messages: list[str]) -> None: | ||
| """Records the output messages for telemetry tracking. | ||
|
|
||
| Args: | ||
| messages: List of output messages | ||
| """ | ||
| self.set_tag_maybe(GEN_AI_OUTPUT_MESSAGES_KEY, safe_json_dumps(messages)) |
There was a problem hiding this comment.
The method record_output_messages allows updating output messages after the scope has been created, but there's no documentation explaining when and why this method should be used versus passing the messages during initialization. The Response object is already passed in init, so the purpose of this additional method is unclear. Consider adding documentation to explain the use case, or removing this method if it's not needed.
| with OutputScope.start(agent_details, tenant_details, response): | ||
| # Log each message | ||
| for message in messages: | ||
| self.logger.info(f"📤 Bot output message: {message}") |
There was a problem hiding this comment.
The emoji usage in log messages ('📥' and '📤') may not render correctly in all logging environments and could cause issues with log parsing tools or text-based terminals. Consider using standard text prefixes like '[INPUT]' and '[OUTPUT]' instead, or make the emoji usage configurable.
| self.logger.info(f"📤 Bot output message: {message}") | |
| self.logger.info(f"[OUTPUT] Bot output message: {message}") |
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT License. |
There was a problem hiding this comment.
There's an inconsistency in copyright headers across the codebase. Most existing files use 'Copyright (c) Microsoft. All rights reserved.' but the new files use 'Copyright (c) Microsoft Corporation.\n# Licensed under the MIT License.' While both are valid, this inconsistency could cause confusion. Consider aligning with the existing codebase standard.
| """Records the input messages for telemetry tracking. | ||
|
|
||
| Args: | ||
| messages: List of input messages |
There was a problem hiding this comment.
The method record_input_messages allows updating input messages after the scope has been created, but there's no documentation explaining when and why this method should be used versus passing the message during initialization. The Request object is already passed in init, so the purpose of this additional method is unclear. Consider adding documentation to explain the use case, or removing this method if it's not needed.
| """Records the input messages for telemetry tracking. | |
| Args: | |
| messages: List of input messages | |
| """Records or updates the input messages for telemetry tracking. | |
| This method is intended for scenarios where the full set of input messages is | |
| not yet available when the :class:`InputScope` is created, or when the initial | |
| request content needs to be updated (for example, after normalization, | |
| aggregation across multiple turns, or other preprocessing). | |
| By default, the constructor records the initial input messages based on | |
| ``request.content``. Calling this method will overwrite the previously | |
| recorded ``GEN_AI_INPUT_MESSAGES_KEY`` value with the provided ``messages``. | |
| Args: | |
| messages: The complete list of input messages to associate with this scope. |
| agent_details = AgentDetails( | ||
| agent_id=activity.recipient.id if activity.recipient else "unknown", | ||
| agent_name=activity.recipient.name if activity.recipient else None, | ||
| conversation_id=activity.conversation.id if activity.conversation else None, | ||
| ) |
There was a problem hiding this comment.
The agent_id is set to "unknown" when activity.recipient is None, but the agent_name in the same case is set to None. This inconsistency could lead to confusion in logs and telemetry. Consider either setting both to "unknown" for consistency, or both to None to indicate missing data.
|
|
||
| class MessageLoggingMiddleware(Middleware): | ||
| """ | ||
| Lightweight middleware for logging input and output messages. |
There was a problem hiding this comment.
The documentation states that the class provides message logging functionality, but it doesn't mention that it also creates OpenTelemetry spans through InputScope and OutputScope. This is a significant aspect of the middleware's behavior that should be documented, as it affects observability infrastructure and trace collection.
| Lightweight middleware for logging input and output messages. | |
| Lightweight middleware that logs input and output messages and creates | |
| OpenTelemetry spans for these messages. | |
| The middleware starts an ``InputScope`` for the turn when a user message | |
| is present and uses an ``OutputScope`` for outgoing bot messages. This | |
| behavior affects observability infrastructure and trace collection, in | |
| addition to providing message logging. |
| class Response: | ||
| """Response details from agent execution.""" | ||
|
|
||
| messages: list[str] |
There was a problem hiding this comment.
nit: Should we name this Content to keep it similar to Request?
Task
Add support to input and output scopes using agents SDK middleware
Solution