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

Infer AWS HTTP request payload size #2841

Open
HoneyryderChuck opened this issue Mar 29, 2023 · 6 comments
Open

Infer AWS HTTP request payload size #2841

HoneyryderChuck opened this issue Mar 29, 2023 · 6 comments
Labels
feature-request A feature should be added or improved. guidance Question that needs advice or information.

Comments

@HoneyryderChuck
Copy link

Describe the feature

The APIs for request context are missing a way to calculate the final payload size for a request. Currently, in the case of SNS publish of SQS send message routines, one passes the payload as :message, but the request isn't fully created yet in the build_request routines but rather encoding during the middleware/plugins handlers, before it is sent. So as a user, one can't know whether the payload goes above the threshold set by the AWS services.

Use Case

I'm building an internal extension to store large SNS/SQS payloads in S3, in the manner described here.

While I can implement the routine if the request fails due to the "message too long" error (for which there is no specific error btw, so I have to resort to rescue Aws::SNS::Errors::InvalidParameter and sniffing the error message), it's still a bit inefficient, and I wanted instead to calculate whether the payload will be a problem before sending.

Proposed Solution

Ideally there'd be an API I could use to force the calculation of the body, or some API that does that behind the scenes to calculate it:

req = build_request(:publish, params)

puts req.context.message_size #=> and if above 256k, I could do smth else

### Other Information

_No response_

### Acknowledgements

- [ ] I may be able to implement this feature request
- [ ] This feature might incur a breaking change

### SDK version used

v3

### Environment details (OS name and version, etc.)

environment independent, I'd say.
@HoneyryderChuck HoneyryderChuck added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Mar 29, 2023
@alextwoods
Copy link
Contributor

I'd recommend using a stubbed client to compute the request size. You can do something like:

stubbed_sns = Aws::SNS::Client.new(stub_responses: true)
stubbed_sns = c.publish(params)
body_size = resp.context.http_request.body.size

@alextwoods alextwoods added guidance Question that needs advice or information. and removed needs-triage This issue or PR still needs to be triaged. feature-request A feature should be added or improved. labels Apr 3, 2023
@mullermp
Copy link
Contributor

mullermp commented Apr 4, 2023

Interesting, I did not know about the extended clients for python and java. I would think we would want our own extended client (possibly just as a customization in the gem, similar to S3's Encryption clients), or some sort of build_request routine like S3 presigned_url that runs up and down the plugins (except sending) to compute something (we could compute the size). But I think the Extended client is actually desirable here, so I think we should keep this a feature request. Since you are building this extension, would you be willing to contribute that into the SDK as an extended client?

@mullermp mullermp added the feature-request A feature should be added or improved. label Apr 4, 2023
@HoneyryderChuck
Copy link
Author

@mullermp do you mean, building this "extended payload" functionality into the official AWS SDK? I think that's something that could indeed be done, however a few questions:

  • would this be enabled by default? or is it an opt-in?
  • what about other non-ruby SDKs? one of the strengths of the SDK is that it provides the same functionality across tech stacks. And while I can build this in the ruby SDK, I don't think I can do it for the remainder of the SDKs, i.e. it should be AWS leading that effort.
  • Is AWS comfortable with accepting such a patch, when the current protocol contains a few "javaisms" there? Shouldn't AWS instead built this "extended" protocol into the services, just like they already do with other features such as FIFO? One of the main drawbacks of this "extended payload" workaround is that it doesn't work very well for cases where you have an SNS-to-firehose subscription (something which I'll have to workaround on my side, when this moves ahead). I could see this ability of accepting bigger payloads as one of those "if the message is above 256k AWS will charge you 0.003$ on top of the call" price bumps clients will gladly accept in order not to think about it.

For the reasons above, I'm not sure whether adding this "extended" functionality to the SDK is the best way going forward (at least I think AWS needs to consider a few things here), and I'd be glad with the ability of simply calculating the payload of a request, so I could keep my patch locally, eventually release it as an extra "add-on" library for the AWS SDK, until the day I could discontinue it because AWS figured out how to accept >256kb payloads :)

@mullermp
Copy link
Contributor

mullermp commented Apr 5, 2023

Thanks for your response.

  • It would be opt-in, because it would be a new client class, i.e. Aws::SNS::ExtendedClient.new

  • I think in the case of Java/Python, these libraries were created and owned by the SNS team, and not us, the AWS SDKs team. I definitely am not asking you to contribute to all of the SDKs 😅. Rather if you're writing it in Ruby, we may as well have others benefit from it, by potentially adding it into this SDK.

  • What do you mean by java-isms? I think we can ruby-ify this behavior.

You bring up a good point about "pushing logic to the service" instead of on the client side, and also billing concerns. I'm going to reach out internally to the owners of the Java/Python implementations and see why those decisions were made and figure out with our leadership whether this is something we should be owning or not.

As far as calculation, the stub_responses approach above works. You could also consider registering a plugin to the Client that checks the size before sending, and then does s3 upload and other stuff instead.

@HoneyryderChuck
Copy link
Author

It would be opt-in, because it would be a new client class, i.e. Aws::SNS::ExtendedClient.new

Sounds reasonable.

I think in the case of Java/Python, these libraries were created and owned by the SNS team, and not us, the AWS SDKs team.

I thought the python client was not AWS maintained. Which one do you mean?

What do you mean by java-isms?

I'm talking about the protocol, particularly the first element of the following array:

[
  "software.amazon.payloadoffloading.PayloadS3Pointer",
  {
    "s3BucketName": "extended-client-bucket",
    "s3Key": "xxxx-xxxxx-xxxxx-xxxxxx"
  }
]

ou could also consider registering a plugin to the Client that checks the size before sending, and then does s3 upload and other stuff instead.

Thx, will consider that. TBH I don't know a lot about how the plugin system works, how to set up a new one middway in the chain, and which particular plugin should I put middleware after. But I can certainly try.

@mullermp
Copy link
Contributor

mullermp commented Apr 6, 2023

I thought both tools are in the awslabs org which is owned by AWS.

I would go and say, build what you're building, and when you're done, share it with me, and I can see what we can do with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. guidance Question that needs advice or information.
Projects
None yet
Development

No branches or pull requests

3 participants