terraform-provider-signalform icon indicating copy to clipboard operation
terraform-provider-signalform copied to clipboard

Detector fails to link Slack notification silently

Open blkperl opened this issue 7 years ago • 6 comments
trafficstars

The Slack notification is silently failing. The Pagerduty one is linked but the Slack one is not linked when I look at them in the SignalFX UI.

This is the code that I have that creates the detectors.

resource "signalform_detector" "site_capacity" {
    provider = "signalform"
    count = "${length(var.regions)}"
    name = "${var.regions[count.index]} stack capacity"
    max_delay = 30
    description = "Alerts when stack capacity in a region is full"
    program_text = <<-EOF
        [.....]
        detect("Site Capacity @ 95%", when(site_count > site_capacity * 0.95, lasting='5s'))
        detect("Site Capacity @ 90%", when(site_count > site_capacity * 0.90, lasting='5s'))
    EOF
    rule {
        description = "active when site count exceeds 95% site capacity"
        severity = "Critical"
        detect_label = "Site Capacity @ 95%"
        notifications = ["PagerDuty,RedactedId"]
    }
    rule {
        description = "active when site count exceeds 90% site capacity"
        severity = "Major"
        detect_label = "CDE Site Capacity @ 90%"
        notifications = ["Slack,RedactedId,#channelname"]
    }
}

blkperl avatar Jan 30 '18 23:01 blkperl

Hi @blkperl actually we never implemented the slack integration in this provider (L263). I'll make a quick patch.

dichiarafrancesco avatar Jan 30 '18 23:01 dichiarafrancesco

https://github.com/Yelp/terraform-provider-signalform/pull/9 out for review. I'll update you as soon as it gets merged. I tested it locally. If it keeps failing with an error that looks like the following, the problem is related to invalid credentials:

  • signalform_detector. site_capacity.0: For the resource <WHATEVER> SignalFx returned status 400:
Error 400 Bad Request

HTTP ERROR 400

Problem accessing /v2/detector. Reason:

    Bad Request

dichiarafrancesco avatar Jan 30 '18 23:01 dichiarafrancesco

Tested #9 and confirmed that it works!

blkperl avatar Jan 31 '18 00:01 blkperl

This could go in a follow up story but I think it should throw an error if the notification type is not defined.

blkperl avatar Jan 31 '18 00:01 blkperl

Hi it's a great idea but unfortunately ValidateFunc only works for primitive types so it makes kinda hard to do validations on lists. thanks for the feedback and for taking some time to test my change. really appreciated that ;)

dichiarafrancesco avatar Jan 31 '18 00:01 dichiarafrancesco

It might be possible to add the validation check to https://github.com/Yelp/signalform-tools, since we have already a command to perform more advanced validation of terraform resources: https://github.com/Yelp/signalform-tools/blob/master/signalform_tools/validate.py

We run this validation as git pre-commit hook (https://pre-commit.com/) for our internal repo at Yelp.

@blkperl If this could work for you and you want to take a stab at it, feel free to go ahead.

poros avatar Jan 31 '18 16:01 poros