aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.62k stars 3.91k forks source link

(lambda): Improve `FilterCriteria` documentation #23214

Open peterwoodworth opened 1 year ago

peterwoodworth commented 1 year ago

Describe the bug

The documentation here leads to a malformed template for EventSource filters

Expected Behavior

I expect the code in the docs to work, and if it doesn't, I expect to have a good enough understanding to work around the limitation in the example.

Current Behavior

The code seen here:

fn.addEventSource(new eventsources.DynamoEventSource(table, {
  startingPosition: lambda.StartingPosition.LATEST,
  filters: [{ eventName: lambda.FilterRule.isEqual('INSERT') }],
}));

will provide this output in CloudFormation

      FilterCriteria:
        Filters:
          - {}

And the example doesn't leave any good hints as to what the right thing to do is

Reproduction Steps

Synthesize the docs

Possible Solution

The correct code is this:

    fn.addEventSource(new DynamoEventSource(table, {
      startingPosition: lambda.StartingPosition.LATEST,
      filters: [FilterCriteria.filter({ eventName: lambda.FilterRule.isEqual('INSERT') })],
    }));

Additional Information/Context

No response

CDK CLI Version

latest

Framework Version

No response

Node.js Version

16

OS

mac

Language

Typescript

Language Version

No response

Other information

We need to explain how to implement different kinds of filters, rather than just providing an example

peterwoodworth commented 1 year ago

@marciocadev this feature you implemented is wonderful, is there any chance you can work on explaining how it works and updating the existing example?

marciocadev commented 1 year ago

Sure, I will update the docs

marciocadev commented 1 year ago

@peterwoodworth I don't know what's going on, in the README.md of the lambda the example is like this

import * as eventsources from '@aws-cdk/aws-lambda-event-sources';
import * as dynamodb from '@aws-cdk/aws-dynamodb';

declare const fn: lambda.Function;
const table = new dynamodb.Table(this, 'Table', {
  partitionKey: {
    name: 'id',
    type: dynamodb.AttributeType.STRING,
  },
  stream: dynamodb.StreamViewType.NEW_IMAGE,
});
fn.addEventSource(new eventsources.DynamoEventSource(table, {
  startingPosition: lambda.StartingPosition.LATEST,
  filters: [lambda.FilterCriteria.filter({ eventName: lambda.FilterRule.isEqual('INSERT') })],
}));

but in the documentation the filters line is like this filters: [{ eventName: lambda.FilterRule.isEqual('INSERT') }],

@TheRealAmazonKendra can you help us to update this doc ?

peterwoodworth commented 1 year ago

@marciocadev thanks for noticing this, it turns out we fixed this example specifically here https://github.com/aws/aws-cdk/pull/23085, we just haven't had a release since so I didn't notice this got fixed. My bad 😅

Regardless, are you still interested in writing up a little bit of documentation on how FilterCriteria should be used in general? I think there's some room for improvement still 🙂

tim-finnigan commented 1 year ago

Since the example got fixed I think we can remove the bug label? And leave this open for improving the FilterCriteria documentation.