[Feature] MCP (Model Context Protocol) Instrumentation Support For Python SDK #30
Conversation
WalkthroughA new OpenTelemetry instrumentation module for the MCP protocol was introduced, including comprehensive context propagation logic, package metadata, versioning, and dependency declarations. The documentation was extensively updated for clarity, completeness, and community engagement, with no changes to core code entities outside the new MCP integration. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant MCPInstrumentor
participant MCPClient
participant InstrumentedStreamReader
participant InstrumentedStreamWriter
participant OpenTelemetry
App->>MCPInstrumentor: instrument()
MCPInstrumentor->>MCPClient: wrap transport functions
MCPClient->>InstrumentedStreamReader: receive message
InstrumentedStreamReader->>OpenTelemetry: extract context from metadata
InstrumentedStreamReader->>App: yield message with context
App->>InstrumentedStreamWriter: send message
InstrumentedStreamWriter->>OpenTelemetry: inject context into metadata
InstrumentedStreamWriter->>MCPClient: send message
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
NVJKKartik
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
python/frameworks/mcp/pyproject.toml (1)
21-21: Remove trailing whitespace-build-backend = "poetry.core.masonry.api" - +build-backend = "poetry.core.masonry.api"README.md (1)
127-136: Format bare URLs properlyBare URLs should be wrapped in angle brackets for proper markdown formatting.
-Website: https://www.futureagi.com/ -Documentation: https://docs.futureagi.com/ +Website: <https://www.futureagi.com/> +Documentation: <https://docs.futureagi.com/> Cookbooks: [How-To-Implement-Observability](https://docs.futureagi.com/cookbook/cookbook8/How-To-Implement-Observability) ## Connect With Us -LinkedIn: https://www.linkedin.com/company/futureagi -Twitter: https://x.com/FutureAGI_ -Reddit: https://www.reddit.com/user/Future_AGI/submitted/ -Substack: https://substack.com/@futureagi +LinkedIn: <https://www.linkedin.com/company/futureagi> +Twitter: <https://x.com/FutureAGI_> +Reddit: <https://www.reddit.com/user/Future_AGI/submitted/> +Substack: <https://substack.com/@futureagi>🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
127-127: Bare URL used
null(MD034, no-bare-urls)
128-128: Bare URL used
null(MD034, no-bare-urls)
133-133: Bare URL used
null(MD034, no-bare-urls)
134-134: Bare URL used
null(MD034, no-bare-urls)
135-135: Bare URL used
null(MD034, no-bare-urls)
136-136: Bare URL used
null(MD034, no-bare-urls)
python/frameworks/mcp/traceai_mcp/__init__.py (1)
122-124: Consider moving imports to module levelImporting inside methods can impact performance, especially in async iterators that may be called frequently.
Move these imports to the top of the file:
from mcp.shared.message import SessionMessage from mcp.types import JSONRPCRequestThis would improve performance and follow Python conventions for import placement.
Also applies to: 155-157
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(3 hunks)python/frameworks/mcp/pyproject.toml(1 hunks)python/frameworks/mcp/traceai_mcp/__init__.py(1 hunks)python/frameworks/mcp/traceai_mcp/package.py(1 hunks)python/frameworks/mcp/traceai_mcp/version.py(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~6-~6: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...hem to standardized trace attributes. traceAI is natively supported by Future AGI, bu...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~7-~7: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...enTelemetry-compatible backend as well. traceAI provides a set of instrumentations for ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~7-~7: Consider using a more concise synonym.
Context: ...machine learning SDKs and frameworks in a variety of languages like Langchain, OpenAI, Anthr...
(A_VARIETY_OF)
[style] ~118-~118: Using many exclamation marks might seem excessive (in this case: 18 exclamation marks for a text that’s 5232 characters long)
Context: ...welcome contributions from the community! To contribute: 1. Fork the repository. ...
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.17.2)
README.md
11-11: Link fragments should be valid
null
(MD051, link-fragments)
16-16: Link fragments should be valid
null
(MD051, link-fragments)
127-127: Bare URL used
null
(MD034, no-bare-urls)
128-128: Bare URL used
null
(MD034, no-bare-urls)
133-133: Bare URL used
null
(MD034, no-bare-urls)
134-134: Bare URL used
null
(MD034, no-bare-urls)
135-135: Bare URL used
null
(MD034, no-bare-urls)
136-136: Bare URL used
null
(MD034, no-bare-urls)
🪛 Ruff (0.11.9)
python/frameworks/mcp/traceai_mcp/__init__.py
108-110: Do not call setattr with a constant attribute value. It is not any safer than normal property access.
Replace setattr with assignment
(B010)
111-111: Do not call setattr with a constant attribute value. It is not any safer than normal property access.
Replace setattr with assignment
(B010)
🔇 Additional comments (2)
python/frameworks/mcp/traceai_mcp/version.py (1)
1-1: LGTM!Standard version declaration following Python packaging best practices.
README.md (1)
106-106: Good catch on the typo fix!Correcting "gpt-4-0" to "gpt-4o" ensures the correct OpenAI model name is used.
| @@ -0,0 +1 @@ | |||
| _instruments = ("mcp >= 0.1.0", "futureagi >= 0.0.1") No newline at end of file | |||
There was a problem hiding this comment.
Version mismatch and missing dependency in pyproject.toml
The MCP version requirement here (>= 0.1.0) conflicts with the one in pyproject.toml (>= 1.8.1). Additionally, the futureagi dependency is not declared in pyproject.toml.
Apply this diff to align with pyproject.toml:
-_instruments = ("mcp >= 0.1.0", "futureagi >= 0.0.1")
+_instruments = ("mcp >= 1.8.1", "futureagi >= 0.0.1")Additionally, ensure the futureagi dependency is added to pyproject.toml if it's required for instrumentation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _instruments = ("mcp >= 0.1.0", "futureagi >= 0.0.1") | |
| _instruments = ("mcp >= 1.8.1", "futureagi >= 0.0.1") |
🤖 Prompt for AI Agents
In python/frameworks/mcp/traceai_mcp/package.py at line 1, update the MCP
version requirement in the _instruments tuple to match the version specified in
pyproject.toml (>= 1.8.1) instead of >= 0.1.0. Also, verify that the futureagi
dependency is declared in pyproject.toml; if it is missing, add it with the
appropriate version to ensure consistency and proper dependency management.
| [tool.poetry.dependencies] | ||
| python = ">3.9, <4.0" | ||
| fi-instrumentation-otel = "0.1.6" | ||
| mcp = ">=1.8.1" | ||
|
|
There was a problem hiding this comment.
Missing futureagi dependency
The package.py file references "futureagi >= 0.0.1" as a required dependency, but it's not declared here.
If futureagi is required for the instrumentation to work correctly, add it to the dependencies:
[tool.poetry.dependencies]
python = ">3.9, <4.0"
fi-instrumentation-otel = "0.1.6"
mcp = ">=1.8.1"
+futureagi = ">=0.0.1"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [tool.poetry.dependencies] | |
| python = ">3.9, <4.0" | |
| fi-instrumentation-otel = "0.1.6" | |
| mcp = ">=1.8.1" | |
| [tool.poetry.dependencies] | |
| python = ">3.9, <4.0" | |
| fi-instrumentation-otel = "0.1.6" | |
| mcp = ">=1.8.1" | |
| futureagi = ">=0.0.1" |
🤖 Prompt for AI Agents
In python/frameworks/mcp/pyproject.toml around lines 12 to 16, the dependency
"futureagi >= 0.0.1" is missing but required by package.py. Add the line
futureagi = ">=0.0.1" under [tool.poetry.dependencies] to declare this
dependency properly.
| ## Table of Contents | ||
| - [Introduction](#introduction) | ||
| - [Features](#features) | ||
| - [Supported Frameworks](#supported-frameworks) | ||
| - [Quickstart](#quickstart) | ||
| - [Contributing](#contributing) | ||
| - [License](#license) | ||
|
|
There was a problem hiding this comment.
Fix missing link fragments in Table of Contents
The links to #introduction and #license sections are broken as these sections don't exist in the document.
Either add the missing sections or update the Table of Contents:
## Table of Contents
-- [Introduction](#introduction)
- [Features](#features)
- [Supported Frameworks](#supported-frameworks)
- [Quickstart](#quickstart)
- [Contributing](#contributing)
-- [License](#license)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Table of Contents | |
| - [Introduction](#introduction) | |
| - [Features](#features) | |
| - [Supported Frameworks](#supported-frameworks) | |
| - [Quickstart](#quickstart) | |
| - [Contributing](#contributing) | |
| - [License](#license) | |
| ## Table of Contents | |
| - [Features](#features) | |
| - [Supported Frameworks](#supported-frameworks) | |
| - [Quickstart](#quickstart) | |
| - [Contributing](#contributing) |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
11-11: Link fragments should be valid
null
(MD051, link-fragments)
16-16: Link fragments should be valid
null
(MD051, link-fragments)
🤖 Prompt for AI Agents
In README.md around lines 10 to 17, the Table of Contents includes links to
#introduction and #license sections that do not exist in the document. To fix
this, either add the missing Introduction and License sections with appropriate
content to the README or remove/update these entries in the Table of Contents to
only include existing sections, ensuring all links point to valid headers.
| reader = getattr(instance, "_incoming_message_stream_reader", None) | ||
| writer = getattr(instance, "_incoming_message_stream_writer", None) | ||
| if reader and writer: | ||
| setattr( | ||
| instance, "_incoming_message_stream_reader", ContextAttachingStreamReader(reader) | ||
| ) | ||
| setattr(instance, "_incoming_message_stream_writer", ContextSavingStreamWriter(writer)) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use direct assignment instead of setattr
Static analysis correctly identifies that using setattr with constant strings is unnecessary and less readable than direct assignment.
reader = getattr(instance, "_incoming_message_stream_reader", None)
writer = getattr(instance, "_incoming_message_stream_writer", None)
if reader and writer:
- setattr(
- instance, "_incoming_message_stream_reader", ContextAttachingStreamReader(reader)
- )
- setattr(instance, "_incoming_message_stream_writer", ContextSavingStreamWriter(writer))
+ instance._incoming_message_stream_reader = ContextAttachingStreamReader(reader)
+ instance._incoming_message_stream_writer = ContextSavingStreamWriter(writer)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| reader = getattr(instance, "_incoming_message_stream_reader", None) | |
| writer = getattr(instance, "_incoming_message_stream_writer", None) | |
| if reader and writer: | |
| setattr( | |
| instance, "_incoming_message_stream_reader", ContextAttachingStreamReader(reader) | |
| ) | |
| setattr(instance, "_incoming_message_stream_writer", ContextSavingStreamWriter(writer)) | |
| reader = getattr(instance, "_incoming_message_stream_reader", None) | |
| writer = getattr(instance, "_incoming_message_stream_writer", None) | |
| if reader and writer: | |
| instance._incoming_message_stream_reader = ContextAttachingStreamReader(reader) | |
| instance._incoming_message_stream_writer = ContextSavingStreamWriter(writer) |
🧰 Tools
🪛 Ruff (0.11.9)
108-110: Do not call setattr with a constant attribute value. It is not any safer than normal property access.
Replace setattr with assignment
(B010)
111-111: Do not call setattr with a constant attribute value. It is not any safer than normal property access.
Replace setattr with assignment
(B010)
🤖 Prompt for AI Agents
In python/frameworks/mcp/traceai_mcp/__init__.py around lines 105 to 111,
replace the setattr calls used to assign to _incoming_message_stream_reader and
_incoming_message_stream_writer with direct attribute assignment on the instance
object, as the attribute names are constant strings and direct assignment
improves readability and satisfies static analysis.
| def _uninstrument(self, **kwargs: Any) -> None: | ||
| unwrap("mcp.client.stdio", "stdio_client") | ||
| unwrap("mcp.server.stdio", "stdio_server") |
There was a problem hiding this comment.
Incomplete uninstrumentation implementation
The _uninstrument method only unwraps stdio transports but leaves HTTP, SSE, and session instrumentation in place.
Add the missing unwrap calls:
def _uninstrument(self, **kwargs: Any) -> None:
+ unwrap("mcp.client.streamable_http", "streamablehttp_client")
+ unwrap("mcp.server.streamable_http", "StreamableHTTPServerTransport.connect")
+ unwrap("mcp.client.sse", "sse_client")
+ unwrap("mcp.server.sse", "SseServerTransport.connect_sse")
unwrap("mcp.client.stdio", "stdio_client")
unwrap("mcp.server.stdio", "stdio_server")
+ unwrap("mcp.server.session", "ServerSession.__init__")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _uninstrument(self, **kwargs: Any) -> None: | |
| unwrap("mcp.client.stdio", "stdio_client") | |
| unwrap("mcp.server.stdio", "stdio_server") | |
| def _uninstrument(self, **kwargs: Any) -> None: | |
| unwrap("mcp.client.streamable_http", "streamablehttp_client") | |
| unwrap("mcp.server.streamable_http", "StreamableHTTPServerTransport.connect") | |
| unwrap("mcp.client.sse", "sse_client") | |
| unwrap("mcp.server.sse", "SseServerTransport.connect_sse") | |
| unwrap("mcp.client.stdio", "stdio_client") | |
| unwrap("mcp.server.stdio", "stdio_server") | |
| unwrap("mcp.server.session", "ServerSession.__init__") |
🤖 Prompt for AI Agents
In python/frameworks/mcp/traceai_mcp/__init__.py around lines 79 to 81, the
_uninstrument method currently only unwraps stdio transports but does not unwrap
HTTP, SSE, and session instrumentation. To fix this, add unwrap calls for the
HTTP client and server, SSE client and server, and session instrumentation to
fully revert all instrumentations applied.
| if request.params: | ||
| meta = request.params.get("_meta") | ||
| if meta: | ||
| ctx = propagate.extract(meta) | ||
| restore = context.attach(ctx) | ||
| try: | ||
| yield item | ||
| continue | ||
| finally: | ||
| context.detach(restore) | ||
| yield item |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling for context operations
The context extraction and attachment operations could fail, potentially causing unhandled exceptions in the async iterator.
Consider wrapping context operations in try-except blocks:
if request.params:
meta = request.params.get("_meta")
if meta:
- ctx = propagate.extract(meta)
- restore = context.attach(ctx)
try:
+ ctx = propagate.extract(meta)
+ restore = context.attach(ctx)
yield item
continue
+ except Exception as e:
+ # Log the error but continue processing
+ # Consider using a logger here
+ yield item
+ continue
finally:
- context.detach(restore)
+ if 'restore' in locals():
+ context.detach(restore)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if request.params: | |
| meta = request.params.get("_meta") | |
| if meta: | |
| ctx = propagate.extract(meta) | |
| restore = context.attach(ctx) | |
| try: | |
| yield item | |
| continue | |
| finally: | |
| context.detach(restore) | |
| yield item | |
| if request.params: | |
| meta = request.params.get("_meta") | |
| if meta: | |
| try: | |
| ctx = propagate.extract(meta) | |
| restore = context.attach(ctx) | |
| yield item | |
| continue | |
| except Exception as e: | |
| # Log the error but continue processing | |
| # Consider using a logger here | |
| yield item | |
| continue | |
| finally: | |
| if 'restore' in locals(): | |
| context.detach(restore) | |
| yield item |
🤖 Prompt for AI Agents
In python/frameworks/mcp/traceai_mcp/__init__.py around lines 134 to 144, the
context extraction and attachment code can raise exceptions that are currently
unhandled. To fix this, wrap the context extraction, attachment, and detachment
logic inside try-except blocks to catch and handle any exceptions gracefully,
ensuring the async iterator continues to function without interruption.
Pull Request
Description
Describe the changes in this pull request:
Checklist
Related Issues
Closes #<issue_number>
Summary by CodeRabbit