Skip to content

feat(contrib/go-sdk): Add intent capture to go-sdk #4209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
jboolean wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(contrib/go-sdk): Add intent capture to go-sdk #4209

jboolean wants to merge 2 commits into from

Conversation

@jboolean
Copy link
Contributor

@jboolean jboolean commented Dec 3, 2025 •
edited
Loading

What does this PR do?

Adds the same functionality as #4123, but for modelcontextprotocol/go-sdk, while that PR was for mark3labs/mcp-go.

In summary, this means modifying the tool schemas to add an intent parameter for the client to pass, then recording that value on the span.

Motivation

Relates to 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!

This was referenced Dec 3, 2025
Merged
Merged
@github-actions github-actions bot added the apm:ecosystem contrib/* related feature requests or bugs label Dec 3, 2025
@jboolean Graphite App
Copy link
Contributor Author

jboolean commented Dec 3, 2025 •
edited
Loading

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

@jboolean jboolean changed the title Add intent capture to go-sdk feat(contrib/go-sdk): Add intent capture to go-sdk Dec 3, 2025
@jboolean jboolean force-pushed the jb/add-intent-capture-to-go-sdk branch from bcc1ff0 to b697f68 Compare December 3, 2025 19:23
@jboolean jboolean marked this pull request as ready for review December 3, 2025 19:23
@jboolean jboolean requested review from a team as code owners December 3, 2025 19:23
@pr-commenter
Copy link

pr-commenter bot commented Dec 3, 2025 •
edited
Loading

Benchmarks

Benchmark execution time: 2025-12-05 22:05:55

Comparing candidate commit 3fc56c6 in PR branch jb/add-intent-capture-to-go-sdk with baseline commit 2dc2312 in branch main.

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

scenario:BenchmarkParallelMetrics/distribution/handle-reused-25

  • 🟥 execution_time [+1.456ns; +6.760ns] or [+2.239%; +10.400%]

@datadog-official
Copy link
Contributor

datadog-official bot commented Dec 3, 2025 •
edited by datadog-datadog-prod-us1 bot
Loading

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

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

@jboolean jboolean force-pushed the jb/add-intent-capture-to-go-sdk branch from b697f68 to b6ef884 Compare December 4, 2025 15:19
@jboolean jboolean requested a review from a team as a code owner December 4, 2025 15:19
@codecov
Copy link

codecov bot commented Dec 4, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.12%. Comparing base (2dc2312) to head (3fc56c6).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

see 74 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.

@jboolean jboolean force-pushed the jb/add-intent-capture-to-go-sdk branch from b6ef884 to 47b647a Compare December 4, 2025 15:58
@jboolean jboolean force-pushed the jb/tracing-for-go-sdk branch from 299d5b6 to b30dc5b Compare December 4, 2025 16:26
@jboolean jboolean force-pushed the jb/add-intent-capture-to-go-sdk branch from 47b647a to b68fef9 Compare December 4, 2025 16:26
genesor
genesor reviewed Dec 4, 2025
View reviewed changes
@jboolean jboolean force-pushed the jb/add-intent-capture-to-go-sdk branch from b68fef9 to d6f341a Compare December 4, 2025 18:07
@jboolean jboolean requested a review from genesor December 4, 2025 18:09
@jboolean jboolean force-pushed the jb/tracing-for-go-sdk branch from b30dc5b to a1b1e1a Compare December 4, 2025 19:23
@jboolean jboolean force-pushed the jb/add-intent-capture-to-go-sdk branch from d6f341a to eaf72fb Compare December 4, 2025 19:23
Base automatically changed from jb/tracing-for-go-sdk to main December 4, 2025 20:54
@jboolean jboolean force-pushed the jb/add-intent-capture-to-go-sdk branch from eaf72fb to d08bbba Compare December 4, 2025 21:06
genesor
genesor approved these changes Dec 5, 2025
View reviewed changes
Copy link
Member

@genesor genesor left a comment

Choose a reason for hiding this comment

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

LGTM, somes nits mostly about code style.

contrib/modelcontextprotocol/go-sdk/option.go Outdated
// Ordering:
// The tracing middleware must run first so that the span is created and can be annotated with the intent.
// The SDK wraps the handler with each successive middleware added, so the last added runs first.
// Confusingly, the go-sdk code has a comment suggesting the opposite, but the actual implementation only reverses the order of a set of middleware added in the same AddReceivingMiddleware call, not middleware added successively as done here.
Copy link
Member

@genesor genesor Dec 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

is the go-sdk team aware of this? It might be a good idea to either fix the comment or the ordering issue in their implementation upstream.

If they update the code and break our implementation that wouldn't be ideal.

Copy link
Contributor Author

@jboolean jboolean Dec 5, 2025

Choose a reason for hiding this comment

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

Submitted a PR modelcontextprotocol/go-sdk#688

@jboolean jboolean force-pushed the jb/add-intent-capture-to-go-sdk branch from d08bbba to 8a141b4 Compare December 5, 2025 15:26
@jboolean jboolean force-pushed the jb/add-intent-capture-to-go-sdk branch from 8a141b4 to 2fdb71b Compare December 5, 2025 15:29
hannahkm
hannahkm approved these changes Dec 5, 2025
View reviewed changes
Copy link
Contributor

@hannahkm hannahkm left a comment

Choose a reason for hiding this comment

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

LP related content (testing, general code structure) lgtm. I'll defer to SDK/IDM to review the details!

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

Reviewers

@genesor genesor genesor approved these changes

@hannahkm hannahkm hannahkm approved these changes

Assignees

No one assigned

Labels

apm:ecosystem contrib/* related feature requests or bugs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jboolean @genesor @hannahkm