feat(conf): add optional field_labels to override Labels #189
Conversation
cropalato
force-pushed
the
master
branch
4 times, most recently
from
ab7b098 to
cd931b9
Compare
Signed-off-by: Ricardo Melo <ricardo@cropa.ca>
|
What about this, is this going to get merged into prometheus alertmanager? Very useful and needed feature for jiralert. |
| @@ -194,6 +197,58 @@ func (c Config) String() string { | |||
| return string(b) | |||
| } | |||
|
|
|||
| // GetJiraFieldKey returns the jira key associated to a field. | |||
There was a problem hiding this comment.
Comment is different vs actual function signature -- GetJiraFieldKey only returns error or nil.
| @@ -194,6 +197,58 @@ func (c Config) String() string { | |||
| return string(b) | |||
| } | |||
|
|
|||
| // GetJiraFieldKey returns the jira key associated to a field. | |||
| func (c *Config) GetJiraFieldKey() error { | |||
There was a problem hiding this comment.
Do you mind moving this function out of the config.go? It would be epic to separate JIRA API integrations vs config logic. Also let's avoid extra config state that does not belong to user config directly - it's ok to store this fieldkey as variable somewhere.
Also probably you mean ValidateJIRAField or something - it's ok to have validateXYZ function in cmd/ somewhere 🤔
| if c.Defaults.FieldLabels != "" { | ||
| rc.FieldLabels = c.Defaults.FieldLabels | ||
| } else { | ||
| rc.FieldLabels = "Labels" |
There was a problem hiding this comment.
We might need constant variable for this
| // Search multiple projects in case issue was moved and further alert firings are desired in existing JIRA. | ||
| projectList := "'" + strings.Join(projects, "', '") + "'" | ||
| query := fmt.Sprintf("project in(%s) and labels=%q order by resolutiondate desc", projectList, issueLabel) | ||
| if r.conf.FieldLabels == "Labels" { | ||
| labelKey = "labels" |
There was a problem hiding this comment.
Why not using labels as a default value (instead of Labels) then?
| # Define the jira field used by jiralert to avoid ticket duplication. Optional (default: Labels) | ||
| field_labels: Labels |
There was a problem hiding this comment.
Hm, what about id_field?
| # Define the jira field used by jiralert to avoid ticket duplication. Optional (default: Labels) | |
| field_labels: Labels | |
| # The jira field used by jiralert for the unique hash and optional grouping and static labels that identify the ticket for reuse. It's recommended to keep using labels field, but some users prefer custom fields (see issue #164). Optional (default: labels). | |
| id_field: labels |
| @@ -171,11 +171,11 @@ func (r *Receiver) Notify(data *alertmanager.Data, hashJiraLabel bool, updateSum | |||
| } | |||
|
|
|||
| if len(data.Alerts.Firing()) == 0 { | |||
| level.Debug(r.logger).Log("msg", "no firing alert; nothing to do.", "label", issueGroupLabel) | |||
| level.Debug(r.logger).Log("msg", "no firing alert; nothing to do.", r.conf.FieldLabels, issueGroupLabel) | |||
There was a problem hiding this comment.
Not sure if we should use IDLabel value for logging key. What if somebody will have custom field called msg? Can we just use id here?
Also I would rename issueGroupLabel everywhere to issueIDLabel because at this point it might be hash, static and maybe group labels. "Group" is a bit misleading given the add_group_labels config field which only referes to Prometheus labels. 🙈 not an ideal name, but not worth changing (unless we want to deprecate)
|
Also please rebase as we updated CI. |
It adds an optional parameter to replace Labels field with a custom field.
It should be enough to fix #164.