feat(metrics): Collect event-driven Automation metrics#929
feat(metrics): Collect event-driven Automation metrics#929boxhock wants to merge 14 commits intoautobrr:developfrom
Conversation
WalkthroughWires a new Prometheus AutomationCollector into MetricsManager, exposes it via GetAutomationCollector, injects it into the automations Service with SetMetricsCollector, and emits per-rule/action counters during rule processing. No public API signature changes. (≈30 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Main
participant MetricsManager
participant AutomationCollector as Collector
participant AutomationService as Service
participant Processor
Main->>MetricsManager: NewMetricsManager()
Note right of MetricsManager: constructs & registers collectors\n(including AutomationCollector)
MetricsManager->>Collector: NewAutomationCollector() & MustRegister
Collector-->>MetricsManager: registered collector
Main->>Service: SetMetricsCollector(automationCollector)
Service-->>Service: store collector
Note over Processor,Service: During per-instance rule processing
Processor->>Service: process rules -> produce ruleRunStats
Service->>Processor: processing completes
Processor->>Collector: CollectMetrics(rule, stats, instanceName)
Collector-->>Collector: increment counters (rule runs, matches,\nactions, not-performed)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (3)📚 Learning: 2025-11-06T12:11:04.963ZApplied to files:
📚 Learning: 2025-12-11T08:40:01.329ZApplied to files:
📚 Learning: 2025-12-28T18:44:10.496ZApplied to files:
🧬 Code graph analysis (1)internal/services/automations/processor.go (1)
🔇 Additional comments (4)
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 (1)
internal/services/automations/service.go (1)
99-101: Consider thread safety forSetMetricsCollector.The setter modifies
s.metricsCollectorwithout synchronization. Since this is called during startup (inmain.go) before the service loop processes rules, it's safe in the current flow. However, if this setter is ever called while the service is running, there could be a data race withcollectRuleRunMetricsreading the field.For future-proofing, consider using
sync.Onceor a mutex, or documenting that this must be called beforeStart().
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cmd/qui/main.gointernal/metrics/collector/automation.gointernal/metrics/manager_test.gointernal/metrics/metrics.gointernal/services/automations/processor.gointernal/services/automations/processor_test.gointernal/services/automations/service.go
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-28T18:44:10.496Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 876
File: internal/logstream/hub_test.go:188-192
Timestamp: 2025-12-28T18:44:10.496Z
Learning: In Go 1.25 (Aug 2025), use wg.Go(func()) to spawn a goroutine and automate the Add/Done lifecycle. Replace manual patterns like wg.Add(1); go func(){ defer wg.Done(); ... }() with wg.Go(func(){ ... }). Ensure the codebase builds with Go 1.25+ and apply this in relevant Go files (e.g., internal/logstream/hub_test.go). If targeting older Go versions, maintain the existing pattern.
Applied to files:
internal/metrics/metrics.gointernal/services/automations/service.gointernal/services/automations/processor_test.gointernal/metrics/collector/automation.gointernal/metrics/manager_test.gointernal/services/automations/processor.gocmd/qui/main.go
📚 Learning: 2025-11-06T12:11:04.963Z
Learnt from: Audionut
Repo: autobrr/qui PR: 553
File: internal/services/crossseed/service.go:1045-1082
Timestamp: 2025-11-06T12:11:04.963Z
Learning: The autobrr/qui project uses a custom go-qbittorrent client library (github.com/autobrr/go-qbittorrent) that supports both "paused" and "stopped" parameters when adding torrents via the options map. Both parameters should be set together when controlling torrent start state, as seen in internal/services/crossseed/service.go and throughout the codebase.
Applied to files:
internal/services/automations/service.gointernal/services/automations/processor_test.gointernal/services/automations/processor.go
📚 Learning: 2025-12-11T08:40:01.329Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 746
File: internal/services/reannounce/service.go:480-481
Timestamp: 2025-12-11T08:40:01.329Z
Learning: In autobrr/qui's internal/services/reannounce/service.go, the hasHealthyTracker, getProblematicTrackers, and getHealthyTrackers functions intentionally match qbrr's lenient tracker health logic (skip unregistered trackers and check if any other tracker is healthy) rather than go-qbittorrent's strict isTrackerStatusOK logic (which treats unregistered as an immediate failure). For multi-tracker torrents, if one tracker is working, reannouncing won't help. The duplication of the health check logic across these three functions is acceptable as it's a simple one-liner, and extracting it would add unnecessary complexity.
Applied to files:
internal/services/automations/service.gointernal/services/automations/processor_test.gointernal/services/automations/processor.go
📚 Learning: 2025-11-25T22:46:03.762Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 632
File: internal/backups/service.go:1401-1404
Timestamp: 2025-11-25T22:46:03.762Z
Learning: In qui's backup service (internal/backups/service.go), background torrent downloads initiated during manifest import intentionally use a fire-and-forget pattern with the shared service context (s.ctx). Per-run cancellation is not needed, as orphaned downloads completing after run deletion are considered harmless and acceptable. This design prioritizes simplicity over per-run lifecycle management for background downloads.
Applied to files:
internal/services/automations/processor_test.go
🧬 Code graph analysis (5)
internal/metrics/metrics.go (1)
internal/metrics/collector/automation.go (2)
AutomationCollector(9-26)NewAutomationCollector(34-119)
internal/services/automations/service.go (2)
internal/metrics/collector/automation.go (1)
AutomationCollector(9-26)internal/models/automation.go (1)
Automation(34-46)
internal/services/automations/processor_test.go (2)
internal/models/automation.go (3)
Automation(34-46)ActionConditions(448-456)CategoryAction(498-506)internal/metrics/collector/automation.go (2)
NewAutomationCollector(34-119)GetAutomationRuleRunLabels(30-32)
internal/metrics/manager_test.go (2)
internal/metrics/metrics.go (1)
NewMetricsManager(22-42)internal/metrics/collector/automation.go (2)
AutomationCollector(9-26)NewAutomationCollector(34-119)
internal/services/automations/processor.go (1)
internal/metrics/collector/automation.go (2)
AutomationCollector(9-26)GetAutomationRuleRunLabels(30-32)
🔇 Additional comments (12)
internal/metrics/collector/automation.go (2)
28-32: High-cardinality label consideration withrule_name.Including
rule_nameas a Prometheus label can lead to high cardinality if users frequently rename rules or create many rules over time. Old label combinations persist until the process restarts. This is acceptable for a moderate number of rules but worth documenting as a known characteristic.
34-118: LGTM - Clean metric registration pattern.The collector initialization is well-structured with consistent naming, appropriate help text, and correct label usage. The explicit registration of each metric provides clarity.
cmd/qui/main.go (1)
711-713: LGTM - Clean metrics wiring integration.The metrics collector is correctly injected into the automation service only when metrics are enabled, following the established pattern in the codebase. The placement after MetricsManager creation and before server start is appropriate.
internal/services/automations/service.go (2)
535-547: LGTM - Defensive nil check and clean iteration.The nil check for
metricsCollectorensures graceful behavior when metrics are disabled. The iteration pattern correctly matches rules to their stats.
663-665: Good use ofdeferfor metrics collection.Using
deferensures metrics are collected regardless of how the function exits (early returns, errors, or normal completion). This guarantees consistent metric emission for each rule run.internal/metrics/metrics.go (2)
32-32: Verify: AutomationCollector self-registers its metrics.Unlike
TorrentCollector(which implements theCollectorinterface and is registered viaregistry.MustRegister),AutomationCollectorinternally registers eachCounterVecin its constructor usingr.MustRegister(...). This is a valid pattern, but it's worth noting the design difference.This means
AutomationCollectordoesn't need to implementDescribe/Collectsince its metrics are individually registered. This works correctly but differs from theTorrentCollectorpattern.
47-50: LGTM - Clean accessor method.The accessor follows the existing pattern with
GetRegistry()and provides a straightforward way to retrieve the automation collector.internal/services/automations/processor_test.go (2)
144-190: LGTM - Comprehensive unit test forCollectMetrics.The test thoroughly validates all 16 metrics are correctly incremented with expected values. The use of
testutil.ToFloat64is the standard approach for asserting Prometheus counter values in tests.
192-249: LGTM - Good integration test with negative assertions.The test validates the end-to-end flow from
processTorrentsthroughCollectMetrics, and importantly verifies that unrelated metrics remain at zero. This negative testing pattern helps catch unintended side effects.internal/services/automations/processor.go (1)
67-85: LGTM - Clean metrics emission implementation.The method correctly uses
Inc()for the total count andAdd()for the individual stat counters. The label generation viaGetAutomationRuleRunLabelsensures consistency with the collector's label definition.The lack of a nil check here is acceptable since the caller (
collectRuleRunMetricsin service.go) guards against nil before invoking this method.internal/metrics/manager_test.go (2)
62-69: LGTM - Good accessor test coverage.The test validates both that the accessor returns a non-nil value and that it's the correct type.
174-188: LGTM - Validates metric descriptor count.The test correctly uses
r.Describe()on the registry (since AutomationCollector's metrics are individually registered) and verifies all 16 metric descriptors are present.
…ude instance name
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
internal/metrics/collector/automation.gointernal/metrics/manager_test.gointernal/services/automations/processor.gointernal/services/automations/processor_test.gointernal/services/automations/service.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/services/automations/service.go
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-06T12:11:04.963Z
Learnt from: Audionut
Repo: autobrr/qui PR: 553
File: internal/services/crossseed/service.go:1045-1082
Timestamp: 2025-11-06T12:11:04.963Z
Learning: The autobrr/qui project uses a custom go-qbittorrent client library (github.com/autobrr/go-qbittorrent) that supports both "paused" and "stopped" parameters when adding torrents via the options map. Both parameters should be set together when controlling torrent start state, as seen in internal/services/crossseed/service.go and throughout the codebase.
Applied to files:
internal/services/automations/processor_test.gointernal/services/automations/processor.go
📚 Learning: 2025-12-11T08:40:01.329Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 746
File: internal/services/reannounce/service.go:480-481
Timestamp: 2025-12-11T08:40:01.329Z
Learning: In autobrr/qui's internal/services/reannounce/service.go, the hasHealthyTracker, getProblematicTrackers, and getHealthyTrackers functions intentionally match qbrr's lenient tracker health logic (skip unregistered trackers and check if any other tracker is healthy) rather than go-qbittorrent's strict isTrackerStatusOK logic (which treats unregistered as an immediate failure). For multi-tracker torrents, if one tracker is working, reannouncing won't help. The duplication of the health check logic across these three functions is acceptable as it's a simple one-liner, and extracting it would add unnecessary complexity.
Applied to files:
internal/services/automations/processor_test.gointernal/services/automations/processor.go
📚 Learning: 2025-11-25T22:46:03.762Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 632
File: internal/backups/service.go:1401-1404
Timestamp: 2025-11-25T22:46:03.762Z
Learning: In qui's backup service (internal/backups/service.go), background torrent downloads initiated during manifest import intentionally use a fire-and-forget pattern with the shared service context (s.ctx). Per-run cancellation is not needed, as orphaned downloads completing after run deletion are considered harmless and acceptable. This design prioritizes simplicity over per-run lifecycle management for background downloads.
Applied to files:
internal/services/automations/processor_test.go
📚 Learning: 2025-12-28T18:44:10.496Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 876
File: internal/logstream/hub_test.go:188-192
Timestamp: 2025-12-28T18:44:10.496Z
Learning: In Go 1.25 (Aug 2025), use wg.Go(func()) to spawn a goroutine and automate the Add/Done lifecycle. Replace manual patterns like wg.Add(1); go func(){ defer wg.Done(); ... }() with wg.Go(func(){ ... }). Ensure the codebase builds with Go 1.25+ and apply this in relevant Go files (e.g., internal/logstream/hub_test.go). If targeting older Go versions, maintain the existing pattern.
Applied to files:
internal/services/automations/processor_test.gointernal/services/automations/processor.gointernal/metrics/collector/automation.gointernal/metrics/manager_test.go
🧬 Code graph analysis (3)
internal/services/automations/processor_test.go (2)
internal/models/automation.go (3)
Automation(34-46)ActionConditions(448-456)CategoryAction(498-506)internal/metrics/collector/automation.go (1)
NewAutomationCollector(14-33)
internal/services/automations/processor.go (1)
internal/metrics/collector/automation.go (1)
AutomationCollector(9-12)
internal/metrics/manager_test.go (2)
internal/metrics/metrics.go (1)
NewMetricsManager(22-42)internal/metrics/collector/automation.go (2)
AutomationCollector(9-12)NewAutomationCollector(14-33)
🔇 Additional comments (4)
internal/metrics/manager_test.go (1)
59-60: LGTM!The test coverage for the new AutomationCollector integration is thorough and follows the existing test patterns in the file. The tests appropriately validate registration, accessor methods, and metric descriptor counts.
Also applies to: 62-69, 174-189
internal/services/automations/processor.go (1)
67-85: LGTM!The
CollectMetricsimplementation correctly translates theruleRunStatscounters into Prometheus metrics. The use ofMustCurryWithfor pre-binding instance and rule labels is elegant and efficient.internal/metrics/collector/automation.go (2)
14-33: Well-designed metrics following Prometheus best practices.The implementation correctly:
- Uses
_totalsuffix for accumulating counters per Prometheus naming conventions- Includes both
instance_nameandrule_namelabels for readable dashboards (addressing previous feedback)- Registers metrics with appropriate namespace/subsystem structure
44-51: Elegant use ofMustCurryWithfor efficient label binding.Pre-binding the instance and rule labels via
MustCurryWithis an excellent design choice. It allows callers to efficiently add multiple action-labeled metrics without repeating the common labels for each action type. This pattern is well-suited to the usage inprocessor.gowhere multiple action metrics are emitted per rule run.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/services/automations/processor.gointernal/services/automations/service.go
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-06T12:11:04.963Z
Learnt from: Audionut
Repo: autobrr/qui PR: 553
File: internal/services/crossseed/service.go:1045-1082
Timestamp: 2025-11-06T12:11:04.963Z
Learning: The autobrr/qui project uses a custom go-qbittorrent client library (github.com/autobrr/go-qbittorrent) that supports both "paused" and "stopped" parameters when adding torrents via the options map. Both parameters should be set together when controlling torrent start state, as seen in internal/services/crossseed/service.go and throughout the codebase.
Applied to files:
internal/services/automations/processor.gointernal/services/automations/service.go
📚 Learning: 2025-12-11T08:40:01.329Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 746
File: internal/services/reannounce/service.go:480-481
Timestamp: 2025-12-11T08:40:01.329Z
Learning: In autobrr/qui's internal/services/reannounce/service.go, the hasHealthyTracker, getProblematicTrackers, and getHealthyTrackers functions intentionally match qbrr's lenient tracker health logic (skip unregistered trackers and check if any other tracker is healthy) rather than go-qbittorrent's strict isTrackerStatusOK logic (which treats unregistered as an immediate failure). For multi-tracker torrents, if one tracker is working, reannouncing won't help. The duplication of the health check logic across these three functions is acceptable as it's a simple one-liner, and extracting it would add unnecessary complexity.
Applied to files:
internal/services/automations/processor.gointernal/services/automations/service.go
📚 Learning: 2025-12-28T18:44:10.496Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 876
File: internal/logstream/hub_test.go:188-192
Timestamp: 2025-12-28T18:44:10.496Z
Learning: In Go 1.25 (Aug 2025), use wg.Go(func()) to spawn a goroutine and automate the Add/Done lifecycle. Replace manual patterns like wg.Add(1); go func(){ defer wg.Done(); ... }() with wg.Go(func(){ ... }). Ensure the codebase builds with Go 1.25+ and apply this in relevant Go files (e.g., internal/logstream/hub_test.go). If targeting older Go versions, maintain the existing pattern.
Applied to files:
internal/services/automations/processor.gointernal/services/automations/service.go
🧬 Code graph analysis (2)
internal/services/automations/processor.go (2)
internal/models/automation.go (1)
Automation(34-46)internal/metrics/collector/automation.go (1)
AutomationCollector(9-12)
internal/services/automations/service.go (1)
internal/metrics/collector/automation.go (1)
AutomationCollector(9-12)
🔇 Additional comments (6)
internal/services/automations/processor.go (1)
11-11: LGTM!The collector import is correctly added to support metrics collection.
internal/services/automations/service.go (5)
24-24: LGTM!The collector import is correctly added.
66-66: LGTM!The metrics collector field is appropriately added to the Service struct.
99-101: LGTM!The setter enables dependency injection of the metrics collector, allowing the service to remain testable without requiring metrics infrastructure.
532-544: LGTM!The helper properly handles nil collectors and iterates through rules to emit metrics. The nil-check prevents panics when metrics are disabled.
661-661: LGTM!The defer placement ensures metrics are collected after rule processing completes, capturing the final state of all rule statistics.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/services/automations/processor.go (1)
66-77: LGTM! Well-aligned with the outcome-focused metrics design.The implementation correctly emits metrics for actions taken rather than conditions evaluated, which aligns with the extensive design discussion in past reviews. The metric updates accurately reflect the rule execution outcomes.
Optional: Add defensive nil check
For robustness, consider adding a nil check at the start of the method:
func (s *ruleRunStats) CollectMetrics(rule *models.Automation, metricsCollector *collector.AutomationCollector, instanceName string) { + if metricsCollector == nil { + return + } metricsCollector.GetAutomationRuleRunTotal(rule.InstanceID, instanceName, rule.ID, rule.Name).Inc()This prevents panics if metrics collection is disabled and the service layer passes nil.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
internal/metrics/collector/automation.gointernal/metrics/manager_test.gointernal/services/automations/processor.gointernal/services/automations/processor_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/metrics/manager_test.go
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-06T12:11:04.963Z
Learnt from: Audionut
Repo: autobrr/qui PR: 553
File: internal/services/crossseed/service.go:1045-1082
Timestamp: 2025-11-06T12:11:04.963Z
Learning: The autobrr/qui project uses a custom go-qbittorrent client library (github.com/autobrr/go-qbittorrent) that supports both "paused" and "stopped" parameters when adding torrents via the options map. Both parameters should be set together when controlling torrent start state, as seen in internal/services/crossseed/service.go and throughout the codebase.
Applied to files:
internal/services/automations/processor.gointernal/services/automations/processor_test.go
📚 Learning: 2025-12-11T08:40:01.329Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 746
File: internal/services/reannounce/service.go:480-481
Timestamp: 2025-12-11T08:40:01.329Z
Learning: In autobrr/qui's internal/services/reannounce/service.go, the hasHealthyTracker, getProblematicTrackers, and getHealthyTrackers functions intentionally match qbrr's lenient tracker health logic (skip unregistered trackers and check if any other tracker is healthy) rather than go-qbittorrent's strict isTrackerStatusOK logic (which treats unregistered as an immediate failure). For multi-tracker torrents, if one tracker is working, reannouncing won't help. The duplication of the health check logic across these three functions is acceptable as it's a simple one-liner, and extracting it would add unnecessary complexity.
Applied to files:
internal/services/automations/processor.gointernal/services/automations/processor_test.go
📚 Learning: 2025-12-28T18:44:10.496Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 876
File: internal/logstream/hub_test.go:188-192
Timestamp: 2025-12-28T18:44:10.496Z
Learning: In Go 1.25 (Aug 2025), use wg.Go(func()) to spawn a goroutine and automate the Add/Done lifecycle. Replace manual patterns like wg.Add(1); go func(){ defer wg.Done(); ... }() with wg.Go(func(){ ... }). Ensure the codebase builds with Go 1.25+ and apply this in relevant Go files (e.g., internal/logstream/hub_test.go). If targeting older Go versions, maintain the existing pattern.
Applied to files:
internal/services/automations/processor.gointernal/metrics/collector/automation.gointernal/services/automations/processor_test.go
🧬 Code graph analysis (2)
internal/services/automations/processor.go (2)
internal/models/automation.go (1)
Automation(34-46)internal/metrics/collector/automation.go (1)
AutomationCollector(9-13)
internal/services/automations/processor_test.go (3)
internal/models/automation.go (3)
Automation(34-46)ActionConditions(448-456)CategoryAction(498-506)internal/metrics/collector/automation.go (1)
NewAutomationCollector(15-41)internal/qbittorrent/sync_manager.go (1)
NewSyncManager(216-237)
🔇 Additional comments (5)
internal/services/automations/processor_test.go (2)
144-182: LGTM! Comprehensive test coverage.The test thoroughly validates that
CollectMetricscorrectly increments each metric type with the expected values from the stats fields.
184-237: LGTM! End-to-end metrics flow validated correctly.The test verifies the complete flow from torrent processing through metrics collection, correctly asserting both the category action that was applied and the action types that remained at zero.
internal/metrics/collector/automation.go (3)
9-41: LGTM! Well-structured collector following Prometheus conventions.The AutomationCollector properly implements the Prometheus metrics pattern with:
- Correct
_totalsuffix for all counter metrics- Appropriate label dimensions for dashboarding (instance_id/name, rule_id/name)
- Clean registration via
MustRegister
43-59: LGTM! Accessor methods correctly return fully-labeled counters.Both methods properly bind all label dimensions and return ready-to-use
prometheus.Counterinstances.
61-68: LGTM! Correct use of currying for multi-label metric.The method properly uses
MustCurryWithto pre-bind instance and rule labels, allowing the caller to specify theactionlabel. This pattern is used correctly inprocessor.go.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
internal/metrics/collector/automation.gointernal/metrics/manager_test.gointernal/services/automations/processor.gointernal/services/automations/processor_test.gointernal/services/automations/service.go
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-06T12:11:04.963Z
Learnt from: Audionut
Repo: autobrr/qui PR: 553
File: internal/services/crossseed/service.go:1045-1082
Timestamp: 2025-11-06T12:11:04.963Z
Learning: The autobrr/qui project uses a custom go-qbittorrent client library (github.com/autobrr/go-qbittorrent) that supports both "paused" and "stopped" parameters when adding torrents via the options map. Both parameters should be set together when controlling torrent start state, as seen in internal/services/crossseed/service.go and throughout the codebase.
Applied to files:
internal/services/automations/service.gointernal/services/automations/processor_test.gointernal/services/automations/processor.go
📚 Learning: 2025-12-11T08:40:01.329Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 746
File: internal/services/reannounce/service.go:480-481
Timestamp: 2025-12-11T08:40:01.329Z
Learning: In autobrr/qui's internal/services/reannounce/service.go, the hasHealthyTracker, getProblematicTrackers, and getHealthyTrackers functions intentionally match qbrr's lenient tracker health logic (skip unregistered trackers and check if any other tracker is healthy) rather than go-qbittorrent's strict isTrackerStatusOK logic (which treats unregistered as an immediate failure). For multi-tracker torrents, if one tracker is working, reannouncing won't help. The duplication of the health check logic across these three functions is acceptable as it's a simple one-liner, and extracting it would add unnecessary complexity.
Applied to files:
internal/services/automations/service.gointernal/services/automations/processor_test.gointernal/services/automations/processor.go
📚 Learning: 2025-12-28T18:44:10.496Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 876
File: internal/logstream/hub_test.go:188-192
Timestamp: 2025-12-28T18:44:10.496Z
Learning: In Go 1.25 (Aug 2025), use wg.Go(func()) to spawn a goroutine and automate the Add/Done lifecycle. Replace manual patterns like wg.Add(1); go func(){ defer wg.Done(); ... }() with wg.Go(func(){ ... }). Ensure the codebase builds with Go 1.25+ and apply this in relevant Go files (e.g., internal/logstream/hub_test.go). If targeting older Go versions, maintain the existing pattern.
Applied to files:
internal/services/automations/service.gointernal/metrics/manager_test.gointernal/services/automations/processor_test.gointernal/services/automations/processor.gointernal/metrics/collector/automation.go
🧬 Code graph analysis (2)
internal/services/automations/processor_test.go (2)
internal/models/automation.go (3)
Automation(34-46)ActionConditions(448-456)CategoryAction(498-506)internal/metrics/collector/automation.go (1)
NewAutomationCollector(16-49)
internal/services/automations/processor.go (1)
internal/metrics/collector/automation.go (1)
AutomationCollector(9-14)
🔇 Additional comments (10)
internal/services/automations/processor_test.go (2)
144-187: LGTM!Comprehensive test coverage for the new CollectMetrics method. The test validates all metric increments for both action-performed and action-not-performed counters.
189-246: LGTM!The test correctly validates the full metrics emission flow through
processTorrentsandCollectMetrics. Past review concerns have been addressed—the redundant manualAddcall was removed and thecategory_appliedassertion is now present.internal/metrics/manager_test.go (3)
59-59: LGTM!Correctly added assertion to verify
AutomationCollectoris registered, consistent with the existing pattern forTorrentCollector.
62-69: LGTM!Standard test pattern for the new
GetAutomationCollectoraccessor, matching the existing test structure for other collectors.
174-189: LGTM!Correctly validates that
AutomationCollectorexposes the expected 4 metric descriptors (RuleRunTotal, RuleRunTorrentsMatchedTotal, RuleRunActionTotal, RuleRunActionNotPerformedTotal).internal/services/automations/processor.go (3)
11-11: LGTM!Correctly imports the new metrics collector package.
61-62: LGTM!Correctly splits the category metrics into distinct counters for "condition not met" vs "blocked by cross-seed protection", addressing the past review feedback about distinguishing these cases.
67-82: LGTM!The
CollectMetricsmethod correctly emits all automation metrics, properly categorizing actions performed vs not performed. The implementation aligns with the new split category metrics.internal/services/automations/service.go (1)
24-24: LGTM!Clean integration of metrics collection into the automation service:
- Proper dependency injection via
SetMetricsCollector- Metrics emission deferred until after rule processing completes
- Nil guard prevents issues when metrics are disabled
- Logging updated to reflect the split category metrics
Also applies to: 66-66, 99-101, 532-544, 661-661, 692-693
internal/metrics/collector/automation.go (1)
1-85: LGTM!Excellent implementation of the AutomationCollector following Prometheus best practices:
- Counter names properly suffixed with
_total- Consistent label sets across metrics
- Correct use of
MustCurryWithfor metrics with additionalactionlabels- Clean constructor pattern with registry registration
- Descriptive help text for all metrics
| RuleRunTorrentsMatchedTotal: prometheus.NewCounterVec(prometheus.CounterOpts{ | ||
| Namespace: "qui", | ||
| Subsystem: "automation", | ||
| Name: "rule_run_torrents_matched_total", | ||
| Help: "Total number of torrents that matched the trackers in the rule", | ||
| }, []string{"instance_id", "instance_name", "rule_id", "rule_name"}), |
There was a problem hiding this comment.
Thoughts on updating this to a guage metric? This way it shows us the most recent number of torrents matched any time it's reported, and we don't have to work backwards from the slope. If the reporting interval is less frequent than the rule execution interval, it won't necessarily tell us exactly how many items were matched every run, however I think that's also true in many cases with an accumulating counter. and the gauge is easier to understand.
| RuleRunTorrentsMatchedTotal: prometheus.NewCounterVec(prometheus.CounterOpts{ | |
| Namespace: "qui", | |
| Subsystem: "automation", | |
| Name: "rule_run_torrents_matched_total", | |
| Help: "Total number of torrents that matched the trackers in the rule", | |
| }, []string{"instance_id", "instance_name", "rule_id", "rule_name"}), | |
| RuleRunTorrentsMatchedTotal: prometheus.NewGaugeVec(prometheus.GaugeOpts{ | |
| Namespace: "qui", | |
| Subsystem: "automation", | |
| Name: "rule_run_torrents_matched", | |
| Help: "Total number of torrents that matched the trackers in the rule", | |
| }, []string{"instance_id", "instance_name", "rule_id", "rule_name"}), |
We'd need to update processor.go
- metricsCollector.GetAutomationRuleRunTorrentsMatchedTotal(rule.InstanceID, instanceName, rule.ID, rule.Name).Add(float64(s.MatchedTrackers))
+ metricsCollector.GetAutomationRuleRunTorrentsMatchedTotal(rule.InstanceID, instanceName, rule.ID, rule.Name).Set(float64(s.MatchedTrackers))
finevan
left a comment
finevan
left a comment
There was a problem hiding this comment.
I'm really excited to try this out. Added one more minor thought, but I trust your judgement on whether or not it's worthwhile
3 participants
It doesn't seem like we're currently collecting event-driven metrics in this project, so I took some liberties in how to structurethat. I'm open to refactoring.
This will expose the following new metrics:
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.