-
-
Notifications
You must be signed in to change notification settings - Fork 236
feat: Add SMART attribute override functionality #835
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
base: master
Are you sure you want to change the base?
Changes from all commits
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,24 +6,32 @@ import ( | |||||
| "strings" | ||||||
| "time" | ||||||
|
|
||||||
| "github.com/analogj/scrutiny/webapp/backend/pkg" | ||||||
| "github.com/analogj/scrutiny/webapp/backend/pkg/models" | ||||||
| "github.com/analogj/scrutiny/webapp/backend/pkg/models/collector" | ||||||
| "github.com/analogj/scrutiny/webapp/backend/pkg/models/measurements" | ||||||
| influxdb2 "github.com/influxdata/influxdb-client-go/v2" | ||||||
| "github.com/influxdata/influxdb-client-go/v2/api" | ||||||
| "github.com/mitchellh/mapstructure" | ||||||
| log "github.com/sirupsen/logrus" | ||||||
| ) | ||||||
|
|
||||||
| //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// | ||||||
| // ////////////////////////////////////////////////////////////////////////////////////////////////////////////////// | ||||||
| // SMART | ||||||
| //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// | ||||||
| // ////////////////////////////////////////////////////////////////////////////////////////////////////////////////// | ||||||
| func (sr *scrutinyRepository) SaveSmartAttributes(ctx context.Context, wwn string, collectorSmartData collector.SmartInfo) (measurements.Smart, error) { | ||||||
| sr.logger.Infof("SaveSmartAttributes called for wwn=%s", wwn) | ||||||
| deviceSmartData := measurements.Smart{} | ||||||
| err := deviceSmartData.FromCollectorSmartInfo(wwn, collectorSmartData) | ||||||
| if err != nil { | ||||||
| sr.logger.Errorln("Could not process SMART metrics", err) | ||||||
| return measurements.Smart{}, err | ||||||
| } | ||||||
|
|
||||||
| // apply host-level attribute overrides before persisting or notifying | ||||||
| attributeOverrides := sr.loadAttributeOverrides() | ||||||
| sr.applyAttributeOverrides(&deviceSmartData, wwn, attributeOverrides) | ||||||
|
Comment on lines
+31
to
+33
|
||||||
|
|
||||||
| tags, fields := deviceSmartData.Flatten() | ||||||
|
|
||||||
| // write point immediately | ||||||
|
|
@@ -196,11 +204,195 @@ func (sr *scrutinyRepository) generateSmartAttributesSubquery(wwn string, durati | |||||
| } | ||||||
|
|
||||||
| partialQueryStr = append(partialQueryStr, `|> aggregateWindow(every: 1d, fn: last, createEmpty: false)`) | ||||||
|
|
||||||
| if selectEntries > 0 { | ||||||
| partialQueryStr = append(partialQueryStr, fmt.Sprintf(`|> tail(n: %d, offset: %d)`, selectEntries, selectEntriesOffset)) | ||||||
| } | ||||||
| partialQueryStr = append(partialQueryStr, "|> schema.fieldsAsCols()") | ||||||
|
|
||||||
| return strings.Join(partialQueryStr, "\n") | ||||||
| } | ||||||
|
|
||||||
| // loadAttributeOverrides retrieves the user-provided overrides from configuration. | ||||||
| func (sr *scrutinyRepository) loadAttributeOverrides() []models.AttributeOverride { | ||||||
| // Load raw maps so we can detect presence of threshold keys even when value is zero. | ||||||
| rawOverrides := []map[string]interface{}{} | ||||||
| if err := sr.appConfig.UnmarshalKey("smart.attribute_overrides", &rawOverrides); err != nil { | ||||||
| sr.logger.Debugf("failed to parse smart.attribute_overrides: %v", err) | ||||||
| return []models.AttributeOverride{} | ||||||
| } | ||||||
|
|
||||||
| overrides := make([]models.AttributeOverride, 0, len(rawOverrides)) | ||||||
| for _, raw := range rawOverrides { | ||||||
| var ao models.AttributeOverride | ||||||
| if err := mapstructure.Decode(raw, &ao); err != nil { | ||||||
| sr.logger.Debugf("failed to decode attribute override entry: %v", err) | ||||||
| continue | ||||||
| } | ||||||
| if _, ok := raw["warn_above"]; ok { | ||||||
| ao.WarnAboveSet = true | ||||||
| } | ||||||
| if _, ok := raw["fail_above"]; ok { | ||||||
| ao.FailAboveSet = true | ||||||
| } | ||||||
| overrides = append(overrides, ao) | ||||||
| } | ||||||
|
|
||||||
| sr.logger.Infof("Loaded %d attribute overrides from config", len(overrides)) | ||||||
|
||||||
| sr.logger.Infof("Loaded %d attribute overrides from config", len(overrides)) | |
| sr.logger.Debugf("Loaded %d attribute overrides from config", len(overrides)) |
Copilot
AI
Dec 7, 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.
[nitpick] This Infof log is inside a loop over all attributes and will generate a log entry for every matched override on every SMART upload. Consider using Debugf instead to reduce log verbosity in production.
| sr.logger.Infof("Applying override to attribute %s: action=%s", attrKey, override.Action) | |
| sr.logger.Debugf("Applying override to attribute %s: action=%s", attrKey, override.Action) |
Copilot
AI
Dec 7, 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.
Returning a pointer to the loop variable o can lead to unexpected behavior. The loop variable is reused in each iteration, so all returned pointers could point to the same memory location with the last iteration's value. Copy the value or return by index: return &overrides[ndx] instead of return &o.
| return &o | |
| return &overrides[ndx] |
Copilot
AI
Dec 7, 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 "ignore" action is handled twice: once at the beginning (lines 305-307) and again in the switch statement (lines 314-315). The first check makes the switch case unreachable. Remove the duplicate case from the switch statement.
| case "ignore": | |
| return setAttributeStatus(attr, pkg.AttributeStatusPassed, "Ignored by attribute override") |
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.
[nitpick] The
Infoflog at this line will log on every SMART attribute save operation, which could be quite frequent in production. Consider usingDebugfinstead for this type of operational logging to avoid log verbosity.