Skip to content

Commit 5e4186a

Browse files
ioanarmioanarm
committed
clean up - review comments
1 parent 145e58e commit 5e4186a

File tree

2 files changed

+70
-113
lines changed

2 files changed

+70
-113
lines changed

‎tools/sift.go

+67-110
Original file line numberDiff line numberDiff line change
@@ -35,100 +35,56 @@ const (
3535
)
3636

3737
type InvestigationRequest struct {
38-
AlertLabels map[string]string `json:"alertLabels,omitempty" gorm:"-"`
38+
AlertLabels map[string]string `json:"alertLabels,omitempty"`
3939
Labels map[string]string `json:"labels"`
4040

41-
Start time.Time `json:"start" gorm:"not null"`
42-
End time.Time `json:"end" gorm:"not null"`
43-
44-
// TODO: Add this when we have a new investigation source field InvestigationSourceTypeMCP.
45-
// InvestigationSource InvestigationSource `json:"investigationSource" gorm:"embedded;embeddedPrefix:investigation_source_"`
41+
Start time.Time `json:"start"`
42+
End time.Time `json:"end"`
4643

4744
QueryURL string `json:"queryUrl"`
4845

49-
Checks []string `json:"checks" gorm:"serializer:json"`
50-
}
51-
52-
// AnalysisStep represents a single step in the analysis process.
53-
type AnalysisStep struct {
54-
CreatedAt time.Time `json:"created" validate:"isdefault"`
55-
// State that the Analysis is entering.
56-
State string `json:"state"`
57-
// The exit message of the step. Can be empty if the step was successful.
58-
ExitMessage string `json:"exitMessage"`
59-
// Runtime statistics for this step
60-
Stats map[string]interface{} `json:"stats,omitempty"`
46+
Checks []string `json:"checks"`
6147
}
6248

