Skip to content

[ELI-688] - Adding Kinesis Data Stream as a buffer for Firehose#609

Open
TOEL2 wants to merge 21 commits intomainfrom
feature/ELI-688-kinesis-stream-buffer
Open

[ELI-688] - Adding Kinesis Data Stream as a buffer for Firehose#609
TOEL2 wants to merge 21 commits intomainfrom
feature/ELI-688-kinesis-stream-buffer

Conversation

@TOEL2
Copy link
Contributor

@TOEL2 TOEL2 commented Mar 18, 2026

Description

Adds this new service in-between our lambda and Firehose to act as a queue of sorts, which will allow us to get around Firehose service limits.

This does not cover tuning of the new stream as the scope of this is already big enough, we should have that as a follow up ticket. For now I'm suggesting we use the on demand setting as this will scale to whatever we need for the time being.

Context

Due to the size of our audit records, when we scale up to 400rps we hit Firehose service limits. It was decided that we cannot condense the audit records any further without losing key information, so this workaround allows us to keep the full record and scale up to the SLA response requirements.

Type of changes

  • Refactoring (non-breaking change)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would change existing functionality)
  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I am familiar with the contributing guidelines
  • I have followed the code style of the project
  • I have added tests to cover my changes
  • I have updated the documentation accordingly
  • This PR is a result of pair or mob programming

Sensitive Information Declaration

To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.

  • I confirm that neither PII/PID nor sensitive data are included in this PR and the codebase changes.

@TOEL2
Copy link
Contributor Author

TOEL2 commented Mar 18, 2026

Turns out that moto is a little bit shallow on firehose and wont actually pass records from a fake kinesis client to a fake firehose client - which in fairness would be a lot to ask!

One option would be to create some bridging code in the test itself to do that passing of the record.

Thinking about this in a slightly different frame - we are sort of wading into the territory of "not the lambda's responsibility anymore" to ensure that audit records reach the s3 bucket, rather in the new world that is a task for firehose to handle. So from that perspective maybe we shouldn't be directly testing for that in our code and we have have to wait until a deployment to dev perhaps to see that working.

For now I think some bridging code achieves the aim of proving that we are writing to the kinesis stream, while preserving existing tests so we'll go with that, happy to discuss.

@TOEL2 TOEL2 requested a review from Copilot March 18, 2026 19:40

This comment was marked as resolved.

@TOEL2 TOEL2 requested a review from Copilot March 18, 2026 20:44

This comment was marked as resolved.

@TOEL2 TOEL2 marked this pull request as ready for review March 18, 2026 21:11
@TOEL2 TOEL2 closed this Mar 18, 2026
@TOEL2 TOEL2 reopened this Mar 18, 2026
@TOEL2
Copy link
Contributor Author

TOEL2 commented Mar 18, 2026

The dismissed checkov warnings were about being overly permissive with a KMS key where it sees "*" but in relation to the key there is only one of them - apparently this is a known issue

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants