-
Notifications
You must be signed in to change notification settings - Fork 350
feat(snmp_zabbix): support dependent item type and LLD lifetime manag… #1378
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
Changes from all commits
152ef0c
a006a94
b8024bd
0294b65
73d9131
09a5179
f32b7e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -170,6 +170,11 @@ func (c *SNMPCollector) collectFromAgent(ctx context.Context, agent string, item | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| func (c *SNMPCollector) collectIndividually(client *gosnmp.GoSNMP, agent string, items []MonitorItem, resultChan chan<- CollectionResult) { | ||||||||||||||||||||||
| for _, item := range items { | ||||||||||||||||||||||
| // Skip dependent items in individual collection, they are handled by their masters | ||||||||||||||||||||||
| if strings.EqualFold(item.Type, "dependent") { | ||||||||||||||||||||||
| continue | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // 使用 Get 而不是 BulkGet | ||||||||||||||||||||||
| pduResult, err := client.Get([]string{item.OID}) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -220,11 +225,14 @@ func (c *SNMPCollector) processSingleResult(agent string, item MonitorItem, resu | |||||||||||||||||||||
| Tags: c.buildTags(agent, item), | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| var masterValue interface{} | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if result.Error != nil { | ||||||||||||||||||||||
| collectionResult.Error = result.Error | ||||||||||||||||||||||
| } else { | ||||||||||||||||||||||
| rawValue := c.convertSNMPValue(result.Value, item) | ||||||||||||||||||||||
| processedValue := rawValue | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if len(item.Preprocessing) > 0 { | ||||||||||||||||||||||
| processed, err := ApplyPreprocessingWithContext( | ||||||||||||||||||||||
| rawValue, | ||||||||||||||||||||||
|
|
@@ -243,6 +251,8 @@ func (c *SNMPCollector) processSingleResult(agent string, item MonitorItem, resu | |||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| masterValue = processedValue | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| finalValue, fields, err := c.processValue(processedValue, item) | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
| collectionResult.Error = err | ||||||||||||||||||||||
|
|
@@ -259,6 +269,66 @@ func (c *SNMPCollector) processSingleResult(agent string, item MonitorItem, resu | |||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| resultChan <- collectionResult | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if len(item.DependentItems) > 0 && result.Error == nil { | ||||||||||||||||||||||
| c.processDependentItems(agent, item.DependentItems, masterValue, resultChan) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| func (c *SNMPCollector) processDependentItems(agent string, dependents []MonitorItem, masterValue interface{}, resultChan chan<- CollectionResult) { | ||||||||||||||||||||||
| for _, depItem := range dependents { | ||||||||||||||||||||||
|
Comment on lines
+278
to
+279
|
||||||||||||||||||||||
| func (c *SNMPCollector) processDependentItems(agent string, dependents []MonitorItem, masterValue interface{}, resultChan chan<- CollectionResult) { | |
| for _, depItem := range dependents { | |
| func (c *SNMPCollector) processDependentItems(agent string, dependentItems []MonitorItem, masterValue interface{}, resultChan chan<- CollectionResult) { | |
| for _, depItem := range dependentItems { |
Copilot
AI
Dec 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error from preprocessing is checked but only logged in debug mode, then the function returns early without sending any result to the channel. This means dependent items with preprocessing errors will silently disappear from metrics. Consider sending an error result to the channel or at least logging at a higher level than debug.
| if c.config.DebugMode { | |
| log.Printf("D! dependent preprocessing failed for %s: %v", item.Key, err) | |
| } | |
| log.Printf("E! dependent preprocessing failed for %s: %v", item.Key, err) | |
| collectionResult.Error = err | |
| resultChan <- collectionResult |
Copilot
AI
Dec 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete error handling. When preprocessing fails for a dependent item, the function returns early without sending any error result through the channel. This means the caller won't be aware of the failure. Consider sending a CollectionResult with the error set before returning.
| } | |
| } | |
| collectionResult.Error = err | |
| resultChan <- collectionResult |
Copilot
AI
Dec 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate statement detected. The line resultChan <- collectionResult appears twice consecutively, which will send the same result to the channel twice. This will cause duplicate metrics to be collected for dependent items. Remove one of the duplicate lines.
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -14,6 +14,13 @@ import ( | |||||||||||||||
| "github.com/gosnmp/gosnmp" | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| const ( | ||||||||||||||||
| // DurationNever 代表永不执行 (-1s) | ||||||||||||||||
| DurationNever = -1 * time.Second | ||||||||||||||||
| // DurationImmediately 代表立即执行 (0s) | ||||||||||||||||
| DurationImmediately = 0 | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| type DiscoveryEngine struct { | ||||||||||||||||
| client *SNMPClientManager | ||||||||||||||||
| template *ZabbixTemplate | ||||||||||||||||
|
|
@@ -55,6 +62,9 @@ type MonitorItem struct { | |||||||||||||||
| DiscoveryIndex string // 从 {#SNMPINDEX} 等宏解析出的唯一索引 | ||||||||||||||||
|
|
||||||||||||||||
| Preprocessing []PreprocessStep `json:"preprocessing,omitempty"` | ||||||||||||||||
|
|
||||||||||||||||
| // DependentItems 存储依赖于该 Item 的子 Item | ||||||||||||||||
| DependentItems []MonitorItem | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| func extractLabelKey(itemKey string) string { | ||||||||||||||||
|
|
@@ -404,6 +414,10 @@ func (d *DiscoveryEngine) extractIndexFromOID(fullOID, baseOID string) string { | |||||||||||||||
| if !strings.HasPrefix(fullOID, baseOID) { | ||||||||||||||||
| return "" | ||||||||||||||||
| } | ||||||||||||||||
| // 如果比 baseOID 长,必须紧跟切分符(.) | ||||||||||||||||
| if len(fullOID) > len(baseOID) && fullOID[len(baseOID)] != '.' { | ||||||||||||||||
| return "" | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // 提取索引部分 | ||||||||||||||||
| if len(fullOID) <= len(baseOID) { | ||||||||||||||||
|
|
@@ -529,6 +543,12 @@ func (d *DiscoveryEngine) ApplyItemPrototypes(discoveries []DiscoveryItem, rule | |||||||||||||||
| var items []MonitorItem | ||||||||||||||||
| prototypes := rule.ItemPrototypes | ||||||||||||||||
|
|
||||||||||||||||
| // Flat list to scope item creation per discovery iteration | ||||||||||||||||
| type GeneratedItem struct { | ||||||||||||||||
| MonitorItem | ||||||||||||||||
| MasterKey string | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| for _, discovery := range discoveries { | ||||||||||||||||
| discoveryIndex := "" | ||||||||||||||||
| if idx, ok := discovery.Macros["{#SNMPINDEX}"]; ok { | ||||||||||||||||
|
|
@@ -538,9 +558,10 @@ func (d *DiscoveryEngine) ApplyItemPrototypes(discoveries []DiscoveryItem, rule | |||||||||||||||
| discoveryIndex = idx | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| var currentScopeItems []GeneratedItem | ||||||||||||||||
|
|
||||||||||||||||
| for _, prototype := range prototypes { | ||||||||||||||||
| if prototype.Status == "DISABLED" || | ||||||||||||||||
| prototype.Status == "UNSUPPORTED" { | ||||||||||||||||
| if prototype.Status == "DISABLED" || prototype.Status == "UNSUPPORTED" { | ||||||||||||||||
| continue | ||||||||||||||||
| } | ||||||||||||||||
| delay := parseZabbixDelay(prototype.Delay) | ||||||||||||||||
|
|
@@ -549,8 +570,15 @@ func (d *DiscoveryEngine) ApplyItemPrototypes(discoveries []DiscoveryItem, rule | |||||||||||||||
| tags[tag.Tag] = tag.Value | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // Expand macros in keys to properly link masters and dependents | ||||||||||||||||
| expandedKey := d.expandMacros(prototype.Key, discovery.Macros) | ||||||||||||||||
| expandedMasterKey := "" | ||||||||||||||||
| if prototype.MasterItem.Key != "" { | ||||||||||||||||
| expandedMasterKey = d.expandMacros(prototype.MasterItem.Key, discovery.Macros) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| item := MonitorItem{ | ||||||||||||||||
| Key: d.expandMacros(prototype.Key, discovery.Macros), | ||||||||||||||||
| Key: expandedKey, | ||||||||||||||||
| OID: d.expandMacros(prototype.SNMPOID, discovery.Macros), | ||||||||||||||||
| Type: ConvertZabbixItemType(prototype.Type), | ||||||||||||||||
| Name: d.expandMacros(prototype.Name, discovery.Macros), | ||||||||||||||||
|
|
@@ -571,10 +599,55 @@ func (d *DiscoveryEngine) ApplyItemPrototypes(discoveries []DiscoveryItem, rule | |||||||||||||||
| default: | ||||||||||||||||
| item.IsLabelProvider = false | ||||||||||||||||
| } | ||||||||||||||||
| items = append(items, item) | ||||||||||||||||
|
|
||||||||||||||||
| currentScopeItems = append(currentScopeItems, GeneratedItem{ | ||||||||||||||||
| MonitorItem: item, | ||||||||||||||||
| MasterKey: expandedMasterKey, | ||||||||||||||||
| }) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // Map for O(1) lookups during dependency linkage | ||||||||||||||||
| localMap := make(map[string]*MonitorItem) | ||||||||||||||||
| var localList []*MonitorItem | ||||||||||||||||
| var dependentList []*GeneratedItem | ||||||||||||||||
|
|
||||||||||||||||
| for i := range currentScopeItems { | ||||||||||||||||
| ptr := ¤tScopeItems[i].MonitorItem | ||||||||||||||||
| localMap[ptr.Key] = ptr | ||||||||||||||||
|
|
||||||||||||||||
| if currentScopeItems[i].MasterKey != "" { | ||||||||||||||||
| dependentList = append(dependentList, ¤tScopeItems[i]) | ||||||||||||||||
| } else { | ||||||||||||||||
| localList = append(localList, ptr) | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| depMap := make(map[string][]string) | ||||||||||||||||
| for _, dep := range dependentList { | ||||||||||||||||
| depMap[dep.MasterKey] = append(depMap[dep.MasterKey], dep.Key) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| var buildTree func(key string) MonitorItem | ||||||||||||||||
| buildTree = func(key string) MonitorItem { | ||||||||||||||||
| item := *localMap[key] | ||||||||||||||||
|
||||||||||||||||
| item := *localMap[key] | |
| itemPtr, ok := localMap[key] | |
| if !ok || itemPtr == nil { | |
| // If the key is not present in the local map, return a zero-value item | |
| return MonitorItem{} | |
| } | |
| item := *itemPtr |
Copilot
AI
Dec 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential stack overflow risk. The buildTree function is recursive and doesn't have any protection against circular dependencies. If the dependency graph contains a cycle (A depends on B, B depends on A), this will cause infinite recursion and a stack overflow. Add cycle detection or a visited set to prevent this.
Copilot
AI
Dec 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default value in the comment doesn't match the code. The comment states "默认删除时间 30d" (default deletion time 30d), but the code uses 7*24*time.Hour which is 7 days, not 30 days. Either update the comment to say "7d" or change the code to use 30 days if that's the intended default.
| deleteTTL := parseStrategy(rule.LifetimeType, rule.Lifetime, 7*24*time.Hour) // 默认删除时间 7d | |
| deleteTTL := parseStrategy(rule.LifetimeType, rule.Lifetime, 7*24*time.Hour) // 默认删除时间 7d |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When master item collection fails, dependent items are silently skipped without any indication. Consider logging a message at debug level when dependent items are skipped due to master collection failure to aid in troubleshooting.