feat(contrib/mcp-go) Make simpler way to include mcp-go tracer

[jboolean]

What does this PR do?

Provides an easier way to add tracing to an mcp server that does not leak implementation details (use of hooks, middleware).

Unfortunately this does use some reflection, which is a bit messy. I can understand pushback on this.

Motivation

I was bothered by how the abstractions were leaked, and put the burden on the user to understand them. Also, I'm thinking about how to write an orchestrion patch, and this will make it easier.

Reviewer's Checklist

• Changed code has unit tests for its functionality at or near 100% coverage.
• System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
• There is a benchmark for any new code, or changes to existing code.
• If this interacts with the agent in a new way, a system test has been added.
• New code is free of linting errors. You can check this by running ./scripts/lint.sh locally.
• Add an appropriate team label so this PR gets put in the right place for the release notes.
• Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

[jboolean]

• feat(contrib/mcp-go) Make simpler way to include mcp-go tracer #4115 👈 (View in Graphite)
• feat(contrib/mcp-go): Trace MCP session initializations with MLObs spans #4101 : 2 other dependent PRs (#4106 , #4122 )
• feat(contrib/mcp-go): Initial mcp-go tracer implementation #4100
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[pr-commenter]

Benchmarks

Benchmark execution time: 2025-11-12 19:58:48

Comparing candidate commit e8a5c51 in PR branch jb/contrib-mcp-go-wrap with baseline commit c704875 in branch jb/contrib-mcp-go-init-span.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 3 metrics, 0 unstable metrics.

[datadog-datadog-prod-us1]

⚠️ Tests

⚠️ Warnings

❄️ 1 New flaky test detected

TestWriter_Flush_MultipleEndpoints from github.com/DataDog/dd-trace-go/v2/internal/telemetry/internal (Datadog)

Failed

=== RUN   TestWriter_Flush_MultipleEndpoints
    writer_test.go:242: 
        	Error Trace:	/home/runner/work/dd-trace-go/dd-trace-go/internal/telemetry/internal/writer_test.go:242
        	Error:      	Should not be zero, but was 0
        	Test:       	TestWriter_Flush_MultipleEndpoints
    writer_test.go:245: 
        	Error Trace:	/home/runner/work/dd-trace-go/dd-trace-go/internal/telemetry/internal/writer_test.go:245
        	Error:      	Not equal: 
...

ℹ️ Info

🧪 All tests passed

_{This comment will be updated automatically if new data arrives.
🔗 Commit SHA: e8a5c51 | Docs | Datadog PR Page | Was this helpful? Give us feedback!}

[genesor]

I don't think this is a good idea.

Reflection is not pretty in a library, on top of that we have no real way of ensuring that the hooks we're creating won't be erased by another WithHooks option if the user has one in place.

If we want to make it simpler we can change the existings funcs to be usable directly as server.ServerOption by wrapping whatever we want to do in already existing ServerOption

• NewToolHandlerMiddleware would become WithToolHandlerMiddleware wrapping server.WithToolHandlerMiddleware

• New WithHooks func wrapping server.WithHooks but adding our own tracing hooks (and handling nil so it's easy to use when you have no hooks and want to create them)

func WithToolHandlerMiddleware() server.ServerOption {
	return server.WithToolHandlerMiddleware(func(next server.ToolHandlerFunc) server.ToolHandlerFunc {
		return func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
			toolSpan, ctx := llmobs.StartToolSpan(ctx, request.Params.Name, llmobs.WithIntegration(string(instrumentation.PackageMark3LabsMCPGo)))

			result, err := next(ctx, request)

			inputJSON, marshalErr := json.Marshal(request)
			if marshalErr != nil {
				instr.Logger().Warn("mcp-go: failed to marshal tool request: %v", marshalErr)
			}
			var outputText string
			if result != nil {
				resultJSON, marshalErr := json.Marshal(result)
				if marshalErr != nil {
					instr.Logger().Warn("mcp-go: failed to marshal tool result: %v", marshalErr)
				}
				outputText = string(resultJSON)
			}

			toolSpan.AnnotateTextIO(string(inputJSON), outputText)

			if err != nil {
				toolSpan.Finish(llmobs.WithError(err))
			} else {
				toolSpan.Finish()
			}

			return result, err
		}
	})
}

func WithHooks(hooks *server.Hooks) server.ServerOption {
	if hooks == nil {
		hooks = new(server.Hooks)
	}

	AddServerHooks(hooks)

	return server.WithHooks(hooks)
}

Making it transparent to the user if they already have hooks in place. They would simple need to change it to the following:

// If they have hooks: 
    srv := server.NewMCPServer("my-server", "1.0.0",
        mcpgotrace.WithHooks(hooks),
        mcpgotrace.WithToolHandlerMiddleware(),
	)

// if they don't have hooks
     srv := server.NewMCPServer("my-server", "1.0.0",
        mcpgotrace.WithHooks(nil),
        mcpgotrace.WithToolHandlerMiddleware(),
	)

cc @rarguelloF what do you think ?

[jboolean]

I like that. I thought about it but wanted to see if I count abstract away hooks/middleware details. But I think you're right and it's a decent middle ground.

[darccio]

For future reference, reflect breaks code pruning: https://appliedgo.net/spotlight/reflection-binary-size/

We try to avoid it in general, as some customers work in really constrained environments, like serverless: #3762