Skip to content

Sql insight ce #3100

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

Merged
merged 6 commits into from
Jul 29, 2025
Merged

Sql insight ce #3100

merged 6 commits into from
Jul 29, 2025

Conversation

BugsGuru
Copy link
Collaborator

@BugsGuru BugsGuru commented Jul 28, 2025

User description

关联的 issue

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

描述你的变更

  1. 新增扫描任务类型:性能指标采集,及其相关指标、名称等变量
  2. 自动清理过期的性能指标数据
  3. /v1/audit_plan_metas [get]接口新增扫描任务的提示信息字段

确认项(pr提交后操作)

Tip

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


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

assign in @iwanghc


Description

  • 在审批计划元数据中新增Tips字段及本地化处理

  • 添加SQL Insight性能指标常量及清理作业逻辑

  • 更新API响应、Swagger及文档定义,支持新增字段

  • 调整任务调用逻辑以便正确记录Raw SQL信息


Diagram Walkthrough

flowchart LR
  A["新增Tips字段"] -- "更新API响应" --> B["更新文档与Swagger"]
  A -- "本地化处理" --> C["Meta结构体扩展"]
  D["新增SQL Insight指标"] -- "定义指标常量" --> E["更新ALLMetric映射"]
  D -- "新增清理作业" --> F["CleanExpiredSqlManageInsightRecords"]
Loading

File Walkthrough

Relevant files

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

github-actions bot commented Jul 28, 2025

PR Code Suggestions ✨

Latest suggestions up to 1ac7ee0
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
实现删除逻辑

建议实现实际的数据库删除逻辑,而不是返回固定的值,以保证定时清理任务能正确地移除过期的 SqlInsight 记录。这样可以防止数据垃圾堆积和潜在的系统资源浪费。

sqle/model/sql_manage_insight_ce.go [12-14]

 func (s *Storage) RemoveExpiredSqlInsightRecord(expiredTime time.Time) (int64, error) {
-	return 0, nil
+	// 示例实现,请根据实际数据库操作进行调整
+	result, err := s.db.Exec("DELETE FROM sql_insight_records WHERE created_at < ?", expiredTime)
+	if err != nil {
+		return 0, err
+	}
+	return result.RowsAffected()
 }
Suggestion importance[1-10]: 7

__

Why: The suggestion improves functionality by replacing the stub implementation with actual deletion logic, which can prevent data build-up. The improved code correctly reflects the intended changes in the snippet.

Medium

Previous suggestions

Suggestions up to commit 3000a86
CategorySuggestion                                                                                                                                    Impact
Possible issue
实现记录删除逻辑

建议实现真正的删除逻辑以移除过期的 SQL Insight 记录,否则可能导致数据无法及时清理,从而引起存储资源浪费的问题。

可通过执行数据库删除操作来实现,避免返回固定值。

sqle/model/sql_manage_insight_ce.go [12-14]

 func (s *Storage) RemoveExpiredSqlInsightRecord(expiredTime time.Time) (int64, error) {
-	return 0, nil
+	result, err := s.db.Exec("DELETE FROM sql_insight_records WHERE created_at < ?", expiredTime)
+	if err != nil {
+		return 0, err
+	}
+	return result.RowsAffected()
 }
Suggestion importance[1-10]: 8

__

Why: The suggestion replaces a stub function that always returns 0 with a proper database deletion operation using s.db.Exec, which is a significant functional improvement.

Medium
General
使用独立过期配置

建议为 SQL Insight 记录设置独立的过期时间变量,而不是复用 Raw SQL 的过期时间,以确保清理策略符合实际需求。
可以新增单独方法或配置项来获取 SQL
Insight 的过期时间,从而更精确地管理数据清理。

sqle/server/clean.go [136-140]

-expiredHours, err := st.GetSqlManageRawSqlExpiredHoursOrDefault()
+expiredHours, err := st.GetSqlManageInsightExpiredHoursOrDefault()
 if err != nil {
-	entry.Errorf("get sql manage raw sql expired hours error: %v", err)
+	entry.Errorf("get sql manage insight expired hours error: %v", err)
 	return
 }
 expiredTime := time.Now().Add(time.Duration(-expiredHours) * time.Hour)
Suggestion importance[1-10]: 7

__

Why: The recommendation to use a dedicated method (GetSqlManageInsightExpiredHoursOrDefault) for fetching the SQL Insight expiration value enhances clarity and aligns the cleanup logic with the correct dataset, though it only moderately impacts functionality.

Medium

BugsGuru added 6 commits July 28, 2025 18:20
- Add new type: TypeMySQLPerformanceCollect for MySQL performance data collection
- Introduce new metrics for SQL insight:
  - MetricNameSqlInsightCollectTime - MetricNameSqlInsightThreadsConnected
  - MetricNameSqlInsightQueries
  - MetricNameSqlInsightQPS
- Update ALLMetric map to include new metrics
- Implement CleanExpiredSqlManageInsightRecords function in CleanJob
- Add RemoveExpiredSqlInsightRecord method to Storage
- Integrate new cleaning job into existing cleanup processes
- Use existing GetSqlManageRawSqlExpiredHoursOrDefault method for expiration time
- Add tips for performance collection audit plan
- Update audit plan metadata structure to include tips
- Localize tips message in English and Chinese
- Modify audit plan response to include tips information
- Add new field 'audit_plan_type_tips' to API documentation in docs.go
- Update swagger.json and swagger.yaml to include the new field
Copy link

PR Code Suggestions ✨

No code suggestions found for the PR.

@BugsGuru BugsGuru marked this pull request as draft July 29, 2025 07:51
@BugsGuru BugsGuru marked this pull request as ready for review July 29, 2025 08:04
@iwanghc iwanghc merged commit 434ad87 into main Jul 29, 2025
5 checks passed
@BugsGuru BugsGuru deleted the sql-insight-ce branch July 29, 2025 08:32
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