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

Create an adapter for DynamoDB event trigger #152

Open
ceilfors opened this issue Jul 5, 2019 · 8 comments
Open

Create an adapter for DynamoDB event trigger #152

ceilfors opened this issue Jul 5, 2019 · 8 comments

Comments

@ceilfors
Copy link
Collaborator

ceilfors commented Jul 5, 2019

This ticket is created from #111.

Is your feature request related to a problem? Please describe.
An adapter for dynamodb stream trigger, similar to what laconia already have for kinesis: https://laconiajs.io/docs/api/adapter#kinesis-app.

Describe the solution you'd like

const laconia = require("@laconia/core");
const dynamodb = require("@laconia/adapter").dynamodb();

const app = async records => {
  console.log(records); // Prints: [{ Message: 'New item!', Id: 101 }]
};

exports.handler = laconia(dynamodb(app));

// Calls handler with an example Kinesis event
exports.handler({
  Records: [
    {
      dynamodb: {
        Keys: {
          Id: {
            N: "101"
          }
        },
        NewImage: {
          Message: {
            S: "New item!"
          },
          Id: {
            N: "101"
          }
        }
      }
    }
  ]
});

Additional context

  • The most common use case of the trigger is to use NewImage. We can support OldImage in another iteartion
  • Watch out on the conversation of DynamoDB data types. Is there anything that we can reuse from DocumentClient? DocumentClient has an automatic conversion from DynamoDB type to JavaScript type.
  • See example stream here: https://docs.aws.amazon.com/lambda/latest/dg/with-ddb.html
@sakthivel-pro
Copy link

@ceilfors I would like to work on this issue.

@Ankcorn
Copy link
Contributor

Ankcorn commented Jul 3, 2020

Hi @sakthivel-tw and @ceilfors,

I see a really good start has been made with this. Would it be alright if I pick it up?

There are a few nuances to this adaptor as a lot of people have multiple entities in a single table. I think Laconia should aim to support that by providing more context in the records array.

perhaps this is what the record should look like:

[{ item: { new: { pk: '1', sk: 'party',  message: 'New Item!'}, old: undefined }, type: 'party-create' }]

aws-lambda-stream has quite a mature implementation handling plenty of the edge cases so it could be used to inform this.
https://github.com/jgilbert01/aws-lambda-stream/blob/adb758f43f385b84f49f3af80e52850774513e9f/test/unit/from/dynamodb.test.js

A special adapter that supports batching and rate-limiting would be especially cool
That would be very useful at work we use dynamodb db streams to send data to some older tech companies who could do with some serverless in there lives so they don't get overwhelmed by the pressure from the stream 😆

@ceilfors
Copy link
Collaborator Author

ceilfors commented Jul 3, 2020

@all-contributors please add @Ankcorn for ideas

@allcontributors
Copy link
Contributor

@ceilfors

I've put up a pull request to add @Ankcorn! 🎉

@ceilfors
Copy link
Collaborator Author

ceilfors commented Jul 3, 2020

@Ankcorn There is no active pull request at the moment by @sakthivel-tw, so definitely happy for you to pick it up!

That's an interesting thought on supporting multiple entities, I presume you're talking about the Single table design? To be honest it's a pattern that I haven't explored yet as I've been living with Faux SQL at the moment. But based on my understanding, most users will have their table designed differently that it'll be quite impossible for a framework to derive what type of entity that's coming. So I think even for simple table, the conversion from DDB Items to Entities should belong to the user land (an Entity Factory). This means, Laconia may simply need to be able to accept an entity factory type of function. This function will then be called when we're adapting the event.

I don't actually know the best practice on the batching and rate-limiting of ddb streams. Is this something that should be handled on AWS level? I'm thinking that there'll be a complexity thing around handling large DDB stream that the user might hit a timeout (we could recurse I guess, which is what Laconia is already doing, see: https://laconiajs.io/docs/guides/long-running-tasks). Is this a better pattern for example: DDB stream -> lambda -> sqs/kinesis -> lambda -> legacy, to throttle the request?

Something to bear in mind, the API for DynamoDB adapter has sadly has a design flaw; it only supports newImage at the moment. If I were to redesign this, I'd make sure that laconia users know what requests they are handling, and even explicitly force them to be aware that they might need to handle different types of DynamoDB events. Something like (not well thought yet..):

require("@laconia/adapter").dynamodb.insert({}).modify((}).remove({})

@Ankcorn
Copy link
Contributor

Ankcorn commented Jul 4, 2020

You are absolutely right about the fact it would be impossible to serve everyone.

My suggestion would have been to check type field -> SK -> table name. But I agree with your suggestion of a factory function would be much better.

Because this is a polling handler like kinesis hitting the timeout is not a concern except for some very niche use cases. You just configure the batch size and the paralysation factor to the current values for your table throughout and workload. If you have a thing that takes 15 mins you can just set the batch size to 1 and then configure the paralysation so the stream doesn't lag behind too much.

My suggestions of ways the rate limit and batch in the stream are totally a case of excitement driven over-engineering and should be ignored.

How would you recommend to handle the different event formats?

My opinion is one of the things that I really like about Laconia is that it just handles things where AWS gets the API wrong in a lovely way.

sns(event)
// vs
JSON.parse(event.Records[0].Sns.Message)

Just saves so many brain cells and reduces the chance of bugs. I once shipped a bug where i typed SNS 😓

My personal preference would be to do this

dynamo(event) -> { type, new, old }

type would be a string with the value insert/update/delete

For insert old would be undefined and for delete new would be undefined.

Obviously this has the drawback of being a breaking change. How much should this impact the decision?

@Ankcorn
Copy link
Contributor

Ankcorn commented Jul 6, 2020

Although to disagree with my previous comment all the code we have to handle db stream events starts with

if(insert) {
// do the insert thing
} else if (modify) {
...
} else {
...}

so maybe the framework abstracting that would be nice but I am not sure if it would just add more complexity

@hugosenari
Copy link
Contributor

hugosenari commented Jul 6, 2020

Hello!! (me)

Wondering if ADT could help here... 🤔 in the end would what @ceilfors proposed with fancy name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants