Support adding custom fields to VictorOps notifications (#1420)

* Support adding custom fields to VictorOps notifications

* Response to feedback

* Added logic to validate victorops custom fields to config load time

* Cleanup victorops notifier of logic duplicated in config check

* rebase and further cleanup from feedback

* another grammer fix

Signed-off-by: Jason Roberts <jroberts@drud.com>

config/notifiers.go

@@ -494,6 +494,7 @@ type VictorOpsConfig struct {
 	StateMessage      string            `yaml:"state_message" json:"state_message"`
 	EntityDisplayName string            `yaml:"entity_display_name" json:"entity_display_name"`
 	MonitoringTool    string            `yaml:"monitoring_tool" json:"monitoring_tool"`
+	CustomFields      map[string]string `yaml:"custom_fields,omitempty" json:"custom_fields,omitempty"`
 }
 
 // UnmarshalYAML implements the yaml.Unmarshaler interface.
@@ -506,6 +507,15 @@ func (c *VictorOpsConfig) UnmarshalYAML(unmarshal func(interface{}) error) error
 	if c.RoutingKey == "" {
 		return fmt.Errorf("missing Routing key in VictorOps config")
 	}
+
+	reservedFields := []string{"routing_key", "message_type", "state_message", "entity_display_name", "monitoring_tool", "entity_id", "entity_state"}
+
+	for _, v := range reservedFields {
+		if _, ok := c.CustomFields[v]; ok {
+			return fmt.Errorf("VictorOps config contains custom field %s which cannot be used as it conflicts with the fixed/static fields", v)
+		}
+	}
+
 	return nil
 }
 

config/notifiers_test.go

@@ -287,6 +287,49 @@ routing_key: ''
 	}
 }
 
+func TestVictorOpsCustomFieldsValidation(t *testing.T) {
+	in := `
+routing_key: 'test'
+custom_fields:
+  entity_state: 'state_message'
+`
+	var cfg VictorOpsConfig
+	err := yaml.UnmarshalStrict([]byte(in), &cfg)
+
+	expected := "VictorOps config contains custom field entity_state which cannot be used as it conflicts with the fixed/static fields"
+
+	if err == nil {
+		t.Fatalf("no error returned, expected:\n%v", expected)
+	}
+	if err.Error() != expected {
+		t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error())
+	}
+
+	in = `
+routing_key: 'test'
+custom_fields:
+  my_special_field: 'special_label'
+`
+
+	err = yaml.UnmarshalStrict([]byte(in), &cfg)
+
+	expected = "special_label"
+
+	if err != nil {
+		t.Fatalf("Unexpected error returned, got:\n%v", err.Error())
+	}
+
+	val, ok := cfg.CustomFields["my_special_field"]
+
+	if !ok {
+		t.Fatalf("Expected Custom Field to have value %v set, field is empty", expected)
+	}
+	if val != expected {
+		t.Errorf("\nexpected custom field my_special_field value:\n%v\ngot:\n%v", expected, val)
+	}
+
+}
+
 func TestPushoverUserKeyIsPresent(t *testing.T) {
 	in := `
 user_key: ''

notify/impl.go

@@ -1285,16 +1285,39 @@ const (
 	victorOpsEventResolve = "RECOVERY"
 )
 
-type victorOpsMessage struct {
-	MessageType       string `json:"message_type"`
-	EntityID          string `json:"entity_id"`
-	EntityDisplayName string `json:"entity_display_name"`
-	StateMessage      string `json:"state_message"`
-	MonitoringTool    string `json:"monitoring_tool"`
-}
-
 // Notify implements the Notifier interface.
 func (n *VictorOps) Notify(ctx context.Context, as ...*types.Alert) (bool, error) {
+
+	var err error
+	var (
+		data   = n.tmpl.Data(receiverName(ctx, n.logger), groupLabels(ctx, n.logger), as...)
+		tmpl   = tmplText(n.tmpl, data, &err)
+		apiURL = n.conf.APIURL.Copy()
+	)
+	apiURL.Path += fmt.Sprintf("%s/%s", n.conf.APIKey, tmpl(n.conf.RoutingKey))
+
+	c, err := commoncfg.NewClientFromConfig(*n.conf.HTTPConfig, "victorops")
+	if err != nil {
+		return false, err
+	}
+
+	buf, err := n.createVictorOpsPayload(ctx, as...)
+	if err != nil {
+		return true, err
+	}
+
+	resp, err := post(ctx, c, apiURL.String(), contentTypeJSON, buf)
+	if err != nil {
+		return true, err
+	}
+
+	defer resp.Body.Close()
+
+	return n.retry(resp.StatusCode)
+}
+
+// Create the JSON payload to be sent to the VictorOps API.
+func (n *VictorOps) createVictorOpsPayload(ctx context.Context, as ...*types.Alert) (*bytes.Buffer, error) {
 	victorOpsAllowedEvents := map[string]bool{
 		"INFO":     true,
 		"WARNING":  true,
@@ -1303,7 +1326,7 @@ func (n *VictorOps) Notify(ctx context.Context, as ...*types.Alert) (bool, error
 
 	key, ok := GroupKey(ctx)
 	if !ok {
-		return false, fmt.Errorf("group key missing")
+		return nil, fmt.Errorf("group key missing")
 	}
 
 	var err error
@@ -1311,11 +1334,10 @@ func (n *VictorOps) Notify(ctx context.Context, as ...*types.Alert) (bool, error
 		alerts = types.Alerts(as...)
 		data   = n.tmpl.Data(receiverName(ctx, n.logger), groupLabels(ctx, n.logger), as...)
 		tmpl   = tmplText(n.tmpl, data, &err)
-		apiURL       = n.conf.APIURL.Copy()
+
 		messageType  = tmpl(n.conf.MessageType)
 		stateMessage = tmpl(n.conf.StateMessage)
 	)
-	apiURL.Path += fmt.Sprintf("%s/%s", n.conf.APIKey, tmpl(n.conf.RoutingKey))
 
 	if alerts.Status() == model.AlertFiring && !victorOpsAllowedEvents[messageType] {
 		messageType = victorOpsEventTrigger
@@ -1330,36 +1352,31 @@ func (n *VictorOps) Notify(ctx context.Context, as ...*types.Alert) (bool, error
 		level.Debug(n.logger).Log("msg", "Truncated stateMessage due to VictorOps stateMessage limit", "truncated_state_message", stateMessage, "incident", key)
 	}
 
-	msg := &victorOpsMessage{
-		MessageType:       messageType,
-		EntityID:          hashKey(key),
-		EntityDisplayName: tmpl(n.conf.EntityDisplayName),
-		StateMessage:      stateMessage,
-		MonitoringTool:    tmpl(n.conf.MonitoringTool),
+	msg := map[string]string{
+		"message_type":        messageType,
+		"entity_id":           hashKey(key),
+		"entity_display_name": tmpl(n.conf.EntityDisplayName),
+		"state_message":       stateMessage,
+		"monitoring_tool":     tmpl(n.conf.MonitoringTool),
 	}
 
 	if err != nil {
-		return false, fmt.Errorf("templating error: %s", err)
+		return nil, fmt.Errorf("templating error: %s", err)
+	}
+
+	// Add custom fields to the payload.
+	for k, v := range n.conf.CustomFields {
+		msg[k] = tmpl(v)
+		if err != nil {
+			return nil, fmt.Errorf("templating error: %s", err)
+		}
 	}
 
 	var buf bytes.Buffer
 	if err := json.NewEncoder(&buf).Encode(msg); err != nil {
-		return false, err
+		return nil, err
 	}
-
-	c, err := commoncfg.NewClientFromConfig(*n.conf.HTTPConfig, "victorops")
-	if err != nil {
-		return false, err
-	}
-
-	resp, err := post(ctx, c, apiURL.String(), contentTypeJSON, &buf)
-	if err != nil {
-		return true, err
-	}
-
-	defer resp.Body.Close()
-
-	return n.retry(resp.StatusCode)
+	return &buf, nil
 }
 
 func (n *VictorOps) retry(statusCode int) (bool, error) {

notify/impl_test.go

@@ -15,6 +15,7 @@ package notify
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -322,3 +323,51 @@ func TestEmailConfigMissingAuthParam(t *testing.T) {
 	require.Error(t, err)
 	require.Equal(t, err.Error(), "missing password for PLAIN auth mechanism; missing password for LOGIN auth mechanism")
 }
+
+func TestVictorOpsCustomFields(t *testing.T) {
+	logger := log.NewNopLogger()
+	tmpl := createTmpl(t)
+
+	url, err := url.Parse("http://nowhere.com")
+
+	require.NoError(t, err, "unexpected error parsing mock url")
+
+	conf := &config.VictorOpsConfig{
+		APIKey:            `12345`,
+		APIURL:            &config.URL{url},
+		EntityDisplayName: `{{ .CommonLabels.Message }}`,
+		StateMessage:      `{{ .CommonLabels.Message }}`,
+		RoutingKey:        `test`,
+		MessageType:       ``,
+		MonitoringTool:    `AM`,
+		CustomFields: map[string]string{
+			"Field_A": "{{ .CommonLabels.Message }}",
+		},
+	}
+
+	notifier := NewVictorOps(conf, tmpl, logger)
+
+	ctx := context.Background()
+	ctx = WithGroupKey(ctx, "1")
+
+	alert := &types.Alert{
+		Alert: model.Alert{
+			Labels: model.LabelSet{
+				"Message": "message",
+			},
+			StartsAt: time.Now(),
+			EndsAt:   time.Now().Add(time.Hour),
+		},
+	}
+
+	msg, err := notifier.createVictorOpsPayload(ctx, alert)
+	require.NoError(t, err)
+
+	var m map[string]string
+	err = json.Unmarshal(msg.Bytes(), &m)
+
+	require.NoError(t, err)
+
+	// Verify that a custom field was added to the payload and templatized.
+	require.Equal(t, "message", m["Field_A"])
+}