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

Add links to pagerduty2 alerts #1961

Merged
merged 3 commits into from
Nov 27, 2018

Conversation

onlynone
Copy link
Contributor

@onlynone onlynone commented Jun 11, 2018

Required for all non-trivial PRs
  • Rebased/mergable
  • Tests pass
  • CHANGELOG.md updated
  • Sign CLA (if not already signed)

This feature allows one to do:

alert()
  .pagerDuty2()
    .link('http://some.example.com/here', 'here')
    .link('http://other.example.com/there', 'there')

To add a list of custom links to pagerduty alerts. They'll be put in the links field of the body sent to https://events.pagerduty.com/v2/enqueue as per the docs here: https://v2.developer.pagerduty.com/docs/send-an-event-events-api-v2. Both the URL and text fields are treated as templates so one can interpolate fields/tags/etc. The link text is optional and will default to the link url when missing.

@onlynone onlynone force-pushed the pagerduty2_add_links branch from 9bbb0ad to 70cc7ce Compare June 11, 2018 18:55
@onlynone onlynone force-pushed the pagerduty2_add_links branch from a459505 to 39052bd Compare August 7, 2018 20:58
 * Also change the signature of PagerDuty2Service.Handler to return
   (alert.Handler, error)
@onlynone onlynone force-pushed the pagerduty2_add_links branch from 39052bd to f17c583 Compare August 7, 2018 21:09
@onlynone onlynone changed the title Work in Progress: Pagerduty2 add links Add links to pagerduty2 alerts Aug 7, 2018
@onlynone
Copy link
Contributor Author

onlynone commented Aug 7, 2018

@stevebang @russorat @nathanielc @timhallinflux I'd love some feedback on this feature.

@onlynone
Copy link
Contributor Author

onlynone commented Aug 9, 2018

It looks like all tests passed but the test just exited with a 1 exit code? I grepped through the output and I see 830 tests run, 829 tests passed, and 1 test skipped.

@onlynone
Copy link
Contributor Author

@stevebang @russorat @nathanielc @timhallinflux Can someone take a look at this? Also, is there a way to trigger another test build run?

@onlynone onlynone force-pushed the pagerduty2_add_links branch from 4ff4d46 to ef22caa Compare November 27, 2018 15:33
@onlynone
Copy link
Contributor Author

@stevebang @russorat @nathanielc @timhallinflux I've merged master and made a tweak to the changelog which forced a new build which succeeded this time. Anything else I can do?

@rbetts
Copy link

rbetts commented Nov 27, 2018

Can you clarify how this fits with: #2081. Thanks for the contribution. Help me sort out what (if at all) overlaps and we will review this for K. 1.5.2.

@rbetts rbetts added this to the 1.5.2 milestone Nov 27, 2018
Copy link
Contributor

@nathanielc nathanielc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I have tested locally. I will also put up a PR to add one more test case.

@nathanielc nathanielc merged commit ef22caa into influxdata:master Nov 27, 2018
nathanielc added a commit that referenced this pull request Nov 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants