Skip to content

jaegermcp: enforce MaxSpanDetailsPerRequest in get_span_details #8174

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

Open
jkowall wants to merge 2 commits into
base: main
Choose a base branch
from
Open

jaegermcp: enforce MaxSpanDetailsPerRequest in get_span_details #8174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e379982
jaegermcp: enforce MaxSpanDetailsPerRequest in get_span_details
jkowall Mar 14, 2026
c47b392
jaegermcp: keep limit validation in one place
jkowall Mar 14, 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
11 changes: 10 additions & 1 deletion cmd/jaeger/internal/extension/jaegermcp/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package jaegermcp

import (
"fmt"

"github.com/asaskevich/govalidator"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/confmap/xconfmap"
Expand All @@ -21,7 +23,7 @@
ServerVersion string `mapstructure:"server_version" valid:"required"`

// MaxSpanDetailsPerRequest limits the number of spans that can be fetched in a single request.
MaxSpanDetailsPerRequest int `mapstructure:"max_span_details_per_request" valid:"range(1|100)"`
Copy link
Member

@yurishkuro yurishkuro Mar 14, 2026

Choose a reason for hiding this comment

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

why remove validation?

MaxSpanDetailsPerRequest int `mapstructure:"max_span_details_per_request"`

// MaxSearchResults limits the number of trace search results.
MaxSearchResults int `mapstructure:"max_search_results" valid:"range(1|1000)"`
Expand All @@ -32,6 +34,13 @@
// if cfg.ServerVersion == "" {
// cfg.ServerVersion = version.Get().GitVersion
// }
if cfg.MaxSpanDetailsPerRequest < 1 {
return fmt.Errorf("max_span_details_per_request must be at least 1")

Check failure on line 38 in cmd/jaeger/internal/extension/jaegermcp/config.go

View workflow job for this annotation

GitHub Actions / stage1-seq / lint-checks / lint 

use-errors-new: replace fmt.Errorf by errors.New (revive)
}
if cfg.MaxSpanDetailsPerRequest > 100 {
Copy link
Member

@yurishkuro yurishkuro Mar 14, 2026

Choose a reason for hiding this comment

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

why do this manually instead of via govalidator.ValidateStruct ?

return fmt.Errorf("max_span_details_per_request must not exceed 100")

Check failure on line 41 in cmd/jaeger/internal/extension/jaegermcp/config.go

View workflow job for this annotation

GitHub Actions / stage1-seq / lint-checks / lint 

use-errors-new: replace fmt.Errorf by errors.New (revive)
}

_, err := govalidator.ValidateStruct(cfg)
return err
}
Expand Down
9 changes: 9 additions & 0 deletions cmd/jaeger/internal/extension/jaegermcp/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ func TestValidate(t *testing.T) {
},
expectError: false,
},
{
name: "Invalid MaxSpanDetailsPerRequest (too low)",
config: &Config{
ServerVersion: "1.0.0",
MaxSpanDetailsPerRequest: 0,
MaxSearchResults: 100,
},
expectError: true,
},
{
name: "Invalid MaxSpanDetailsPerRequest (too high)",
config: &Config{
Expand Down
16 changes: 13 additions & 3 deletions cmd/jaeger/internal/extension/jaegermcp/internal/handlers/get_span_details.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,18 @@ type queryServiceGetTracesInterface interface {
// This tool fetches full OTLP details (attributes, events, links, status) for specific spans
// within a trace, allowing deep inspection of individual span data for debugging and analysis.
type getSpanDetailsHandler struct {
queryService queryServiceGetTracesInterface
queryService queryServiceGetTracesInterface
maxSpanDetailsPerRequest int
}

// NewGetSpanDetailsHandler creates a new get_span_details handler and returns the handler function.
func NewGetSpanDetailsHandler(
queryService *querysvc.QueryService,
maxSpanDetailsPerRequest int,
) mcp.ToolHandlerFor[types.GetSpanDetailsInput, types.GetSpanDetailsOutput] {
h := &getSpanDetailsHandler{
queryService: queryService,
queryService: queryService,
maxSpanDetailsPerRequest: maxSpanDetailsPerRequest,
}
return h.handle
}
Expand Down Expand Up @@ -115,7 +118,7 @@ func (h *getSpanDetailsHandler) handle(
}

// buildQuery converts GetSpanDetailsInput to querysvc.GetTraceParams.
func (*getSpanDetailsHandler) buildQuery(input types.GetSpanDetailsInput) (querysvc.GetTraceParams, error) {
func (h *getSpanDetailsHandler) buildQuery(input types.GetSpanDetailsInput) (querysvc.GetTraceParams, error) {
// Validate input
if input.TraceID == "" {
return querysvc.GetTraceParams{}, errors.New("trace_id is required")
Expand All @@ -125,6 +128,13 @@ func (*getSpanDetailsHandler) buildQuery(input types.GetSpanDetailsInput) (query
return querysvc.GetTraceParams{}, errors.New("span_ids is required and must not be empty")
}

if len(input.SpanIDs) > h.maxSpanDetailsPerRequest {
return querysvc.GetTraceParams{}, fmt.Errorf(
"span_ids must not exceed %d items",
Copy link
Member

@yurishkuro yurishkuro Mar 14, 2026

Choose a reason for hiding this comment

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

Suggested change
"span_ids must not exceed %d items",
"Number of input Span IDs must not exceed %d items",

h.maxSpanDetailsPerRequest,
)
}

traceID, err := parseTraceID(input.TraceID)
if err != nil {
return querysvc.GetTraceParams{}, fmt.Errorf("invalid trace_id: %w", err)
Expand Down
49 changes: 36 additions & 13 deletions cmd/jaeger/internal/extension/jaegermcp/internal/handlers/get_span_details_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,15 @@ import (

// Helper types and functions are defined in test_helpers.go

const testMaxSpanDetailsPerRequest = 20

func newTestGetSpanDetailsHandler(mock queryServiceGetTracesInterface) *getSpanDetailsHandler {
return &getSpanDetailsHandler{
queryService: mock,
maxSpanDetailsPerRequest: testMaxSpanDetailsPerRequest,
}
}

func TestGetSpanDetailsHandler_Handle_Success(t *testing.T) {
traceID := testTraceID
spanID1 := "span001"
Expand Down Expand Up @@ -59,7 +68,7 @@ func TestGetSpanDetailsHandler_Handle_Success(t *testing.T) {

mock := newMockYieldingTraces(testTrace)

handler := &getSpanDetailsHandler{queryService: mock}
handler := newTestGetSpanDetailsHandler(mock)

input := types.GetSpanDetailsInput{
TraceID: traceID,
Expand Down Expand Up @@ -115,7 +124,7 @@ func TestGetSpanDetailsHandler_Handle_SingleSpan(t *testing.T) {

mock := newMockYieldingTraces(testTrace)

handler := &getSpanDetailsHandler{queryService: mock}
handler := newTestGetSpanDetailsHandler(mock)

input := types.GetSpanDetailsInput{
TraceID: traceID,
Expand Down Expand Up @@ -143,7 +152,7 @@ func TestGetSpanDetailsHandler_Handle_FiltersBySpanIDs(t *testing.T) {

mock := newMockYieldingTraces(testTrace)

handler := &getSpanDetailsHandler{queryService: mock}
handler := newTestGetSpanDetailsHandler(mock)

input := types.GetSpanDetailsInput{
TraceID: traceID,
Expand All @@ -166,7 +175,7 @@ func TestGetSpanDetailsHandler_Handle_FiltersBySpanIDs(t *testing.T) {
}

func TestGetSpanDetailsHandler_Handle_MissingTraceID(t *testing.T) {
handler := NewGetSpanDetailsHandler(nil)
handler := NewGetSpanDetailsHandler(nil, testMaxSpanDetailsPerRequest)

input := types.GetSpanDetailsInput{
SpanIDs: []string{spanIDToHex("span001")},
Expand All @@ -179,7 +188,7 @@ func TestGetSpanDetailsHandler_Handle_MissingTraceID(t *testing.T) {
}

func TestGetSpanDetailsHandler_Handle_MissingSpanIDs(t *testing.T) {
handler := NewGetSpanDetailsHandler(nil)
handler := NewGetSpanDetailsHandler(nil, testMaxSpanDetailsPerRequest)

input := types.GetSpanDetailsInput{
TraceID: testTraceID,
Expand All @@ -192,8 +201,22 @@ func TestGetSpanDetailsHandler_Handle_MissingSpanIDs(t *testing.T) {
assert.Contains(t, err.Error(), "span_ids is required")
}

func TestGetSpanDetailsHandler_Handle_TooManySpanIDs(t *testing.T) {
handler := NewGetSpanDetailsHandler(nil, 2)

input := types.GetSpanDetailsInput{
TraceID: testTraceID,
SpanIDs: []string{spanIDToHex("span001"), spanIDToHex("span002"), spanIDToHex("span003")},
}

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

require.Error(t, err)
assert.Contains(t, err.Error(), "span_ids must not exceed 2 items")
}

func TestGetSpanDetailsHandler_Handle_InvalidTraceID(t *testing.T) {
handler := NewGetSpanDetailsHandler(nil)
handler := NewGetSpanDetailsHandler(nil, testMaxSpanDetailsPerRequest)

input := types.GetSpanDetailsInput{
TraceID: "invalid-trace-id",
Expand All @@ -209,7 +232,7 @@ func TestGetSpanDetailsHandler_Handle_InvalidTraceID(t *testing.T) {
func TestGetSpanDetailsHandler_Handle_TraceNotFound(t *testing.T) {
mock := newMockYieldingEmpty()

handler := &getSpanDetailsHandler{queryService: mock}
handler := newTestGetSpanDetailsHandler(mock)

input := types.GetSpanDetailsInput{
TraceID: testTraceID,
Expand All @@ -225,7 +248,7 @@ func TestGetSpanDetailsHandler_Handle_TraceNotFound(t *testing.T) {
func TestGetSpanDetailsHandler_Handle_QueryError(t *testing.T) {
mock := newMockYieldingError(errors.New("database connection failed"))

handler := &getSpanDetailsHandler{queryService: mock}
handler := newTestGetSpanDetailsHandler(mock)

input := types.GetSpanDetailsInput{
TraceID: testTraceID,
Expand Down Expand Up @@ -262,7 +285,7 @@ func TestGetSpanDetailsHandler_Handle_PartialResults(t *testing.T) {
},
}

handler := &getSpanDetailsHandler{queryService: mock}
handler := newTestGetSpanDetailsHandler(mock)

input := types.GetSpanDetailsInput{
TraceID: traceID,
Expand Down Expand Up @@ -300,7 +323,7 @@ func TestGetSpanDetailsHandler_Handle_MultipleIterations(t *testing.T) {
},
}

handler := &getSpanDetailsHandler{queryService: mock}
handler := newTestGetSpanDetailsHandler(mock)

input := types.GetSpanDetailsInput{
TraceID: traceID,
Expand Down Expand Up @@ -335,7 +358,7 @@ func TestGetSpanDetailsHandler_Handle_WithParentSpanID(t *testing.T) {

mock := newMockYieldingTraces(testTrace)

handler := &getSpanDetailsHandler{queryService: mock}
handler := newTestGetSpanDetailsHandler(mock)

input := types.GetSpanDetailsInput{
TraceID: traceID,
Expand All @@ -360,7 +383,7 @@ func TestGetSpanDetailsHandler_Handle_NoMatchingSpans(t *testing.T) {

mock := newMockYieldingTraces(testTrace)

handler := &getSpanDetailsHandler{queryService: mock}
handler := newTestGetSpanDetailsHandler(mock)

input := types.GetSpanDetailsInput{
TraceID: traceID,
Expand Down Expand Up @@ -389,7 +412,7 @@ func TestGetSpanDetailsHandler_Handle_PartialMissingSpans(t *testing.T) {

mock := newMockYieldingTraces(testTrace)

handler := &getSpanDetailsHandler{queryService: mock}
handler := newTestGetSpanDetailsHandler(mock)

input := types.GetSpanDetailsInput{
TraceID: traceID,
Expand Down
2 changes: 1 addition & 1 deletion cmd/jaeger/internal/extension/jaegermcp/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func (s *server) registerTools() {
}, searchTracesHandler)

// Get span details tool
getSpanDetailsHandler := handlers.NewGetSpanDetailsHandler(s.queryAPI)
getSpanDetailsHandler := handlers.NewGetSpanDetailsHandler(s.queryAPI, s.config.MaxSpanDetailsPerRequest)
mcp.AddTool(s.mcpServer, &mcp.Tool{
Name: "get_span_details",
Description: "Fetch full details (attributes, events, links, status) for specific spans.",
Expand Down
Loading