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

When not wrapping fallback to base64 encoding in case invalid SQS chars are being used #2646

Merged
merged 9 commits into from
Dec 2, 2024

Conversation

danielmarbach
Copy link
Contributor

@danielmarbach danielmarbach commented Nov 28, 2024

This is a follow-up on:

Which also addresses:

Originally, DoNotWrapOutoingMessages was designed to no longer base64 encode and wrap the message with the transport message. The idea was to purely hand over whatever we got to the transport. So in cases of raw dispatches when the message contains characters that are not compliant with the SQS message encoding constraints, the transport would throw.

This has consequences for scenarios like sending metrics when the payload uses raw bytes that may be incompatible with the text restrictions of SQS. By discovering that an encoded payload has characters that are not suitable to be sent and then falling back to base64 encoding, those scenarios are handled.

The transport pump already assumes when it receives non-native messages that the body might be base4 encoded and therefore gracefully falls back to reading those encoded special cases too without any extra code required.

The consequence of the change is that when not wrapping, the payload needs to be inspected for illegal characters which has computational overhead. The current implementation uses the most straightforward approach using a source-gened regex that should be reasonably fast enough. Future optimizations could be done by checking the byte array directly for invalid byte sequences / code points.

@lailabougria lailabougria marked this pull request as ready for review November 28, 2024 14:45
@andreasohlund
Copy link
Member

The regex adds 12ms when used so that all seems fine

image

@andreasohlund andreasohlund merged commit 291c8cc into master Dec 2, 2024
3 checks passed
@andreasohlund andreasohlund deleted the do-not-wrap-invalid-chars branch December 2, 2024 11:19
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.

3 participants