signalfx / signalfx-go

Go client library and instrumentation bindings for SignalFx
https://www.signalfx.com
Apache License 2.0
14 stars 48 forks source link

Notifications [detector] #27

Closed doctornkz closed 5 years ago

doctornkz commented 5 years ago

We have a different meaning of notification field in detector's code and from terraform-provider documentation. Documentation:

notifications - (Optional) List of strings specifying where notifications will be sent when an incident occurs. 

Code:

Notifications []map[string]interface{} `json:"notifications,omitempty"`

So, what do we have List of strings or List of Maps ? List of strings more preferable, cos more easier to process it with cty library.

doctornkz commented 5 years ago

Hmm. I am wrong API gives me

    "notifications" : [ {
      "type" : "Team",
      "team" : "****"
    } ],

It's List of Maps, definitely

cory-signalfx commented 5 years ago

It's definitely a list of maps. The Terraform provider — for reasons that predate me — uses this string format. Soon, in this PR the Terraform provider will use this librfary under the hood instead of it's current map[string]interface{}.

doctornkz commented 5 years ago

Thank you, friend. Right now it seems impossible to create detector with Map objects:

Error: signalfx_detector.sfx-D-*****: rule.0.notifications.2 must be a single value, not a map

I tried that section inside tf config file :

    notifications = [{ team = "****", type = "Team" }, { email = "*******@olx.com", type = "Email" }, { channel = "#alerts_critical", credentialId = "****", type = "Slack" }]
  }
}

8(

cory-signalfx commented 5 years ago

Yes, that's because the string form is used in Terraform! Here are the docs for that.

cory-signalfx commented 5 years ago

You'll want something like this on the Terraform side:

rule {
        …
        notifications = ["Team,***", "Email,*******@olx.com"]
    }

But on the client lib side, the aforementioned list of maps is the way.

doctornkz commented 5 years ago

No problem friends, I gonna implement wrapper around signalFX's notification object. Thank you for your help and time!

cory-signalfx commented 5 years ago

🙇