63-
type AnalysisEvent struct {
64-
StartTime time.Time `json:"startTime"`
65-
EndTime time.Time `json:"endTime"`
66-
Name string `json:"name"`
67-
Description string `json:"description,omitempty"`
49+
// Interesting: The analysis complete with results that indicate a probable cause for failure.
50+
type AnalysisResult struct {
51+
Successful bool `json:"successful"`
52+
Interesting bool `json:"interesting"`
53+
Message string `json:"message"`
6854
Details map[string]interface{} `json:"details"`
6955
}
7056

71-
// Interesting: The analysis complete with results that indicate a probable cause for failure.
72-
type AnalysisResult struct {
73-
Successful bool `json:"successful"`
74-
Interesting bool `json:"interesting"`
75-
Message string `json:"message"`
76-
MarkdownSummary string `json:"-" gorm:"-"`
77-
Details map[string]interface{} `json:"details"`
78-
Events []AnalysisEvent `json:"events,omitempty" gorm:"serializer:json"`
57+
type AnalysisMeta struct {
58+
Items []Analysis `json:"items"`
7959
}
8060

8161
// An Analysis struct provides the status and results
8262
// of running a specific type of check.
8363
type Analysis struct {
84-
ID uuid.UUID `json:"id" gorm:"primarykey;type:char(36)" validate:"isdefault"`
85-
CreatedAt time.Time `json:"created" validate:"isdefault"`
86-
UpdatedAt time.Time `json:"modified" validate:"isdefault"`
64+
ID uuid.UUID `json:"id"`
65+
CreatedAt time.Time `json:"created"`
66+
UpdatedAt time.Time `json:"modified"`
8767

88-
Status AnalysisStatus `json:"status" gorm:"default:pending;index:idx_analyses_stats,priority:100"`
89-
StartedAt *time.Time `json:"started" validate:"isdefault"`
68+
Status AnalysisStatus `json:"status"`
69+
StartedAt *time.Time `json:"started"`
9070

9171
// Foreign key to the Investigation that created this Analysis.
92-
InvestigationID uuid.UUID `json:"investigationId" gorm:"index:idx_analyses_stats,priority:10"`
72+
InvestigationID uuid.UUID `json:"investigationId"`
9373

9474
// Name is the name of the check that this analysis represents.
9575
Name string `json:"name"`
9676
Title string `json:"title"`
97-
Steps []AnalysisStep `json:"steps" gorm:"foreignKey:AnalysisID;constraint:OnDelete:CASCADE"`
98-
Result AnalysisResult `json:"result" gorm:"embedded;embeddedPrefix:result_"`
99-
}
100-
101-
type DatasourceConfig struct {
102-
LokiDatasource DatasourceInfo `json:"lokiDatasource" gorm:"not null;embedded;embeddedPrefix:loki_"`
103-
PrometheusDatasource DatasourceInfo `json:"prometheusDatasource" gorm:"not null;embedded;embeddedPrefix:prometheus_"`
104-
TempoDatasource DatasourceInfo `json:"tempoDatasource" gorm:"not null;embedded;embeddedPrefix:tempo_"`
105-
PyroscopeDatasource DatasourceInfo `json:"pyroscopeDatasource" gorm:"not null;embedded;embeddedPrefix:pyroscope_"`
106-
}
107-
108-
type DatasourceInfo struct {
109-
Uid string `json:"uid"`
110-
}
111-
112-
// AnalysisMeta represents metadata about the analyses
113-
type AnalysisMeta struct {
114-
CountsByStage map[string]interface{} `json:"countsByStage"`
115-
Items []Analysis `json:"items"`
77+
Result AnalysisResult `json:"result"`
11678
}
11779

11880
type Investigation struct {
119-
ID uuid.UUID `json:"id" gorm:"primarykey;type:char(36)" validate:"isdefault"`
120-
CreatedAt time.Time `json:"created" gorm:"index" validate:"isdefault"`
121-
UpdatedAt time.Time `json:"modified" validate:"isdefault"`
122-
123-
TenantID string `json:"tenantId" gorm:"index;not null;size:256"`
81+
ID uuid.UUID `json:"id"`
82+
CreatedAt time.Time `json:"created"`
83+
UpdatedAt time.Time `json:"modified"`
12484

125-
Datasources DatasourceConfig `json:"datasources" gorm:"embedded;embeddedPrefix:datasources_"`
85+
TenantID string `json:"tenantId"`
12686

127-
Name string `json:"name"`
128-
RequestData InvestigationRequest `json:"requestData" gorm:"not null;embedded;embeddedPrefix:request_"`
129-
130-
// TODO: Add this when we want to extract discovered inputs for later usage
131-
// Inputs Inputs `json:"inputs" gorm:"serializer:json"`
87+
Name string `json:"name"`
13288

13389
// GrafanaURL is the Grafana URL to be used for datasource queries
13490
// for this investigation.
@@ -141,15 +97,9 @@ type Investigation struct {
14197
// investigation failed.
14298
FailureReason string `json:"failureReason,omitempty"`
14399

144-
// Analyses contains metadata about the investigation's analyses
145100
Analyses AnalysisMeta `json:"analyses"`
146101
}
147102

148-
type RequestData struct {
149-
Labels map[string]string `json:"labels"`
150-
Checks []string `json:"checks"`
151-
}
152-
153103
// SiftClient represents a client for interacting with the Sift API
154104
type SiftClient struct {
155105
client *http.Client
@@ -254,7 +204,7 @@ func getAnalysis(ctx context.Context, args GetAnalysisParams) (*Analysis, error)
254204
// GetAnalysis is a tool for retrieving a specific analysis from an investigation
255205
var GetAnalysis = mcpgrafana.MustTool(
256206
"get_analysis",
257-
"Retrieves a specific analysis from a Sift investigation by their UUIDs. Both the investigation ID and analysis ID should be provided as strings in UUID format.",
207+
"Retrieves a specific analysis from an investigation by its UUID. The investigation ID and analysis ID should be provided as strings in UUID format.",
258208
getAnalysis,
259209
)
260210

@@ -290,17 +240,17 @@ var ListInvestigations = mcpgrafana.MustTool(
290240
listInvestigations,
291241
)
292242

293-
// RunErrorPatternLogsParams defines the parameters for running an ErrorPatternLogs check
294-
type RunErrorPatternLogsParams struct {
243+
// FindErrorPatternLogsParams defines the parameters for running an ErrorPatternLogs check
244+
type FindErrorPatternLogsParams struct {
295245
Name string `json:"name" jsonschema:"required,description=The name of the investigation"`
296246
Labels map[string]string `json:"labels" jsonschema:"required,description=Labels to scope the analysis"`
297247
Start time.Time `json:"start,omitempty" jsonschema:"description=Start time for the investigation. Defaults to 30 minutes ago if not specified."`
298248
End time.Time `json:"end,omitempty" jsonschema:"description=End time for the investigation. Defaults to now if not specified."`
299249
QueryURL string `json:"queryUrl,omitempty" jsonschema:"description=Optional query URL for the investigation"`
300250
}
301251

302-
// runErrorPatternLogs creates an investigation with ErrorPatternLogs check, waits for it to complete, and returns the analysis
303-
func runErrorPatternLogs(ctx context.Context, args RunErrorPatternLogsParams) (*Analysis, error) {
252+
// findErrorPatternLogs creates an investigation with ErrorPatternLogs check, waits for it to complete, and returns the analysis
253+
func findErrorPatternLogs(ctx context.Context, args FindErrorPatternLogsParams) (*Analysis, error) {
304254
client, err := siftClientFromContext(ctx)
305255
if err != nil {
306256
return nil, fmt.Errorf("creating Sift client: %w", err)
@@ -316,14 +266,13 @@ func runErrorPatternLogs(ctx context.Context, args RunErrorPatternLogsParams) (*
316266
}
317267

318268
investigation := &Investigation{
319-
Name: args.Name,
320-
RequestData: requestData,
321-
GrafanaURL: client.url,
322-
Status: InvestigationStatusPending,
269+
Name: args.Name,
270+
GrafanaURL: client.url,
271+
Status: InvestigationStatusPending,
323272
}
324273

325274
// Create the investigation and wait for it to complete
326-
completedInvestigation, err := client.createInvestigation(ctx, investigation)
275+
completedInvestigation, err := client.createInvestigation(ctx, investigation, requestData)
327276
if err != nil {
328277
return nil, fmt.Errorf("creating investigation: %w", err)
329278
}
@@ -350,24 +299,24 @@ func runErrorPatternLogs(ctx context.Context, args RunErrorPatternLogsParams) (*
350299
return errorPatternLogsAnalysis, nil
351300
}
352301

353-
// RunErrorPatternLogs is a tool for running an ErrorPatternLogs check
354-
var RunErrorPatternLogs = mcpgrafana.MustTool(
355-
"run_error_pattern_logs",
356-
"Creates a Sift investigation with ErrorPatternLogs check, waits for it to complete, and returns the analysis results. This tool triggers an investigation with the ErrorPatternLogs check in the relevant Loki datasource. It investigates if the there are elevated errors rates in the logs compared to the last day's average and returns the error pattern found, if any.",
357-
runErrorPatternLogs,
302+
// FindErrorPatternLogs is a tool for running an ErrorPatternLogs check
303+
var FindErrorPatternLogs = mcpgrafana.MustTool(
304+
"find_error_pattern_logs",
305+
"Creates an investigation to search for error patterns in logs, waits for it to complete, and returns the analysis results. This tool triggers an investigation in the relevant Loki datasource to determine if there are elevated error rates compared to the last day's average, and returns the error pattern found, if any.",
306+
findErrorPatternLogs,
358307
)
359308

360-
// RunSlowRequestsCheckParams defines the parameters for running an SlowRequests check
361-
type RunSlowRequestsCheckParams struct {
309+
// FindSlowRequestsParams defines the parameters for running an SlowRequests check
310+
type FindSlowRequestsParams struct {
362311
Name string `json:"name" jsonschema:"required,description=The name of the investigation"`
363312
Labels map[string]string `json:"labels" jsonschema:"required,description=Labels to scope the analysis"`
364313
Start time.Time `json:"start,omitempty" jsonschema:"description=Start time for the investigation. Defaults to 30 minutes ago if not specified."`
365314
End time.Time `json:"end,omitempty" jsonschema:"description=End time for the investigation. Defaults to now if not specified."`
366315
QueryURL string `json:"queryUrl,omitempty" jsonschema:"description=Optional query URL for the investigation"`
367316
}
368317

369-
// runSlowRequests creates an investigation with SlowRequests check, waits for it to complete, and returns the analysis
370-
func runSlowRequests(ctx context.Context, args RunSlowRequestsCheckParams) (*Analysis, error) {
318+
// findSlowRequests creates an investigation with SlowRequests check, waits for it to complete, and returns the analysis
319+
func findSlowRequests(ctx context.Context, args FindSlowRequestsParams) (*Analysis, error) {
371320
client, err := siftClientFromContext(ctx)
372321
if err != nil {
373322
return nil, fmt.Errorf("creating Sift client: %w", err)
@@ -383,14 +332,13 @@ func runSlowRequests(ctx context.Context, args RunSlowRequestsCheckParams) (*Ana
383332
}
384333

385334
investigation := &Investigation{
386-
Name: args.Name,
387-
RequestData: requestData,
388-
GrafanaURL: client.url,
389-
Status: InvestigationStatusPending,
335+
Name: args.Name,
336+
GrafanaURL: client.url,
337+
Status: InvestigationStatusPending,
390338
}
391339

392340
// Create the investigation and wait for it to complete
393-
completedInvestigation, err := client.createInvestigation(ctx, investigation)
341+
completedInvestigation, err := client.createInvestigation(ctx, investigation, requestData)
394342
if err != nil {
395343
return nil, fmt.Errorf("creating investigation: %w", err)
396344
}
@@ -417,20 +365,20 @@ func runSlowRequests(ctx context.Context, args RunSlowRequestsCheckParams) (*Ana
417365
return slowRequestsAnalysis, nil
418366
}
419367

420-
// RunSlowRequestsCheck is a tool for running an SlowRequests check
421-
var RunSlowRequestsCheck = mcpgrafana.MustTool(
422-
"run_slow_requests_check",
423-
"Creates a Sift investigation with SlowRequests check in the relevant Tempo datasources, waits for it to complete, and returns the analysis results.",
424-
runSlowRequests,
368+
// FindSlowRequests is a tool for running an SlowRequests check
369+
var FindSlowRequests = mcpgrafana.MustTool(
370+
"find_slow_requests",
371+
"Creates an investigation to search for slow requests in the relevant Tempo datasources, waits for it to complete, and returns the analysis results.",
372+
findSlowRequests,
425373
)
426374

427375
// AddSiftTools registers all Sift tools with the MCP server
428376
func AddSiftTools(mcp *server.MCPServer) {
429377
GetInvestigation.Register(mcp)
430378
GetAnalysis.Register(mcp)
431379
ListInvestigations.Register(mcp)
432-
RunErrorPatternLogs.Register(mcp)
433-
RunSlowRequestsCheck.Register(mcp)
380+
FindErrorPatternLogs.Register(mcp)
381+
FindSlowRequests.Register(mcp)
434382
}
435383

436384
// makeRequest is a helper method to make HTTP requests and handle common response patterns
@@ -486,16 +434,25 @@ func (c *SiftClient) getInvestigation(ctx context.Context, id uuid.UUID) (*Inves
486434
return &investigationResponse.Data, nil
487435
}
488436

489-
func (c *SiftClient) createInvestigation(ctx context.Context, investigation *Investigation) (*Investigation, error) {
437+
func (c *SiftClient) createInvestigation(ctx context.Context, investigation *Investigation, requestData InvestigationRequest) (*Investigation, error) {
490438
// Set default time range to last 30 minutes if not provided
491-
if investigation.RequestData.Start.IsZero() {
492-
investigation.RequestData.Start = time.Now().Add(-30 * time.Minute)
439+
if requestData.Start.IsZero() {
440+
requestData.Start = time.Now().Add(-30 * time.Minute)
493441
}
494-
if investigation.RequestData.End.IsZero() {
495-
investigation.RequestData.End = time.Now()
442+
if requestData.End.IsZero() {
443+
requestData.End = time.Now()
444+
}
445+
446+
// Create the payload including the necessary fields for the API
447+
payload := struct {
448+
Investigation
449+
RequestData InvestigationRequest `json:"requestData"`
450+
}{
451+
Investigation: *investigation,
452+
RequestData: requestData,
496453
}
497454

498-
jsonData, err := json.Marshal(investigation)
455+
jsonData, err := json.Marshal(payload)
499456
if err != nil {
500457
return nil, fmt.Errorf("marshaling investigation: %w", err)
501458
}

‎tools/sift_cloud_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,9 @@ func TestCloudSiftInvestigations(t *testing.T) {
6767
// Verify all required fields are present
6868
assert.NotEmpty(t, result.Name, "Investigation should have a name")
6969
assert.NotEmpty(t, result.TenantID, "Investigation should have a tenant ID")
70-
assert.NotNil(t, result.Datasources, "Investigation should have datasources")
71-
assert.NotNil(t, result.RequestData, "Investigation should have request data")
72-
assert.NotNil(t, result.Analyses, "Investigation should have analyses")
70+
assert.NotNil(t, result.GrafanaURL, "Investigation should have a Grafana URL")
71+
assert.NotNil(t, result.Status, "Investigation should have a status")
72+
assert.NotNil(t, result.FailureReason, "Investigation should have a failure reason")
7373
})
7474

7575
// Test getting a non-existent investigation

0 commit comments

Comments
 (0)