Skip to content

Commit 74320e3

Browse files
authored
refactor: remove the unnecessary tool and parameter, implement unit tests (#39)
1 parent b8f424c commit 74320e3

File tree

13 files changed

+740
-203
lines changed

13 files changed

+740
-203
lines changed

.github/workflows/CI.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ jobs:
6767
- name: Lint
6868
run: make lint
6969

70+
- name: Test
71+
run: make test
72+
7073
- name: Build Docker images
7174
run: make docker
7275

CLAUDE.md

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ make build-image # Build Docker image skywalking-mcp:latest
3232
make clean # Remove build artifacts
3333
```
3434

35-
No unit tests exist yet. CI runs license checks, lint, and docker build.
35+
Unit tests exist for selected transport/context behavior. CI runs license checks, lint, and docker build.
3636

3737
## Architecture
3838

@@ -41,12 +41,11 @@ No unit tests exist yet. CI runs license checks, lint, and docker build.
4141
Three MCP transport modes as cobra subcommands: `stdio`, `sse`, `streamable`.
4242

4343
The SkyWalking OAP URL is resolved in priority order:
44-
- **stdio**: `set_skywalking_url` session tool > `--sw-url` flag > `http://localhost:12800/graphql`
45-
- **SSE/HTTP**: `SW-URL` HTTP header > `--sw-url` flag > `http://localhost:12800/graphql`
44+
- **All transports**: `--sw-url` flag > `http://localhost:12800/graphql`
4645

47-
The `set_skywalking_url` tool is only available in stdio mode (single client, well-defined session). SSE and HTTP transports use per-request headers instead.
46+
SSE and HTTP transports always use the configured server URL.
4847

49-
Basic auth is configured via `--sw-username` / `--sw-password` flags. Both flags (and the `set_skywalking_url` tool) support `${ENV_VAR}` syntax to resolve credentials from environment variables (e.g. `--sw-password ${MY_SECRET}`).
48+
Basic auth is configured via `--sw-username` / `--sw-password` flags. The startup flags support `${ENV_VAR}` syntax to resolve credentials from environment variables (e.g. `--sw-password ${MY_SECRET}`).
5049

5150
Each transport injects the OAP URL and auth into the request context via `WithSkyWalkingURLAndInsecure()` and `WithSkyWalkingAuth()`. Tools extract them downstream using `skywalking-cli`'s `contextkey.BaseURL{}`, `contextkey.Username{}`, and `contextkey.Password{}`.
5251

@@ -99,4 +98,4 @@ Tool handlers should return `(mcp.NewToolResultError(...), nil)` for expected qu
9998

10099
## CI & Merge Policy
101100

102-
Squash-merge only. PRs to `main` require 1 approval and passing `Required` status check (license + lint + docker build). Go 1.25.
101+
Squash-merge only. PRs to `main` require 1 approval and passing `Required` status check (license + lint + docker build). Go 1.25.

Makefile

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ PLATFORMS ?= linux/amd64
3131
MULTI_PLATFORMS ?= linux/amd64,linux/arm64
3232
OUTPUT ?= --load
3333
IMAGE_TAGS ?= -t $(IMAGE):$(VERSION) -t $(IMAGE):latest
34+
GO_TEST_FLAGS ?=
35+
GO_TEST_PKGS ?= ./...
3436

3537
.PHONY: all
3638
all: build ;
@@ -48,6 +50,14 @@ build: ## Build the binary.
4850
-X ${VERSION_PATH}.date=${BUILD_DATE}" \
4951
-o bin/swmcp cmd/skywalking-mcp/main.go
5052

53+
.PHONY: test
54+
test: ## Run unit tests.
55+
go test $(GO_TEST_FLAGS) $(GO_TEST_PKGS)
56+
57+
.PHONY: test-cover
58+
test-cover: ## Run unit tests with coverage output in coverage.txt.
59+
go test $(GO_TEST_FLAGS) -coverprofile=coverage.txt $(GO_TEST_PKGS)
60+
5161
$(GO_LINT):
5262
@$(GO_LINT) version > /dev/null 2>&1 || go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.64.0
5363
$(LICENSE_EYE):
@@ -139,7 +149,7 @@ PUSH_RELEASE_SCRIPTS := ./scripts/push-release.sh
139149
release-push-candidate:
140150
${PUSH_RELEASE_SCRIPTS}
141151

142-
.PHONY: lint fix-lint
152+
.PHONY: lint fix-lint test test-cover
143153
.PHONY: license-header fix-license-header dependency-license fix-dependency-license
144154
.PHONY: release-binary release-source release-sign release-assembly
145155
.PHONY: release-push-candidate docker-build-multi

README.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,11 @@ bin/swmcp stdio --sw-url http://localhost:12800 --sw-username admin --sw-passwor
6565
bin/swmcp sse --sse-address localhost:8000 --base-path /mcp --sw-url http://localhost:12800
6666
```
6767

68+
Transport URL behavior:
69+
70+
- `stdio`, `sse`, and `streamable` all use the configured `--sw-url` value (or the default `http://localhost:12800/graphql`).
71+
- `sse` and `streamable` ignore request-level URL override headers.
72+
6873
### Usage with Cursor, Copilot, Claude Code
6974

7075
```json
@@ -128,7 +133,6 @@ SkyWalking MCP provides the following tools to query and analyze SkyWalking OAP
128133

129134
| Category | Tool Name | Description |
130135
|--------------|--------------------------------|---------------------------------------------------------------------------------------------------|
131-
| **Session** | `set_skywalking_url` | Set the SkyWalking OAP server URL and optional basic auth credentials for the current session (stdio mode only). Supports `${ENV_VAR}` syntax for credentials. |
132136
| **Trace** | `query_traces` | Query traces with multi-condition filtering (service, endpoint, state, tags, and time range via start/end/step). Supports `full`, `summary`, and `errors_only` views with performance insights. |
133137
| **Log** | `query_logs` | Query logs with filters for service, instance, endpoint, trace ID, tags, and time range. Supports cold storage and pagination. |
134138
| **MQE** | `execute_mqe_expression` | Execute MQE (Metrics Query Expression) to query and calculate metrics data. Supports calculations, aggregations, TopN, trend analysis, and multiple result types. |
@@ -176,4 +180,4 @@ SkyWalking MCP provides the following prompts for guided analysis workflows:
176180

177181
[Apache 2.0 License.](/LICENSE)
178182

179-
[mcp]: https://modelcontextprotocol.io/
183+
[mcp]: https://modelcontextprotocol.io/

internal/swmcp/server.go

Lines changed: 8 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -37,18 +37,13 @@ import (
3737
)
3838

3939
// newMCPServer creates a new MCP server with all tools, resources, and prompts registered.
40-
// When stdio is true, session management tools (set_skywalking_url) are also registered,
41-
// since stdio has a single client and session semantics are well-defined.
42-
func newMCPServer(stdio bool) *server.MCPServer {
40+
func newMCPServer() *server.MCPServer {
4341
s := server.NewMCPServer(
4442
"skywalking-mcp", "0.1.0",
4543
server.WithResourceCapabilities(true, true),
4644
server.WithPromptCapabilities(true),
4745
server.WithLogging(),
4846
)
49-
if stdio {
50-
AddSessionTools(s)
51-
}
5247
tools.AddTraceTools(s)
5348
tools.AddLogTools(s)
5449
tools.AddMQETools(s)
@@ -131,63 +126,31 @@ func withConfiguredAuth(ctx context.Context) context.Context {
131126
return ctx
132127
}
133128

134-
// urlFromHeaders extracts URL for a request.
135-
// URL is sourced from Header > configured value > Default.
136-
func urlFromHeaders(req *http.Request) string {
137-
urlStr := req.Header.Get("SW-URL")
138-
if urlStr == "" {
139-
return configuredSkyWalkingURL()
140-
}
141-
142-
return tools.FinalizeURL(urlStr)
143-
}
144-
145-
// applySessionOverrides checks for a session in the context and applies any
146-
// URL or auth overrides that were set via the set_skywalking_url tool.
147-
func applySessionOverrides(ctx context.Context) context.Context {
148-
session := SessionFromContext(ctx)
149-
if session == nil {
150-
return ctx
151-
}
152-
if url := session.URL(); url != "" {
153-
ctx = context.WithValue(ctx, contextkey.BaseURL{}, url)
154-
}
155-
if username := session.Username(); username != "" {
156-
ctx = WithSkyWalkingAuth(ctx, username, session.Password())
157-
}
158-
return ctx
159-
}
160-
161129
// EnhanceStdioContextFunc returns a StdioContextFunc that enriches the context
162-
// with SkyWalking settings from the global configuration and a per-session store.
130+
// with SkyWalking settings from the global configuration.
163131
func EnhanceStdioContextFunc() server.StdioContextFunc {
164-
session := &Session{}
165132
return func(ctx context.Context) context.Context {
166-
ctx = WithSession(ctx, session)
167133
ctx = WithSkyWalkingURLAndInsecure(ctx, configuredSkyWalkingURL(), false)
168134
ctx = withConfiguredAuth(ctx)
169-
ctx = applySessionOverrides(ctx)
170135
return ctx
171136
}
172137
}
173138

174139
// EnhanceSSEContextFunc returns a SSEContextFunc that enriches the context
175-
// with SkyWalking settings from SSE request headers and CLI-configured auth.
140+
// with SkyWalking settings from the CLI configuration and configured auth.
176141
func EnhanceSSEContextFunc() server.SSEContextFunc {
177-
return func(ctx context.Context, req *http.Request) context.Context {
178-
urlStr := urlFromHeaders(req)
179-
ctx = WithSkyWalkingURLAndInsecure(ctx, urlStr, false)
142+
return func(ctx context.Context, _ *http.Request) context.Context {
143+
ctx = WithSkyWalkingURLAndInsecure(ctx, configuredSkyWalkingURL(), false)
180144
ctx = withConfiguredAuth(ctx)
181145
return ctx
182146
}
183147
}
184148

185149
// EnhanceHTTPContextFunc returns a HTTPContextFunc that enriches the context
186-
// with SkyWalking settings from HTTP request headers and CLI-configured auth.
150+
// with SkyWalking settings from the CLI configuration and configured auth.
187151
func EnhanceHTTPContextFunc() server.HTTPContextFunc {
188-
return func(ctx context.Context, req *http.Request) context.Context {
189-
urlStr := urlFromHeaders(req)
190-
ctx = WithSkyWalkingURLAndInsecure(ctx, urlStr, false)
152+
return func(ctx context.Context, _ *http.Request) context.Context {
153+
ctx = WithSkyWalkingURLAndInsecure(ctx, configuredSkyWalkingURL(), false)
191154
ctx = withConfiguredAuth(ctx)
192155
return ctx
193156
}

0 commit comments

Comments
 (0)