修复aws RecordID 指标重复报错问题 #37
Conversation
Walkthrough在Metrics.Collect中为同一批次添加了基于RecordID的去重守护（seenIDs），并在amazon提供者的ListRecords中将RecordID赋值移动到每个资源记录的循环内，防止同名标签导致Prometheus重复采集错误。 Changes
Sequence Diagram(s)（已跳过：本次变更为错误修复/改进，未生成序列图） Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/provider/amazon.go (1)
143-143: 正确移除了外层的 RecordID 赋值将 RecordID 赋值从外层循环移除是正确的，因为这会导致同一 ResourceRecordSet 下的所有 ResourceRecord 共享相同的 ID。
♻️ 可选：清理注释代码
建议直接删除注释行而非保留，以保持代码整洁：
recordInfo := Record{ CloudProvider: a.account.CloudProvider, CloudName: a.account.CloudName, DomainName: domain, - // RecordID: public.GetID(), RecordType: string(record.Type), RecordWeight: fmt.Sprintf("%d", record.Weight), RecordStatus: oneStatus("enable"),pkg/export/export_gauge.go (1)
190-206: 建议为自定义记录添加同样的重复检测当前重复检测逻辑仅应用于云服务商的证书信息（lines 174-183），但自定义记录（custom records）部分缺少相同的保护机制。为保持一致性并避免潜在的重复 RecordID 问题，建议添加类似的防御逻辑。
♻️ 建议的改进方案
// get custom record cert info list from cache if len(public.Config.CustomRecords) != 0 { recordCertInfoCacheKey := public.RecordCertInfo + "_" + public.CustomRecords var recordCerts []provider.RecordCert recordCertInfoCacheValue, err := public.CertCache.Get(recordCertInfoCacheKey) if err != nil { logger.Error(fmt.Sprintf("[ %s ] get record list failed: %v", recordCertInfoCacheKey, err)) return } err = json.Unmarshal(recordCertInfoCacheValue, &recordCerts) if err != nil { logger.Error(fmt.Sprintf("[ %s ] json.Unmarshal error: %v", recordCertInfoCacheKey, err)) return } + seenIDs := make(map[string]bool) for _, v := range recordCerts { + if v.RecordID == "" { + continue + } + if seenIDs[v.RecordID] { + logger.Error(fmt.Sprintf("【Duplicate Found】Custom Record: %s, RecordID: %s is duplicated!", v.FullRecord, v.RecordID)) + continue + } + seenIDs[v.RecordID] = true ch <- prometheus.MustNewConstMetric(c.metrics[public.RecordCertInfo], prometheus.GaugeValue, float64(v.DaysUntilExpiry), v.CloudProvider, v.CloudName, v.DomainName, v.RecordID, v.FullRecord, v.SubjectCommonName, v.SubjectOrganization, v.SubjectOrganizationalUnit, v.IssuerCommonName, v.IssuerOrganization, v.IssuerOrganizationalUnit, v.CreatedDate, v.ExpiryDate, fmt.Sprintf("%t", v.CertMatched), v.ErrorMsg) } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/export/export_gauge.gopkg/provider/amazon.go
🧰 Additional context used
🧬 Code graph analysis (2)
pkg/provider/amazon.go (1)
public/public.go (1)
GetID(101-103)
pkg/export/export_gauge.go (1)
public/logger/logger.go (1)
Error(38-40)
🔇 Additional comments (2)
pkg/provider/amazon.go (1)
163-168: 正确修复了 RecordID 重复问题将 RecordID 的生成移至内层循环是正确的解决方案。现在每个 ResourceRecord 都会获得唯一的 ID，避免了多个记录值共享同一 RecordID 导致的 Prometheus 指标冲突。
这个改动有效解决了 issue #36 中报告的 "collected before with the same name and label values" 错误。
pkg/export/export_gauge.go (1)
174-184: 有效的重复检测防御逻辑添加的
seenIDs映射能够有效检测并跳过重复的 RecordID，防止 Prometheus 采集错误。这是一个很好的防御层，即使在数据源层面已经修复的情况下，仍能捕获可能出现的重复。逻辑实现正确：
- 空 RecordID 检查避免了无效数据
- 重复检测及时记录错误并跳过
- seenIDs 的作用域限定在每个云账号批次内，符合预期
|
话说，这里是不是已经用随机ID填充了，为啥还会重复呢，好奇怪： cloud_dns_exporter/pkg/provider/amazon.go Line 143 in 1a76524 |
|
cloud_dns_exporter/pkg/provider/amazon.go Lines 164 to 167 in 1a76524 如果record.ResourceRecords 是多个值会共享前面的同一个随机ID |
2 participants
Fixes #36
Summary by CodeRabbit
发布说明
✏️ Tip: You can customize this high-level summary in your review settings.