feat(mcp): respect TRACELOOP_TRACE_CONTENT=false in standard MCP instrumentor

[milkisbad]

The standard MCP instrumentor was unconditionally recording span input/output attributes, unlike the FastMCP instrumentor which already honored the TRACELOOP_TRACE_CONTENT env var. Guards all content attributes (TRACELOOP_ENTITY_INPUT, TRACELOOP_ENTITY_OUTPUT, MCP_RESPONSE_VALUE) behind the same _should_send_prompts() check, enabling workflows that cannot store prompts/responses to opt out. Closes #4188.

• I have added tests that cover my changes.
• If adding a new instrumentation or changing an existing one, I've added screenshots from some observability platform showing the change.
• PR name follows conventional commits format: feat(instrumentation): ... or fix(instrumentation): ....
• (If applicable) I have updated the documentation accordingly.

screens

Details

- disabled confidential information error pass: - diabled confidential information happy path: - enabled confidential information error pass: - enabled confidential information happy path:

Summary by CodeRabbit

• New Features
  • Centralized gating for prompt/content tracing: when enabled, tool and MCP spans include input/output and response value details (using cleaned output when configured).
  • When disabled, sensitive content (including tool error text and response/error descriptions) is suppressed, while operational metadata and span identity still appear.
• Tests
  • Added span-inspection coverage for TRACELOOP_TRACE_CONTENT across tool spans, MCP method spans, and error paths (ensuring no redacted sentinel text leaks).
• Documentation
  • Updated README with a “what is and isn’t gated” breakdown of always-on versus content-gated span fields.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Centralize TRACELOOP_TRACE_CONTENT handling into utils.should_send_prompts() helper and constant SUPPRESSED_ERROR_DESCRIPTION; update FastMCP and MCP instrumentors to conditionally emit prompt/result span attributes (input/output/response value), conditionally record exceptions, and conditionally set detailed error text; add comprehensive tests covering attribute suppression on tool spans and MCP method spans, attribute inclusion when enabled, and error-path redaction to verify sensitive data is not leaked when content tracing is disabled.

Changes

Trace content control for MCP instrumentation

Layer / File(s)	Summary
Shared utility helper and constant packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/utils.py	Add os import, introduce SUPPRESSED_ERROR_DESCRIPTION constant, and implement should_send_prompts() -> bool that reads TRACELOOP_TRACE_CONTENT environment variable (defaulting to "true").
FastMCP instrumentation with conditional error handling packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py	Import should_send_prompts and SUPPRESSED_ERROR_DESCRIPTION; disable SDK exception recording on spans; conditionally record exceptions and set error status text based on _should_send_prompts(); delegate _should_send_prompts() to shared helper.
MCP instrumentation with conditional span attributes and error suppression packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/instrumentation.py	Import should_send_prompts; add McpInstrumentor._should_send_prompts() delegate; gate TRACELOOP_ENTITY_INPUT in tool-call and MCP-method handlers; gate TRACELOOP_ENTITY_OUTPUT and error status text in result handling (suppress exception recording when disabled); gate MCP_RESPONSE_VALUE and error text in stream writer.
Documentation and comprehensive tests for trace content behavior packages/opentelemetry-instrumentation-mcp/README.md, packages/opentelemetry-instrumentation-mcp/tests/test_trace_content.py	Add README section explaining which trace data is always captured versus gated by TRACELOOP_TRACE_CONTENT; add four async tests verifying suppression of input/output attributes on tool and MCP-method spans when disabled while retaining span identity, inclusion when enabled, and error sentinel string redaction from span status and exception attributes when disabled.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A flag to guide what spans may speak,
Prompts stay hidden when we're discreet,
Errors whisper, details meek,
FastMCP and MCP now compete—
To trace content, or silent, neat.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title accurately describes the main change: implementing TRACELOOP_TRACE_CONTENT=false support in the standard MCP instrumentor, which is the core objective of this PR.
Linked Issues check	✅ Passed	The PR fully satisfies issue #4188's requirement to implement TRACELOOP_TRACE_CONTENT=false support in the standard MCP instrumentor with proper gating of content attributes and error paths.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to implementing TRACELOOP_TRACE_CONTENT support: utilities for content gating, instrumentation updates, error handling, tests, and documentation.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

🧹 Nitpick comments (1)

packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/utils.py (1)

13-14: ⚡ Quick win

Consider adding a docstring to document the environment variable behavior.

The function correctly implements the TRACELOOP_TRACE_CONTENT check with a default of "true", but lacks documentation. A docstring would clarify that this controls whether prompt/response content is recorded in spans and that the default is to include content.

📝 Suggested docstring

 def should_send_prompts() -> bool:
+    """Check if prompt/response content should be recorded in spans.
+    
+    Returns True unless TRACELOOP_TRACE_CONTENT is explicitly set to 'false' (case-insensitive).
+    This allows workflows that cannot store sensitive content to opt out.
+    """
     return (os.getenv("TRACELOOP_TRACE_CONTENT") or "true").lower() == "true"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/utils.py`
around lines 13 - 14, Add a concise docstring to the should_send_prompts
function that documents the TRACELOOP_TRACE_CONTENT environment variable
behavior: explain that the function returns True when TRACELOOP_TRACE_CONTENT is
set to "true" (case-insensitive), that the default is "true" when the variable
is unset, and that enabling it causes prompt/response content to be recorded in
spans; place this docstring directly above the should_send_prompts function
definition for visibility.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/utils.py`:
- Around line 13-14: Add a concise docstring to the should_send_prompts function
that documents the TRACELOOP_TRACE_CONTENT environment variable behavior:
explain that the function returns True when TRACELOOP_TRACE_CONTENT is set to
"true" (case-insensitive), that the default is "true" when the variable is
unset, and that enabling it causes prompt/response content to be recorded in
spans; place this docstring directly above the should_send_prompts function
definition for visibility.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 663c86e0-a619-4e47-93aa-3f6db5b03ba7

