Netflix-Skunkworks / raven-python-lambda

Sentry/Raven SDK Integration For AWS Lambda (python) and Serverless
Apache License 2.0
47 stars 15 forks source link

Moving lambda functionality into raven #7

Closed ashwoods closed 7 years ago

ashwoods commented 7 years ago

I'd be interested in moving this functionality into raven.contrib. Any objections? @kevgliss

kevgliss commented 7 years ago

That is the ultimate goal of this project. What do you need to me to get this moved over?

ashwoods commented 7 years ago

Mostly official permission: IANAL but the licenses are probably not compatible. I'll be porting parts to raven.contrib.lambda. Modify the interface a bit to match the library a bit. All help is welcome :)

kevgliss commented 7 years ago

Cool, let me me know when you have a PR going. I will follow along and help test.

ashwoods commented 7 years ago

I've tried to focus on the API from a user point-of-view and have something simple in core first. I'll be implementing a plugin architecture for raven in the near future, where I would want to tackle the more specific serverless/apex/chalice integrations.

But as a non-lambda user I might be overlooking something, not sure if most people use framework+django/flask more than just "raw" functions.

I'd appreciate any comments: https://github.com/getsentry/raven-python/blob/lambda-integration/raven/contrib/aws_lambda/__init__.py

I'm particularly not sure about the handler(event, context, sentry) magic.

kevgliss commented 7 years ago

So all lambda functions are "raw" functions. Essentially you pass an entrypoint or "handler" in lambda terminology and that function get's invoked.

Regarding def handler(event, context, sentry) what's the rationale around passing the sentry object in?

Although each lambda function only has one entry point, the logic of the function itself could be broken up in to several other functions. If the sentry object could be used directly would be easiest. Otherwise you would have to keep passing the sentry object around.

ashwoods commented 7 years ago

Wanted to avoid end users having to initialize a client explicitly by default. Think I'll avoid magic by defining a get_client utility function.

mikegrima commented 7 years ago

To inject some additional complexity :)

I have recently created a new transport that supports SQS message proxying. I submitted a PR to raven-python here: https://github.com/getsentry/raven-python/pull/1095.

However, per the comments, it was suggested that this be an independent library. I have added the SQS poller to this project via #14 .

Because of the way that lambda works, if this were to be incorporated with raven-python, the SQS transport should be imported as well, as it can alleviate some fundamental issues with using Sentry with AWS.

danbovey commented 7 years ago

Looks like this was merged, so for anybody wishing to use the integration in Raven itself, the docs are here: https://github.com/getsentry/raven-python/blob/master/docs/integrations/lambda.rst

jwkvam commented 6 years ago

It doesn't seem like the awslambda module has feature parity with this, are there plans to do this? It doesn't have memory or timeout monitoring.

kevgliss commented 6 years ago

@ashwoods any ideas? @jwkvam fwiw we are still using (and supporting) this code base until parity is achieved.

ashwoods commented 6 years ago

No plans until after raven 7.0, so it makes sense to keep this package until then. It might make more sense after :)