Skip to content

[feat] Contribute mcp Instrumentor #435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 46 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
d8a0ee3
Unit Test and MCP Instrumentor
Johnnyl202 Jul 23, 2025
19b25ce
Merge branch 'main' into MCP-Instrumentor
liustve Jul 23, 2025
b2bad7c
Fix lint errors
Johnnyl202 Jul 23, 2025
49649e7
Testing coverage and dependency issues
Johnnyl202 Jul 24, 2025
442a45a
Code Cleanup and To Test Dependencies
Johnnyl202 Jul 24, 2025
10d91ac
Fixed mcp supported python
Johnnyl202 Jul 24, 2025
0e6e339
Added copywrite headers
Johnnyl202 Jul 24, 2025
6a5f9d6
Testing lint checker
Johnnyl202 Jul 24, 2025
db1af61
Testing lint checker 2
Johnnyl202 Jul 24, 2025
a6ba4c4
Testing lint checker3
Johnnyl202 Jul 25, 2025
4ea1757
Test lint check4
Johnnyl202 Jul 25, 2025
3f576c2
Added input/output types, further code cleanup, got rid of pylint dis…
Johnnyl202 Jul 25, 2025
f54609e
Working Version of Cleaned Up Code
Johnnyl202 Jul 25, 2025
f8e7172
Contract Test
Johnnyl202 Jul 29, 2025
1a9ce26
Restore pyproject.toml file
Johnnyl202 Jul 29, 2025
fb9d1c7
pyproject.toml
Johnnyl202 Jul 29, 2025
7b9a8f4
Update pyproject.toml
Johnnyl202 Jul 29, 2025
de0ec1d
Fixed pyproject.toml for mcpinstrumentor
Johnnyl202 Jul 29, 2025
43526f6
Fixed pyproject.toml for mcpinstrumentor
Johnnyl202 Jul 29, 2025
bd28ccc
adde semconv file, client span name changed, new folder name, mcpinst…
Johnnyl202 Jul 31, 2025
70b0da3
updated instrumentation README.md,revert some changes in distro/pypro…
Johnnyl202 Jul 31, 2025
5f4f773
Merge branch 'main' into MCP-Instrumentor
liustve Aug 1, 2025
0af52d9
Fixed span attributes, updated mcp model, lint, semconv changes
Johnnyl202 Aug 1, 2025
0d10a82
Merge branch 'MCP-Instrumentor' of github.com:JohnnyL202/aws-otel-pyt…
Johnnyl202 Aug 1, 2025
775f0b1
lint and test coverage
Johnnyl202 Aug 2, 2025
141f93c
Merge branch 'main' into MCP-Instrumentor
liustve Aug 2, 2025
176fd90
lint and test coverage
Johnnyl202 Aug 3, 2025
0ea9695
lint and contract test coverage
Johnnyl202 Aug 4, 2025
e6c479e
test coverage and lint
Johnnyl202 Aug 4, 2025
1f4f476
test coverage and lint
Johnnyl202 Aug 4, 2025
8621644
test coverage and lint
Johnnyl202 Aug 4, 2025
c78df18
Merge branch 'main' into MCP-Instrumentor
liustve Aug 5, 2025
5f70570
fixes
liustve Aug 5, 2025
064c71c
add otel propagation library
liustve Aug 6, 2025
c97c8df
Fix MCP instrumentation for distributed tracing
Johnnyl202 Aug 6, 2025
dc2c730
add further tool instrumentation
liustve Aug 6, 2025
76c2405
cleanup code
liustve Aug 7, 2025
323b87a
add more span attribute logic
liustve Aug 7, 2025
3902c60
add span support for notifications
liustve Aug 7, 2025
51aaba9
add support for notifications + refactoring
liustve Aug 7, 2025
9c0e57a
add rpc service name for MCP server
liustve Aug 7, 2025
dc91c62
removed typo
liustve Aug 7, 2025
d7666ca
modify contract tests
liustve Aug 8, 2025
aab4e7d
add session id extraction logic
liustve Aug 10, 2025
5c1aeec
add further trace propagation for responses
liustve Aug 10, 2025
de1020e
Revert "add further trace propagation for responses"
liustve Aug 10, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file.
2 changes: 1 addition & 1 deletion aws-opentelemetry-distro/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,4 @@ include = [
]

