Kinesis Record Observer

[polymorfiq]

Adds an observer for records being added to a Kinesis stream

[snowplowcla]

Thanks for your pull request. Is this your first contribution to a Snowplow open source project? Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://github.com/snowplow/snowplow/wiki/CLA to learn more and sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

[polymorfiq]

I signed it!

[snowplowcla]

Confirmed! @fingerco has signed the Individual Contributor License Agreement. Thanks so much

[alexanderdean]

Thanks @fingerco ! Assigning to @chuwy and @rzats to review

[polymorfiq]

Thanks @rzats for the review! I've made the requested fixes.

Note: I removed the union type for AWS Region, Access Key ID and Secret Access Key. The reason for this is that when a string was entered in those ["null", "string"] fields, the Mediator would fail silently before reaching the observer. Not sure if this a bug or I was using it incorrectly.

BUT if you now leave those parameters blank, the Kinesis Consumer uses the machine's configured AWS settings for Access Key ID, Secret Access Key and Region (through environment variables or otherwise).

[alexanderdean]

Regarding other AWS authentication strategies (which is the only thing I think I was asked on?) - I agree that we should handle this in a concerted way across all AWS components, involving schema updates, not do it in an ad hoc way.

[polymorfiq]

Thanks for your review, @chuwy! I believe I have made all the requested changes.

I am at a loss for how to properly test this module, at the moment. Given its reliance on Kinesis Streams. Perhaps the callback that is passed to Gilt's module could be moved out into a method or into the receive method (Gilt simply passing a message to self instead of parent) and then tested like any other Akka actor.

Let me know if you'd like me to take further action on this! Glad it's looking good!

[chuwy]

Hey @fingerco. That's looks perfect now. If you're interested on how to write tests dedendent on AWS services you can have a look at our existsing test suite, but this is optional so far, you can leave it for us. Thanks for your contribution.

[rzats]

Hello again @fingerco! Did you ever test the observer in a real-world environment (i.e. hooked up to a Kinesis stream and processing events)? I've tried sending some events to a Kinesis stream in a test environment and while these events could subsequently be processed by a raw getRecords() call, they were never recognized by the observer. It looks like this might be a fundamental issue with the consumer library you're using (gfc-aws-kinesis).

[polymorfiq]

Hello @rzats!

I apologize for my late response. Been extremely busy with work these past few weeks!

I did hook up a production AWS Kinesis stream and watch as this observer processed events from the stream accurately.

If you send me the steps you're using so that I can reproduce the issue, I'll take a look and see if I can work out where it's going wrong!

[rzats]

Hey @fingerco! I've taken another look at the test code earlier and it looks like the issue was caused by the test environment - apparently KCL consumer initialization can take a while (~25-30s), but I wasn't factoring the startup time into the tests. Sorry about that! The observer looks good and should be released in v0.2.0.

[rzats]

Included in #80, closing

[alexanderdean]

Don't close these tickets manually - they will auto-close when the milestone is released. Please re-open...

[alexanderdean]

Oh apologies, this is a PR

[alexanderdean]

When a PR is incorporated into a final release PR, it's fine yes to close the child PR.