Jira Integration: Allow configuring issue update via parameter

[holger-waschke]

This change allows disabling the update of issue descriptions.

It´s a non breaking change the default is still to update everything. Only if you set the parameter disable_update_description explicit to true the description won´t get updated anymore.

We need this feature as there a lot of jira automations which updates the description itself, jiralert would revert the changes back to the original templated description state once the repeat_interval kicks in and the alert is still active, so it´s handy to have the possibility to disable it.

I added a new unit test for this function to test.

Exempel config

receivers:
- name: jira_alarm
  jira_configs:
  - project: MON
    issue_type: Bug
    resolve_transition: "Geschlossen"
    reopen_transition: "Erfasst"
    reopen_duration: "1h"
    disable_update_description: true

[cjedro]

this is also important due to upcoming rate-limiting https://developer.atlassian.com/cloud/jira/platform/rate-limiting/

[holger-waschke]

this is also important due to upcoming rate-limiting https://developer.atlassian.com/cloud/jira/platform/rate-limiting/

tbh this sounds to me like it will be mandatory to have some kind of back off mechanism to avoid running into these enforced rate limits?

[cjedro]

@holger-waschke i think this is already implemented

alertmanager/notify/jira/jira.go

Line 63 in f7ff687

retrier: &notify.Retrier{RetryCodes: []int{http.StatusTooManyRequests}},

[fluktuid]

I'm with you, but this handles every alarm separate.
So in the worst-case when many alarms are open and firing, the rate-limit could result in no new alarms opened.
This could happen because of the push-mechanism that re-sends old alarms for updates.

I think generally an optional circuit-breaker could solve this.
Spontaneously I would think about two queues:

1. more important (e.g. new alarms)
2. less important (e.g. update/close alarms)

Maybe we should discuss this in an issue.

[Spaceman1701]

I like this idea, but I'm wondering if we should have a more generic implementation. I'm less familiar with the Jira integration than I should be, honestly. Are there any other fields which we might want to avoid updating on repeat notifications? I'm wondering if we should allow fields to have sub-keys that indicate if we should update them or not.

I also think the discussion about rate-limiting is important enough to move to a dedicated issue. It's really a problem for any receiver, so it's something we should ideally solve generically.

[holger-waschke]

I like this idea, but I'm wondering if we should have a more generic implementation. I'm less familiar with the Jira integration than I should be, honestly. Are there any other fields which we might want to avoid updating on repeat notifications? I'm wondering if we should allow fields to have sub-keys that indicate if we should update them or not.

I also think the discussion about rate-limiting is important enough to move to a dedicated issue. It's really a problem for any receiver, so it's something we should ideally solve generically.

we´re using this Jira API Endpoint to update existing issues
https://developer.atlassian.com/cloud/jira/platform/rest/v2/api-group-issues/#api-rest-api-2-issue-issueidorkey-put

In the request body we send the fields we would like to update. which are the same as creating a new issue so basically everything.
from my experience what should be configurable (whats most relevant to users) is the summary and description. customfields, priority and other stuff is more static in most cases. (for example customfield hostname or environment will not change)

[Spaceman1701]

from my experience what should be configurable (whats most relevant to users) is the summary and description. customfields, priority and other stuff is more static in most cases. (for example customfield hostname or environment will not change)

That makes sense to me... The thing I'm worried about is that we'll end up having multiple disable_update_* keys floating around (which seems a little ugly to me).

What do you think about introducing a new JiraField type in the config that we can either deserialize from string (to maintain backwards compatibility) or from an object with keys that control updating? It's a bit more complexity, but I think it's a bit cleaner in the long run.

This would result in config like:

receivers:
- name: jira_alarm
  jira_configs:
  - project: MON
    issue_type: Bug
    resolve_transition: "Geschlossen"
    reopen_transition: "Erfasst"
    reopen_duration: "1h"
    description:
      disable_update: true

We could then reuse the type for summary or any other field that might end up needing this behavior.

cc @siavashs and @grobinson-grafana since they might have a different opinion.

[holger-waschke]

@Spaceman1701
I implemented your suggestion. The unmarshalling remains backward-compatible, so nothing breaks — both formats are now supported:

# Old/legacy format (just a string)
summary: '{{ template "jira.default.summary" . }}'
description: '{{ template "jira.default.description" . }}'
# New format (object with fields)
summary:
  template: '{{ template "jira.default.summary" . }}'
  disable_update: true
description:
  template: '{{ template "jira.default.description" . }}'
  disable_update: true

The jira_test required a fair amount of refactoring to accommodate this, but it feels justified.

[Spaceman1701]

Thanks for making those changes! I like the implementation, this is looking good to me.

Last thing is to make sure the docs reflect this change.

[Spaceman1701]

why did this change? I see that Description is also any, so I think this is fine. Just answering here in github is enough that someone will be able to learn the answer if they come looking for it in the future 😄

[holger-waschke]

any is an alias for interface{} and we´re using it in combination with the MarshalJSON function

// MarshalJSON merges the struct issueFields and issueFields.CustomField together.
func (i issueFields) MarshalJSON() ([]byte, error) {
	jsonFields := map[string]interface{}{}

	if i.Summary != nil {
		jsonFields["summary"] = i.Summary
	}

	if i.Description != nil {
		jsonFields["description"] = i.Description
	}

to ensure that the field is nil and not an empty string when updating existing issues with the fields not being included. parsing an empty string would just update the field to an empty string so we have to make sure it´s omitted from the JSON completely. I´m not quite sure if a string pointer could be used here as well I tested it and I think it would need quite some reworking.