Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

SQS Transport Capability #1095

Closed
wants to merge 1 commit into from
Closed

SQS Transport Capability #1095

wants to merge 1 commit into from

Conversation

mikegrima
Copy link

Hello:

This PR adds a SQSTransport class to the Transports.

What does this do?
Instead of forwarding messages directly to Sentry, it forwards them over to an SQS queue. Then, an SQS poller pops off the messages before sending them onto Sentry.

I just created some simple code on GitHub here that does this: https://github.com/Netflix-Skunkworks/raven-sqs-proxy

Why is this useful?
The primary use case of this is with AWS Lambda functions. For lambda functions to write to an AWS Sentry instance, it would need to reside within a VPC. Running lambdas inside of a VPC requires that the lambda be assigned an ENI.

There are many use-cases where running a lambda within a VPC is not feasible, such as:

  1. An AWS account doesn't have a useful VPC (special purpose accounts)
  2. An AWS account doesn't have a VPC that is peered to a VPC where Sentry is running
  3. Cross-region use cases where Sentry lives in an internal VPC without external connectivity
  4. ENI Exhaustion Concerns: It is possible to exhaust the ENIs within a VPC if you have many, many lambdas running. This can break new deployments within a VPC.

For any of the above use cases, SQS alleviates the drawbacks. In this case, the lambda is still able to take advantage of Sentry by posting to an SQS queue. A VPC ENI need not be required. Security to the SQS queue is handled via IAM Roles.

Potential Concern
This PR simply passes the headers with the DSN credentials onto SQS. If this is a concern, I can add a KMS option to encrypt credentials before passing onto SQS.

Please let me know if there are additional changes that can be made or if you have any questions about my approach.

Thank You

@dcramer
Copy link
Member

dcramer commented Sep 29, 2017

I'd personally rather this be a separate library that just depended on the raven package. It's a pattern we're taking in a lot of situations to avoid too tightly coupling things into the core lib that aren't heavily used.

Aside, we intend to solve the 'async' issue natively at some point with a uniform binary you can install on a system.

@dcramer
Copy link
Member

dcramer commented Sep 29, 2017

(also if this was talked about elsewhere, ignore me, i just happened to be paying attention to my inbox tonight)

@mikegrima
Copy link
Author

mikegrima commented Sep 29, 2017

I went ahead and moved this plugin over to our raven-python-lambda project here in this PR: Netflix-Skunkworks/raven-python-lambda#14

There was an initial discussion taking place here: Netflix-Skunkworks/raven-python-lambda#7 with regards to raven incorporating much of that project's logic.

Not sure what additional complexity is added by supporting SQS proxying, but if raven were to take in Lambda support, it should also take in SQS support for the reasons mentioned above.

@dcramer
Copy link
Member

dcramer commented Sep 29, 2017

@mikegrima its more around us having to maintain it. lambda I think makes a bit more sense given the requirement of it, but if e.g. sqs is required for lambda, thats a different story. Just my two cents, as I'm not actively involved in these projects

@mikegrima
Copy link
Author

Closing this as we have this implemented in our raven-python-lambda project.

@mikegrima mikegrima closed this Jul 12, 2018
@mikegrima mikegrima deleted the sqsTransport branch July 12, 2018 16:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants