Copilot commented on code in PR #26:
URL: https://github.com/apache/skywalking-mcp/pull/26#discussion_r2923984139
##
internal/prompts/trace.go:
##
@@ -0,0 +1,202 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you under
+// the Apache License, Version 2.0 (the "License"); you may
+// not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package prompts
+
+import (
+ "context"
+ "fmt"
+
+ "github.com/mark3labs/mcp-go/mcp"
+)
+
+func traceInvestigationHandler(_ context.Context, request
mcp.GetPromptRequest) (*mcp.GetPromptResult, error) {
+ args := request.Params.Arguments
+ serviceID := args["service_id"]
+ traceState := args["trace_state"]
+ duration := args["duration"]
+
+ if duration == "" {
+ duration = defaultDuration
+ }
+ if traceState == "" {
+ traceState = "all"
+ }
+
+ // Use the dynamic tool instructions
+ toolInstructions := generateToolInstructions("trace_investigation")
+
+ prompt := fmt.Sprintf(`Investigate traces with filters:
service_id="%s", trace_state="%s", duration="%s".
+
+%s
+
+**Analysis Steps:**
+
+**Find Problematic Traces**
+- First use query_traces with view="summary" to get overview
+- Look for patterns in error traces, slow traces, or anomalies
+- Note trace IDs that need deeper investigation
+
+**Deep Dive on Specific Traces**
+- Use query_traces with the identified trace_id
Review Comment:
This prompt instructs using query_traces with a "duration" filter, but
query_traces now accepts start/end/step instead. Update the prompt to produce
start/end (e.g. end="now" and start derived from duration) so it matches the
actual tool parameters.
```suggestion
prompt := fmt.Sprintf(`Investigate traces with filters:
service_id="%s", trace_state="%s", time_range="%s".
%s
**Analysis Steps:**
**Find Problematic Traces**
- First use query_traces with end="now", start derived from the requested
time_range (e.g., now minus the time_range), and view="summary" to get an
overview
- Look for patterns in error traces, slow traces, or anomalies
- Note trace IDs that need deeper investigation
**Deep Dive on Specific Traces**
- Use query_traces with the identified trace_id and the same start/end time
window
```
##
internal/tools/log.go:
##
@@ -65,13 +68,26 @@ func buildLogQueryCondition(req *LogQueryRequest)
*api.LogQueryCondition {
paging := BuildPagination(req.PageNum, req.PageSize)
+ order := api.OrderDes
+ if req.QueryOrder == "ASC" {
+ order = api.OrderAsc
+ }
+
cond := &api.LogQueryCondition{
ServiceID: &req.ServiceID,
ServiceInstanceID: &req.ServiceInstanceID,
EndpointID:&req.EndpointID,
- RelatedTrace: &api.TraceScopeCondition{TraceID:
req.TraceID},
QueryDuration: &duration,
Paging:paging,
+ QueryOrder:&order,
+ }
+
+ if req.TraceID != "" || req.SegmentID != "" || req.SpanID != nil {
+ cond.RelatedTrace = &api.TraceScopeCondition{
+ TraceID: req.TraceID,
+ SegmentID: &req.SegmentID,
+ SpanID:req.SpanID,
+ }
Review Comment:
RelatedTrace always sets SegmentID to &req.SegmentID when any trace scope
filter is used. When only trace_id is provided (or only span_id), this passes
an empty segment_id pointer, which can unintentionally filter out all logs.
Only set SegmentID when req.SegmentID != "" (and similarly avoid setting
span/segment pointers unless provided).
```suggestion
traceScope := &api.TraceScopeCondition{}
if req.TraceID != "" {
traceScope.TraceID = req.TraceID
}
if req.SegmentID != "" {
// Only set SegmentID when it is actually provided to
avoid filtering by an empty segment.
traceScope.SegmentID = &req.SegmentID
}
if req.SpanID != nil {
traceScope.SpanID = req.SpanID
}
