feat: Metadata: 自定义指标查询路由过期时间根据关联集群获取 --story=128171911 #1079
Conversation
unique0lai
requested review from
EASYGOING45 and
Copilot
and removed request for
EASYGOING45
There was a problem hiding this comment.
Pull Request Overview
This PR implements dynamic metric retention time based on associated VM clusters for custom metric query routing. Instead of using a fixed global expiration time, the system now queries each VM cluster's retention configuration and filters metrics accordingly.
Key Changes:
- Replaced global metric expiration time with per-cluster retention times fetched from VM cluster configurations
- Added support for grouping metrics by retention time to optimize batch queries
- Renamed method from
getTableIdClusterIdtogetTableIdBcsClusterIdfor clarity
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/bk-monitor-worker/internal/metadata/service/spaceredis.go | Core implementation: queries VM cluster retention times, groups metrics by retention period, and filters based on cluster-specific expiration rules |
| pkg/bk-monitor-worker/internal/metadata/service/spaceredis_test.go | Added comprehensive test coverage for retention time logic with three test scenarios (30-day, 60-day, 90-day retention) |
| pkg/bk-monitor-worker/config/metadata_config.go | Added QueryDbTableIdBatchSize configuration parameter for controlling table_id batch size |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if err := storage.NewAccessVMRecordQuerySet(db).Select(storage.AccessVMRecordDBSchema.VmClusterId, storage.AccessVMRecordDBSchema.ResultTableId).BkTenantIdEq(bkTenantId).All(&accessVmRecordList); err != nil { | ||
| return nil, err | ||
| } | ||
| if len(accessVmRecordList) == 0 { | ||
| return nil, nil | ||
| } |
There was a problem hiding this comment.
The query fetches all AccessVMRecord entries for the tenant without filtering by the specific tableIds being processed. This can retrieve unnecessary records when tableIds is a subset of all tables. Add .ResultTableIdIn(tsGroupTableId...) to the query to only fetch records for the tables actually being processed.
| if !ok { | ||
| continue | ||
| } | ||
| vmClusterRetentionTimeMap[cluster.ClusterID] = int(retentionTime.(float64)) |
There was a problem hiding this comment.
Type assertion to float64 will panic if the retention_time value in JSON is an integer or other type. Use a safer type assertion with the ok pattern: retentionTimeFloat, ok := retentionTime.(float64); if !ok { continue }, or handle multiple numeric types.
| vmClusterRetentionTimeMap[cluster.ClusterID] = int(retentionTime.(float64)) | |
| var retentionInt int | |
| switch v := retentionTime.(type) { | |
| case float64: | |
| retentionInt = int(v) | |
| case int: | |
| retentionInt = v | |
| case int64: | |
| retentionInt = int(v) | |
| case uint: | |
| retentionInt = int(v) | |
| case uint64: | |
| retentionInt = int(v) | |
| case string: | |
| // Try to parse string to int | |
| if parsed, err := strconv.Atoi(v); err == nil { | |
| retentionInt = parsed | |
| } else { | |
| continue | |
| } | |
| default: | |
| continue | |
| } | |
| vmClusterRetentionTimeMap[cluster.ClusterID] = retentionInt |
1 participant
No description provided.