Skip to content

Conversation

@apetti1920
Copy link
Contributor

@apetti1920 apetti1920 commented Nov 6, 2025

Problem

The list_traces tool was failing with multiple related issues:

Issue 1: Invalid QueryType Value (400 Bad Request)

ListTraces failed: proto: (line 1:58): invalid value for enum field queryType: {
(category=INVALID_REQUEST_ERROR code=BAD_REQUEST)

Issue 2: Nil Pointer Dereference Panic

panic: runtime error: invalid memory address or nil pointer dereference
time.Time.locabs({0x40003950e0, 0xdc6c84, 0x11})

Root Cause

go-swagger generates broken code when using allOf with enum types in OpenAPI specs. The generated Datav1ListTracesRequest struct has:

QueryType struct {
    ListTracesRequestQueryType
} `json:"query_type,omitempty"`

This embedded anonymous struct doesn't marshal correctly to JSON. Instead of:

{"query_type": "SERVICE_OPERATION"}

It produces:

{"query_type": {"ListTracesRequestQueryType": "SERVICE_OPERATION"}}

The API expects the plain string enum value, not a nested object.

Additionally, attempts to use unsafe.Pointer to cast between types caused memory corruption because the struct layouts were incompatible, leading to nil pointer dereferences when marshaling time fields.

Solution

Implemented a complete workaround that bypasses the broken generated client code:

  1. Custom Request Wrapper: Created listTracesRequestBody that embeds the generated model by VALUE (not pointer) and stores queryType separately
  2. Custom JSON Marshaling: Implemented MarshalBinary() that produces correctly formatted JSON with query_type as a plain string
  3. Direct HTTP Calls: Bypassed the generated client entirely and made direct HTTP requests with properly marshaled JSON
  4. Reflection-based Transport Extraction: Used reflection to extract the HTTP client and base URL from the runtime transport

Code Changes

mcp-server/pkg/tools/traces/traces.go:

  • Added httpClient and baseURL fields to Tools struct
  • Updated NewTools() to extract HTTP client and base URL using reflection
  • Created listTracesRequestBody wrapper type (embedding by value, not pointer)
  • Implemented custom MarshalBinary() and Validate() methods
  • Replaced generated client call with direct HTTP POST request
  • Added proper error handling and response parsing

mcp-server/pkg/tools/traces/traces_test.go:

  • Added comprehensive serialization tests
  • Verifies query_type serializes as a string, not an object
  • Tests both SERVICE_OPERATION and TRACE_IDS query types
  • Updated to use value embedding instead of pointer embedding

Testing

go test ./mcp-server/pkg/tools/traces/... -v
go build ./mcp-server/...

All tests pass, confirming:

  • ✅ JSON serializes correctly with query_type as a string value
  • ✅ No nested object structure
  • ✅ No nil pointer dereferences
  • ✅ Compatible with Chronosphere API expectations
  • ✅ Direct HTTP calls work without the broken generated client

Technical Details

Why Direct HTTP Calls?

Multiple approaches were attempted:

  1. unsafe.Pointer cast: Caused memory corruption due to incompatible struct layouts
  2. Reflection-based method override: Go's static method dispatch prevented this
  3. json.RawMessage: Type system prevented assignment to typed field
  4. Direct HTTP calls: Clean solution that completely bypasses the broken code

Struct Layout

The key insight was embedding by VALUE:

type listTracesRequestBody struct {
    models.Datav1ListTracesRequest  // Embedded by value
    queryType models.ListTracesRequestQueryType
}

This allows us to:

  • Implement custom MarshalBinary() that's actually called
  • Avoid pointer indirection issues
  • Maintain type safety
  • Produce correct JSON output

Reflection Usage

Since httptransport.Runtime has unexported fields, we use reflection to extract:

  • The underlying *http.Client for making requests
  • The Host field for constructing the base URL

This is safe because we have fallback defaults if reflection fails.

Notes

  • This is a workaround for a known go-swagger bug with allOf + enum types
  • The solution is production-ready and tested
  • If the OpenAPI spec or go-swagger code generation improves, this workaround can be removed
  • The direct HTTP approach is actually simpler and more maintainable than the generated client for this endpoint
  • Auth headers and other transport concerns are handled by the extracted HTTP client

@chucknthem
Copy link
Collaborator

hey @apetti1920, thanks for contributing a work around. We're going to try and fix the schema in #118 instead. I'm at kubecon this week so will be a bit delayed getting the fix out, but we should have it merged and rolled out within 1 week.

@chucknthem chucknthem closed this Dec 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants