Skip to content

Conversation

@jboolean
Copy link
Contributor

@jboolean jboolean commented Dec 2, 2025

What does this PR do?

Implements tracing of modelcontextprotocol/go-sdk​ with the same functionality as mark3labs/mcp-go​.

Motivation

Closes https://datadoghq.atlassian.net/browse/MLOB-4639

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!

Copy link
Contributor Author

jboolean commented Dec 2, 2025

@github-actions github-actions bot added the apm:ecosystem contrib/* related feature requests or bugs label Dec 2, 2025
@jboolean jboolean changed the title Tracing for go-sdk feat(contrib/go-sdk): Tracing for go-sdk Dec 2, 2025
@jboolean jboolean changed the title feat(contrib/go-sdk): Tracing for go-sdk feat(contrib/go-sdk): Initial go-sdk tracer implementation Dec 2, 2025
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Dec 2, 2025

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

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

@pr-commenter
Copy link

pr-commenter bot commented Dec 2, 2025

Benchmarks

Benchmark execution time: 2025-12-04 20:07:05

Comparing candidate commit a1b1e1a in PR branch jb/tracing-for-go-sdk with baseline commit 5682bdb in branch main.

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

@jboolean jboolean force-pushed the jb/tracing-for-go-sdk branch 5 times, most recently from d680e25 to e62619f Compare December 2, 2025 15:34
@jboolean jboolean force-pushed the jb/tracing-for-go-sdk branch from e62619f to 79e412f Compare December 2, 2025 18:10
@jboolean jboolean requested a review from hannahkm December 2, 2025 18:32
@jboolean jboolean marked this pull request as ready for review December 2, 2025 18:34
@jboolean jboolean requested review from a team as code owners December 2, 2025 18:34
@jboolean jboolean force-pushed the jb/contrib-mcp-go-intent-capture branch from 4285bb9 to f99dd20 Compare December 3, 2025 19:19
@jboolean jboolean force-pushed the jb/tracing-for-go-sdk branch from d58bedd to 299d5b6 Compare December 3, 2025 19:19
@jboolean jboolean requested a review from darccio December 3, 2025 19:24
Copy link
Contributor

@rarguelloF rarguelloF left a comment

Choose a reason for hiding this comment

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

LGTM, left a comment about a potential improvement on the llmobs SDK (which can be done/discussed in a future PR)

Comment on lines +93 to +96
type textIOSpan interface {
AnnotateTextIO(input, output string, opts ...llmobs.AnnotateOption)
Finish(opts ...llmobs.FinishSpanOption)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

if this kind of code is gonna be common when using our llmobs sdk, it might be worth it to include something in the core llmobs package itself to make life easier for all users.

Maybe AnnotateTextIO should be available in all span types, including AnySpan, WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be nice if llmobs just exported the textIOSpan interface it already has (llmobs.go:389)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, that's not an interface.
I didn't see an elegant way to publish a nice, reusable interface that's widely useful. Played with it for a bit.

We could add it to all span types but it looks like llm spans don't support it and use messages instead and I don't want methods that are not applicable.

I'm not going to go further down this rabbit hole for now.

@jboolean jboolean requested a review from genesor December 4, 2025 15:58
Base automatically changed from jb/contrib-mcp-go-intent-capture to main December 4, 2025 16:15
@dd-mergequeue dd-mergequeue bot requested a review from a team as a code owner December 4, 2025 16:15
@jboolean jboolean force-pushed the jb/tracing-for-go-sdk branch from 299d5b6 to b30dc5b Compare December 4, 2025 16:26
@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.13%. Comparing base (21d8bbb) to head (a1b1e1a).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
Files with missing lines Coverage Δ
instrumentation/packages.go 9.23% <ø> (ø)
internal/stacktrace/contribs_generated.go 100.00% <100.00%> (ø)

... and 76 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dd-devflow-routing-codex
Copy link

dd-devflow-routing-codex bot commented Dec 4, 2025

View all feedbacks in Devflow UI.

2025-12-04 19:02:37 UTC ℹ️ Start processing command devflow:merge


2025-12-04 19:02:42 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 45m (p90).


2025-12-04 19:03:02 UTCMergeQueue: This merge request has conflicts

This merge request conflicts with another merge request ahead in the queue.

The merge requests in front of this one are:

@dd-devflow-routing-codex
Copy link

dd-devflow-routing-codex bot commented Dec 4, 2025

View all feedbacks in Devflow UI.

2025-12-04 19:26:14 UTC ℹ️ Start processing command devflow:merge


2025-12-04 19:26:25 UTC ℹ️ MergeQueue: waiting for PR to be ready

This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.


2025-12-04 20:10:19 UTC ℹ️ MergeQueue: merge request added to the queue

The expected merge time in main is approximately 45m (p90).


2025-12-04 20:54:40 UTC ℹ️ MergeQueue: This merge request was merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

apm:ecosystem contrib/* related feature requests or bugs devflow:merge mergequeue-status: done

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants