You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "Fine0830 (via GitHub)" <gi...@apache.org> on 2026/03/16 09:53:39 UTC
[PR] refactor(mcp): remove OpenAI dependency, add Streamable HTTP transport, and refactor tools/prompts [skywalking-banyandb]
Fine0830 opened a new pull request, #1010:
URL: https://github.com/apache/skywalking-banyandb/pull/1010
Refactors the BanyanDB MCP server with the following changes:
- **Remove OpenAI dependency**: Replaced the OpenAI-based query generation with a pure LLM-prompt approach using the MCP protocol itself.
- **Add Streamable HTTP transport**: The MCP server now supports both `stdio` and `streamable-http` transports, configurable via the `TRANSPORT` environment variable. This enables deployment as a persistent HTTP service alongside the traditional stdio mode.
- **Refactor tools and prompts**: Consolidated and simplified the tool definitions and prompt generation logic.
- **Update documentation**: Updated `docs/operation/mcp/setup.md` and `docs/operation/mcp/build.md` to reflect the new configuration options (transport mode, `MCP_PORT`, etc.).
- **Clean up licenses**: Removed license files for OpenAI and related transitive dependencies that are no longer needed.
Signed-off-by: Qiuxia Fan <qi...@apache.org>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(mcp): remove OpenAI dependency, add Streamable HTTP transport, and refactor tools/prompts [skywalking-banyandb]
Posted by "hanahmily (via GitHub)" <gi...@apache.org>.
hanahmily commented on code in PR #1010:
URL: https://github.com/apache/skywalking-banyandb/pull/1010#discussion_r2974401794
##########
docs/operation/mcp/setup.md:
##########
@@ -131,6 +127,89 @@ curl http://localhost:17913/api/healthz
grpcurl -plaintext localhost:17912 list
```
+## HTTP Transport Mode
+
+By default the MCP server communicates over standard I/O (`TRANSPORT=stdio`), which is suitable for desktop clients such as Claude Desktop. Set `TRANSPORT=http` to expose the server as an HTTP endpoint instead. This mode uses the MCP [Streamable HTTP](https://modelcontextprotocol.io/specification/2025-11-25/basic/transports) transport, which supports both streaming (SSE) and direct JSON responses.
+
+### Start in HTTP Mode
+
+```bash
+# Default port 3000
+TRANSPORT=http BANYANDB_ADDRESS=localhost:17900 node dist/index.js
+
+# Custom port
+TRANSPORT=http MCP_PORT=8080 BANYANDB_ADDRESS=localhost:17900 node dist/index.js
+```
+
+The server prints the listening address on startup:
+
+```
+BanyanDB MCP HTTP server listening on :3000/mcp
+```
+
+The single endpoint is `POST /mcp` (also handles `GET /mcp` for server-to-client notifications). All other paths return 404.
Review Comment:
The HTTP handler only processes requests after fully reading the body via `req.on('end', ...)`. There's no explicit GET handling. If the transport doesn't support GET, this documentation claim is incorrect.
##########
mcp/src/index.ts:
##########
@@ -33,11 +38,86 @@ dotenv.config();
setupGlobalErrorHandlers();
const BANYANDB_ADDRESS = process.env.BANYANDB_ADDRESS || 'localhost:17900';
-const LLM_API_KEY = process.env.LLM_API_KEY;
-const LLM_BASE_URL = process.env.LLM_BASE_URL;
+const TRANSPORT = process.env.TRANSPORT || 'stdio';
+const mcpPortRaw = parseInt(process.env.MCP_PORT || '3000', 10);
+if (!Number.isFinite(mcpPortRaw) || mcpPortRaw <= 0 || mcpPortRaw > 65535) {
+ log.error(`Invalid MCP_PORT value "${process.env.MCP_PORT}": must be an integer between 1 and 65535. Defaulting to 3000.`);
Review Comment:
Port validation logs error but doesn't exit. This could mask configuration errors
##########
mcp/src/index.ts:
##########
@@ -33,11 +38,86 @@ dotenv.config();
setupGlobalErrorHandlers();
const BANYANDB_ADDRESS = process.env.BANYANDB_ADDRESS || 'localhost:17900';
-const LLM_API_KEY = process.env.LLM_API_KEY;
-const LLM_BASE_URL = process.env.LLM_BASE_URL;
+const TRANSPORT = process.env.TRANSPORT || 'stdio';
+const mcpPortRaw = parseInt(process.env.MCP_PORT || '3000', 10);
+if (!Number.isFinite(mcpPortRaw) || mcpPortRaw <= 0 || mcpPortRaw > 65535) {
+ log.error(`Invalid MCP_PORT value "${process.env.MCP_PORT}": must be an integer between 1 and 65535. Defaulting to 3000.`);
+}
+const MCP_PORT = Number.isFinite(mcpPortRaw) && mcpPortRaw > 0 && mcpPortRaw <= 65535 ? mcpPortRaw : 3000;
+
+// Prompt schema for generate_BydbQL — used with registerPrompt which requires a type cast
+// due to MCP SDK 1.x Zod v3/v4 compatibility layer causing TS2589 deep type instantiation.
+const generateBydbQLPromptSchema = {
+ description: z.string().describe(
+ "Natural language description of the query (e.g., 'list the last 30 minutes service_cpm_minute', 'show the last 30 zipkin spans order by time')",
+ ),
+ resource_type: z.enum(['stream', 'measure', 'trace', 'property']).optional().describe('Optional resource type hint: stream, measure, trace, or property'),
+ resource_name: z.string().optional().describe('Optional resource name hint (stream/measure/trace/property name)'),
+ group: z.string().optional().describe('Optional group hint, for example the properties group'),
+} as const;
+
+type QueryHints = {
+ description?: string;
+ BydbQL?: string;
+ resource_type?: string;
+ resource_name?: string;
+ group?: string;
+};
+
+function normalizeQueryHints(args: unknown): QueryHints {
+ if (!args || typeof args !== 'object') {
+ return {};
+ }
-async function main() {
- // Create MCP server
+ const rawArgs = args as Record<string, unknown>;
+ return {
+ description: typeof rawArgs.description === 'string' ? rawArgs.description.trim() : undefined,
+ BydbQL: typeof rawArgs.BydbQL === 'string' ? rawArgs.BydbQL.trim() : typeof rawArgs.bydbql === 'string' ? rawArgs.bydbql.trim() : undefined,
Review Comment:
Why do you use two different casings? Could you choose one? It looks like lowercase matches the parameter's convention.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(mcp): remove OpenAI dependency, add Streamable HTTP transport, and refactor tools/prompts [skywalking-banyandb]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1010:
URL: https://github.com/apache/skywalking-banyandb/pull/1010#discussion_r2958716134
##########
mcp/src/index.ts:
##########
@@ -33,11 +38,82 @@ dotenv.config();
setupGlobalErrorHandlers();
const BANYANDB_ADDRESS = process.env.BANYANDB_ADDRESS || 'localhost:17900';
-const LLM_API_KEY = process.env.LLM_API_KEY;
-const LLM_BASE_URL = process.env.LLM_BASE_URL;
+const TRANSPORT = process.env.TRANSPORT || 'stdio';
+const MCP_PORT = parseInt(process.env.MCP_PORT || '3000', 10);
+
Review Comment:
`MCP_PORT` is parsed with `parseInt(...)` but not validated. If `MCP_PORT` is unset/empty/non-numeric, this becomes `NaN` and `httpServer.listen(NaN)` will throw at runtime. Consider validating `Number.isFinite(MCP_PORT)` (and >0) and either defaulting or failing fast with a clear error message.
##########
mcp/src/index.ts:
##########
@@ -33,11 +38,82 @@ dotenv.config();
setupGlobalErrorHandlers();
const BANYANDB_ADDRESS = process.env.BANYANDB_ADDRESS || 'localhost:17900';
-const LLM_API_KEY = process.env.LLM_API_KEY;
-const LLM_BASE_URL = process.env.LLM_BASE_URL;
+const TRANSPORT = process.env.TRANSPORT || 'stdio';
+const MCP_PORT = parseInt(process.env.MCP_PORT || '3000', 10);
Review Comment:
PR description mentions `streamable-http` as a transport option, but the implementation/docs key off `TRANSPORT === 'http'`. To avoid confusion and config mistakes, either accept both values (e.g., `http` and `streamable-http`) or align the documentation/PR description and code to one canonical value.
##########
mcp/src/index.ts:
##########
@@ -359,15 +367,67 @@ async function main() {
}
}
+ if (name === 'get_generate_bydbql_prompt') {
+ const queryHints = normalizeQueryHints(args);
+ if (!queryHints.description) {
+ throw new Error('description is required');
+ }
+
+ const { groups, resourcesByGroup } = await loadQueryContext(banyandbClient);
+ const prompt = generateBydbQL(queryHints.description, queryHints, groups, resourcesByGroup);
+
+ return { content: [{ type: 'text', text: prompt }] };
+ }
+
throw new Error(`Unknown tool: ${name}`);
});
- // Start the server
- const transport = new StdioServerTransport();
- await server.connect(transport);
+ return server;
+}
+
+async function main() {
+ const banyandbClient = new BanyanDBClient(BANYANDB_ADDRESS);
+
+ log.info('Using built-in BydbQL prompt generation with natural language support and BanyanDB schema hints. Use the list_groups_schemas tool to discover available resources and guide your queries.');
+
+ if (TRANSPORT === 'http') {
+ const httpServer = createServer((req, res) => {
+ if (req.url !== '/mcp') {
+ res.writeHead(404, { 'Content-Type': 'application/json' });
+ res.end(JSON.stringify({ error: 'Not found' }));
+ return;
+ }
+
+ let body = '';
+ req.on('data', (chunk) => { body += chunk; });
+ req.on('end', async () => {
+ try {
+ const parsedBody = body ? JSON.parse(body) : undefined;
+ const transport = new StreamableHTTPServerTransport({ sessionIdGenerator: undefined });
+ const mcpServer = createMcpServer(banyandbClient);
+ await mcpServer.connect(transport);
+ await transport.handleRequest(req, res, parsedBody);
+ } catch (error) {
+ if (!res.headersSent) {
+ res.writeHead(500, { 'Content-Type': 'application/json' });
+ res.end(JSON.stringify({ error: error instanceof Error ? error.message : String(error) }));
+ }
+ }
Review Comment:
`JSON.parse(body)` failures (malformed JSON) are currently caught and returned as a 500. This is a client error and should return a 400 with a clear message; otherwise it looks like a server fault and makes debugging harder for MCP clients.
##########
mcp/src/index.ts:
##########
@@ -359,15 +367,67 @@ async function main() {
}
}
+ if (name === 'get_generate_bydbql_prompt') {
+ const queryHints = normalizeQueryHints(args);
+ if (!queryHints.description) {
+ throw new Error('description is required');
+ }
+
+ const { groups, resourcesByGroup } = await loadQueryContext(banyandbClient);
+ const prompt = generateBydbQL(queryHints.description, queryHints, groups, resourcesByGroup);
+
+ return { content: [{ type: 'text', text: prompt }] };
+ }
+
throw new Error(`Unknown tool: ${name}`);
});
- // Start the server
- const transport = new StdioServerTransport();
- await server.connect(transport);
+ return server;
+}
+
+async function main() {
+ const banyandbClient = new BanyanDBClient(BANYANDB_ADDRESS);
+
+ log.info('Using built-in BydbQL prompt generation with natural language support and BanyanDB schema hints. Use the list_groups_schemas tool to discover available resources and guide your queries.');
+
+ if (TRANSPORT === 'http') {
+ const httpServer = createServer((req, res) => {
+ if (req.url !== '/mcp') {
+ res.writeHead(404, { 'Content-Type': 'application/json' });
+ res.end(JSON.stringify({ error: 'Not found' }));
+ return;
+ }
Review Comment:
In HTTP transport mode the handler checks `req.url !== '/mcp'`. This will 404 for valid requests that include a query string (e.g. `/mcp?…`) which some clients/transports may use. Prefer parsing `new URL(req.url ?? '', 'http://localhost')` and matching on `.pathname === '/mcp'`.
##########
mcp/src/index.ts:
##########
@@ -93,28 +199,55 @@ async function main() {
},
{
name: 'list_resources_bydbql',
+ description: 'Fetch streams, measures, traces, or properties from BanyanDB using a BydbQL query.',
+ inputSchema: {
Review Comment:
`list_resources_bydbql`’s description says it fetches streams/measures/traces/properties, but the underlying `BanyanDBClient.query()` can also return TOPN results. Consider making the description generic (e.g., “execute a BydbQL query”) or explicitly include TOPN to match actual behavior.
##########
mcp/package.json:
##########
@@ -22,8 +22,7 @@
"license": "Apache-2.0",
"dependencies": {
"@modelcontextprotocol/sdk": "^1.26.0",
- "dotenv": "^16.4.5",
- "openai": "^4.20.0"
+ "dotenv": "^16.4.5"
},
Review Comment:
`src/index.ts` imports `zod`, and `@modelcontextprotocol/sdk` declares `zod` as a non-optional peer dependency, but `package.json` doesn’t list `zod` in dependencies. This can break installs (e.g., pnpm won’t hoist a transitive peer the same way) and cause runtime module resolution failures. Add `zod` as an explicit dependency (and keep `package-lock.json` in sync).
##########
mcp/src/query/llm-prompt.ts:
##########
@@ -19,10 +19,8 @@
import type { ResourcesByGroup } from './types.js';
-/**
- * Generate the LLM prompt for converting natural language to BydbQL queries.
- */
-export function generateQueryPrompt(
+// Generates a BydbQL query based on the provided natural language description and available schema information.
+export function generateBydbQL(
description: string,
args: Record<string, unknown>,
Review Comment:
The prompt generator instructs the LLM to return JSON with a `bydbql` field, but the tool interface (`list_resources_bydbql`) expects an input field named `BydbQL` (capital B). This mismatch makes the “prompt → tool call” workflow error-prone. Consider standardizing on one key (preferably `BydbQL` everywhere) or supporting both aliases when normalizing inputs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(mcp): remove OpenAI dependency, add Streamable HTTP transport, and refactor tools/prompts [skywalking-banyandb]
Posted by "hanahmily (via GitHub)" <gi...@apache.org>.
hanahmily merged PR #1010:
URL: https://github.com/apache/skywalking-banyandb/pull/1010
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(mcp): remove OpenAI dependency, add Streamable HTTP transport, and refactor tools/prompts [skywalking-banyandb]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1010:
URL: https://github.com/apache/skywalking-banyandb/pull/1010#issuecomment-4067853010
## [Codecov](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1010?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 30.17%. Comparing base ([`3530dd9`](https://app.codecov.io/gh/apache/skywalking-banyandb/commit/3530dd9b5cefaa4fef60eec91ecbfbdfcaf243c5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`d3f4cfb`](https://app.codecov.io/gh/apache/skywalking-banyandb/commit/d3f4cfba00d7173291e4a6a4dda95f1c64aad92b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 159 commits behind head on main.
> :exclamation: There is a different number of reports uploaded between BASE (3530dd9) and HEAD (d3f4cfb). Click for more details.
>
> <details><summary>HEAD has 1 upload less than BASE</summary>
>
>| Flag | BASE (3530dd9) | HEAD (d3f4cfb) |
>|------|------|------|
>||1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #1010 +/- ##
===========================================
- Coverage 45.97% 30.17% -15.81%
===========================================
Files 328 124 -204
Lines 55505 12883 -42622
===========================================
- Hits 25520 3887 -21633
+ Misses 27909 8622 -19287
+ Partials 2076 374 -1702
```
| [Flag](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1010/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [pkg](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1010/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `30.17% <ø> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1010?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org