Skip to content

Prometheus: Export all torrent counters, add more metrics#1348

Closed
Zmegolaz wants to merge 89 commits intoautobrr:developfrom
Zmegolaz:dynamic_prom_status_metrics
Closed

Prometheus: Export all torrent counters, add more metrics#1348
Zmegolaz wants to merge 89 commits intoautobrr:developfrom
Zmegolaz:dynamic_prom_status_metrics

Conversation

@Zmegolaz
Copy link
Copy Markdown

@Zmegolaz Zmegolaz commented Jan 28, 2026

There are quite a lot of torrent statuses which might be interesting, but currently aren't exposed via the Prometheus endpoint. This patch adds them all, dynamically instead of listing each one.

It also restructures the metrics, creating one metric for all statuses with a label for the status, which is a bit more like how Prometheus metrics are usually structured, it allows for simpler queries later on. I'm aware that this breaks existing metrics, but I didn't find any good way to create the descriptions dynamically. Let me know if you still prefer the old format and I'll find a way to rewrite this.

I've also added more metrics:

"qbittorrent_session_dl_rate_limit_bytes",
"qbittorrent_session_up_rate_limit_bytes",
"qbittorrent_dht_nodes_count",
"qbittorrent_torrents_category_count",
"qbittorrent_torrents_category_bytes",
"qbittorrent_torrents_tag_count",
"qbittorrent_torrents_tag_bytes",
"qbittorrent_peer_connections_count",
"qbittorrent_alt_speed_limits_enabled",

I have a Grafana dashboard which use these metrics I can publish if this PR is accepted.

Summary by CodeRabbit

  • Refactor
    • Consolidated per-status torrent metrics into a single metric with a status label; added session download/upload rate-limit, DHT node count, total peer connections, alternate speed-limits flag, and per-category torrent counts and sizes for richer monitoring. Empty category/tag keys are normalized to a placeholder.
  • Tests
    • Updated test expectations to reflect the expanded set of metric descriptors.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 28, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Replaces five per-status torrent metrics with a single status-labeled metric and expands the collector to emit session-level and aggregation metrics (DL/UL limits, DHT nodes, total peer connections, ALT speed flag, per-category and per-tag counts/sizes), normalizing empty keys to "_".

Changes

Cohort / File(s) Summary
Torrent Collector
internal/metrics/collector/torrent.go
Replaced five per-status descriptors with a single qbittorrent_torrents_status_count metric (adds status label). Added descriptors and emissions for session download/upload rate limits, DHT nodes, total peer connections, ALT speed limits flag, per-category counts & sizes, and per-tag counts & sizes. Normalizes empty category/tag keys to _. Describe() and Collect() updated accordingly; removed old per-status descriptors.
Test update
internal/metrics/manager_test.go
Updated TestTorrentCollector_Describe expected descriptor count from 11 to 18 to match the expanded descriptor set.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested labels

metrics

Suggested reviewers

  • s0up4200

Poem

🐇 I hopped through descriptors, one by one,