[tool.hatch.build.targets.wheel]
packages = ["src/amazon"]
packages = ["src/amazon"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# MCP Instrumentor

OpenTelemetry MCP instrumentation package.

## Installation

```bash
pip install mcpinstrumentor
```

## Usage

```python
from mcpinstrumentor import MCPInstrumentor

MCPInstrumentor().instrument()
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
import logging
from typing import Any, Collection

from openinference.instrumentation.mcp.package import _instruments
from wrapt import register_post_import_hook, wrap_function_wrapper

from opentelemetry import trace
from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
from opentelemetry.instrumentation.utils import unwrap


def setup_logger_two():
logger = logging.getLogger("loggertwo")
logger.setLevel(logging.DEBUG)
handler = logging.FileHandler("loggertwo.log", mode="w")
handler.setLevel(logging.DEBUG)
formatter = logging.Formatter("%(asctime)s - %(levelname)s - %(message)s")
handler.setFormatter(formatter)
if not logger.handlers:
logger.addHandler(handler)
return logger


logger_two = setup_logger_two()


class MCPInstrumentor(BaseInstrumentor):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can you organize your functions top-to-bottom (i.e. most important functions at top _wrap_send_request and _wrap_handle_request and then helper functions)

"""
An instrumenter for MCP.
"""

def instrumentation_dependencies(self) -> Collection[str]:
return _instruments

def _instrument(self, **kwargs: Any) -> None:
tracer_provider = kwargs.get("tracer_provider")
if tracer_provider:
self.tracer_provider = tracer_provider
else:
self.tracer_provider = None
register_post_import_hook(
lambda _: wrap_function_wrapper(
"mcp.shared.session",
"BaseSession.send_request",
self._wrap_send_request,
),
"mcp.shared.session",
)
register_post_import_hook(
lambda _: wrap_function_wrapper(
"mcp.server.lowlevel.server",
"Server._handle_request",
self._wrap_handle_request,
),
"mcp.server.lowlevel.server",
)

def _uninstrument(self, **kwargs: Any) -> None:
unwrap("mcp.shared.session", "BaseSession.send_request")
unwrap("mcp.server.lowlevel.server", "Server._handle_request")

def handle_attributes(self, span, request, is_client=True):
import mcp.types as types

operation = "Server Handle Request"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
operation = "Server Handle Request"
operation = "Server Handle Request"
span_name = "unknown"

if isinstance(request, types.ListToolsRequest):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have duplicate code where we check isinstance(request, [type]) in get_name. In my opinion we should delete get_name and consolidate the logic into this function.

Suggested change
if isinstance(request, types.ListToolsRequest):
if isinstance(request, types.ListToolsRequest):
span_name = "tools/list"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion! I understand your point but I feel like we should keep it because each function handles distinct logic specific to client or server. To me, that's easier to read and if things go wrong, it is also easier to test fixes like I can just add an attribute to the client function to see what changes on CloudWatch.

operation = "ListTool"
span.set_attribute("mcp.list_tools", True)
elif isinstance(request, types.CallToolRequest):
if hasattr(request, "params") and hasattr(request.params, "name"):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this error if we directly check for request.params.name?

operation = request.params.name
span.set_attribute("mcp.call_tool", True)
if is_client:
self._add_client_attributes(span, operation, request)
else:
self._add_server_attributes(span, operation, request)

def _add_client_attributes(self, span, operation, request):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The attribute names we are setting should be defined here. Import to use the variable name instead of the explicit string.

If we are defining our own attribute like "tool.name", we should also create and use a variable for it. (PS should this be gen_ai.tool.name?)

span.set_attribute("span.kind", "CLIENT")
span.set_attribute("aws.remote.service", "Appsignals MCP Server")
span.set_attribute("aws.remote.operation", operation)
if hasattr(request, "params") and hasattr(request.params, "name"):
span.set_attribute("tool.name", request.params.name)

def _add_server_attributes(self, span, operation, request):
span.set_attribute("server_side", True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above. If "server_side" is defined by you then you should create a variable, otherwise import the predefined one.

span.set_attribute("aws.span.kind", "SERVER")
if hasattr(request, "params") and hasattr(request.params, "name"):
span.set_attribute("tool.name", request.params.name)

def _inject_trace_context(self, request_data, span_ctx):
if "params" not in request_data:
request_data["params"] = {}
if "_meta" not in request_data["params"]:
request_data["params"]["_meta"] = {}
request_data["params"]["_meta"]["trace_context"] = {"trace_id": span_ctx.trace_id, "span_id": span_ctx.span_id}
Copy link
Contributor

@liustve liustve Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we rename trace_context to traceparent and inject a W3C trace header string instead of the dictionary?
When the trace context gets received we should parse the string for the trace id and parent span id.
See: https://www.w3.org/TR/trace-context/#trace-context-http-headers-format

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thank you!


# Send Request Wrapper
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment

def _wrap_send_request(self, wrapped, instance, args, kwargs):
"""
Changes made:
The wrapper intercepts the request before sending, injects distributed tracing context into the
request's params._meta field and creates OpenTelemetry spans. The wrapper does not change anything
else from the original function's behavior because it reconstructs the request object with the same
type and calling the original function with identical parameters.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: probably don't need to talk about how it preserves the function's behavior. More importantly, what does the wrapper introduce? (adds attributes, etc.) same for the server wrapper

"""

async def async_wrapper():
if self.tracer_provider is None:
tracer = trace.get_tracer("mcp.client")
else:
tracer = self.tracer_provider.get_tracer("mcp.client")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider moving the above logic into a separate function since its duplicated in the server. You could pass in either mcp.client or mcp.server.

with tracer.start_as_current_span("client.send_request", kind=trace.SpanKind.CLIENT) as span:
span_ctx = span.get_span_context()
request = args[0] if len(args) > 0 else kwargs.get("request")
if request:
req_root = request.root if hasattr(request, "root") else request

self.handle_attributes(span, req_root, True)
request_data = request.model_dump(by_alias=True, mode="json", exclude_none=True)
self._inject_trace_context(request_data, span_ctx)
# Reconstruct request object with injected trace context
modified_request = type(request).model_validate(request_data)
if len(args) > 0:
new_args = (modified_request,) + args[1:]
result = await wrapped(*new_args, **kwargs)
else:
kwargs["request"] = modified_request
result = await wrapped(*args, **kwargs)
else:
result = await wrapped(*args, **kwargs)
return result

return async_wrapper()

def _get_span_name(self, req):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should delete this function, see above comment

span_name = "unknown"
import mcp.types as types

if isinstance(req, types.ListToolsRequest):
span_name = "tools/list"
elif isinstance(req, types.CallToolRequest):
if hasattr(req, "params") and hasattr(req.params, "name"):
span_name = f"tools/{req.params.name}"
else:
span_name = "unknown"
return span_name

# Handle Request Wrapper
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment

async def _wrap_handle_request(self, wrapped, instance, args, kwargs):
"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be helpful if you can add a comment of what the request object looks like.

Copy link
Author

@Johnnyl202 Johnnyl202 Jul 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks for the suggestion!

Changes made:
This wrapper intercepts requests before processing, extracts distributed tracing context from
the request's params._meta field, and creates server-side OpenTelemetry spans linked to the client spans.
The wrapper also does not change the original function's behavior by calling it with identical parameters
ensuring no breaking changes to the MCP server functionality.
"""
req = args[1] if len(args) > 1 else None
trace_context = None

if req and hasattr(req, "params") and req.params and hasattr(req.params, "meta") and req.params.meta:
trace_context = req.params.meta.trace_context
if trace_context:

if self.tracer_provider is None:
tracer = trace.get_tracer("mcp.server")
else:
tracer = self.tracer_provider.get_tracer("mcp.server")
trace_id = trace_context.get("trace_id")
span_id = trace_context.get("span_id")
span_context = trace.SpanContext(
trace_id=trace_id,
span_id=span_id,
is_remote=True,
trace_flags=trace.TraceFlags(trace.TraceFlags.SAMPLED),
trace_state=trace.TraceState(),
)
span_name = self._get_span_name(req)
with tracer.start_as_current_span(
span_name,
kind=trace.SpanKind.SERVER,
context=trace.set_span_in_context(trace.NonRecordingSpan(span_context)),
) as span:
self.handle_attributes(span, req, False)
result = await wrapped(*args, **kwargs)
return result
else:
return await wrapped(*args, **kwargs)
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
[build-system]
requires = ["hatchling"]
build-backend = "hatchling.build"

[project]
name = "amazon-opentelemetry-distro-mcpinstrumentor"
version = "0.1.0"
description = "OpenTelemetry MCP instrumentation for AWS Distro"
readme = "README.md"
license = "Apache-2.0"
requires-python = ">=3.9"
authors = [
{ name = "Johnny Lin", email = "[email protected]" },
]
classifiers = [
"Development Status :: 4 - Beta",
"Intended Audience :: Developers",
"License :: OSI Approved :: Apache Software License",
"Programming Language :: Python",
"Programming Language :: Python :: 3",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://pypi.org/project/mcp/

It looks likes this mcp does not support Python 3.9.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will go ahead and fix that. Thanks!

"Programming Language :: Python :: 3.9",
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
"Programming Language :: Python :: 3.12",
"Programming Language :: Python :: 3.13",
]
dependencies = [
"openinference-instrumentation-mcp",
"opentelemetry-api",
"opentelemetry-instrumentation",
"opentelemetry-sdk",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could be missing some dependencies here (like mcp or openinference), check dependencies that had to be installed manually outside of this list and add them here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^^
Please add mcp as a dependency.
We should not have openinference as a dependency as the implementation does not require anything in their library. Please remove

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Thank you!

"wrapt"
]

[project.optional-dependencies]
instruments = ["mcp"]

[project.entry-points.opentelemetry_instrumentor]
mcp = "mcpinstrumentor:MCPInstrumentor"

[tool.hatch.build.targets.sdist]
include = [
"mcpinstrumentor.py",
"README.md"
]

[tool.hatch.build.targets.wheel]
packages = ["."]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid including development-specific logging configurations and test setups in PR commits.

As a rule, before pushing:

  • Review your commit diff to ensure only intended changes are included
  • Remove any temporary debugging code, verbose logging, or test-specific configurations
  • Keep commits focused on the actual feature/fix being implemented

Empty file.
Loading
Loading