[feat] Contribute mcp Instrumentor #435

Johnnyl202 · 2025-07-23T01:08:18Z

Add MCPInstrumentor class

Provides OpenTelemetry support for MCP by injecting and extracting trace context. Enables span creation for both client-side (send_request) and server-side (_handle_request) flows to support end-to-end observability.

Add test_mcpinstrumentor.py
Implements a comprehensive unit test suite that:

Verifies trace context injection

Ensures proper fallback behavior when no context is present

Simulates end-to-end client-server communication with trace propagation

Tests behavior of instrumentation internals, including tracer provider setup

Add contract_test

Validate the number of spans created and the correct attributes they have

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

ezhang6811 · 2025-07-23T19:32:40Z

.flake8

@@ -19,6 +19,9 @@ exclude =
  CVS
  .venv*/
  venv*/
+  **/venv*/
+  **/.venv*/
+  aws-opentelemetry-distro/src/amazon/opentelemetry/distro/mcpinstrumentor/venv


This file is used to exclude certain directories from checks. You can remove these as venvs should normally be created in the root directory (covered by above paths)

Agreed to this. Thank you for suggestion!

ezhang6811 · 2025-07-23T19:33:14Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/mcpinstrumentor/loggertwo.log

remove this from committed files

ezhang6811 · 2025-07-23T19:34:41Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/mcpinstrumentor/mcpinstrumentor.py

+    return logger
+
+
+loggertwo = setup_loggertwo()


Remove this logger setup code

Got it, thank you!

ezhang6811 · 2025-07-23T19:37:08Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/mcpinstrumentor/mcpinstrumentor.py

+
+        return async_wrapper()
+
+    def getname(self, req):


nit: make function name more descriptive, like _span_name_from_request

Will do. Thanks for suggestion!

ezhang6811 · 2025-07-23T19:38:33Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/mcpinstrumentor/mcpinstrumentor.py

+        return _instruments
+
+    def _instrument(self, **kwargs: Any) -> None:
+        tracer_provider = kwargs.get("tracer_provider")  # Move this line up


remove inline comment

ezhang6811 · 2025-07-23T21:45:39Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/mcpinstrumentor/pyproject.toml

+  "opentelemetry-instrumentation",
+  "opentelemetry-semantic-conventions",
+  "wrapt",
+  "opentelemetry-sdk",


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.

^^
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

Got it. Thank you!

ezhang6811 · 2025-07-23T21:46:54Z

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_mcpinstrumentor.py

+import sys
+import unittest
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), "../../../../../../src"))


can absolute path be used instead here? seems unreliable

Fixed! Thanks for suggestion.

ezhang6811 · 2025-07-23T22:02:38Z

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_mcpinstrumentor.py

+        # Verify - should handle missing request gracefully
+        self.assertIsNone(req3)
+
+    def test_end_to_end_trace_context_propagation(self):


IMO context propagation should be tested in contract/integration tests, better to just test basic functionality here.

ezhang6811 · 2025-07-23T22:07:48Z

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_mcpinstrumentor.py

+        )
+
+        # Verify the tool call request structure matches AppSignal expectations
+        request_data = appsignal_request.model_dump()


these assertions don't really have to do with the instrumentation logic

Thanks for the suggestion! This is part of the mock workflow but I have made a new version of it.

ezhang6811 · 2025-07-23T22:09:59Z

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_mcpinstrumentor.py

+        }
+
+        # Verify expected response structure
+        self.assertTrue(expected_list_apps_result["success"])


Is any behavior being validated here? It seems like we're just checking certain attributes in a hardcoded attribute

Thanks for the suggestion. Removed this line!

liustve · 2025-07-24T12:13:42Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/mcpinstrumentor/mcpinstrumentor.py

+logger_two = setup_logger_two()
+
+
+class MCPInstrumentor(BaseInstrumentor):


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)

liustve · 2025-07-24T12:18:22Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/mcpinstrumentor/mcpinstrumentor.py

+            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}


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

Got it, thank you!

liustve · 2025-07-24T12:21:12Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/mcpinstrumentor/pyproject.toml

+  "opentelemetry-instrumentation",
+  "opentelemetry-semantic-conventions",
+  "wrapt",
+  "opentelemetry-sdk",


^^
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

liustve · 2025-07-24T12:22:33Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/mcpinstrumentor/pyproject.toml

+  "Intended Audience :: Developers",
+  "License :: OSI Approved :: Apache Software License",
+  "Programming Language :: Python",
+  "Programming Language :: Python :: 3",


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

It looks likes this mcp does not support Python 3.9.

Will go ahead and fix that. Thanks!

liustve · 2025-07-24T12:25:40Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/mcpinstrumentor/mcpinstrumentor.py