📥 Commits

Reviewing files that changed from the base of the PR and between ed254dc and d888241.

📒 Files selected for processing (3)

• packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py
• packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/instrumentation.py
• packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/utils.py

[YuxiangJiangCT]

Thanks for taking this — the gating on TRACELOOP_ENTITY_INPUT / TRACELOOP_ENTITY_OUTPUT / MCP_RESPONSE_VALUE reads cleanly.

One thing that might be in scope given the motivation ("workflows that cannot store prompts/responses"): two error-path set_status calls still write response content text into the span status, and they sit outside the should_send_prompts() guard:

• _execute_and_handle_result: Status(StatusCode.ERROR, f"{result.content[0].text}")
• InstrumentedStreamWriter.send (the ResponseStreamWriter span): Status(StatusCode.ERROR, f"{request.result['content'][0]['text']}")

So with TRACELOOP_TRACE_CONTENT=false, an error response's text still ends up in the span status description. Might be worth gating those too, or setting a generic message when content is disabled. The Status(..., str(e)) ones look fine — those are exception messages, not response content.

Since _execute_and_handle_result is shared by both the tool and mcp-method paths, gating it there covers both. Happy to open a small follow-up if you'd rather keep this PR focused.

[milkisbad]

Changed. It was a bit more complicated than what you wrote. I've identified 2 cases

• 1st is tool errors via Tool Execution Errors - my understanding is that it is not guarded against confidential information
• 2nd is there is error on Protocol Errors. i.e. in fastmcp providing wrong argument for tool results in pydantic model the error would look like (i've put the confidential info in input_value='ber':

1 validation error for Model
foo
  value is not "bar", got "ber" [type=not_a_bar, **input_value='ber'**, input_type=str]

decided to fix both, but it required touching a lot of places.

[milkisbad]

providing updated screenshots:

Details

- disabled confidential information error pass: - diabled confidential information happy path: - enabled confidential information error pass: - enabled confidential information happy path:

[milkisbad]

@YuxiangJiangCT making sure you got the notification

[YuxiangJiangCT]

@milkisbad nice, this is more thorough than what I flagged. Both isError branches are behind should_send_prompts() now, and good call extending it to the exception path. I'd written off str(e) as safe, but your pydantic input_value=... example is exactly why it isn't.

One asymmetry I noticed: _handle_tool_call and _handle_mcp_method both set record_exception=False / set_status_on_exception=False, but the ResponseStreamWriter span doesn't. So if the wrapped send raises in there, the SDK would still auto-record the exception and set status from str(exc) with no should_send_prompts() gate. Intentional (that path can't surface content-bearing exceptions), or worth the same treatment?

[milkisbad]

One asymmetry I noticed: _handle_tool_call and _handle_mcp_method both set record_exception=False / set_status_on_exception=False, but the ResponseStreamWriter span doesn't. So if the wrapped send raises in there, the SDK would still auto-record the exception and set status from str(exc) with no should_send_prompts() gate. Intentional (that path can't surface content-bearing exceptions), or worth the same treatment?

@YuxiangJiangCT Hmm could you help me out with this one - not too confident on the stream paths. Any concrete path you have in mind?
We do handle expected errors correctly. So the only tracebacks would be like stream serialization specific? can't think of probable scenario where that would leak data and I do not want to mask too much right now

[YuxiangJiangCT]

Yeah, I can't give you a concrete, probable one, so I'd leave it as is. With content off, both lines in send that touch the payload (serialize(request.result) and the content[0]['text'] status) are already short-circuited by should_send_prompts(), so the only thing still handling the response inside that span is the wrapped send itself, i.e. your serialization case. I can't construct a realistic traceback there that carries the prompt/response text, and anything else that escapes would be structural or transport-level with a generic message rather than payload. Agreed it's not worth masking real errors for a hypothetical; if a concrete one ever shows up it can be its own follow-up. The tool/method gating is where it matters, and that looks solid.

[milkisbad]

@YuxiangJiangCT oke how do follow from now on?

[YuxiangJiangCT]

Nothing more from me, I'm good with it as-is. The next step is really out of my hands though: it needs a maintainer with write access to approve and kick off CI, and mine doesn't count for that. Tagging one on the PR is probably the fastest nudge. Glad I could help on this one.

[Necmttn]

This is the right privacy split for MCP instrumentation: always trace metadata, gate content. I would make the metadata/content boundary explicit in docs and tests: method/tool name, server id, duration, status, and error class are always safe; prompt, response value, and serialized payload require TRACELOOP_TRACE_CONTENT=true. Local raw retention can stay a separate opt-in.

---

Generated with ax.

[milkisbad]

This is the right privacy split for MCP instrumentation: always trace metadata, gate content. I would make the metadata/content boundary explicit in docs and tests: method/tool name, server id, duration, status, and error class are always safe; prompt, response value, and serialized payload require TRACELOOP_TRACE_CONTENT=true. Local raw retention can stay a separate opt-in.

Hi @Necmttn agreed, docs and tests updated. As far as I understand the current logic satisfies the split deifinition.