Skip to content

Add initial Activity / Metric support #183

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
Merged
merged 4 commits into from
Apr 2, 2025
Merged

Add initial Activity / Metric support #183

merged 4 commits into from
Apr 2, 2025

Conversation

stephentoub
Copy link
Contributor

samsp-msft
samsp-msft reviewed Apr 1, 2025
View reviewed changes
halter73
halter73 reviewed Apr 1, 2025
View reviewed changes
@samsp-msft samsp-msft mentioned this pull request Apr 1, 2025
Open
@samsp-msft
Copy link
Contributor

Tagging @lmolkova for looking at how to formalize the semantic conventions.

samsp-msft
samsp-msft reviewed Apr 1, 2025
View reviewed changes
lmolkova
lmolkova reviewed Apr 1, 2025
View reviewed changes
@JamesNK
Copy link

JamesNK commented Apr 2, 2025

If MCP becomes a big deal then at some point in the future there will probably be some OTEL semantic conventions for MCP traces/metrics. Undoubtably our names won't line up.

I don't think there is anything that can be done about that. But something to keep in mind.

WeihanLi
WeihanLi reviewed Apr 2, 2025
View reviewed changes
@samsp-msft
Copy link
Contributor

If MCP becomes a big deal then at some point in the future there will probably be some OTEL semantic conventions for MCP traces/metrics. Undoubtably our names won't line up.

I don't think there is anything that can be done about that. But something to keep in mind.

I created an issue and copilot generated doc based on these to kickstart the semantic convention process.
open-telemetry/semantic-conventions#2043
open-telemetry/semantic-conventions#2045

@lmolkova
Copy link
Contributor

lmolkova commented Apr 2, 2025 •
edited
Loading

I created an issue and copilot generated doc based on these to kickstart the semantic convention process. open-telemetry/semantic-conventions#2043 open-telemetry/semantic-conventions#2045

unfortunately AI cannot replace people when writing semantic conventions - it should read contributing.md and probably didn't. OTel also advises against using AI as a primary contributor.

halter73
halter73 approved these changes Apr 2, 2025
View reviewed changes
eiriktsarpalis
eiriktsarpalis reviewed Apr 2, 2025
View reviewed changes
@stephentoub
Add initial Activity / Metric support
16c17dd 
- Creates an Activity around every outgoing send / incoming receive handling
- Creates a histogram for each incoming operation, tagged with request/notification details
@stephentoub
Update based on feedback
493a7fb
@stephentoub
Address feedback
f944439
@stephentoub stephentoub force-pushed the mcpsessiondiagnostics branch from c9d76f7 to f944439 Compare April 2, 2025 13:44
@stephentoub
Copy link
Contributor Author

@samsp-msft, are you good with this for now?

@stephentoub stephentoub merged commit 4b38d3e into modelcontextprotocol:main Apr 2, 2025
8 checks passed
@stephentoub stephentoub deleted the mcpsessiondiagnostics branch April 2, 2025 15:09
@samsp-msft samsp-msft mentioned this pull request Apr 2, 2025
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JamesNK JamesNK JamesNK left review comments

@lmolkova lmolkova lmolkova left review comments

@WeihanLi WeihanLi WeihanLi left review comments

@samsp-msft samsp-msft samsp-msft left review comments

@halter73 halter73 halter73 approved these changes

@eiriktsarpalis eiriktsarpalis eiriktsarpalis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@stephentoub @samsp-msft @JamesNK @lmolkova @halter73 @eiriktsarpalis @WeihanLi