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

Cloudwatch SQS Event Target #1786

Closed
dbettin opened this issue Feb 19, 2019 · 12 comments · Fixed by #2683 or MechanicalRock/tech-radar#14 · May be fixed by MechanicalRock/cdk-constructs#5, MechanicalRock/cdk-constructs#6 or MechanicalRock/cdk-constructs#7
Labels
@aws-cdk/aws-events Related to CloudWatch Events @aws-cdk/aws-sqs Related to Amazon Simple Queue Service feature-request A feature should be added or improved.

Comments

@dbettin
Copy link

dbettin commented Feb 19, 2019

Is it possible to have a SQS queue as an event target via a EventRule?

@rix0rrr
Copy link
Contributor

rix0rrr commented Feb 19, 2019

It's not built in to Queue right now, but you can write a new class that implements IEventRuleTarget and put a method like this on it:

https://github.com/awslabs/aws-cdk/blob/master/packages/@aws-cdk/aws-sns/lib/topic-base.ts#L282

(Reference your queue instead).

@rix0rrr rix0rrr added feature-request A feature should be added or improved. @aws-cdk/aws-events Related to CloudWatch Events gap @aws-cdk/aws-sqs Related to Amazon Simple Queue Service labels Feb 19, 2019
@wangchunyang
Copy link

Unfortunately, software.amazon.awscdk.services.events.EventRuleTargetProps doesn't support to set message group id which is required for sending message to FIFO queue. To fix the gap, we can use AWS CloudFormation Library CfnRule and CfnRule.TargetProperty which has an property named CfnRule.SqsParametersProperty.

@Visorgood
Copy link

Very important feature for us! We want to send messages to an SQS as soon as StepFunction execution is complete. Currently we have to have a custom Lambda inside SF to do that.

@made2591
Copy link
Contributor

I would like to work on this if it is ok for everyone: I'm currently into the topic for a personal project, and I think I can make some experiment on this

@eladb
Copy link
Contributor

eladb commented May 29, 2019

That would be great but sync up with @rix0rrr since he plans a bit of a refactor in this area in the coming days.

@made2591
Copy link
Contributor

I think I drafted a solution, the Event Rule was generated correctly with the SQS Target. I didn't write the test yet...

@rix0rrr
Copy link
Contributor

rix0rrr commented May 30, 2019

Sounds great! Feel free to create a draft PR for it, I will have a look.

@rix0rrr
Copy link
Contributor

rix0rrr commented May 30, 2019

I'm thinking the model should be a mix between this: https://github.com/awslabs/aws-cdk/blob/master/packages/%40aws-cdk/aws-events-targets/lib/ecs-ec2-task.ts

And this: https://github.com/awslabs/aws-cdk/blob/master/packages/%40aws-cdk/aws-events-targets/lib/sns.ts

Specifically, I'm thinking the SQS target takes an

  readonly message?: events.RuleTargetInput;

As well as fields that go into SqsParameters: https://docs.aws.amazon.com/AmazonCloudWatchEvents/latest/APIReference/API_SqsParameters.html

(Seems to be only messageGroupId)

@made2591
Copy link
Contributor

made2591 commented May 30, 2019

@rix0rrr thank you, it would really be appreciated! I just wrote the test using SNS as model. I'm building right now and I will run them ^^

For everyone reading about this issue, if your use case is around triggering events from S3 event creation directly to SQS you can use successfully addObjectCreatedNotification in the @aws-cdk/aws-s3

Don't forget to provide to SQS the right policy to let S3 publish correctly: I'm working on a working sample here

@made2591
Copy link
Contributor

@rix0rrr I implemented the SqsQueueProps exactly like this. I didn't focus on parameters yet, just build the skeleton for the binding: I used the grantSendMessages inside. What would you like to get from the ecs-task definition?

@rix0rrr
Copy link
Contributor

rix0rrr commented May 30, 2019

What I wanted to get from that example is: we have multiple props on SqsQueueProps, some of them go into the Input and others go into SqsParameters, and the distinction is invisible to consumers.

If you've already got that, great!

Also, it seems to me it's not possible to get messageGroupId from the event contents, is that right? Or am I misunderstanding something...?

@made2591
Copy link
Contributor

made2591 commented May 30, 2019

Ok, regarding parameters I got your point from 10k feets, but I'm confused 🤔I had a look at https://docs.aws.amazon.com/AmazonCloudWatchEvents/latest/APIReference/API_EcsParameters.html and some of them seem to be not present in https://github.com/awslabs/aws-cdk/blob/master/packages/%40aws-cdk/aws-events-targets/lib/ecs-ec2-task.ts - like the containerOverrides.

Then in the bind, I saw that the input takes from the props, together with networkConfiguration...maybe this is what you were meaning by mixing Input and SqsParameters. Still, by trying to emulate the logic in ECS, I don't understand why some properties like LaunchType are not present.

Also, it seems to me it's not possible to get messageGroupId from the event contents, is that right? Or am I misunderstanding something...?

Regarding this, I still have to investigate. However, I packaged also the tests with just the message - by following almost the SNS topic and I run the tests and they seem to be fine. Do you think I can already make a pull request even if parameters are not present yet?

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