Skip to content

fix(memory): accept string-form error filters in trace search #8217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yurishkuro merged 9 commits into from
Mar 27, 2026
Merged

fix(memory): accept string-form error filters in trace search #8217

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2da7a1d
fix(jaegermcp): use PutBool instead of PutStr for error attribute in …
YoungHypo Mar 21, 2026
61a51b0
fix(jaegermcp): fix test assertions for error bool attribute
YoungHypo Mar 21, 2026
271b35b
fix(memory): accept string-form error filters in trace search
YoungHypo Mar 23, 2026
0c0afb3
Merge branch 'main' into haibo-fix-error-type
YoungHypo Mar 23, 2026
4247cbd
fix(memory): change variable name by copilot suggestion
YoungHypo Mar 23, 2026
37edd99
test(memory): add TestFindTraces_ErrorAttributeInvalidType
YoungHypo Mar 23, 2026
223f28b
Merge branch 'main' into haibo-fix-error-type
YoungHypo Mar 23, 2026
4d2f672
Merge branch 'main' into haibo-fix-error-type
YoungHypo Mar 24, 2026
600299e
Merge branch 'main' into haibo-fix-error-type
YoungHypo Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/jaeger/internal/extension/jaegermcp/internal/handlers/search_traces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func (h *searchTracesHandler) buildQuery(input types.SearchTracesInput) (querysv

// If WithErrors is requested, add error attribute filter
if input.WithErrors {
attributes.PutStr("error", "true")
attributes.PutBool("error", true)
}
Comment thread
yurishkuro marked this conversation as resolved.

return querysvc.TraceQueryParams{
Expand Down
38 changes: 38 additions & 0 deletions cmd/jaeger/internal/extension/jaegermcp/internal/handlers/search_traces_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (

"github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegermcp/internal/types"
"github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerquery/querysvc"
"github.com/jaegertracing/jaeger/internal/storage/v2/memory"
)

// mockQueryService and createTestTrace are defined in test_helpers.go
Expand Down Expand Up @@ -187,6 +188,43 @@ func TestSearchTracesHandler_Handle_WithErrorsFilter(t *testing.T) {
assert.True(t, output.Traces[0].HasErrors)
}

func TestSearchTracesHandler_Handle_WithErrorsFilter_UsingMemoryStore(t *testing.T) {
store, err := memory.NewStore(memory.Configuration{MaxTraces: 10})
require.NoError(t, err)

require.NoError(t, store.WriteTraces(context.Background(), createTestTrace(
"trace111",
"test",
"/ok",
false,
)))
require.NoError(t, store.WriteTraces(context.Background(), createTestTrace(
"trace222",
"test",
"/error",
true,
)))

handler := &searchTracesHandler{
queryService: querysvc.NewQueryService(store, store, querysvc.QueryServiceOptions{}),
maxResults: 100,
}

input := types.SearchTracesInput{
StartTimeMin: "-1h",
ServiceName: "test",
WithErrors: true,
}

_, output, err := handler.handle(context.Background(), &mcp.CallToolRequest{}, input)

require.NoError(t, err)
assert.True(t, output.Traces[0].HasErrors)
assert.Equal(t, "test", output.Traces[0].RootService)
assert.Equal(t, "/error", output.Traces[0].RootSpanName)
assert.Empty(t, output.Error)
Comment thread
yurishkuro marked this conversation as resolved.
}
Comment on lines +191 to +227
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new memory-store test validates returned results, but it does not actually assert that the handler encodes the error query attribute as a boolean. If the memory backend accepts both string and bool (as it now does), this test won’t catch regressions back to string encoding. Consider asserting the query attribute type/value (via a mock query service) or tightening the memory backend behavior for this handler path so the test enforces boolean semantics.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@YoungHypo YoungHypo Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disagree. This test is intended to verify the end-to-end MCP -> QueryService -> memory behavior, not to enforce a boolean encoding in the handler.

The handler encoding is already covered by TestSearchTracesHandler_Handle_WithErrorsFilter, which explicitly checks that WithErrors is translated to the string query attribute error="true".

For this PR, string encoding is the intended behavior, and the memory backend change is specifically to make that existing query convention work correctly.


func TestSearchTracesHandler_Handle_SearchDepthDefault(t *testing.T) {
testTrace := createTestTrace("trace444", "test", "/test", false)

Expand Down
Loading