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 Amazon Web Services (AWS) SNS Support #418

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mazurio
Copy link

@mazurio mazurio commented Oct 22, 2016

  • Added as discussed in Send alerts to AWS SNS #331
  • Supports AWS Lambda through SNS
  • Supports SMS through SNS
  • Uses default AWS credentials
  • Supports AWS Regions from SNS_REGION config.
  • Target is an ARN of the SNS Topic.

@mazurio
Copy link
Author

mazurio commented Oct 22, 2016

@scobal let me know what you think and whether this could make the next release. Thanks.

@mazurio mazurio changed the title Add Amazon Web Services (AWS) SNS Support WIP: Add Amazon Web Services (AWS) SNS Support Oct 22, 2016
@mazurio mazurio changed the title WIP: Add Amazon Web Services (AWS) SNS Support Add Amazon Web Services (AWS) SNS Support Oct 22, 2016
@mazurio
Copy link
Author

mazurio commented Oct 22, 2016

Testing:

screen shot 2016-10-22 at 19 48 03

@TyBrown
Copy link

TyBrown commented Oct 22, 2016

Looks great... Have you thought about adding a link back to the Seyren Check in the msg... something like:
String checkUrl = seyrenConfig.getBaseUrl() + "/#/checks/" + check.getId();

@mazurio
Copy link
Author

mazurio commented Oct 22, 2016

@TyBrown I thought about it - we use SNS to send SMS and Seyren is not accessible from the phone unfortunately. Do you think it would be useful here @TyBrown ?

@TyBrown
Copy link

TyBrown commented Oct 22, 2016

@mazurio Given your particular use case, it might be prudent to add a configuration option that would allow the URL to be inserted or left out... As you know, SNS is very flexible, and can be used to forward messages to many different destinations, many of which I can see a high use case for having a link to the check. I can figure out the implementation details if you don't want to work on that part of it, though figured you already have a PR open... up to you. :)

@mazurio
Copy link
Author

mazurio commented Oct 22, 2016

Makes sense :) I will add it tomorrow as it's quite late here already - unless you want to add it first then feel free to do so!

@mazurio
Copy link
Author

mazurio commented Oct 23, 2016

@TyBrown @scobal Pushed checkUrl to SNS. It will now print something like:

Seyren notification 'Test' changed state to 'ERROR' http://localhost:8080/seyren/#/checks/580c93a8124350039a163b32

@TyBrown
Copy link

TyBrown commented Oct 23, 2016

@mazurio LGTM! :)

@alenkacz alenkacz self-assigned this Oct 28, 2016
@Mousius
Copy link

Mousius commented Jul 19, 2017

@alenkacz any update on this? Looks like it was good to merge last year?

@alenkacz alenkacz removed their assignment Oct 25, 2023
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.

4 participants