Skip to content
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

Multiple services for PagerDuty alert #82

Closed
oldmantaiter opened this issue Dec 7, 2015 · 6 comments
Closed

Multiple services for PagerDuty alert #82

oldmantaiter opened this issue Dec 7, 2015 · 6 comments

Comments

@oldmantaiter
Copy link

Some users might want alerts to take different escalation paths (eg in Pagerduty to multiple rotations), so adding support to reference "named" alert handlers of the same type would be useful.

I was thinking I could attempt to tackle this when I get some time, or at least open a discussion on the best way to accomplish this (while staying in the style guidelines of the project).

Very cool project by the way, really like the DSL and how it is structured.

EDIT: Not sure if this already exists, haven't had a whole bunch of time to go through the code. So if it does, please ignore this request, I was basing this off configuration examples.

@nathanielc
Copy link
Contributor

Do you want to provide some examples of how the DSL would look for 'named' alert handlers? I like the idea, if we can decide on a DSL structure we like I can point you in the right direction for implementing a solution. Thanks

@oldmantaiter
Copy link
Author

@nathanielc It would be along the lines of how the slack channel is configured. So it looks to be something that is just pagerduty specific at this point.

Based on VictorOps and how that is specified, it could either be done that way (adding another function to be chained in the DSL) or in the config.

As for examples:

Main config

[pagerduty.<name>] # Defaults to 'default'
  enabled = true
  service-key = "xxxxxxx"

DSL

.alert()
  .pagerDuty("<name>")

This would allow a bit of readability when configuring which schedule to escalate to. As I said above, we could also just have a chained function like routingKey for VictorOps and pass in the service key for Pagerduty in there, but that is less readable due to the random nature of the service key.

Let me know your thoughts. I have no problem with either way, but potentially adding the ability for "named" outputs that could be referenced (not only for Pagerduty) might be of use?

@nathanielc
Copy link
Contributor

I like the design. The configuration will probably change to something like:

[pagerduty]
  enabled = true
  [pagerduty.schedules]
    schedule_name = "service_key"
    another_schedule  = "different_service_key"

Which would match with a config structure like:

type Config struct {
    // Whether PagerDuty integration is enabled.
    Enabled bool `toml:"enabled"`
    // The PagerDuty API URL, should not need to be changed.
    URL string `toml:"url"`
    // Whether every alert should automatically go to PagerDuty
    Global bool `toml:"global"`
    // Schedules name -> service key
    Schedules map[string]string
}

Is schedule the right name? I am not very familiar with PagerDuty so I could be using the wrong name here.

This change for PagerDuty should be pretty easy, just need to update the both pipeline/alert.go and 'alert.go and then the service in services/pagerduty.

@oldmantaiter
Copy link
Author

Sounds good. For PagerDuty, it would be called a service, which gets mapped to an escalation policy, which can be mapped to multiple schedules.

I'll start taking a look soon into adding this. Still getting familiar with the project so want to make sure I can adequately test.

@oldmantaiter oldmantaiter changed the title Multiple alert handlers of same type Multiple services in PagerDuty Alert Dec 8, 2015
@oldmantaiter oldmantaiter changed the title Multiple services in PagerDuty Alert Multiple services for PagerDuty alert Dec 8, 2015
@mattfinlayson
Copy link
Contributor

This is working fine for us in production, let me know if if needs any more love.

@oldmantaiter
Copy link
Author

@savagegus I have not had the time to eval Kapacitor fully yet, so if it works for you I'll take your word for it and close out this issue. Thanks! This will make it easier when we get around to using it.

nathanielc added a commit that referenced this issue Jun 9, 2016
…47a777

bbd5bb6 Make struct decoding also handle empty Primitives
66416ff Decode empty Primitives into nullable values successfully
5b80cc5 Clean up slice decoding handling
75869ce Unify two switch arms
1946733 Properly encode struct fields having toml tags without a name
0e5f512 Don't treat non-empty strings of whitespace as empty for omitempty
e27e134 add bool empty option
a4eecd4 Remove extra lexer advance
0c4ce10 In Decode, reuse slices when possible
dacf173 Merge remote-tracking branch 'yourkarma/master'
166915e Merge pull request #82 from mjibson/fix-decode-omit
3e3bd42 Don't panic when failing to parse a timestamp
001f7af Fix no-op utf-8 validity test
2678c1e Add tests for ignored fields
2fe0945 Flesh out anonymous field encoding
4cc516a Merge remote-tracking branch 'shawnps/gofmt'
f772cd8 Merge pull request #112 from stapelberg/inaccessible-go1.6
77ccfcd Bugfix: update check for inaccessible fields for Go 1.6
312db06 Merge pull request #93 from bep/parse-panic
782628a gofmt -s
5c4df71 Merge pull request #108 from kezhuw/fix_endless_loop
c3bcd45 Fix endless loop in table name lexing
851e5be Panic instead of os.Exit for illegal state situations in parser
110f954 Make new destination slice when length doesn't match.
54c24c1 Use correct name during decode with omit options
056c9bc Merge pull request #81 from bbuck/omitempty
aa708eb Clean up, remove zero as 'empty' and add 'omitzero' option
d918309 Support for omitempty, as well as tests for omitempty.
443a628 Merge pull request #72 from binary132/fix-readme
9baf8a8 Updated link for TOML v0.2.0
f706d00 Support quoted keys.
7eda3e2 Remove escape for '/'.
0f9db13 Forbid '#' in table names.
a6db6cf Simplify lexer for Unicode escapes and add support for `\U`.
3644d30 Fix typo. Thanks @ChrisHines
32ee81d Various formatting fixes. 80 cols.
0eaa740 Fix #66.
3883ac1 Merge pull request #59 from fromonesrc/patch-1
237e946 Merge pull request #57 from gisakulabs/UnmarshalTOML
b2c5eb4 Merge pull request #61 from halostatue/multiline
1956abe Implement multiline strings and raw multiline strings.
73199af Support single-line raw strings.
71fac5b Fixed comment typo
ac8879e Fix readme typo on Decode method
67ade19 Modified the `Unmarshaler` interface to `.UnmarshalTOML(v interface{})`
bc95534 Added support for UnmarshalTOML() interface.

git-subtree-dir: vendor/github.com/BurntSushi/toml
git-subtree-split: 747a77770ca4730759d5944e3a7fe869d452648b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants