Skip to content

Sql insight api #3089

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.

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
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Sql insight api #3089

wants to merge 2 commits into from

Conversation

BugsGuru
Copy link
Collaborator

@BugsGuru BugsGuru commented Jul 16, 2025

User description

关联的 issue

https://github.com/actiontech/sqle-ee/issues/2450

描述你的变更

新增性能洞察功能相关接口定义

确认项(pr提交后操作)

Tip

请在指定复审人之前,确认并完成以下事项,完成后✅


  • 我已完成自测
  • 我已记录完整日志方便进行诊断
  • 我已在关联的issue里补充了实现方案
  • 我已在关联的issue里补充了测试影响面
  • 我已确认了变更的兼容性,如果不兼容则在issue里标记 not_compatible
  • 我已确认了是否要更新文档,如果要更新则在issue里标记 need_update_doc


Description

  • 新增 SQL 性能洞察 API 接口

  • 新增相关 SQL 与事务关联接口

  • 更新 Swagger 文档和接口说明

  • 扩展数据结构和错误返回处理


Diagram Walkthrough

flowchart LR
  A["app.go 新增端点"] --> B["sql_manage_insight.go 实现接口"]
  A --> C["sql_manage_insight_ce.go 返回错误"]
  B --> D["Swagger 文档更新"]
Loading

File Walkthrough

Relevant files

@actiontech-bot actiontech-bot requested a review from iwanghc July 16, 2025 09:18
Copy link

github-actions bot commented Jul 16, 2025

PR Code Suggestions ✨

Latest suggestions up to dda2b43
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
优化切片类型使用方式

建议将 Points 的类型从指针切片改为非指针切片,这样可以减少 nil 检查并且提高代码健壮性,防止在遍历时出现 nil 指针引发 panic。或者在使用前确保对
nil 值进行了充分检查。

sqle/api/controller/v1/sql_manage_insight.go [24-27]

 type Line struct {
-	LineName string        `json:"line_name"` // 线条名称
-	Points   *[]ChartPoint `json:"points"`    // 图表线条数据
+	LineName string       `json:"line_name"` // 线条名称
+	Points   []ChartPoint `json:"points"`    // 图表线条数据
 }
Suggestion importance[1-10]: 6

__

Why: 建议将 Points 由指针切片改为普通切片,减少了对 nil 值的检查,从而提高了代码健壮性和可读性,是一项小而有效的改进。

Low
检查 nil 指针处理

请确保新增的 Status 字段在使用前有合适的初始值或者在操作时进行 nil 检查,避免出现 nil 指针解引用而导致
panic。建议在业务逻辑中明确该字段是否允许为空,并做相应处理。

sqle/api/controller/v1/sql_manage.go [277-282]

 type ChartPoint struct {
 	X     *string             `json:"x"`
 	Y     *float64            `json:"y"`
 	Infos []map[string]string `json:"info"`
-	Status *int               `json:"status"`
+	Status int                `json:"status"` // 或者保持指针类型,并在使用时做 nil 检查
 }
Suggestion importance[1-10]: 5

__

Why: 该建议提醒开发者为新添的 Status 字段处理 nil 检查以规避潜在的 panic 问题,但将指针改为非指针可能不符合设计预期,因此改动较为主观,影响有限。

Low

Previous suggestions

Suggestions
CategorySuggestion                                                                                                                                    Impact
Possible issue
统一枚举值使用

请统一接口中使用的枚举值。在代码中枚举值未包含 "top_sql_trend",而 swagger
文档中包含此枚举,这可能引发业务逻辑不一致的问题。建议将代码中的枚举更新为
"comprehensive_trend,slow_sql_trend,top_sql_trend,active_session_trend" 以确保前后端一致性。

sqle/api/controller/v1/sql_manage_insight.go [36-41]

 type GetSqlPerformanceInsightsReq struct {
 	InstanceName string     `query:"instance_name" json:"instance_name" valid:"required"`
 	StartTime    string     `query:"start_time" json:"start_time" valid:"required"`
 	EndTime      string     `query:"end_time" json:"end_time" valid:"required"`
-	MetricName   MetricName `query:"metric_name" json:"metric_name" enums:"comprehensive_trend,slow_sql_trend,active_session_trend" valid:"required"`
+	MetricName   MetricName `query:"metric_name" json:"metric_name" enums:"comprehensive_trend,slow_sql_trend,top_sql_trend,active_session_trend" valid:"required"`
 }
Suggestion importance[1-10]: 8

__

Why: This suggestion fixes a potential inconsistency by adding the missing "top_sql_trend" in the enum values, aligning the code with the swagger documentation and preventing business logic mismatches.

Medium

Copy link

github-actions bot commented Jul 17, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
统一枚举值使用

请统一接口中使用的枚举值。在代码中枚举值未包含 "top_sql_trend",而 swagger
文档中包含此枚举,这可能引发业务逻辑不一致的问题。建议将代码中的枚举更新为
"comprehensive_trend,slow_sql_trend,top_sql_trend,active_session_trend" 以确保前后端一致性。

sqle/api/controller/v1/sql_manage_insight.go [36-41]

 type GetSqlPerformanceInsightsReq struct {
 	InstanceName string     `query:"instance_name" json:"instance_name" valid:"required"`
 	StartTime    string     `query:"start_time" json:"start_time" valid:"required"`
 	EndTime      string     `query:"end_time" json:"end_time" valid:"required"`
-	MetricName   MetricName `query:"metric_name" json:"metric_name" enums:"comprehensive_trend,slow_sql_trend,active_session_trend" valid:"required"`
+	MetricName   MetricName `query:"metric_name" json:"metric_name" enums:"comprehensive_trend,slow_sql_trend,top_sql_trend,active_session_trend" valid:"required"`
 }
Suggestion importance[1-10]: 8

__

Why: This suggestion fixes a potential inconsistency by adding the missing "top_sql_trend" in the enum values, aligning the code with the swagger documentation and preventing business logic mismatches.

Medium

Data *SqlPerformanceInsights `json:"data"`
}

type SqlPerformanceInsights struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

给结构体的字段加下注释吧,不太好看懂字段的含义

Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
统一枚举值使用

请统一接口中使用的枚举值。在代码中枚举值未包含 "top_sql_trend",而 swagger
文档中包含此枚举,这可能引发业务逻辑不一致的问题。建议将代码中的枚举更新为
"comprehensive_trend,slow_sql_trend,top_sql_trend,active_session_trend" 以确保前后端一致性。

sqle/api/controller/v1/sql_manage_insight.go [36-41]

 type GetSqlPerformanceInsightsReq struct {
 	InstanceName string     `query:"instance_name" json:"instance_name" valid:"required"`
 	StartTime    string     `query:"start_time" json:"start_time" valid:"required"`
 	EndTime      string     `query:"end_time" json:"end_time" valid:"required"`
-	MetricName   MetricName `query:"metric_name" json:"metric_name" enums:"comprehensive_trend,slow_sql_trend,active_session_trend" valid:"required"`
+	MetricName   MetricName `query:"metric_name" json:"metric_name" enums:"comprehensive_trend,slow_sql_trend,top_sql_trend,active_session_trend" valid:"required"`
 }
Suggestion importance[1-10]: 8

__

Why: This suggestion fixes a potential inconsistency by adding the missing "top_sql_trend" in the enum values, aligning the code with the swagger documentation and preventing business logic mismatches.

Medium

Copy link

PR Code Suggestions ✨

No code suggestions found for the PR.

Copy link

PR Code Suggestions ✨

No code suggestions found for the PR.

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