+
+    # Handle Request Wrapper
+    async def _wrap_handle_request(self, wrapped, instance, args, kwargs):
+        """


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

Got it, thanks for the suggestion!

liustve · 2025-07-24T12:34:11Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/mcpinstrumentor/mcpinstrumentor.py

+        import mcp.types as types
+
+        operation = "Server Handle Request"
+        if isinstance(request, types.ListToolsRequest):


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"

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.

liustve · 2025-07-24T12:35:01Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/mcpinstrumentor/mcpinstrumentor.py

+    def handle_attributes(self, span, request, is_client=True):
+        import mcp.types as types
+
+        operation = "Server Handle Request"


Suggested change

operation = "Server Handle Request"

operation = "Server Handle Request"

span_name = "unknown"

liustve · 2025-07-24T12:35:21Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/mcpinstrumentor/mcpinstrumentor.py

+
+        return async_wrapper()
+
+    def _get_span_name(self, req):


Should delete this function, see above comment

liustve · 2025-07-24T12:40:49Z

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/loggertwo.log

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

liustve · 2025-07-24T12:41:00Z

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_mcpinstrumentor.py

+import unittest
+from unittest.mock import MagicMock
+
+project_root = os.path.abspath(os.path.join(os.path.dirname(__file__), "../../../.."))


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

Got it, thank you!

…able self use

…rumentor as an entry point

…ject.toml

liustve · 2025-08-01T11:24:01Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/instrumentation/mcp/semconv.py

+    """Fallback operation name for unrecognized MCP operations."""
+
+
+class MCPTraceContext:


These are not semantic conventions. Please read https://opentelemetry.io/docs/specs/semconv/ I recommend you have a separate class for variables like these.

liustve · 2025-08-01T11:24:11Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/instrumentation/mcp/semconv.py

+
+
+class MCPEnvironmentVariables:
+    """Environment variable names for MCP instrumentation configuration."""


These are not semantic conventions. Please read https://opentelemetry.io/docs/specs/semconv/
I recommend you have a separate class for variables like these.

liustve · 2025-08-01T11:25:27Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/instrumentation/mcp/semconv.py

+    """
+
+    # AWS-specific Remote Service Attributes
+    AWS_REMOTE_SERVICE = "aws.remote.service"


These AWS attributes are already defined in

aws-otel-python-instrumentation/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/_aws_attribute_keys.py

Line 8 in 2388f5f

AWS_REMOTE_SERVICE: str = "aws.remote.service"

.

Was able to completely removed this. Thank you!

liustve · 2025-08-01T11:32:32Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/instrumentation/mcp/semconv.py

+        return f"mcp.call_tool.{tool_name}"
+
+    # Server-side span names
+    TOOLS_INITIALIZE = "tools/initialize"


