feat: add PromQL and trace diagnosis tools - #1499
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds read-only observability diagnosis capabilities to the Dubbo Admin MCP server by introducing PromQL query tools and a Jaeger trace lookup tool, plus configuration and agent prompt guidance to support intelligent diagnosis workflows.
Changes:
- Add MCP tools for Prometheus instant/range PromQL queries with bounded responses and query limits.
- Add MCP tool for Jaeger trace lookup by TraceID with normalized/bounded trace models and provider configuration.
- Expose observability capabilities (configured providers + limits) and document/update agent prompt policies for safe tool usage.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/mcp/tools/trace/tools.go | Adds MCP entrypoint for TraceID lookup with validation and provider selection. |
| pkg/mcp/tools/trace/provider.go | Defines provider interface and factory for trace backends. |
| pkg/mcp/tools/trace/model.go | Introduces normalized trace/span/event models returned to MCP callers. |
| pkg/mcp/tools/trace/jaeger.go | Implements Jaeger query client and normalization with size/field limits. |
| pkg/mcp/tools/trace/jaeger_test.go | Adds unit tests for Jaeger provider behavior and TraceID validation. |
| pkg/mcp/tools/observability.go | Adds tool to report configured Prometheus/trace providers and query limits. |
| pkg/mcp/tools/metrics/tools.go | Adds MCP entrypoints for instant and range PromQL queries. |
| pkg/mcp/tools/metrics/prometheus.go | Implements bounded Prometheus client, normalization, and enforced limits. |
| pkg/mcp/tools/metrics/prometheus_test.go | Adds unit tests for Prometheus query/normalization and limit enforcement. |
| pkg/mcp/tools/metrics/model.go | Introduces normalized Prometheus response models and limit descriptors. |
| pkg/mcp/server.go | Adds tool call timing/logging and nil-result handling. |
| pkg/mcp/register.go | Registers new observability MCP tools. |
| pkg/mcp/register_test.go | Verifies observability tools are registered with expected required fields. |
| pkg/mcp/DEVELOPMENT.md | Documents the new observability diagnosis tools and configuration. |
| pkg/config/observability/tracing.go | Adds tracing provider config types, validation, and sanitization. |
| pkg/config/observability/tracing_test.go | Adds tests for tracing validation and token sanitization. |
| pkg/config/observability/config.go | Wires tracing config into observability config validation/sanitization. |
| app/dubbo-admin/dubbo-admin.yaml | Adds commented example tracing provider configuration. |
| ai/prompts/agentThink.txt | Adds diagnosis rules guiding when/how to use observability tools. |
| ai/prompts/agentObserve.txt | Adds observation summarization requirements for metrics/traces. |
| ai/prompts/agentAct.txt | Adds strict tool selection policy for observability queries and safety constraints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+276
to
+285
| func parsePrometheusStep(value string) (time.Duration, error) { | ||
| if parsed, err := time.ParseDuration(value); err == nil { | ||
| return parsed, nil | ||
| } | ||
| seconds, err := strconv.ParseFloat(value, 64) | ||
| if err != nil || seconds <= 0 { | ||
| return 0, fmt.Errorf("step must be a positive duration or number of seconds") | ||
| } | ||
| return time.Duration(seconds * float64(time.Second)), nil | ||
| } |
Comment on lines
+172
to
+179
| if upstream.Status != "success" { | ||
| message := upstream.Error | ||
| if message == "" { | ||
| message = "unknown prometheus error" | ||
| } | ||
| logPrometheusFailure(queryType, query, startedAt, resp.StatusCode, upstream.ErrorType, nil) | ||
| return nil, fmt.Errorf("prometheus %s error: %s", upstream.ErrorType, message) | ||
| } |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Contributor
|
@YuZhangLarry Please help review the PR. |
|
@robocanic ，No problem, this can be merged. |
Contributor
|
@thunguo please merge the ai branch and resolve the conflicts. |
|
15 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please provide a description of this PR:
Add PromQL and trace diagnosis tools.
Related #1341
To help us figure out who should review this PR, please put an X in all the areas that this PR affects.
Please check any characteristics that apply to this pull request.