feat(mcp): add mcp official sdk instrumentation #829
Conversation
|
Warning Rate limit exceeded@nirga has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
WalkthroughAdds a new MCP OpenTelemetry instrumentation package, integrates it into the Traceloop SDK, extends semantic conventions with MCP attributes and a SESSION span kind, provides tests and sample apps, and includes build/configuration files for the new package. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Traceloop as Traceloop SDK
participant McpInst as McpInstrumentation
participant MCP as MCP SDK
participant OTel as OpenTelemetry
App->>Traceloop: initialize({ instrumentModules: { mcp } })
Traceloop->>McpInst: new McpInstrumentation(config)
McpInst->>McpInst: init() registers patches for connect/request/close
Note over App,MCP: client connects -> session span lifecycle
App->>MCP: client.connect()
MCP->>McpInst: patched connect handler
McpInst->>OTel: startSpan("SESSION")
McpInst->>McpInst: store session context
Note over App,MCP: tool call -> tool span with optional content
App->>MCP: client.request(toolCall)
MCP->>McpInst: patched request handler
McpInst->>OTel: startSpan(toolSpan) with parent=session
McpInst->>McpInst: extract attributes (traceContent? input/output)
McpInst->>MCP: invoke original request
MCP-->>McpInst: response
McpInst->>OTel: set attributes (`mcp.request.id`, `mcp.response.value`) and endSpan()
App->>MCP: client.close()
MCP->>McpInst: patched close handler
McpInst->>OTel: endSpan("SESSION")
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
galkleinman
force-pushed
the
gk/mcp-instrumentation
branch
from
d9fb1ec to
94d74ff
Compare
Previously, the mcp.client.session span was created in connect() but immediately ended when the connection completed. This meant all subsequent request() calls had no parent session span, creating a flat structure instead of a hierarchy. Changes: - Store session context and span on client instance after connect() - Use stored session context as parent for all request spans - Added close() wrapper to properly end session span on disconnect - Session span now accurately represents the client lifetime Tests: - Added test for session span creation and lifecycle - Added test for tool call spans with correct attributes - All 6 tests passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Capture the MCP client/server name and add it to the session span
using the TRACELOOP_WORKFLOW_NAME attribute, formatted as "{name}.mcp".
This matches the Python implementation pattern and provides better
identification of MCP sessions in traces.
Changes:
- Extract _clientInfo.name from Client instance during connect()
- Set TRACELOOP_WORKFLOW_NAME attribute on session span
- Updated tests to verify workflow name attribute
Example: Client with name "calculator-client" will have
TRACELOOP_WORKFLOW_NAME = "calculator-client.mcp"
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
…nown span kind
Align with Python FastMCP instrumentation pattern:
1. Add TRACELOOP_WORKFLOW_NAME to tool and request spans:
- Tool spans now include workflow name (client/server name)
- All request spans (tools/call, resources/list, etc.) get workflow name
- Matches Python implementation where workflow name is set on both
server spans and tool spans
2. Use TraceloopSpanKindValues.UNKNOWN enum instead of string literal:
- Changed from "unknown" to TraceloopSpanKindValues.UNKNOWN
- Consistent with other span kind assignments
Changes:
- Extract client/server info from _clientInfo or server._serverInfo
- Set TRACELOOP_WORKFLOW_NAME on all request spans
- Use enum for UNKNOWN span kind
- Updated tests to verify workflow name on tool spans
All 6 tests passing ✓
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Reverted enhancements to match the Python MCP client implementation precisely. The Python client instrumentation does NOT set: - TRACELOOP_WORKFLOW_NAME on any spans - TRACELOOP_SPAN_KIND on generic method spans (only on tool calls) - TRACELOOP_ENTITY_NAME on generic method spans (only on tool calls) Changes: 1. Session span: - Use "session" string literal instead of enum - Remove TRACELOOP_WORKFLOW_NAME 2. Tool call spans: - Use "tool" string literal instead of enum - Keep TRACELOOP_ENTITY_NAME (tool name) - Remove TRACELOOP_WORKFLOW_NAME 3. Generic method spans (tools/list, resources/read, etc): - Remove TRACELOOP_SPAN_KIND - Remove TRACELOOP_ENTITY_NAME - Keep TRACELOOP_ENTITY_INPUT/OUTPUT All 6 tests passing ✓ Sample app verified ✓ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed fea22f7 in 2 minutes and 2 seconds. Click for details.
- Reviewed
140lines of code in2files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/instrumentation-mcp/src/instrumentation.ts:248
- Draft comment:
Removed workflow name attribute logic. Confirm that omitting TRACELOOP_WORKFLOW_NAME for sessions is intentional. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. packages/instrumentation-mcp/src/instrumentation.ts:347
- Draft comment:
Hardcoding span kind values ('session' and 'tool') instead of using constants may reduce maintainability. Consider defining module-level constants for these values. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. packages/instrumentation-mcp/src/instrumentation.ts:345
- Draft comment:
Tool call spans now only set attributes when method === 'tools/call'. Verify that non-tool methods do not require additional span kind or entity attributes. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
4. packages/instrumentation-mcp/test/instrumentation.test.ts:248
- Draft comment:
Removed assertions for workflow name (TRACELOOP_WORKFLOW_NAME) in session spans. Ensure test expectations match the new design. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure that test expectations match the new design, which violates the rule against asking the author to ensure that their change is tested. It does not provide a specific suggestion or ask for a specific test to be written.
Workflow ID: wflow_kPUvECCAvJM5BTgX
You can customize 
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 0503b89 in 3 minutes and 5 seconds. Click for details.
- Reviewed
2122lines of code in19files - Skipped
2files when reviewing. - Skipped posting
12draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/ai-semantic-conventions/src/SemanticAttributes.ts:64
- Draft comment:
New MCP attributes (MCP_RESPONSE_VALUE and MCP_REQUEST_ID) have been added. Confirm that the naming convention aligns with other attributes in the file. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm the naming convention, which is against the rules. It does not provide a specific suggestion or point out a clear issue with the code. Therefore, it should be removed.
2. packages/instrumentation-mcp/src/instrumentation.ts:515
- Draft comment:
Consider handling multiple error messages in MCP responses. Currently only the first content text is used for error status; if multiple error details exist, evaluate if they should be aggregated. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% The comment uses the word "Consider" which is a soft suggestion. It's asking the author to "evaluate if they should be aggregated" - this is speculative and not pointing to a definite issue. The current implementation of taking the first error message is a reasonable default behavior. Without knowing the MCP protocol specification or seeing evidence that multiple error messages are common and should be aggregated, this is purely a "nice to have" suggestion. According to the rules, I should NOT make speculative comments like "If X, then Y is an issue" - only comment if it's definitely an issue. This comment is speculative about whether multiple error messages exist and whether they should be aggregated. Perhaps the MCP protocol does commonly return multiple error messages in the content array, and this could be a legitimate issue where important error information is being lost. The comment might be based on knowledge of the MCP protocol that I don't have. Even if multiple error messages are possible in the MCP protocol, the comment doesn't provide evidence that this is actually a problem or that the current behavior is incorrect. It's phrased as "Consider" and "evaluate if" which makes it clearly speculative. Without strong evidence that this is a real issue, this falls under the category of optional code quality suggestions that are too speculative to keep. This comment should be deleted. It's a speculative suggestion using "Consider" and "evaluate if" language, without providing evidence that the current behavior is incorrect or that multiple error messages are actually a problem in practice. It doesn't identify a clear, definite issue that requires a code change.
3. packages/instrumentation-mcp/test/instrumentation.test.ts:120
- Draft comment:
Tests cover key MCP flows. It may be useful to add more edge-case tests (e.g., unexpected response structures) to further validate error handling. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% This comment is suggesting the author add more tests. It's not pointing out a bug or issue with the code that was written. It's a suggestion to expand test coverage with additional edge cases. According to the rules, I should not keep comments that are purely informative or that don't clearly require a code change. This is more of a "nice to have" suggestion rather than identifying a specific problem with the current implementation. The test as written appears to be valid and functional. The comment is essentially asking for more comprehensive testing, which is speculative about what other edge cases might exist. The comment could be seen as actionable since it's suggesting specific improvements to test coverage. Test coverage improvements are legitimate code quality suggestions. However, it's not identifying a specific missing test case or a concrete problem - it's vaguely suggesting "more edge-case tests" without specifying what those should be. While test coverage suggestions can be valuable, this comment is too vague and speculative. It doesn't identify a specific missing test case or a concrete gap in coverage. It's essentially saying "you could test more things" without being specific about what those things are or why they're important. This violates the rule against speculative comments and comments that aren't clearly actionable. This comment should be deleted. It's a vague suggestion to add more tests without identifying specific missing test cases or concrete issues. It's not actionable enough and falls into the category of speculative suggestions that don't meet the criteria for keeping.
4. packages/sample-app/package.json:39
- Draft comment:
New run scripts for MCP samples (run:mcp, run:mcp:real, run:mcp:working) have been added. Ensure that documentation and version consistency are maintained across packages. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure documentation and version consistency, which falls under the rule of not asking the author to ensure or verify things. It doesn't provide a specific suggestion or point out a specific issue with the code.
5. packages/sample-app/src/sample_mcp.ts:25
- Draft comment:
The sample app demonstrates MCP instrumentation with a full in-memory client-server setup. Consider adding a note to document necessary environment variables or potential error scenarios. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is suggesting to add documentation, which is not a specific code suggestion or a request for a test. It seems to be purely informative and does not align with the rules for acceptable comments.
6. packages/sample-app/src/sample_mcp_simple.ts:15
- Draft comment:
The simplified MCP sample uses mocked transport requests. Verify that the simulated responses cover critical operation types and that the mocked logic remains in sync with production behavior. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to verify that the simulated responses cover critical operation types and that the mocked logic remains in sync with production behavior. This falls under the category of asking the author to ensure the behavior is intended and tested, which violates the rules.
7. packages/traceloop-sdk/package.json:49
- Draft comment:
There appears to be a version mismatch for '@modelcontextprotocol/sdk' (here '^1.0.4' versus newer versions seen in sample-app). Verify that all packages use a consistent version to avoid compatibility issues. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. packages/traceloop-sdk/src/lib/interfaces/initialize-options.interface.ts:90
- Draft comment:
The MCP module is now included in instrumentModules. Confirm that the typing for 'mcp' (using 'typeof mcp') correctly reflects the module's exports and is consistent with usage in manual instrumentation. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. packages/traceloop-sdk/src/lib/tracing/index.ts:127
- Draft comment:
MCP instrumentation is now initialized along with other instrumentations. Ensure that any manual instrumentation for the MCP SDK is well-tested in various module systems (CommonJS vs ESM). - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure that manual instrumentation for the MCP SDK is well-tested in various module systems. This falls under the rule of not asking the author to ensure that their change is tested, which is not allowed.
10. packages/traceloop-sdk/src/lib/tracing/span-processor.ts:116
- Draft comment:
The allowed instrumentation libraries array includes '@traceloop/instrumentation-chroma', but elsewhere the instrumentation for ChromaDB is imported as '@traceloop/instrumentation-chromadb'. Consider updating this string for consistency. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. packages/instrumentation-mcp/package.json:8
- Draft comment:
Typo detected in the 'repository' field: "openllmetry-js" appears to be a misspelling. If the intended repository name is "opentelemetry-js", please update it. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 5% vs. threshold = 50% The comment assumes "openllmetry-js" is a typo for "opentelemetry-js", but the homepage URL on line 60 uses the exact same "openllmetry-js" name consistently. This is strong evidence that "openllmetry-js" is intentional. The project appears to be related to OpenTelemetry instrumentation for LLM/AI applications (based on the package name, dependencies, and MCP description), so "openllmetry" (OpenTelemetry + LLM) makes perfect sense as a portmanteau. The comment is incorrect and should be deleted. Could there be a scenario where both the repository field and homepage field contain the same typo? That seems unlikely but theoretically possible. However, given the AI/LLM context of this package, the name "openllmetry" appears to be a deliberate branding choice. While it's theoretically possible both fields have the same typo, the consistency across multiple fields and the clear LLM/AI context of the package (MCP, ai-semantic-conventions) make it overwhelmingly likely that "openllmetry-js" is the correct, intentional name. The comment is based on an incorrect assumption. The comment should be deleted. "openllmetry-js" appears to be the correct, intentional repository name (a portmanteau of OpenTelemetry + LLM), as evidenced by its consistent use in both the repository and homepage fields, and the AI/LLM context of the package.
12. packages/instrumentation-mcp/package.json:60
- Draft comment:
Typo detected in the 'homepage' URL: the repository segment "openllmetry-js" may be a misspelling. Ensure it matches the intended repository name. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_9fTFnp3cHnxTH83S
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed c288fc9 in 39 seconds. Click for details.
- Reviewed
16lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/instrumentation-mcp/test/instrumentation.test.ts:23
- Draft comment:
Unused 'TraceloopSpanKindValues' import was removed. This cleanup is good if it isn’t needed elsewhere. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_ZeVqwrBuH79DkGld
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed c44b472 in 1 minute and 9 seconds. Click for details.
- Reviewed
12lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/instrumentation-mcp/package.json:8
- Draft comment:
Consider using the object format for the repository field (e.g. { type: 'git', url: 'https://github.com/traceloop/openllmetry-js.git' }) to align with npm standards. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. packages/instrumentation-mcp/package.json:45
- Draft comment:
Using 'workspace:*' for @traceloop/ai-semantic-conventions may cause issues outside the monorepo. Consider pinning a specific version for published packages. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
Workflow ID: wflow_U7v9pKma5jqU6rkm
You can customize
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/traceloop-sdk/src/lib/tracing/index.ts (1)
260-291: Extend global traceContent gating to MCP instrumentation for privacyWhen
shouldSendTraces()isfalse, the SDK currently disables content logging for several LLM-related instrumentations by callingsetConfig({ traceContent: false }), but the new MCP instrumentation is not included in that block. Given that MCP spans can also contain prompts and responses, this means MCP content may still be traced even when global trace content is disabled via_configuration.traceContentorTRACELOOP_TRACE_CONTENT.To keep behavior and privacy guarantees consistent, consider updating the block to include MCP:
if (!shouldSendTraces()) { openAIInstrumentation?.setConfig({ traceContent: false, }); @@ togetherInstrumentation?.setConfig({ traceContent: false, }); + mcpInstrumentation?.setConfig({ + traceContent: false, + }); }This ensures MCP content respects the same global trace-content switch as the other content-bearing instrumentations.
Based on learnings, centralizing content/PII gating in the SDK (rather than per instrumentation package) is the intended pattern, so MCP should follow the same rule.
🧹 Nitpick comments (4)
packages/sample-app/src/sample_mcp_simple.ts (1)
42-112: Consider using typed interfaces for request/response.While the
anytype assertions are pragmatic for a sample, consider defining typed interfaces for the MCP request/response structure to improve type safety and demonstrate best practices.Example approach:
+interface McpRequest { + method: string; + params?: Record<string, unknown>; +} + +interface ToolListResponse { + tools: Array<{ name: string; description: string; inputSchema: unknown }>; +} + -(client as any).request = async function (request: any) { +(client as any).request = async function (request: McpRequest): Promise<unknown> { const method = request.method; const params = request.params;packages/instrumentation-mcp/test/instrumentation.test.ts (1)
200-257: Session span test looks solid; consider asserting span kind constant as well.The
"should create session spans for client connect"test uses real in-memory transports and manual instrumentation and correctly asserts:
- A
mcp.client.sessionspan is created.TRACELOOP_SPAN_KINDis"session".TRACELOOP_ENTITY_NAMEis"mcp.client.session".Optionally, you could also assert on the OTEL span kind (
SpanKind.CLIENT) to catch regressions where the session span’s kind changes unintentionally, but the current coverage is already good.packages/instrumentation-mcp/src/instrumentation.ts (2)
309-405: Request wrapper and result handling are generally solid; consider two small refinements.The
requestwrapper and_handleResultcover the main requirements:
- Tool and other MCP methods are distinguished via span names (
<tool>.toolvs<method>.mcp).- Tool spans are tagged with
TRACELOOP_SPAN_KIND="tool"andTRACELOOP_ENTITY_NAME=<tool_name>.- Inputs/outputs are serialized into
TRACELOOP_ENTITY_INPUT/OUTPUTonly when_shouldSendPrompts()is true.- Errors from thrown exceptions are recorded with
recordExceptionandSpanStatusCode.ERROR.- Result objects with
isErrorset flip status to error while still ending the span infinally.Two optional refinements:
Record exceptions for
isErrorresponses as well.
In_handleResult, whenmcpResult.isErroris true you currently only set the status to error. If you want error backends to treat these like other failures, you could also callspan.recordException(new Error(errorMessage))there.Use the parent context when making the span current.
In_wrapRequest, you computeparentContext(session or active) forstartSpanbut then buildexecContextfromcontext.active()again. Usingtrace.setSpan(parentContext, span)would ensure any baggage or upstream context onparentContextis also visible while the original request runs, though this is a minor nuance.Both are non-blocking, but would further tighten the behavior.
Also applies to: 472-506
508-690: Input/output extraction is thoughtful and privacy-aware; consider leveraging MCP-specific attributes.The
_extractCleanInput/_extractCleanOutputhelpers:
- Special-case MCP methods (
tools/call,tools/list,resources/*,prompts/*) to emit structured, compact data.- Omit underscored keys and fall back to type names for un-serializable values.
- Truncate
resources/readtext to 1000 chars to limit payload size.- Surface only the relevant tool arguments and first content item result for
tools/call.This nicely satisfies the requirement to extract request/response data while being careful about payload size and PII.
Given that
SpanAttributesnow includes MCP-specific attributes (e.g.,MCP_RESPONSE_VALUE,MCP_REQUEST_ID), you may optionally want to:
- Populate
MCP_RESPONSE_VALUEdirectly from the primary tool result (in addition to the JSON-encodedTRACELOOP_ENTITY_OUTPUT).- If/when a stable request ID exists in MCP messages, set
MCP_REQUEST_IDfrom it.That would make MCP spans easier to query by these first-class attributes while keeping the existing entity input/output JSON for richer detail.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
packages/ai-semantic-conventions/src/SemanticAttributes.ts(2 hunks)packages/instrumentation-mcp/.prettierignore(1 hunks)packages/instrumentation-mcp/README.md(1 hunks)packages/instrumentation-mcp/eslint.config.cjs(1 hunks)packages/instrumentation-mcp/package.json(1 hunks)packages/instrumentation-mcp/rollup.config.js(1 hunks)packages/instrumentation-mcp/src/index.ts(1 hunks)packages/instrumentation-mcp/src/instrumentation.ts(1 hunks)packages/instrumentation-mcp/src/types.ts(1 hunks)packages/instrumentation-mcp/test/instrumentation.test.ts(1 hunks)packages/instrumentation-mcp/tsconfig.json(1 hunks)packages/sample-app/package.json(2 hunks)packages/sample-app/src/sample_mcp.ts(1 hunks)packages/sample-app/src/sample_mcp_simple.ts(1 hunks)packages/sample-app/src/test_edit_only.ts(0 hunks)packages/sample-app/src/test_generate_only.ts(0 hunks)packages/traceloop-sdk/package.json(2 hunks)packages/traceloop-sdk/src/lib/interfaces/initialize-options.interface.ts(2 hunks)packages/traceloop-sdk/src/lib/tracing/index.ts(4 hunks)packages/traceloop-sdk/src/lib/tracing/span-processor.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/sample-app/src/test_edit_only.ts
- packages/sample-app/src/test_generate_only.ts
🧰 Additional context used
📓 Path-based instructions (7)
packages/{instrumentation-*,traceloop-sdk}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Import AI/LLM semantic attribute constants from @traceloop/ai-semantic-conventions rather than hardcoding strings
Files:
packages/traceloop-sdk/src/lib/tracing/span-processor.tspackages/instrumentation-mcp/src/types.tspackages/traceloop-sdk/src/lib/interfaces/initialize-options.interface.tspackages/instrumentation-mcp/src/index.tspackages/traceloop-sdk/src/lib/tracing/index.tspackages/instrumentation-mcp/src/instrumentation.tspackages/instrumentation-mcp/test/instrumentation.test.ts
packages/traceloop-sdk/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/traceloop-sdk/**/*.{ts,tsx}: Use the provided decorators (@workflow, @task, @agent) for workflow/task/agent spans instead of re-implementing them
For manual LLM operations, use trace.withLLMSpan from @traceloop/node-server-sdk
Files:
packages/traceloop-sdk/src/lib/tracing/span-processor.tspackages/traceloop-sdk/src/lib/interfaces/initialize-options.interface.tspackages/traceloop-sdk/src/lib/tracing/index.ts
packages/instrumentation-*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place each provider integration in its own package under packages/instrumentation-[provider]/
Files:
packages/instrumentation-mcp/tsconfig.jsonpackages/instrumentation-mcp/src/types.tspackages/instrumentation-mcp/package.jsonpackages/instrumentation-mcp/src/index.tspackages/instrumentation-mcp/rollup.config.jspackages/instrumentation-mcp/src/instrumentation.tspackages/instrumentation-mcp/eslint.config.cjspackages/instrumentation-mcp/README.mdpackages/instrumentation-mcp/test/instrumentation.test.ts
packages/instrumentation-*/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/instrumentation-*/**/*.{ts,tsx}: Instrumentation classes must extend InstrumentationBase and register hooks using InstrumentationModuleDefinition
Instrumentations must create spans with appropriate AI/LLM semantic attributes for calls they wrap
Instrumentations must extract request/response data and token usage from wrapped calls
Instrumentations must capture and record errors appropriately
Do not implement anonymous telemetry collection in instrumentation packages; telemetry is collected only in the SDK
Files:
packages/instrumentation-mcp/src/types.tspackages/instrumentation-mcp/src/index.tspackages/instrumentation-mcp/src/instrumentation.tspackages/instrumentation-mcp/test/instrumentation.test.ts
packages/ai-semantic-conventions/src/SemanticAttributes.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Define all AI/LLM span attribute constants in packages/ai-semantic-conventions/src/SemanticAttributes.ts
Files:
packages/ai-semantic-conventions/src/SemanticAttributes.ts
packages/*/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
Use workspace:* for intra-repo package dependencies in package.json
Files:
packages/instrumentation-mcp/package.jsonpackages/sample-app/package.jsonpackages/traceloop-sdk/package.json
packages/traceloop-sdk/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
When adding a new instrumentation package, add it to the main SDK dependencies
Files:
packages/traceloop-sdk/package.json
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/traceloop-sdk/package.json : When adding a new instrumentation package, add it to the main SDK dependencies
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/traceloop-sdk/src/lib/node-server-sdk.ts : Register new instrumentation packages during SDK initialization
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/{instrumentation-*,traceloop-sdk}/**/*.{ts,tsx} : Import AI/LLM semantic attribute constants from traceloop/ai-semantic-conventions rather than hardcoding strings
Applied to files:
packages/traceloop-sdk/src/lib/tracing/span-processor.tspackages/instrumentation-mcp/tsconfig.jsonpackages/instrumentation-mcp/src/types.tspackages/sample-app/src/sample_mcp_simple.tspackages/ai-semantic-conventions/src/SemanticAttributes.tspackages/instrumentation-mcp/package.jsonpackages/traceloop-sdk/src/lib/interfaces/initialize-options.interface.tspackages/instrumentation-mcp/.prettierignorepackages/sample-app/package.jsonpackages/instrumentation-mcp/src/index.tspackages/traceloop-sdk/src/lib/tracing/index.tspackages/instrumentation-mcp/rollup.config.jspackages/instrumentation-mcp/src/instrumentation.tspackages/instrumentation-mcp/eslint.config.cjspackages/instrumentation-mcp/README.mdpackages/traceloop-sdk/package.jsonpackages/instrumentation-mcp/test/instrumentation.test.tspackages/sample-app/src/sample_mcp.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/traceloop-sdk/**/*.{ts,tsx} : For manual LLM operations, use trace.withLLMSpan from traceloop/node-server-sdk
Applied to files:
packages/traceloop-sdk/src/lib/tracing/span-processor.tspackages/sample-app/src/sample_mcp_simple.tspackages/ai-semantic-conventions/src/SemanticAttributes.tspackages/traceloop-sdk/src/lib/interfaces/initialize-options.interface.tspackages/traceloop-sdk/src/lib/tracing/index.tspackages/instrumentation-mcp/src/instrumentation.tspackages/instrumentation-mcp/README.mdpackages/traceloop-sdk/package.jsonpackages/instrumentation-mcp/test/instrumentation.test.tspackages/sample-app/src/sample_mcp.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/instrumentation-*/**/*.{ts,tsx} : Instrumentations must create spans with appropriate AI/LLM semantic attributes for calls they wrap
Applied to files:
packages/traceloop-sdk/src/lib/tracing/span-processor.tspackages/instrumentation-mcp/tsconfig.jsonpackages/instrumentation-mcp/src/types.tspackages/ai-semantic-conventions/src/SemanticAttributes.tspackages/instrumentation-mcp/package.jsonpackages/instrumentation-mcp/src/index.tspackages/traceloop-sdk/src/lib/tracing/index.tspackages/instrumentation-mcp/src/instrumentation.tspackages/traceloop-sdk/package.jsonpackages/instrumentation-mcp/test/instrumentation.test.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/traceloop-sdk/src/lib/node-server-sdk.ts : Register new instrumentation packages during SDK initialization
Applied to files:
packages/traceloop-sdk/src/lib/tracing/span-processor.tspackages/sample-app/src/sample_mcp_simple.tspackages/instrumentation-mcp/package.jsonpackages/traceloop-sdk/src/lib/interfaces/initialize-options.interface.tspackages/instrumentation-mcp/src/index.tspackages/traceloop-sdk/src/lib/tracing/index.tspackages/instrumentation-mcp/src/instrumentation.tspackages/instrumentation-mcp/README.mdpackages/traceloop-sdk/package.jsonpackages/instrumentation-mcp/test/instrumentation.test.tspackages/sample-app/src/sample_mcp.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/traceloop-sdk/package.json : When adding a new instrumentation package, add it to the main SDK dependencies
Applied to files:
packages/traceloop-sdk/src/lib/tracing/span-processor.tspackages/sample-app/src/sample_mcp_simple.tspackages/instrumentation-mcp/package.jsonpackages/instrumentation-mcp/src/index.tspackages/traceloop-sdk/src/lib/tracing/index.tspackages/instrumentation-mcp/src/instrumentation.tspackages/instrumentation-mcp/README.mdpackages/traceloop-sdk/package.json
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/traceloop-sdk/**/*.{ts,tsx} : Use the provided decorators (workflow, task, agent) for workflow/task/agent spans instead of re-implementing them
Applied to files:
packages/traceloop-sdk/src/lib/tracing/span-processor.tspackages/sample-app/src/sample_mcp_simple.tspackages/ai-semantic-conventions/src/SemanticAttributes.tspackages/traceloop-sdk/src/lib/tracing/index.tspackages/traceloop-sdk/package.jsonpackages/instrumentation-mcp/test/instrumentation.test.tspackages/sample-app/src/sample_mcp.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/instrumentation-*/**/*.{ts,tsx} : Do not implement anonymous telemetry collection in instrumentation packages; telemetry is collected only in the SDK
Applied to files:
packages/traceloop-sdk/src/lib/tracing/span-processor.tspackages/instrumentation-mcp/tsconfig.jsonpackages/instrumentation-mcp/src/types.tspackages/sample-app/src/sample_mcp_simple.tspackages/instrumentation-mcp/package.jsonpackages/instrumentation-mcp/.prettierignorepackages/instrumentation-mcp/src/index.tspackages/traceloop-sdk/src/lib/tracing/index.tspackages/instrumentation-mcp/rollup.config.jspackages/instrumentation-mcp/src/instrumentation.tspackages/instrumentation-mcp/README.mdpackages/traceloop-sdk/package.jsonpackages/instrumentation-mcp/test/instrumentation.test.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/instrumentation-*/**/*.{ts,tsx} : Instrumentation classes must extend InstrumentationBase and register hooks using InstrumentationModuleDefinition
Applied to files:
packages/traceloop-sdk/src/lib/tracing/span-processor.tspackages/instrumentation-mcp/tsconfig.jsonpackages/instrumentation-mcp/src/types.tspackages/instrumentation-mcp/src/index.tspackages/traceloop-sdk/src/lib/tracing/index.tspackages/instrumentation-mcp/src/instrumentation.tspackages/instrumentation-mcp/test/instrumentation.test.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/instrumentation-*/**/*.{ts,tsx} : Instrumentations must extract request/response data and token usage from wrapped calls
Applied to files:
packages/traceloop-sdk/src/lib/tracing/span-processor.tspackages/instrumentation-mcp/tsconfig.jsonpackages/instrumentation-mcp/src/types.tspackages/instrumentation-mcp/package.jsonpackages/instrumentation-mcp/src/index.tspackages/traceloop-sdk/src/lib/tracing/index.tspackages/instrumentation-mcp/src/instrumentation.tspackages/traceloop-sdk/package.jsonpackages/instrumentation-mcp/test/instrumentation.test.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/instrumentation-*/**/*.{ts,tsx} : Instrumentations must capture and record errors appropriately
Applied to files:
packages/traceloop-sdk/src/lib/tracing/span-processor.tspackages/instrumentation-mcp/tsconfig.jsonpackages/instrumentation-mcp/src/types.tspackages/instrumentation-mcp/src/index.tspackages/traceloop-sdk/src/lib/tracing/index.tspackages/instrumentation-mcp/src/instrumentation.tspackages/instrumentation-mcp/test/instrumentation.test.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Use ts-mocha as the TypeScript test runner for package tests
Applied to files:
packages/instrumentation-mcp/tsconfig.jsonpackages/instrumentation-mcp/test/instrumentation.test.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/ai-semantic-conventions/src/SemanticAttributes.ts : Define all AI/LLM span attribute constants in packages/ai-semantic-conventions/src/SemanticAttributes.ts
Applied to files:
packages/ai-semantic-conventions/src/SemanticAttributes.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/*/package.json : Use workspace:* for intra-repo package dependencies in package.json
Applied to files:
packages/traceloop-sdk/package.json
📚 Learning: 2025-07-28T14:30:22.017Z
Learnt from: galkleinman
Repo: traceloop/openllmetry-js PR: 620
File: packages/instrumentation-llamaindex/package.json:43-45
Timestamp: 2025-07-28T14:30:22.017Z
Learning: Lerna automatically converts `workspace:*` dependencies to actual published versions during the `lerna publish` command, so `workspace:*` in package.json files is safe and expected in Lerna-managed monorepos.
Applied to files:
packages/traceloop-sdk/package.json
🧬 Code graph analysis (2)
packages/instrumentation-mcp/src/types.ts (1)
packages/instrumentation-mcp/src/index.ts (1)
McpInstrumentationConfig(18-18)
packages/instrumentation-mcp/src/instrumentation.ts (3)
packages/instrumentation-mcp/src/types.ts (1)
McpInstrumentationConfig(19-30)packages/ai-semantic-conventions/src/SemanticAttributes.ts (1)
SpanAttributes(17-67)packages/ai-semantic-conventions/src/index.ts (1)
CONTEXT_KEY_ALLOW_TRACE_CONTENT(20-22)
🪛 GitHub Actions: CI
packages/sample-app/src/sample_mcp_simple.ts
[warning] 1-1: Prettier formatting issues found. Run 'pnpm prettier --write' to fix.
packages/instrumentation-mcp/src/instrumentation.ts
[warning] 1-1: Prettier formatting issues found. Run 'pnpm prettier --write' to fix.
packages/instrumentation-mcp/test/instrumentation.test.ts
[warning] 1-1: Prettier formatting issues found. Run 'pnpm prettier --write' to fix.
packages/sample-app/src/sample_mcp.ts
[warning] 1-1: Prettier formatting issues found. Run 'pnpm prettier --write' to fix.
🔇 Additional comments (25)
packages/instrumentation-mcp/.prettierignore (1)
1-2: Prettier ignore entries are appropriateIgnoring
distandnode_modulesfor this package is standard and looks good as-is.packages/instrumentation-mcp/tsconfig.json (1)
1-10: TypeScript project configuration looks consistentExtending the shared base config, emitting to
dist, and includingsrc+testis in line with the other instrumentation packages; no changes needed.packages/instrumentation-mcp/src/types.ts (1)
17-30: McpInstrumentationConfig shape and docs look goodExtending
InstrumentationConfigand addingtraceContent?: booleanplus anexceptionLoggerhook matches the rest of the SDK’s patterns; this interface looks ready to be used by the instrumentation implementation, assumingundefinedis treated as “log content” (default true) there.packages/instrumentation-mcp/src/index.ts (1)
1-18: Public exports for MCP instrumentation are clean and minimalRe-exporting
McpInstrumentationandMcpInstrumentationConfigfrom the package root matches the pattern of other instrumentation packages and provides a straightforward public API.packages/traceloop-sdk/src/lib/tracing/span-processor.ts (1)
150-165: MCP instrumentation name confirmedThe MCP instrumentation correctly declares its name as
"@traceloop/instrumentation-mcp"(atpackages/instrumentation-mcp/src/instrumentation.ts:71), which exactly matches the entry added totraceloopInstrumentationLibraries. The span filtering will work as intended.packages/ai-semantic-conventions/src/SemanticAttributes.ts (2)
63-66: LGTM! MCP semantic attributes follow conventions.The new MCP-specific attributes follow the established semantic convention patterns and naming scheme. The attribute keys use the
mcp.*namespace consistently.
124-124: LGTM! SESSION span kind addition is consistent.The SESSION span kind value is properly added to the enum and follows the existing pattern for span kind values.
packages/traceloop-sdk/package.json (2)
75-75: LGTM! MCP instrumentation dependency added correctly.The new instrumentation package is properly added to dependencies using
workspace:*, which follows the established pattern and coding guidelines. Based on learnings, this will be converted to actual version duringlerna publish.
99-99: LGTM! MCP SDK devDependency added appropriately.The MCP SDK is correctly placed in devDependencies with a specific version constraint, appropriate for testing and development purposes.
packages/traceloop-sdk/src/lib/interfaces/initialize-options.interface.ts (2)
14-14: LGTM! MCP SDK import follows established pattern.The type import for the MCP SDK follows the same pattern used for other optional instrumentation modules in this file.
102-102: LGTM! MCP module option added consistently.The
mcpfield is properly added toinstrumentModules, following the same optional pattern as other instrumentation modules likeopenAI,anthropic, etc.packages/sample-app/src/sample_mcp_simple.ts (2)
16-24: LGTM! Correct initialization pattern for ESM modules.The sample correctly imports the MCP SDK before calling
traceloop.initialize()and passes it viainstrumentModules, which is the proper pattern for ESM module instrumentation.
1-1: Fix Prettier formatting issues.The CI pipeline reports Prettier formatting issues that must be resolved before merging.
Run the following command to fix:
pnpm prettier --write packages/sample-app/src/sample_mcp_simple.ts⛔ Skipped due to learnings
Learnt from: CR Repo: traceloop/openllmetry-js PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-08-24T22:08:07.023Z Learning: Applies to packages/traceloop-sdk/**/*.{ts,tsx} : For manual LLM operations, use trace.withLLMSpan from traceloop/node-server-sdkLearnt from: CR Repo: traceloop/openllmetry-js PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-08-24T22:08:07.023Z Learning: Applies to packages/traceloop-sdk/src/lib/node-server-sdk.ts : Register new instrumentation packages during SDK initializationLearnt from: galzilber Repo: traceloop/openllmetry-js PR: 643 File: packages/traceloop-sdk/test/datasets-final.test.ts:97-105 Timestamp: 2025-08-12T13:57:05.901Z Learning: The traceloop-sdk uses a response transformer (`transformApiResponse` in `packages/traceloop-sdk/src/lib/utils/response-transformer.ts`) that converts snake_case API responses to camelCase for SDK interfaces. Raw API responses use snake_case but SDK consumers see camelCase fields.Learnt from: galzilber Repo: traceloop/openllmetry-js PR: 643 File: packages/traceloop-sdk/test/datasets-final.test.ts:129-143 Timestamp: 2025-08-12T13:57:42.538Z Learning: The Traceloop SDK includes a response transformer utility (transformApiResponse) that automatically converts snake_case API responses to camelCase for SDK consumers. This means interface definitions and tests should use camelCase field names (like datasetId, datasetSlug, createdAt, updatedAt) rather than the raw API's snake_case format.Learnt from: CR Repo: traceloop/openllmetry-js PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-08-24T22:08:07.023Z Learning: Applies to packages/traceloop-sdk/package.json : When adding a new instrumentation package, add it to the main SDK dependenciesLearnt from: nirga Repo: traceloop/openllmetry-js PR: 623 File: packages/traceloop-sdk/src/lib/images/image-uploader.ts:71-78 Timestamp: 2025-08-10T06:42:06.412Z Learning: In the Traceloop SDK's ImageUploader class (packages/traceloop-sdk/src/lib/images/image-uploader.ts), the Authorization header with bearer token IS required when uploading image data to the URL returned by the Traceloop API's `/v2/traces/{traceId}/spans/{spanId}/images` endpoint. This is not a pre-signed URL pattern but a Traceloop-specific API endpoint that maintains the same authentication requirements.packages/sample-app/src/sample_mcp.ts (3)
16-33: LGTM! Proper ESM instrumentation pattern demonstrated.The sample correctly demonstrates the required pattern for instrumenting ESM modules:
- Import MCP SDK modules before
traceloop.initialize()- Pass the module via
instrumentModulesThis is essential for Next.js and other ESM environments.
41-92: LGTM! Well-structured MCP server setup.The server configuration with tools (add, multiply) and resources (Calculator Info) provides a clear demonstration of MCP capabilities and will help users understand how to structure their own MCP servers.
1-1: Fix Prettier formatting issues.The CI pipeline reports Prettier formatting issues that must be resolved before merging.
Run the following command to fix:
pnpm prettier --write packages/sample-app/src/sample_mcp.ts⛔ Skipped due to learnings
Learnt from: CR Repo: traceloop/openllmetry-js PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-08-24T22:08:07.023Z Learning: Applies to packages/traceloop-sdk/**/*.{ts,tsx} : For manual LLM operations, use trace.withLLMSpan from traceloop/node-server-sdkLearnt from: CR Repo: traceloop/openllmetry-js PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-08-24T22:08:07.023Z Learning: Applies to packages/traceloop-sdk/src/lib/node-server-sdk.ts : Register new instrumentation packages during SDK initializationLearnt from: galzilber Repo: traceloop/openllmetry-js PR: 643 File: packages/traceloop-sdk/test/datasets-final.test.ts:97-105 Timestamp: 2025-08-12T13:57:05.901Z Learning: The traceloop-sdk uses a response transformer (`transformApiResponse` in `packages/traceloop-sdk/src/lib/utils/response-transformer.ts`) that converts snake_case API responses to camelCase for SDK interfaces. Raw API responses use snake_case but SDK consumers see camelCase fields.Learnt from: galzilber Repo: traceloop/openllmetry-js PR: 643 File: packages/traceloop-sdk/test/datasets-final.test.ts:129-143 Timestamp: 2025-08-12T13:57:42.538Z Learning: The Traceloop SDK includes a response transformer utility (transformApiResponse) that automatically converts snake_case API responses to camelCase for SDK consumers. This means interface definitions and tests should use camelCase field names (like datasetId, datasetSlug, createdAt, updatedAt) rather than the raw API's snake_case format.Learnt from: CR Repo: traceloop/openllmetry-js PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-08-24T22:08:07.023Z Learning: Applies to packages/traceloop-sdk/package.json : When adding a new instrumentation package, add it to the main SDK dependenciesLearnt from: nirga Repo: traceloop/openllmetry-js PR: 623 File: packages/traceloop-sdk/src/lib/images/image-uploader.ts:71-78 Timestamp: 2025-08-10T06:42:06.412Z Learning: In the Traceloop SDK's ImageUploader class (packages/traceloop-sdk/src/lib/images/image-uploader.ts), the Authorization header with bearer token IS required when uploading image data to the URL returned by the Traceloop API's `/v2/traces/{traceId}/spans/{spanId}/images` endpoint. This is not a pre-signed URL pattern but a Traceloop-specific API endpoint that maintains the same authentication requirements.packages/instrumentation-mcp/package.json (1)
1-61: LGTM! Package manifest follows established patterns.The package manifest correctly follows the structure of other instrumentation packages in the monorepo:
- Uses
workspace:*for internal dependencies (as per coding guidelines)- Includes appropriate scripts, keywords, and metadata
- Specifies correct Node version constraints
- Declares proper file exports and publish configuration
packages/instrumentation-mcp/rollup.config.js (1)
1-39: LGTM! Rollup configuration follows established pattern.The configuration correctly mirrors the structure used in other instrumentation packages:
- Proper bundle helper with external dependency detection
- Dual output formats (CJS and ESM)
- TypeScript declarations generated separately
- Test files excluded from builds
- Sourcemaps enabled for debugging
packages/instrumentation-mcp/eslint.config.cjs (1)
1-29: LGTM! ESLint configuration follows established pattern.The configuration properly:
- Extends the root ESLint config for consistency
- Defines appropriate ignore patterns (node_modules, dist)
- Sets up placeholder rule blocks for future customization
- Uses FlatCompat for compatibility with legacy configs
packages/instrumentation-mcp/test/instrumentation.test.ts (2)
1-357: Run Prettier to satisfy CI.CI reports Prettier issues for this file. Please run
pnpm prettier --write packages/instrumentation-mcp/test/instrumentation.test.tsand commit the result so the pipeline passes.
259-356: Tool span E2E test is comprehensive and aligns with instrumentation behavior.This test nicely exercises the full client/server flow:
- Uses
manualInstrumenton the MCP client module.- Spins up a real
McpServerwith a registered tool and anInMemoryTransportpair.- Verifies both session and tool spans, including:
- span kinds (
"session"/"tool"viaTRACELOOP_SPAN_KIND),- entity name (
"test_tool"),- serialized input (
tool_name,arguments) and output (result) viaTRACELOOP_ENTITY_INPUT/OUTPUT.This gives strong coverage of the core MCP tool-call semantics and
traceContentbehavior (when enabled by default).packages/instrumentation-mcp/src/instrumentation.ts (4)
1-708: Run Prettier on this file as well.CI reports Prettier formatting issues here. Please run:
pnpm prettier --write packages/instrumentation-mcp/src/instrumentation.tsand commit the formatted output so the pipeline passes.
67-111: Instrumentation wiring and module definitions look correct and follow guidelines.
McpInstrumentationcorrectly:
- Extends
InstrumentationBaseand wires the package name/version.- Returns
InstrumentationNodeModuleDefinitionentries for both client and server, including/index.jssubpaths, covering common import styles.- Delegates to
patchClient/patchServerand their unpatch counterparts.This aligns with the instrumentation guidelines for this repo and provides a clean, predictable lifecycle.
237-307: Session span lifecycle is well-structured and error-safe.
_wrapConnectand_wrapClosetogether provide a clean session model:
connectstarts amcp.client.sessionspan, stores both the session context and span on the instance, and keeps the span open across the session.- Normal connect and close paths set
SpanStatusCode.OK.- Errors on connect or close record the exception, set
SpanStatusCode.ERROR, end the span, and clear the stored symbols to avoid leaks.This gives robust tracking of client sessions while handling both sync and async connect/close flows.
692-707:traceContentgating via context key and config matches repo conventions.
_shouldSendPromptscorrectly:
- Checks the current context for
CONTEXT_KEY_ALLOW_TRACE_CONTENT, honoring any per-request override.- Falls back to
this._config.traceContentwhen the context key is unset.- Defaults to
truewhen neither is provided.This matches the documented pattern in this repo for controlling whether prompt/response content is attached to spans.
| "tslib": "^2.8.1" | ||
| }, | ||
| "devDependencies": { | ||
| "@modelcontextprotocol/sdk": "^1.0.4", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify MCP SDK version compatibility.
Ensure that version ^1.0.4 of @modelcontextprotocol/sdk is the latest stable version and compatible with the instrumentation implementation.
🌐 Web query:
What is the latest stable version of @modelcontextprotocol/sdk npm package?
💡 Result:
The latest stable version is 1.22.0 (released Nov 13, 2025). [1][2]
Sources:
[1] GitHub releases for modelcontextprotocol/typescript-sdk.
[2] ReleaseAlert listing for @modelcontextprotocol/sdk.
🌐 Web query:
@modelcontextprotocol/sdk breaking changes between version 1.0.4 and 1.22.0 migration guide
💡 Result:
Summary of notable breaking or migration-relevant changes from @modelcontextprotocol/sdk v1.0.4 → v1.22.0 and migration steps
Key changes you must review and adapt for:
- registerTool now accepts ZodType for input/output schemas (may change typings and how you pass schemas). [1]
- Decoupled request payloads & removal of passthrough iteration (SEP-1319) — request/response shapes and iteration behavior were refactored; check code that relied on passthrough semantics. [1]
- Default values for elicitation schemas (SEP-1034) and compatibility updates (SEP-1330) — elicitation schema handling may now inject defaults; validate your elicitation handlers. [1]
- New formal format rules for tool names (SEP-986) — tool name validation/format stricter; update existing tool names if they violate the new format. [1]
- Auth / scope handling changes (alignment with SEP-835 and related fixes) — authorization flows and scope parsing/management have been adjusted; confirm your auth integrations and 401/403 handling. [1]
- Node LTS and runtime changes — SDK now targets a newer Node LTS; update your Node runtime if necessary. [1]
- Typecheck / typing fixes and request payload typing changes — TypeScript consumers may need small code changes to satisfy stricter typechecks. [1]
- Zod-related updates (zod v4 support in later beta; v3 compatibility notes) — if you use zod schemas, check peer/required zod versions and adjust imports/usage accordingly. [1]
- Client APIs: small feature additions to ClientSessionGroup (Python SDK example: pass-through/expose additional params to call_tool / connect_to_server) — verify any advanced client session uses. [3]
Practical migration steps
- Read the full changelog/compare between your v1.0.4 tag and v1.22.0; scan SEP numbers referenced above for relevant protocol semantics. Start with the SDK releases page. [1]
- Update TypeScript typings and compiles: run tsc/tests and fix any type errors introduced by the decoupling/typecheck fixes. [1]
- Review all registerTool calls and tool-name strings; migrate schema arguments to ZodType where appropriate. [1]
- Re-run auth flows and tests that exercise 401/403 and scope escalation; adjust code that parsed WWW-Authenticate scope or relied on passthrough iteration. [1]
- If you use zod, check/upgrade zod to the required peer version (v3.25+ or v4 for beta features) and test schema behavior. [1]
- Upgrade Node to the LTS version required by the newer SDK and run integration tests (SSE/stdio transports, elicitation flows). [1]
- For Python consumers (if applicable), review the Python SDK v1.22.0 release notes for ClientSessionGroup changes. [3]
References
- modelcontextprotocol/typescript-sdk releases (includes v1.22.0 changelog and SEPs mentioned). [1]
- @modelcontextprotocol/sdk v1.0.4 package info (npm). [2]
- modelcontextprotocol/python-sdk v1.22.0 release notes (ClientSessionGroup notes). [3]
If you want, I can:
- produce a line-by-line diff of breaking API changes by comparing your code usage to the SDK’s public types (I can fetch the compare URL and parse relevant commits), or
- fetch the full commit/PR list between 1.0.4 and 1.22.0 and extract exact code-level changes you’ll need to make. Which do you prefer?
---
🏁 Script executed:
```shell
#!/bin/bash
# Find instrumentation-mcp source files and examine SDK usage
find packages/instrumentation-mcp -type f \( -name "*.ts" -o -name "*.js" \) ! -path "*/node_modules/*" ! -path "*/dist/*" | head -20
Length of output: 239
🏁 Script executed:
#!/bin/bash
# Search for MCP SDK imports and API usage in instrumentation-mcp
rg "@modelcontextprotocol/sdk|registerTool|Client|Server|Tool" packages/instrumentation-mcp -n -C3 --type ts --type jsLength of output: 27425
🏁 Script executed:
#!/bin/bash
# Check the main entry point and structure of instrumentation-mcp
cat packages/instrumentation-mcp/package.json | grep -A5 -B5 '"main"\|"exports"'Length of output: 305
🏁 Script executed:
#!/bin/bash
# Examine the instrumentation-mcp implementation files
ls -la packages/instrumentation-mcp/src/ 2>/dev/null || echo "src directory not found, checking structure:"
find packages/instrumentation-mcp -maxdepth 3 -type f -name "*.ts" -o -name "*.js" | grep -v node_modules | head -15Length of output: 540
🏁 Script executed:
#!/bin/bash
# Look for any version constraints or compatibility notes in the package
cat packages/instrumentation-mcp/package.jsonLength of output: 1633
Update @modelcontextprotocol/sdk devDependency to a more recent version.
The registerTool ZodType support added in v1.22.0 is backwards-compatible with existing plain-object schema definitions, so the current test code remains compatible. However, the pinned version ^1.0.4 in devDependencies is significantly outdated. Version 1.22.0 (released Nov 13, 2025) includes multiple improvements including request payload decoupling, TypeScript typecheck fixes, and stricter tool name format validation. While the instrumentation's version constraint (>=1.0.0) remains broad, updating to ^1.22.0 or a recent stable version will ensure tests run against the latest SDK with latest bugfixes and features. No code changes are required since the test patterns using registerTool with plain descriptive objects remain compatible.
🤖 Prompt for AI Agents
In packages/instrumentation-mcp/package.json around line 49, the devDependency
"@modelcontextprotocol/sdk" is pinned to the outdated version "^1.0.4"; update
the version string to a recent release that includes registerTool ZodType
support and fixes (for example "^1.22.0"), then reinstall dependencies
(npm/yarn/pnpm install) and commit the updated package.json and lockfile so
CI/tests run against the newer SDK.
| [![NPM Published Version][npm-img]][npm-url] | ||
| [![Apache License][license-image]][license-image] |
There was a problem hiding this comment.
Fix license badge link to point at LICENSE URL
The Apache license badge currently links to the image itself instead of the LICENSE page:
[![Apache License][license-image]][license-image]Consider updating it to use the defined license-url reference:
-[![Apache License][license-image]][license-image]
+[![Apache License][license-image]][license-url]🤖 Prompt for AI Agents
In packages/instrumentation-mcp/README.md around lines 3-4, the Apache license
badge is linking to the image reference itself rather than the LICENSE page;
update the badge link target to use the defined license-url reference (e.g.,
change the trailing reference from [license-image] to [license-url]) so the
badge points to the LICENSE page instead of the image file.
| it("should respect traceContent config", async () => { | ||
| // Create instrumentation with traceContent disabled | ||
| instrumentation.setConfig({ traceContent: false }); | ||
|
|
||
| const { Client } = await import( | ||
| "@modelcontextprotocol/sdk/client/index.js" | ||
| ); | ||
|
|
||
| const client = new Client( | ||
| { | ||
| name: "test-client", | ||
| version: "1.0.0", | ||
| }, | ||
| { | ||
| capabilities: {}, | ||
| }, | ||
| ); | ||
|
|
||
| // Mock the request method | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (client as any).request = async function (request: any) { | ||
| if (request.method === "tools/call") { | ||
| return { | ||
| content: [ | ||
| { | ||
| type: "text", | ||
| text: "sensitive data", | ||
| }, | ||
| ], | ||
| }; | ||
| } | ||
| }; | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| await (client as any).request({ | ||
| method: "tools/call", | ||
| params: { | ||
| name: "test_tool", | ||
| arguments: { data: "sensitive" }, | ||
| }, | ||
| }); | ||
|
|
||
| await new Promise((resolve) => setTimeout(resolve, 100)); | ||
|
|
||
| const spans = memoryExporter.getFinishedSpans(); | ||
| const toolSpans = spans.filter((s) => s.name === "test_tool.tool"); | ||
|
|
||
| if (toolSpans.length > 0) { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const toolSpan = toolSpans[0] as any; | ||
| // When traceContent is false, input/output should not be present | ||
| assert.strictEqual( | ||
| toolSpan.attributes[SpanAttributes.TRACELOOP_ENTITY_INPUT], | ||
| undefined, | ||
| ); | ||
| assert.strictEqual( | ||
| toolSpan.attributes[SpanAttributes.TRACELOOP_ENTITY_OUTPUT], | ||
| undefined, | ||
| ); | ||
| } | ||
|
|
||
| // Reset config | ||
| instrumentation.setConfig({ traceContent: true }); | ||
| }); | ||
|
|
||
| it("should handle errors in tool calls", async () => { | ||
| const { Client } = await import( | ||
| "@modelcontextprotocol/sdk/client/index.js" | ||
| ); | ||
|
|
||
| const client = new Client( | ||
| { | ||
| name: "test-client", | ||
| version: "1.0.0", | ||
| }, | ||
| { | ||
| capabilities: {}, | ||
| }, | ||
| ); | ||
|
|
||
| // Mock the request method to return an error | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (client as any).request = async function (request: any) { | ||
| if (request.method === "tools/call") { | ||
| return { | ||
| content: [ | ||
| { | ||
| type: "text", | ||
| text: "Error: Something went wrong", | ||
| }, | ||
| ], | ||
| isError: true, | ||
| }; | ||
| } | ||
| }; | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const result = await (client as any).request({ | ||
| method: "tools/call", | ||
| params: { | ||
| name: "error_tool", | ||
| arguments: {}, | ||
| }, | ||
| }); | ||
|
|
||
| assert.strictEqual(result.isError, true); | ||
|
|
||
| await new Promise((resolve) => setTimeout(resolve, 100)); | ||
|
|
||
| const spans = memoryExporter.getFinishedSpans(); | ||
| const toolSpans = spans.filter((s) => s.name === "error_tool.tool"); | ||
|
|
||
| if (toolSpans.length > 0) { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const toolSpan = toolSpans[0] as any; | ||
| // Span should have error status | ||
| assert.strictEqual(toolSpan.status.code, 2); // SpanStatusCode.ERROR | ||
| } | ||
| }); |
There was a problem hiding this comment.
traceContent and error tests currently bypass the instrumentation wrapper.
Both "should respect traceContent config" and "should handle errors in tool calls" override request on the client instance:
(client as any).request = async function (...) { ... }
Because the instrumentation wraps Client.prototype.request, this per-instance override completely sidesteps the wrapper, so no spans are produced and these tests pass even if the instrumentation ignores traceContent or never sets error status. The if (toolSpans.length > 0) guard then causes the assertions to be skipped in the common case.
I recommend changing these tests so they exercise the real instrumented path, e.g.:
- Use the same
InMemoryTransport+McpServer.registerToolpattern you already use in the later E2E test, with a tool that returns sensitive data / an error response. - Or otherwise avoid overriding
requeston the instance; instead, let the actual client/server implementation run under instrumentation.
That way, the tests will reliably fail if traceContent handling or error propagation regress.
| "run:mcp": "npm run build && node dist/src/sample_mcp.js", | ||
| "run:mcp:real": "npm run build && node dist/src/sample_mcp_real.js", | ||
| "run:mcp:working": "npm run build && node dist/src/sample_mcp_working.js", |
There was a problem hiding this comment.
🧩 Analysis chain
Confirm MCP sample entrypoints and MCP SDK wiring
The new MCP scripts and dependency look consistent with the rest of the sample app, but they rely on specific entrypoints:
run:mcp→dist/src/sample_mcp.jsrun:mcp:real→dist/src/sample_mcp_real.jsrun:mcp:working→dist/src/sample_mcp_working.js
Please verify that:
- Matching TypeScript sources exist under
packages/sample-app/src/(e.g.,sample_mcp.ts,sample_mcp_real.ts,sample_mcp_working.ts), and - After a build, the corresponding JS files are emitted under
packages/sample-app/dist/src/.
You may also want to double-check that @modelcontextprotocol/sdk@^1.22.0 matches the APIs used in those samples.
Also applies to: 65-65
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking MCP sample TypeScript sources..."
fd 'sample_mcp' packages/sample-app/src -e ts -e tsx || echo "No MCP TS sources found under packages/sample-app/src"
echo
echo "If you've already built the sample app, checking for built MCP JS entrypoints..."
if [ -d packages/sample-app/dist/src ]; then
ls -1 packages/sample-app/dist/src | grep -E 'sample_mcp(_real|_working)?\.js' || echo "Expected MCP JS entrypoints not found in packages/sample-app/dist/src"
else
echo "packages/sample-app/dist/src does not exist. Run the build for sample-app before re-running this check."
fiLength of output: 310
I need to verify the actual package.json content to understand the mismatch between expected and actual source files.
I need to check the actual package.json to see what scripts are defined and identify the discrepancy.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Reading package.json lines 35-70 (to see scripts around lines 41-43 and 65) ==="
sed -n '35,70p' packages/sample-app/package.json
echo
echo "=== Searching for all MCP-related TypeScript files under packages/sample-app/src ==="
find packages/sample-app/src -name "*mcp*" -type f | sort
echo
echo "=== Checking if there are any other sample files we might have missed ==="
ls -la packages/sample-app/src/ 2>/dev/null | head -20Length of output: 159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Reading package.json lines 35-70 (to see scripts around lines 41-43 and 65) ==="
sed -n '35,70p' packages/sample-app/package.json
echo
echo "=== Searching for all MCP-related TypeScript files under packages/sample-app/src ==="
find packages/sample-app/src -name "*mcp*" -type f | sort
echo
echo "=== Full directory listing of packages/sample-app/src/ ==="
ls -la packages/sample-app/src/Length of output: 4346
MCP scripts reference non-existent TypeScript source files
Verification confirms a critical issue: The package.json scripts point to files that do not exist in the codebase.
Expected files (referenced in scripts):
sample_mcp_real.ts→ referenced byrun:mcp:realscript — NOT FOUNDsample_mcp_working.ts→ referenced byrun:mcp:workingscript — NOT FOUND
Actual files present:
sample_mcp.ts✓ (correctly referenced byrun:mcp)sample_mcp_simple.ts(exists but not referenced by any script)
Running npm run run:mcp:real or npm run run:mcp:working will fail because the TypeScript sources do not exist. Either create the missing sample_mcp_real.ts and sample_mcp_working.ts files, or update the package.json scripts to reference files that actually exist (such as renaming sample_mcp_simple.ts and adjusting scripts accordingly).
🤖 Prompt for AI Agents
In packages/sample-app/package.json around lines 41-43 the npm scripts
run:mcp:real and run:mcp:working point to compiled JS files that originate from
TypeScript sources that do not exist (sample_mcp_real.ts and
sample_mcp_working.ts); fix by either creating the missing TypeScript files with
the intended implementations (sample_mcp_real.ts and sample_mcp_working.ts) and
ensuring they are included in the build, or modify the scripts to point to
existing sources (for example change the script targets to the compiled outputs
of sample_mcp_simple.ts or sample_mcp.ts) and update any build configuration if
necessary so the dist/src filenames match the script paths.
| import { McpInstrumentation } from "@traceloop/instrumentation-mcp"; | ||
| import { |
There was a problem hiding this comment.
🧩 Analysis chain
MCP instrumentation integration matches existing pattern; ensure SDK dependency and types
Importing McpInstrumentation, storing it in mcpInstrumentation, and wiring it into both initInstrumentations and manuallyInitInstrumentations is consistent with the other instrumentation modules.
Please verify that:
packages/traceloop-sdk/package.jsondeclares@traceloop/instrumentation-mcpas a dependency (ideallyworkspace:*, like the other instrumentation packages), andInitializeOptions["instrumentModules"]includes anmcpfield with the correct MCP module type soinstrumentModules?.mcpis type-safe.
Based on learnings, this keeps the new instrumentation package properly wired into the SDK dependencies and initialization path.
Also applies to: 49-49, 127-128, 239-243
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking SDK dependency on @traceloop/instrumentation-mcp..."
if [ -f packages/traceloop-sdk/package.json ]; then
cat packages/traceloop-sdk/package.json | jq '.dependencies["@traceloop/instrumentation-mcp"]'
else
echo "packages/traceloop-sdk/package.json not found"
fi
echo
echo "Searching InitializeOptions.instrumentModules definition (including mcp)..."
fd 'initialize-options' packages/traceloop-sdk/src/lib -e ts || true
rg -n "InitializeOptions" packages/traceloop-sdk/src/lib -C3 || true
rg -n "instrumentModules" packages/traceloop-sdk/src/lib/interfaces -C3 || true
rg -n "\bmcp\b" packages/traceloop-sdk/src/lib/interfaces -C3 || trueLength of output: 10959
I need to verify one additional concern about privacy/configuration gating for MCP that was noted in the original scratchpad.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking traceContent gating logic for MCP instrumentation..."
rg -n "traceContent" packages/traceloop-sdk/src/lib/tracing/index.ts -C5 || true
echo
echo "Lines 239-243 context (startTracing gating block):"
sed -n '235,250p' packages/traceloop-sdk/src/lib/tracing/index.ts || true
echo
echo "Checking where MCP is initialized/used in tracing/index.ts:"
rg -n "mcpInstrumentation\|MCP" packages/traceloop-sdk/src/lib/tracing/index.ts -B2 -A2 || trueLength of output: 441
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking traceContent gating logic for MCP instrumentation..."
rg -n "traceContent" packages/traceloop-sdk/src/lib/tracing/index.ts -C5 || true
echo
echo "Lines 235-250 context (startTracing gating block):"
sed -n '235,250p' packages/traceloop-sdk/src/lib/tracing/index.ts || true
echo
echo "Checking where MCP is initialized/used in tracing/index.ts:"
rg -n "mcpInstrumentation\|MCP" packages/traceloop-sdk/src/lib/tracing/index.ts -B2 -A2 || trueLength of output: 1953
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking anthropic instrumentation initialization..."
rg -n "anthropicInstrumentation" packages/traceloop-sdk/src/lib/tracing/index.ts -B2 -A2 || trueLength of output: 888
MCP instrumentation dependency and types are correct; add missing traceContent privacy gating
Verified that:
@traceloop/instrumentation-mcpis correctly declared asworkspace:*inpackages/traceloop-sdk/package.jsonInitializeOptions.instrumentModules.mcpis properly typed
However, mcpInstrumentation is missing from the traceContent: false gating block (lines 266–291). When shouldSendTraces() returns false, the SDK sets traceContent: false on eight other instrumentations but omits both anthropic and mcp. This creates a privacy inconsistency where trace content could be sent despite user configuration.
Add both anthropicInstrumentation and mcpInstrumentation to the privacy gating block:
if (!shouldSendTraces()) {
openAIInstrumentation?.setConfig({
traceContent: false,
});
// ... existing instrumentations ...
anthropicInstrumentation?.setConfig({
traceContent: false,
});
mcpInstrumentation?.setConfig({
traceContent: false,
});
}🤖 Prompt for AI Agents
In packages/traceloop-sdk/src/lib/tracing/index.ts around lines 266 to 291, the
privacy gating block that sets traceContent: false when shouldSendTraces()
returns false omits anthropicInstrumentation and mcpInstrumentation; update that
block to call anthropicInstrumentation?.setConfig({ traceContent: false }) and
mcpInstrumentation?.setConfig({ traceContent: false }) alongside the other
instrumentations so trace content is consistently disabled when
shouldSendTraces() is false.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 36a3c4e in 1 minute and 20 seconds. Click for details.
- Reviewed
119lines of code in2files - Skipped
0files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/instrumentation-mcp/src/instrumentation.ts:146
- Draft comment:
Refactored the wrapping of the 'close' method into a single-line call. This formatting change improves readability—ensure consistent style across similar wrappers. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/instrumentation-mcp/src/instrumentation.ts:172
- Draft comment:
Debug logging calls in patchClient/unpatchClient and similar methods have been reformatted into multi-line templates. This is a stylistic change; just ensure consistent formatting throughout the codebase. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. packages/instrumentation-mcp/src/instrumentation.ts:244
- Draft comment:
The span attribute setting in _wrapConnect has been consolidated into a single-line call. This improves clarity without changing functionality. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. packages/instrumentation-mcp/test/instrumentation.test.ts:59
- Draft comment:
Import statements have been condensed from a multi-line to a single-line format for improved consistency. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. packages/instrumentation-mcp/test/instrumentation.test.ts:341
- Draft comment:
The formatting for JSON.parse calls used in verifying span attributes has been reformatted for clarity. This change is purely stylistic. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_8E7KUUziQEaLfgOu
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed dda8013 in 1 minute and 31 seconds. Click for details.
- Reviewed
101lines of code in2files - Skipped
0files when reviewing. - Skipped posting
6draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/sample-app/src/sample_mcp.ts:17
- Draft comment:
Multiline dynamic imports improve readability. Ensure this formatting is consistent with the project style. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/sample-app/src/sample_mcp.ts:124
- Draft comment:
Reformatted object literal for client.callTool improves readability. This style is clear and aligns with common formatting practices. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. packages/sample-app/src/sample_mcp.ts:149
- Draft comment:
Logging changes for trace structure are now multiline for better clarity. Consider consolidating related log entries into a single multiline string if appropriate. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. packages/sample-app/src/sample_mcp.ts:180
- Draft comment:
Using a fixed setTimeout delay to wait for trace export works in samples, but consider a more robust synchronization mechanism in production. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. packages/sample-app/src/sample_mcp_simple.ts:42
- Draft comment:
Overriding the client's request method using 'as any' bypasses type safety. Consider defining a proper interface or type for improved maintainability. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. packages/sample-app/src/sample_mcp_simple.ts:173
- Draft comment:
Multiline formatting for logging trace structure enhances clarity. Verify that all log messages remain consistently formatted across the sample. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_e6KUqZOG0RlTnptu
You can customize
Important
Adds MCP instrumentation for automatic tracing of client and server operations with new configuration options and sample applications.
instrumentation.ts.traceContentconfig option to control request/response content tracing.MCP_RESPONSE_VALUEandMCP_REQUEST_IDinSemanticAttributes.ts.README.mdfor MCP instrumentation.sample_mcp.tsandsample_mcp_simple.tsdemonstrating MCP integration.package.jsonto include MCP dependencies and scripts.eslint.config.cjsandrollup.config.jsfor MCP package configuration.instrumentation.test.tsfor testing MCP instrumentation functionality.This description was created by
for dda8013. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
traceContent)Documentation
Samples
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.