-
Notifications
You must be signed in to change notification settings - Fork 132
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
jiralert: Improved docs and config. #32
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Bartek Plotka <[email protected]>
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.
Lots of nitpicking (apologies for that) and a more general comment: I (and I'm sure lots of others) find it really useful to have a sample configuration file as a starting point. If I was a Java-only (or Python-only or whatever) developer I'd find it really inconvenient to parse Golang documentation in order to extract a working example for a basic configuration.
This is even worse regarding the removal of the template file: we are now asking people who may not even be coders to figure out Golang's template language plus Alertmanager's data structures in order to write a bunch of templates, unless they are fine with all their tickets being named "Alert" and having an empty description.
At the very least, the sample configuration and template file contents should be included verbatim into the documentation. (Even though, as you noticed, said documentation can disappear without trace for weeks at a time from GoDoc.)
Summary string `yaml:"summary" json:"summary"` | ||
// ReopenState specifies the state to transition into when reopening a closed issue. | ||
// This state has to exists for the chosen project. |
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 state has to exists for the chosen project. | |
// This state must exist for the chosen project and should ideally be reachable from all other states. |
You can find more docs in [the configuration itself](/pkg/config/config.go) | ||
|
||
Each receiver must have: | ||
* a unique name (matching the Alertmanager receiver name) |
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 unique name (matching the Alertmanager receiver name) | |
* a unique name (matching the Alertmanager receiver name), |
Each receiver must have: | ||
* a unique name (matching the Alertmanager receiver name) | ||
* JIRA API access fields (URL, username and password), | ||
* handful of required issue fields (such as the JIRA project and issue summary), |
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.
* handful of required issue fields (such as the JIRA project and issue summary), | |
* required issue fields (such as the JIRA project and issue summary), |
* a unique name (matching the Alertmanager receiver name) | ||
* JIRA API access fields (URL, username and password), | ||
* handful of required issue fields (such as the JIRA project and issue summary), | ||
* some optional issue fields (e.g. priority) and a `fields` map for other (standard or custom) JIRA fields. |
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.
* some optional issue fields (e.g. priority) and a `fields` map for other (standard or custom) JIRA fields. | |
* optional issue fields (e.g. priority), including a `fields` map for other (standard or custom) JIRA fields. |
type ReceiverConfig struct { | ||
// Name represents unique name for a receiver. | ||
// If Iiralert is used with Alertmanager, name it as Alertmanager receiver that sends alert via webhook to Jiralert for | ||
// desired propagation. |
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.
I'd replace all this with
// Name is a unique receiver name. It must match an Alertmanager receiver name. Required.
Priority string `yaml:"priority" json:"priority"` | ||
// Description specifies Golang template invocation for generating the issue description. |
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.
// Description specifies Golang template invocation for generating the issue description. | |
// Description is a plain string; or Golang template invocation to generate the issue description. |
WontFixResolution string `yaml:"wont_fix_resolution" json:"wont_fix_resolution"` | ||
// Fields specifies standard or custom field values to set on created issue. |
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.
// Fields specifies standard or custom field values to set on created issue. | |
// Fields specifies standard or custom field values to set on created issue. E.g. | |
// * TextField: "Random text" | |
// * SelectList: { "value": "red" } | |
// * MultiSelect: [{"value": "red" }, {"value": "blue" }, {"value": "green" }] |
@@ -123,8 +144,13 @@ func (rc *ReceiverConfig) UnmarshalYAML(unmarshal func(interface{}) error) error | |||
|
|||
// Config is the top-level configuration for JIRAlert's config file. | |||
type Config struct { | |||
// Default specifies default values to be used in place of any ReceiverConfig' empty field. |
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.
// Default specifies default values to be used in place of any ReceiverConfig' empty field. | |
// Default specifies default values to be used in place of any ReceiverConfig empty field. |
} | ||
return &Template{tmpl: tmpl}, nil | ||
|
||
level.Info(logger).Log("msg", "no template was passed.") |
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.
level.Info(logger).Log("msg", "no template was passed.") | |
level.Info(logger).Log("msg", "no template file provided") |
Receivers []*ReceiverConfig `yaml:"receivers,omitempty" json:"receivers,omitempty"` | ||
|
||
// Template specifies an optional file with template definitions. |
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.
I have an issue with defaulting to no template file: with no templating, JIRAlert will always create identical JIRA tickets (except the label, but that's not even intended for human consumption). Why would anyone want that at all? Much less as default behavior?
(I know about the deployment issue someone brought up, but I have a hard time believing there exists a widely used deployment tool that trips up on Golang templates and provides no workaround for that. What if the binary contains Golang templates as strings?)
Let me get back to this, rebase and improve 🤗 |
Changes:
example
not needed (if we can use goDocs)Thoughts @free ? (:
Signed-off-by: Bartek Plotka [email protected]