feat(filemanager): move SQS and DLQ to stateful stack #101

mmalenic · 2024-02-09T03:41:33Z

Closes #86

I think this one I'll merge before #98, so that #98 can implement the createFilemanager() function inside the stateless stack.

Note that the architecture is slightly different to the diagram. The Lambda function depends on an IQueue construct directly, rather than an SQS url, and has the option to send events to the DLQ. ~~So the final diagram could look more like this, where the DLQ receives events from both the SQS queue and the filemanager Lambda function:~~ The diagram is now like the original design.

Changes

Implement the EventSource construct, which contains an EventBridge rule matching S3 buckets that pushes events to an SQS queue.
- Add a DLQ with a CloudWatch alarm that triggers when any events go to the DLQ.
Make the stateless stack depend on the stateful stack, e.g. see https://blog.serverlessadvocate.com/serverless-aws-cdk-pipeline-best-practices-patterns-part-1-ab80962f109d#1913

brainstorm · 2024-02-09T04:12:50Z

config/constants.ts

@@ -98,6 +97,9 @@ export const getEnvironmentConfig = (
            securityGroupProps: {
              ...orcaBusStatefulConfig.securityGroupProps,
            },
+            eventSourceProps: {


IIRC the event sources will eventually have to be dynamic? As in not requiring a de-deploy when we want to monitor and ingest new buckets... I guess that's a future feature, but just keeping tabs open on the topic as I guess that this will eventually have to go to some Rust AWS SDK lambda instead of CDK? ... thinking out loud, no need to address this in this PR.

Hmm, could potentially have env variables that contain an SQS url that the Lambda function uses. Although, wouldn't every new bucket need to somehow be registered with the SQS queue, requiring CDK? Not sure how SQS could discover these on its own, unless there's just a blanket "all buckets" EventBridge rule.

Exactly. @reisingerf, what were your thoughts from previous meetings on this? I'm not sure what are the requirements for this.

The idea is that buckets of interest are configured to send events to the EventBus, to make those discoverable.

Marko's statefull stack will contain (and be expanded with) Event Rules that then funnel events from those buckets of interest to the SQS queue.
The stateless FM stack does not need to change at all for this, as it simply consumes events from the SQS.

Does that make sense?

Ok, so the dev story of adding a new "bucket of interest" is declaring it statically in CDK (on the EventBus construct/stack in this case) or those Event Rules will be lax enough to just process events from a new bucket? Or some other mechanism I'm not understanding? :-S

"adding a new bucket" is simply adding a new Rule to the stateful stack to forward the events to SQS. This should always be possible, even if there is no such bucket (as it's a simple pattern matching on the EventBus side).

How actual events get to the EventBus is a different story and outside the scope of the stack. This would be the responsibility of whoever controls the bucket in question.

Also, could be nice to have the ability to add multiple EventBridge rules to the same SQS queue within this PR. Not sure if this is super useful though because multiple buckets can already be defined for the same rule.

Hm, about that...
I need to brush up on the Rule syntax, but if I understand the code correctly then the rule filters for a known list of buckets and a given prefix, right?

That prefix would have to be the same across all the listed buckets though?
If yes, then I think I'd prefer a rule per bucket / concern... easier to keep track of and maintain independence.

Yes, it's one prefix for all the buckets. I think you're right, it might make more sense to have it defined per bucket.

bin/orcabus.ts

…e rule for all buckets

…ck directly and use ARNs instead

reisingerf · 2024-02-14T00:20:34Z

lib/workload/orcabus-stateless-stack.ts

+  /**
+   * The dead letter queue ARN.
+   */
+  deadLetterQueueArn: string;


Why do you want to pass on the DLQ?

…e this is automatically handled by SQS retries

…-sqs # Conflicts: # config/constants.ts # lib/pipeline/orcabus-stateful-pipeline-stack.ts # test/stateful/eventSourceConstruct.test.ts

mmalenic · 2024-02-15T04:45:25Z

Updates

Queue name is passed in static config so that the stateless and stateful pipeline can be separate.
EventBridges rules are configurable per bucket instead of for all buckets.
Dead letter queue is only applied to the SQS queue within the EventSource construct.

reisingerf · 2024-02-15T05:20:24Z

test/stateful/eventSourceConstruct.test.ts

+});
+
+test('Test EventSource created props', () => {
+  new EventSource(stack, 'TestDatabaseConstruct', {


I guess TestDatabaseConstruct should be updated to TestEventSourceConstruct...
(also for the following tests)

reisingerf

LGTM

mmalenic added 3 commits February 9, 2024 08:42

feat: event source construct

8e41f28

feat(stateful): update cloudwatch alarm in event source

683c1ea

feat: add event source and stateful dependency to stateless stack

f33526d

mmalenic requested review from williamputraintan, victorskl, reisingerf and brainstorm February 9, 2024 03:41

mmalenic self-assigned this Feb 9, 2024

mmalenic added the filemanager an issue relating to the filemanager label Feb 9, 2024

mmalenic added the feature New feature label Feb 9, 2024

brainstorm approved these changes Feb 9, 2024

View reviewed changes

test: add event source unit tests

83c9225

reisingerf requested changes Feb 11, 2024

View reviewed changes

bin/orcabus.ts Outdated Show resolved Hide resolved

mmalenic added 2 commits February 12, 2024 15:31

refactor(stateful): configure event source rules per-bucket rather on…

4c2f7dc

…e rule for all buckets

refactor: avoid making the stateless stack depend on the stateful sta…

941d086

…ck directly and use ARNs instead

mmalenic force-pushed the feat/filemanager-sqs branch from 5f0181e to 941d086 Compare February 12, 2024 05:50

mmalenic requested a review from reisingerf February 12, 2024 21:02

reisingerf reviewed Feb 14, 2024

View reviewed changes

mmalenic added 3 commits February 14, 2024 14:23

refactor: remove deadLetterQueue ARN from event source outputs becaus…

88d1b52

…e this is automatically handled by SQS retries

Merge branch 'main' of github.com:umccr/orcabus into feat/filemanager…

fe3c166

…-sqs # Conflicts: # config/constants.ts # lib/pipeline/orcabus-stateful-pipeline-stack.ts # test/stateful/eventSourceConstruct.test.ts

refactor: use static config for queue name, add event rule permissions

4c19c15

mmalenic requested a review from reisingerf February 15, 2024 04:36

mmalenic mentioned this pull request Feb 15, 2024

add notifications for CloudWatch alarms #104

Open

reisingerf reviewed Feb 15, 2024

View reviewed changes

style(test): fix EventSource test naming

fd9af7c

reisingerf approved these changes Feb 15, 2024

View reviewed changes

mmalenic merged commit a3e74d9 into main Feb 15, 2024
2 checks passed

mmalenic deleted the feat/filemanager-sqs branch February 15, 2024 05:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(filemanager): move SQS and DLQ to stateful stack #101

feat(filemanager): move SQS and DLQ to stateful stack #101

mmalenic commented Feb 9, 2024 •

edited

Loading

brainstorm Feb 9, 2024

mmalenic Feb 12, 2024

brainstorm Feb 12, 2024 •

edited

Loading

reisingerf Feb 12, 2024

brainstorm Feb 12, 2024

reisingerf Feb 12, 2024 •

edited

Loading

mmalenic Feb 12, 2024 •

edited

Loading

reisingerf Feb 12, 2024

mmalenic Feb 12, 2024

reisingerf Feb 14, 2024

mmalenic commented Feb 15, 2024

reisingerf Feb 15, 2024

reisingerf left a comment

feat(filemanager): move SQS and DLQ to stateful stack #101

feat(filemanager): move SQS and DLQ to stateful stack #101

Conversation

mmalenic commented Feb 9, 2024 • edited Loading

Changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brainstorm Feb 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reisingerf Feb 12, 2024 • edited Loading

Choose a reason for hiding this comment

mmalenic Feb 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmalenic commented Feb 15, 2024

Updates

Choose a reason for hiding this comment

reisingerf left a comment

Choose a reason for hiding this comment

mmalenic commented Feb 9, 2024 •

edited

Loading

brainstorm Feb 12, 2024 •

edited

Loading

reisingerf Feb 12, 2024 •

edited

Loading

mmalenic Feb 12, 2024 •

edited

Loading