From the documentation:
MCP begins with lifecycle management through a capability negotiation handshake. As described in the [lifecycle management](https://modelcontextprotocol.io/docs/learn/architecture#lifecycle-management) section, the client sends an initialize request to establish the connection and negotiate supported features.

So naming it tools/initialize does not make too much sense to me since it has nothing to do with mcp tools

liustve · 2025-08-01T11:35:13Z

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/instrumentation/mcp/semconv.py

+        return f"tools/{tool_name}"
+
+
+class MCPOperations:


These are not semantic conventions. Please read https://opentelemetry.io/docs/specs/semconv/
I recommend you have a separate class for variables like these.

Got it thanks!

liustve · 2025-08-01T11:39:38Z

...opentelemetry-distro/src/amazon/opentelemetry/distro/instrumentation/mcp/mcp_instrumentor.py

+from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
+from opentelemetry.instrumentation.utils import unwrap
+from .semconv import MCPAttributes, MCPSpanNames, MCPOperations, MCPTraceContext, MCPEnvironmentVariables
+_instruments = ("mcp >= 1.6.0",)


Suggested change

_instruments = ("mcp >= 1.6.0",)

liustve · 2025-08-01T11:40:12Z

...opentelemetry-distro/src/amazon/opentelemetry/distro/instrumentation/mcp/mcp_instrumentor.py

+        else:
+            return await wrapped(*args, **kwargs)
+
+    def _generate_mcp_attributes(self, span: trace.Span, request: ClientRequest, is_client: bool) -> None:


This can also be a static method

This calls other static methods (add_client_attributes and add_server_attributes), so I think keeping _generate_mcp_attributes as an instance method is cleaner. Otherwise, we’d have to call them like MCPInstrumentor.add_client_attributes, which feels less natural in this context. Let me know if I'm wrong. Thank you!

Fixed! Thanks!

liustve · 2025-08-01T11:41:37Z

aws-opentelemetry-distro/pyproject.toml

@@ -89,12 +89,16 @@ dependencies = [
 # If a new patch is added into the list, it must also be added into tox.ini, dev-requirements.txt and _instrumentation_patch
 patch = [
  "botocore ~= 1.0",
+  "mcp >= 1.1.0"


Well this doesn't make sense, your code indicates that your instrumentation library instruments mcp >= 1.6.0 but here you have mcp >= 1.1.0. So which is it?

_instruments = ("mcp >= 1.6.0",)

Sorry, this is my mistake. I was able to see that there's no module named "mcp.server.lowlevel" with version 1.1. The minimum version for my instrumentor to be imported is 1.3. However, there's some significant changes to how client message handling is done in the version of 1.6. Explanation from openinference: Arize-ai/openinference#1563 . I will change it to mcp >= 1.6.0. Thank you for pointing it out!

liustve · 2025-08-01T11:42:10Z

contract-tests/images/applications/mcp/requirements.txt

@@ -0,0 +1,2 @@
+mcp>=1.1.0


see above comment

liustve · 2025-08-01T11:42:53Z

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_mcpinstrumentor.py

+            )
+
+
+if __name__ == "__main__":


Why? pytest doesn't work?

Fixed, was running with python before when I was writing the tests. Sorry about that. Thanks for pointing it out!

liustve · 2025-08-01T11:49:57Z

...opentelemetry-distro/src/amazon/opentelemetry/distro/instrumentation/mcp/mcp_instrumentor.py

+# SPDX-License-Identifier: Apache-2.0
+from typing import Any, Callable, Collection, Dict, Tuple
+
+from mcp import ClientRequest


can you lazy load this import like you did with mcp.types. In your unit tests you should patch this import with a Mock so that it doesn't fail.

Fixed! Thanks!

…hon-instrumentation into MCP-Instrumentor

This reverts commit 5c1aeec.

Unit Test and MCP Instrumentor

d8a0ee3

Johnnyl202 requested a review from a team as a code owner July 23, 2025 01:08

liustve and others added 2 commits July 22, 2025 21:55

Merge branch 'main' into MCP-Instrumentor

19b25ce

Fix lint errors

b2bad7c

ezhang6811 reviewed Jul 23, 2025

View reviewed changes

Testing coverage and dependency issues

49649e7

liustve reviewed Jul 24, 2025

View reviewed changes

Johnnyl202 and others added 17 commits July 24, 2025 15:15

Code Cleanup and To Test Dependencies

442a45a

Fixed mcp supported python

10d91ac

Added copywrite headers

0e6e339

Testing lint checker

6a5f9d6

Testing lint checker 2

db1af61

Testing lint checker3

a6ba4c4

Test lint check4

4ea1757

Added input/output types, further code cleanup, got rid of pylint dis…

3f576c2

…able self use

Working Version of Cleaned Up Code

f54609e

Contract Test

f8e7172

Restore pyproject.toml file

1a9ce26

pyproject.toml

fb9d1c7

Update pyproject.toml

7b9a8f4

Fixed pyproject.toml for mcpinstrumentor

de0ec1d

Fixed pyproject.toml for mcpinstrumentor

43526f6

adde semconv file, client span name changed, new folder name, mcpinst…

bd28ccc

…rumentor as an entry point

updated instrumentation README.md,revert some changes in distro/pypro…

70b0da3

…ject.toml

liustve changed the title ~~Unit Test and MCP Instrumentor~~ [feat] Contribute mcp Instrumentor Aug 1, 2025

Merge branch 'main' into MCP-Instrumentor

5f4f773

liustve reviewed Aug 1, 2025

View reviewed changes

Johnnyl202 added 2 commits August 1, 2025 15:23

Fixed span attributes, updated mcp model, lint, semconv changes

0af52d9

Merge branch 'MCP-Instrumentor' of github.com:JohnnyL202/aws-otel-pyt…

0d10a82

…hon-instrumentation into MCP-Instrumentor

Johnnyl202 and others added 22 commits August 1, 2025 18:54

lint and test coverage

775f0b1

Merge branch 'main' into MCP-Instrumentor

141f93c

lint and test coverage

176fd90

lint and contract test coverage

0ea9695

test coverage and lint

e6c479e

test coverage and lint

1f4f476

test coverage and lint

8621644

Merge branch 'main' into MCP-Instrumentor

c78df18

fixes

5f70570

add otel propagation library

064c71c

Fix MCP instrumentation for distributed tracing

c97c8df

add further tool instrumentation

dc2c730

cleanup code

76c2405

add more span attribute logic

323b87a

add span support for notifications

3902c60

add support for notifications + refactoring

51aaba9

add rpc service name for MCP server

9c0e57a

removed typo

dc91c62

modify contract tests

d7666ca

add session id extraction logic

aab4e7d

add further trace propagation for responses

5c1aeec

Revert "add further trace propagation for responses"

de1020e

This reverts commit 5c1aeec.

		logger_two = setup_logger_two()


		class MCPInstrumentor(BaseInstrumentor):

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

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

		"""Fallback operation name for unrecognized MCP operations."""


		class MCPTraceContext:



		class MCPEnvironmentVariables:
		"""Environment variable names for MCP instrumentation configuration."""

[feat] Contribute mcp Instrumentor #435

Are you sure you want to change the base?

[feat] Contribute mcp Instrumentor #435

Uh oh!

Conversation

Johnnyl202 commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liustve Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Johnnyl202 Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Johnnyl202 commented Jul 23, 2025 •

edited

Loading

liustve Jul 24, 2025 •

edited

Loading

Johnnyl202 Jul 29, 2025 •

edited

Loading