Five into one, then new counts begun.
Limits, nodes, and tags all in a row,
Empty names turned to underscores to show.
A rabbit applauds metrics' steady glow.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: exporting all torrent counters dynamically and adding new metrics, matching the PR's primary objective.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Zmegolaz Zmegolaz changed the title Prometheus: Export all torrent counters Prometheus: Export all torrent counters, add more metrics Jan 29, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@internal/metrics/collector/torrent.go`:
- Around line 345-427: Add a configurable cardinality cap (default ~500) and
enforce it when iterating the label-heavy maps counts.Categories,
counts.CategorySizes, counts.TrackerTransfers, counts.Tags, and counts.TagSizes
in the collector (the loop blocks that emit c.torrentsPerCategoryDesc,
c.torrentsSizePerCategoryDesc, c.torrentsPerTrackerDesc,
c.torrentsSizePerTrackerDesc, c.torrentsPerTagDesc, c.torrentsSizePerTagDesc);
maintain a per-label-type counter, increment on each metric emitted, and break
the loop once the cap is reached (optionally record/log the number skipped), and
make the cap configurable via the collector config or a constant so it can be
tuned.

Comment on lines +345 to +427
// Counts: number/size per category, tracker, tag
if response != nil && response.Counts != nil {
counts := response.Counts

// Categories -> map[string]int
if counts.Categories != nil {
for cat, cnt := range counts.Categories {
ch <- prometheus.MustNewConstMetric(
c.torrentsPerCategoryDesc,
prometheus.GaugeValue,
float64(cnt),
instanceIDStr,
instanceName,
cat,
)
}
}
// CategorySizes -> map[string]int64
if counts.CategorySizes != nil {
for cat, sz := range counts.CategorySizes {
ch <- prometheus.MustNewConstMetric(
c.torrentsSizePerCategoryDesc,
prometheus.GaugeValue,
float64(sz),
instanceIDStr,
instanceName,
cat,
)
}
}
// TrackerTransfers -> map[string]TrackerTransferStats
if counts.TrackerTransfers != nil {
for tr, tt := range counts.TrackerTransfers {
ch <- prometheus.MustNewConstMetric(
c.torrentsPerTrackerDesc,
prometheus.GaugeValue,
float64(tt.Count),
instanceIDStr,
instanceName,
tr,
)
ch <- prometheus.MustNewConstMetric(
c.torrentsSizePerTrackerDesc,
prometheus.GaugeValue,
float64(tt.TotalSize),
instanceIDStr,
instanceName,
tr,
)
}
}
// Tags: Tags -> map[string]int
if counts.Tags != nil {
for tag, cnt := range counts.Tags {
if tag == "" {
tag = "_"
}
ch <- prometheus.MustNewConstMetric(
c.torrentsPerTagDesc,
prometheus.GaugeValue,
float64(cnt),
instanceIDStr,
instanceName,
tag,
)
}
}
// TagSizes -> map[string]int64
if counts.TagSizes != nil {
for tag, sz := range counts.TagSizes {
if tag == "" {
tag = "_"
}
ch <- prometheus.MustNewConstMetric(
c.torrentsSizePerTagDesc,
prometheus.GaugeValue,
float64(sz),
instanceIDStr,
instanceName,
tag,
)
}
}
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "torrent.go" | head -20

Repository: autobrr/qui

Length of output: 94


🏁 Script executed:

wc -l ./internal/metrics/collector/torrent.go

Repository: autobrr/qui

Length of output: 98


🏁 Script executed:

sed -n '345,427p' ./internal/metrics/collector/torrent.go

Repository: autobrr/qui

Length of output: 2030


🏁 Script executed:

sed -n '300,350p' ./internal/metrics/collector/torrent.go

Repository: autobrr/qui

Length of output: 1240


🏁 Script executed:

sed -n '1,50p' ./internal/metrics/collector/torrent.go

Repository: autobrr/qui

Length of output: 1671


🏁 Script executed:

grep -n "func.*Collect" ./internal/metrics/collector/torrent.go

Repository: autobrr/qui

Length of output: 437


🏁 Script executed:

sed -n '189,350p' ./internal/metrics/collector/torrent.go

Repository: autobrr/qui

Length of output: 4211


🏁 Script executed:

find . -type f -name "*.go" | xargs grep -l "type.*SyncManager" | head -5

Repository: autobrr/qui

Length of output: 306


🏁 Script executed:

rg -A 30 "type Counts struct" ./internal/qbittorrent/

Repository: autobrr/qui

Length of output: 37


🏁 Script executed:

rg -n "Counts.*struct" ./internal/qbittorrent/ -A 10

Repository: autobrr/qui

Length of output: 2195


🏁 Script executed:

rg -n "Categories\s*=" ./internal/qbittorrent/sync_manager.go | head -20

Repository: autobrr/qui

Length of output: 228


🏁 Script executed:

sed -n '2900,2950p' ./internal/qbittorrent/sync_manager.go

Repository: autobrr/qui

Length of output: 1842


🏁 Script executed:

rg -n "counts.Tags\s*=" ./internal/qbittorrent/sync_manager.go

Repository: autobrr/qui

Length of output: 37


🏁 Script executed:

rg -n "counts\.Tags" ./internal/qbittorrent/sync_manager.go -B 5 | head -40

Repository: autobrr/qui

Length of output: 687


🏁 Script executed:

sed -n '2800,2900p' ./internal/qbittorrent/sync_manager.go

Repository: autobrr/qui

Length of output: 3641


🏁 Script executed:

rg -n "maxLabelValues\|cardinality\|limit.*label" ./internal/metrics/

Repository: autobrr/qui

Length of output: 37


Implement cardinality limits for category/tracker/tag metrics to prevent Prometheus series explosion.

Categories, trackers, and tags are user-controlled values that can create unbounded metric series. The current code iterates through counts.Categories, counts.CategorySizes, counts.TrackerTransfers, counts.Tags, and counts.TagSizes without limits, risking excessive memory usage and scrape instability in Prometheus. Implement a configurable cap (e.g., 500 series per label type) and break iteration once reached.

Example implementation (apply to all five label-heavy blocks)
-		if response != nil && response.Counts != nil {
-			counts := response.Counts
+		if response != nil && response.Counts != nil {
+			counts := response.Counts
+			const maxLabelValues = 500
@@
 			if counts.Categories != nil {
-				for cat, cnt := range counts.Categories {
+				emitted := 0
+				for cat, cnt := range counts.Categories {
+					if emitted >= maxLabelValues {
+						break
+					}
 					ch <- prometheus.MustNewConstMetric(
 						c.torrentsPerCategoryDesc,
 						prometheus.GaugeValue,
 						float64(cnt),
 						instanceIDStr,
 						instanceName,
 						cat,
 					)
+					emitted++
 				}
 			}
@@
 			if counts.CategorySizes != nil {
-				for cat, sz := range counts.CategorySizes {
+				emitted := 0
+				for cat, sz := range counts.CategorySizes {
+					if emitted >= maxLabelValues {
+						break
+					}
 					ch <- prometheus.MustNewConstMetric(
 						c.torrentsSizePerCategoryDesc,
 						prometheus.GaugeValue,
 						float64(sz),
 						instanceIDStr,
 						instanceName,
 						cat,
 					)
+					emitted++
 				}
 			}
@@
 			if counts.TrackerTransfers != nil {
-				for tr, tt := range counts.TrackerTransfers {
+				emitted := 0
+				for tr, tt := range counts.TrackerTransfers {
+					if emitted >= maxLabelValues {
+						break
+					}
 					ch <- prometheus.MustNewConstMetric(
 						c.torrentsPerTrackerDesc,
 						prometheus.GaugeValue,
 						float64(tt.Count),
 						instanceIDStr,
 						instanceName,
 						tr,
 					)
 					ch <- prometheus.MustNewConstMetric(
 						c.torrentsSizePerTrackerDesc,
 						prometheus.GaugeValue,
 						float64(tt.TotalSize),
 						instanceIDStr,
 						instanceName,
 						tr,
 					)
+					emitted++
 				}
 			}
@@
 			if counts.Tags != nil {
-				for tag, cnt := range counts.Tags {
+				emitted := 0
+				for tag, cnt := range counts.Tags {
+					if emitted >= maxLabelValues {
+						break
+					}
 					if tag == "" {
 						tag = "_"
 					}
 					ch <- prometheus.MustNewConstMetric(
 						c.torrentsPerTagDesc,
 						prometheus.GaugeValue,
 						float64(cnt),
 						instanceIDStr,
 						instanceName,
 						tag,
 					)
+					emitted++
 				}
 			}
@@
 			if counts.TagSizes != nil {
-				for tag, sz := range counts.TagSizes {
+				emitted := 0
+				for tag, sz := range counts.TagSizes {
+					if emitted >= maxLabelValues {
+						break
+					}
 					if tag == "" {
 						tag = "_"
 					}
 					ch <- prometheus.MustNewConstMetric(
 						c.torrentsSizePerTagDesc,
 						prometheus.GaugeValue,
 						float64(sz),
 						instanceIDStr,
 						instanceName,
 						tag,
 					)
+					emitted++
 				}
 			}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Counts: number/size per category, tracker, tag
if response != nil && response.Counts != nil {
counts := response.Counts
// Categories -> map[string]int
if counts.Categories != nil {
for cat, cnt := range counts.Categories {
ch <- prometheus.MustNewConstMetric(
c.torrentsPerCategoryDesc,
prometheus.GaugeValue,
float64(cnt),
instanceIDStr,
instanceName,
cat,
)
}
}
// CategorySizes -> map[string]int64
if counts.CategorySizes != nil {
for cat, sz := range counts.CategorySizes {
ch <- prometheus.MustNewConstMetric(
c.torrentsSizePerCategoryDesc,
prometheus.GaugeValue,
float64(sz),
instanceIDStr,
instanceName,
cat,
)
}
}
// TrackerTransfers -> map[string]TrackerTransferStats
if counts.TrackerTransfers != nil {
for tr, tt := range counts.TrackerTransfers {
ch <- prometheus.MustNewConstMetric(
c.torrentsPerTrackerDesc,
prometheus.GaugeValue,
float64(tt.Count),
instanceIDStr,
instanceName,
tr,
)
ch <- prometheus.MustNewConstMetric(
c.torrentsSizePerTrackerDesc,
prometheus.GaugeValue,
float64(tt.TotalSize),
instanceIDStr,
instanceName,
tr,
)
}
}
// Tags: Tags -> map[string]int
if counts.Tags != nil {
for tag, cnt := range counts.Tags {
if tag == "" {
tag = "_"
}
ch <- prometheus.MustNewConstMetric(
c.torrentsPerTagDesc,
prometheus.GaugeValue,
float64(cnt),
instanceIDStr,
instanceName,
tag,
)
}
}
// TagSizes -> map[string]int64
if counts.TagSizes != nil {
for tag, sz := range counts.TagSizes {
if tag == "" {
tag = "_"
}
ch <- prometheus.MustNewConstMetric(
c.torrentsSizePerTagDesc,
prometheus.GaugeValue,
float64(sz),
instanceIDStr,
instanceName,
tag,
)
}
}
// Counts: number/size per category, tracker, tag
if response != nil && response.Counts != nil {
counts := response.Counts
const maxLabelValues = 500
// Categories -> map[string]int
if counts.Categories != nil {
emitted := 0
for cat, cnt := range counts.Categories {
if emitted >= maxLabelValues {
break
}
ch <- prometheus.MustNewConstMetric(
c.torrentsPerCategoryDesc,
prometheus.GaugeValue,
float64(cnt),
instanceIDStr,
instanceName,
cat,
)
emitted++
}
}
// CategorySizes -> map[string]int64
if counts.CategorySizes != nil {
emitted := 0
for cat, sz := range counts.CategorySizes {
if emitted >= maxLabelValues {
break
}
ch <- prometheus.MustNewConstMetric(
c.torrentsSizePerCategoryDesc,
prometheus.GaugeValue,
float64(sz),
instanceIDStr,
instanceName,
cat,
)
emitted++
}
}
// TrackerTransfers -> map[string]TrackerTransferStats
if counts.TrackerTransfers != nil {
emitted := 0
for tr, tt := range counts.TrackerTransfers {
if emitted >= maxLabelValues {
break
}
ch <- prometheus.MustNewConstMetric(
c.torrentsPerTrackerDesc,
prometheus.GaugeValue,
float64(tt.Count),
instanceIDStr,
instanceName,
tr,
)
ch <- prometheus.MustNewConstMetric(
c.torrentsSizePerTrackerDesc,
prometheus.GaugeValue,
float64(tt.TotalSize),
instanceIDStr,
instanceName,
tr,
)
emitted++
}
}
// Tags: Tags -> map[string]int
if counts.Tags != nil {
emitted := 0
for tag, cnt := range counts.Tags {
if emitted >= maxLabelValues {
break
}
if tag == "" {
tag = "_"
}
ch <- prometheus.MustNewConstMetric(
c.torrentsPerTagDesc,
prometheus.GaugeValue,
float64(cnt),
instanceIDStr,
instanceName,
tag,
)
emitted++
}
}
// TagSizes -> map[string]int64
if counts.TagSizes != nil {
emitted := 0
for tag, sz := range counts.TagSizes {
if emitted >= maxLabelValues {
break
}
if tag == "" {
tag = "_"
}
ch <- prometheus.MustNewConstMetric(
c.torrentsSizePerTagDesc,
prometheus.GaugeValue,
float64(sz),
instanceIDStr,
instanceName,
tag,
)
emitted++
}
}
🤖 Prompt for AI Agents
In `@internal/metrics/collector/torrent.go` around lines 345 - 427, Add a
configurable cardinality cap (default ~500) and enforce it when iterating the
label-heavy maps counts.Categories, counts.CategorySizes,
counts.TrackerTransfers, counts.Tags, and counts.TagSizes in the collector (the
loop blocks that emit c.torrentsPerCategoryDesc, c.torrentsSizePerCategoryDesc,
c.torrentsPerTrackerDesc, c.torrentsSizePerTrackerDesc, c.torrentsPerTagDesc,
c.torrentsSizePerTagDesc); maintain a per-label-type counter, increment on each
metric emitted, and break the loop once the cap is reached (optionally
record/log the number skipped), and make the cap configurable via the collector
config or a constant so it can be tuned.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this is a valid concern, I'd say that if people use hundreds of tags/categories they can probably handle that many metrics as well, or should at least blame themselves :). But I'd appreciate input from the maintainers if they think we should create such a limit.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Zmegolaz, that's a fair perspective! You're right that users who actively manage hundreds of categories/tags are likely monitoring their infrastructure.

The main risk with unbounded cardinality isn't just the number of series—it's that Prometheus stores every unique label combination, and this can grow faster than expected when labels interact (e.g., instance × category × tracker). Even experienced users can accidentally create cardinality explosions through automation or integrations. That said, you know your user base better than I do.

I'll leave this as a suggestion for the maintainers to consider. If they're comfortable with the current approach, that's perfectly reasonable given the use case. Thanks for considering it!

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@internal/metrics/collector/torrent.go`:
- Around line 382-401: The tracker label loop over counts.TrackerTransfers
treats empty tracker keys differently than categories/tags — normalize tr to "_"
when tr == "" before creating metrics; update the loop that iterates
counts.TrackerTransfers (where tr and tt are used to call
c.torrentsPerTrackerDesc and c.torrentsSizePerTrackerDesc with instanceIDStr and
instanceName) to replace empty string keys with "_" so labels are consistent
with other normalization.
🧹 Nitpick comments (1)
internal/metrics/collector/torrent.go (1)

