-
-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add support for scraping tags in radarr #174
Conversation
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.
A few nits on to keep naming/labeling consistent, but the core logic lgtm! Glad to approve and merge once resolved.
Thanks for the PR!
internal/arr/collector/radarr.go
Outdated
@@ -72,6 +73,12 @@ func NewRadarrCollector(c *config.ArrConfig) *radarrCollector { | |||
[]string{"quality"}, | |||
prometheus.Labels{"url": c.URL}, | |||
), | |||
movieTags: prometheus.NewDesc( |
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.
Can we match the naming convention of the other metrics?
Something like movieTagsMetric?
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.
Updated.
internal/arr/model/radarr.go
Outdated
@@ -16,3 +16,9 @@ type Movie []struct { | |||
} `json:"movieFile"` | |||
QualityProfileID int `json:"qualityProfileId"` | |||
} | |||
|
|||
type TagObjects []struct { |
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.
This is specific to radarr, let's call this TagMovies
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.
Updated.
internal/arr/collector/radarr.go
Outdated
@@ -72,6 +73,12 @@ func NewRadarrCollector(c *config.ArrConfig) *radarrCollector { | |||
[]string{"quality"}, | |||
prometheus.Labels{"url": c.URL}, | |||
), | |||
movieTags: prometheus.NewDesc( | |||
"radarr_movie_tag_total", | |||
"Tags by total", |
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.
Same here, can we match the other metrics?
"Total number of downloaded movies by tag"
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.
Updated.
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.
Looks good from here, @rtrox I'll wait for your sign off too.
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.
Lgtm too! Thanks for the quick follow-up.
This change adds a new metric, the number of movies in a tag
No known limitations
Proof of functionality:
Description of the change
Add a new metric which scrapes radarr tags and the movies inside them
Benefits
Additional metrics are always nice?
Possible drawbacks
Any drawbacks that come from adding a new metric.