dubbo icon indicating copy to clipboard operation
dubbo copied to clipboard

Dubbo MCP Integration

Open RainYuY opened this issue 6 months ago • 8 comments

What is the purpose of the change?

Ref: #15332

Checklist

  • [x] Make sure there is a GitHub_issue field for the change.
  • [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • [x] Write necessary unit-test to verify your logic correction. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
  • [x] Make sure gitHub actions can pass. Why the workflow is failing and how to fix it?

RainYuY avatar May 20 '25 12:05 RainYuY

Codecov Report

:x: Patch coverage is 36.83867% with 967 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 60.77%. Comparing base (dea0206) to head (6076567).

Files with missing lines Patch % Lines
...transport/DubboMcpStreamableTransportProvider.java 28.53% 228 Missing and 20 partials :warning:
...va/org/apache/dubbo/mcp/core/McpServiceFilter.java 12.00% 153 Missing and 1 partial :warning:
...ache/dubbo/mcp/tool/DubboOpenApiToolConverter.java 30.09% 120 Missing and 24 partials :warning:
...e/dubbo/mcp/core/McpApplicationDeployListener.java 12.28% 97 Missing and 3 partials :warning:
...pache/dubbo/mcp/tool/DubboServiceToolRegistry.java 57.61% 71 Missing and 18 partials :warning:
...ava/org/apache/dubbo/mcp/util/TypeSchemaUtils.java 51.38% 77 Missing and 11 partials :warning:
...bo/mcp/transport/DubboMcpSseTransportProvider.java 61.72% 30 Missing and 1 partial :warning:
...pache/dubbo/mcp/core/McpServiceExportListener.java 48.14% 25 Missing and 3 partials :warning:
...g/apache/dubbo/mcp/tool/DubboMcpGenericCaller.java 47.16% 25 Missing and 3 partials :warning:
...java/org/apache/dubbo/config/nested/McpConfig.java 0.00% 23 Missing :warning:
... and 5 more
Additional details and impacted files
@@             Coverage Diff              @@
##                3.3   #15406      +/-   ##
============================================
- Coverage     61.06%   60.77%   -0.30%     
- Complexity    11700    11729      +29     
============================================
  Files          1924     1938      +14     
  Lines         87086    88617    +1531     
  Branches      13115    13378     +263     
============================================
+ Hits          53182    53858     +676     
- Misses        28460    29239     +779     
- Partials       5444     5520      +76     
Flag Coverage Δ
integration-tests-java21 32.46% <1.37%> (-0.40%) :arrow_down:
integration-tests-java8 32.53% <0.39%> (-0.43%) :arrow_down:
samples-tests-java21 32.05% <1.17%> (-0.53%) :arrow_down:
samples-tests-java8 29.85% <0.19%> (-0.49%) :arrow_down:
unit-tests-java11 59.01% <17.64%> (-0.03%) :arrow_down:
unit-tests-java17 58.51% <36.51%> (-0.27%) :arrow_down:
unit-tests-java21 58.50% <36.51%> (-0.29%) :arrow_down:
unit-tests-java8 59.03% <17.64%> (+<0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

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

codecov-commenter avatar May 20 '25 14:05 codecov-commenter

PTAL @AlbumenJ @CrazyHZM

RainYuY avatar May 24 '25 05:05 RainYuY

There are many Chinese comments in the change commit.

Done.Translation completed.

RainYuY avatar May 26 '25 12:05 RainYuY

Dear Dubbo team: Once a Dubbo service interface is registered as an MCP (Mesh Control Plane) tool, is its metadata stored exclusively in local memory? Furthermore, are there any plans to register MCP tools with the MCP register in Nacos version 3? Is the Streamable HTTP protocol supported?

SpursJiang avatar May 26 '25 15:05 SpursJiang

There are many Chinese comments in the change commit.

Done.Translation completed.

no, there are still chinese comments. i.e. McpConstant.java

zrlw avatar May 27 '25 01:05 zrlw

There are many Chinese comments in the change commit.

Done.Translation completed.

no, there are still chinese comments. i.e. McpConstant.java

You're right. done.

RainYuY avatar May 27 '25 02:05 RainYuY

Dear Dubbo team: Once a Dubbo service interface is registered as an MCP (Mesh Control Plane) tool, is its metadata stored exclusively in local memory? Furthermore, are there any plans to register MCP tools with the MCP register in Nacos version 3? Is the Streamable HTTP protocol supported?

  1. Is its metadata stored exclusively in local memory? Yes.

  2. Furthermore, are there any plans to register MCP tools with the MCP registry in Nacos version 3? Under consideration.

  3. Is the Streamable HTTP protocol supported? No, but I believe we will support it in the future.Now just sse.

RainYuY avatar May 27 '25 02:05 RainYuY

Dear Dubbo team: Once a Dubbo service interface is registered as an MCP (Mesh Control Plane) tool, is its metadata stored exclusively in local memory? Furthermore, are there any plans to register MCP tools with the MCP register in Nacos version 3? Is the Streamable HTTP protocol supported?

  1. Is its metadata stored exclusively in local memory? Yes.
  2. Furthermore, are there any plans to register MCP tools with the MCP registry in Nacos version 3? Under consideration.
  3. Is the Streamable HTTP protocol supported? No, but I believe we will support it in the future.Now just sse.

OK👍

SpursJiang avatar May 27 '25 08:05 SpursJiang

Since the mcp-java-sdk has not yet implemented server-side streamable support, I suggest we begin reviewing this Pull Request and proceed with using SSE as the first RC version. Once the streamable specification is implemented, we can release a second version. @AlbumenJ @zrlw @CrazyHZM @oxsean @heliang666s

RainYuY avatar Jul 05 '25 04:07 RainYuY

Currently, this PR lacks a solution for multiple instances of MCP Server, which involves session sharing issues in cluster mode. However, the multi-instance solution is an enterprise-level solution, which can be implemented after this PR is merged

conghuhu avatar Jul 21 '25 02:07 conghuhu

check the reviews. @RainYuY

zrlw avatar Jul 24 '25 02:07 zrlw

@wcy666103 spotless:apply to fix code formatting issue.

zrlw avatar Aug 06 '25 02:08 zrlw

@zrlw @AlbumenJ @CrazyHZM @songxiaosheng @heliang666s @EarthChen PTAL

RainYuY avatar Oct 13 '25 05:10 RainYuY

A small suggestion: Disable MCP load by default

Others LGTM

Done

RainYuY avatar Oct 17 '25 23:10 RainYuY