88-99: Consider clarifying rate limit metric names.

The metric names qbittorrent_session_dl_rate_limit_bytes and qbittorrent_session_up_rate_limit_bytes end with _bytes, but the help text says "bytes/sec". This could be slightly confusing since _bytes typically implies a total byte count rather than a rate.

Consider renaming to _bytes_per_second or keeping as-is with clear documentation. This is a minor naming consideration and not a blocker.

Comment thread internal/metrics/collector/torrent.go Outdated
Comment on lines +382 to +401
if counts.TrackerTransfers != nil {
for tr, tt := range counts.TrackerTransfers {
ch <- prometheus.MustNewConstMetric(
c.torrentsPerTrackerDesc,
prometheus.GaugeValue,
float64(tt.Count),
instanceIDStr,
instanceName,
tr,
)
ch <- prometheus.MustNewConstMetric(
c.torrentsSizePerTrackerDesc,
prometheus.GaugeValue,
float64(tt.TotalSize),
instanceIDStr,
instanceName,
tr,
)
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Inconsistent empty string handling for tracker labels.

Categories and tags normalize empty strings to "_", but the tracker iteration does not apply this normalization. If a tracker key could ever be empty, this would result in inconsistent behavior.

🔧 Proposed fix to add empty string normalization
 			if counts.TrackerTransfers != nil {
 				for tr, tt := range counts.TrackerTransfers {
+					if tr == "" {
+						tr = "_"
+					}
 					ch <- prometheus.MustNewConstMetric(
 						c.torrentsPerTrackerDesc,
 						prometheus.GaugeValue,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if counts.TrackerTransfers != nil {
for tr, tt := range counts.TrackerTransfers {
ch <- prometheus.MustNewConstMetric(
c.torrentsPerTrackerDesc,
prometheus.GaugeValue,
float64(tt.Count),
instanceIDStr,
instanceName,
tr,
)
ch <- prometheus.MustNewConstMetric(
c.torrentsSizePerTrackerDesc,
prometheus.GaugeValue,
float64(tt.TotalSize),
instanceIDStr,
instanceName,
tr,
)
}
}
if counts.TrackerTransfers != nil {
for tr, tt := range counts.TrackerTransfers {
if tr == "" {
tr = "_"
}
ch <- prometheus.MustNewConstMetric(
c.torrentsPerTrackerDesc,
prometheus.GaugeValue,
float64(tt.Count),
instanceIDStr,
instanceName,
tr,
)
ch <- prometheus.MustNewConstMetric(
c.torrentsSizePerTrackerDesc,
prometheus.GaugeValue,
float64(tt.TotalSize),
instanceIDStr,
instanceName,
tr,
)
}
}
🤖 Prompt for AI Agents
In `@internal/metrics/collector/torrent.go` around lines 382 - 401, The tracker
label loop over counts.TrackerTransfers treats empty tracker keys differently
than categories/tags — normalize tr to "_" when tr == "" before creating
metrics; update the loop that iterates counts.TrackerTransfers (where tr and tt
are used to call c.torrentsPerTrackerDesc and c.torrentsSizePerTrackerDesc with
instanceIDStr and instanceName) to replace empty string keys with "_" so labels
are consistent with other normalization.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/metrics/collector/torrent.go`:
- Around line 336-338: The code currently replaces empty category/tag label
values with "_" (the conditional around variables like cat and tag), causing
loss of fidelity between actual empty labels and literal "_" values; remove the
conditional branches that set cat = "_" and tag = "_" in the metric emission
code (the blocks that check if cat == "" or tag == "" and overwrite them) so the
original label values (including empty strings) are preserved when calling the
metric recording helpers or Prometheus label constructors (look for uses of
variables named cat and tag in the torrent metrics emission functions and delete
the replacement logic while leaving the original variable values passed
through).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 789ae844-5bd6-45da-84de-aa751b89ac6f

📥 Commits

Reviewing files that changed from the base of the PR and between 2bf1a3f and 6b09bec.

📒 Files selected for processing (1)
  • internal/metrics/collector/torrent.go

Comment on lines +336 to +338
if cat == "" {
cat = "_"
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Does Prometheus allow empty label values, and does github.com/prometheus/client_golang treat duplicate samples with the same metric name and label values in a single scrape as an error?

💡 Result:

Yes, Prometheus allows empty label values. They are considered equivalent to labels that do not exist, as stated in the official data model documentation. However, while Prometheus accepts and normalizes them during ingestion, empty labels are generally discouraged and may be dropped or invalid in certain contexts like remote_write. No, github.com/prometheus/client_golang does not treat duplicate samples with the same metric name and label values in a single scrape as an error. The library's registry checks for consistency during metric collection (Gather), and duplicate samples with identical labels and values within the same family are expected to be merged or handled silently without error during exposition. Errors like "collected metric ... was collected before with the same name and label values" occur only if values differ or due to races/inconsistencies, not identical duplicates. Any duplicate handling (e.g., incrementing counters like prometheus_target_scrapes_sample_duplicate_timestamp_total) happens at the Prometheus server scrape level, not in client_golang.

Citations:


🏁 Script executed:

# First, check if the file exists and get its line count
wc -l internal/metrics/collector/torrent.go

Repository: autobrr/qui

Length of output: 96


🏁 Script executed:

# Read the code around the mentioned line ranges (336-338, 352-354, 368-370, 384-386)
sed -n '330,390p' internal/metrics/collector/torrent.go | cat -n

Repository: autobrr/qui

Length of output: 2030


🏁 Script executed:

# Search for the empty string to underscore rewriting pattern
rg -n 'if (cat|tag) == ""' internal/metrics/collector/torrent.go

Repository: autobrr/qui

Length of output: 150


🏁 Script executed:

# Also search for underscore assignment patterns
rg -n '= "_"' internal/metrics/collector/torrent.go

Repository: autobrr/qui

Length of output: 134


🏁 Script executed:

sed -n '445,510p' internal/metrics/collector/torrent.go | cat -n

Repository: autobrr/qui

Length of output: 2029


Correct the line numbers and remove the empty string to underscore rewriting to preserve metric data fidelity.

The code at lines 452–453, 468–469, 484–485, and 500–501 collapses empty category and tag labels to _. Prometheus accepts empty label values, and this aliasing causes loss of distinction: if the source data contains both an uncategorized item (empty string) and an item actually tagged _, they become indistinguishable in the metrics. Remove these conditionals to preserve the original label values.

Proposed fix
 			// Categories -> map[string]int
 			if counts.Categories != nil {
 				for cat, cnt := range counts.Categories {
-					if cat == "" {
-						cat = "_"
-					}
 					ch <- prometheus.MustNewConstMetric(
 						c.torrentsPerCategoryDesc,
 						prometheus.GaugeValue,
 						float64(cnt),
 						instanceIDStr,
 						instanceName,
 						cat,
 					)
 			// CategorySizes -> map[string]int64
 			if counts.CategorySizes != nil {
 				for cat, sz := range counts.CategorySizes {
-					if cat == "" {
-						cat = "_"
-					}
 					ch <- prometheus.MustNewConstMetric(
 						c.torrentsSizePerCategoryDesc,
 						prometheus.GaugeValue,
 						float64(sz),
 						instanceIDStr,
 						instanceName,
 						cat,
 					)
 			// Tags: Tags -> map[string]int
 			if counts.Tags != nil {
 				for tag, cnt := range counts.Tags {
-					if tag == "" {
-						tag = "_"
-					}
 					ch <- prometheus.MustNewConstMetric(
 						c.torrentsPerTagDesc,
 						prometheus.GaugeValue,
 						float64(cnt),
 						instanceIDStr,
 						instanceName,
 						tag,
 					)
 			// TagSizes -> map[string]int64
 			if counts.TagSizes != nil {
 				for tag, sz := range counts.TagSizes {
-					if tag == "" {
-						tag = "_"
-					}
 					ch <- prometheus.MustNewConstMetric(
 						c.torrentsSizePerTagDesc,
 						prometheus.GaugeValue,
 						float64(sz),
 						instanceIDStr,
 						instanceName,
 						tag,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/metrics/collector/torrent.go` around lines 336 - 338, The code
currently replaces empty category/tag label values with "_" (the conditional
around variables like cat and tag), causing loss of fidelity between actual
empty labels and literal "_" values; remove the conditional branches that set
cat = "_" and tag = "_" in the metric emission code (the blocks that check if
cat == "" or tag == "" and overwrite them) so the original label values
(including empty strings) are preserved when calling the metric recording
helpers or Prometheus label constructors (look for uses of variables named cat
and tag in the torrent metrics emission functions and delete the replacement
logic while leaving the original variable values passed through).

s0up4200 and others added 25 commits March 28, 2026 22:54
- Fix UI location: "Cross-Seed → Auto → Webhook / autobrr" (not Global)
- Add missing /apply fields: startPaused, skipIfExists, findIndividualEpisodes
- Mark torrentData as required, instanceIds as optional for consistency
…, fix free-space projection (autobrr#1116)

Co-authored-by: soup <s0up4200@pm.me>
s0up4200 and others added 28 commits March 28, 2026 22:54
Add redirects for Chrome and Firefox extension store pages:
- /chrome-extension -> Chrome Web Store
- /firefox-extension -> Firefox Add-ons

Add Browser Extensions section to intro page describing
the right-click torrent adding functionality.
@Zmegolaz Zmegolaz force-pushed the dynamic_prom_status_metrics branch from 5a15f9f to fb3c1b2 Compare March 28, 2026 21:54
@Zmegolaz Zmegolaz closed this Mar 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.