-
Notifications
You must be signed in to change notification settings - Fork 235
Add SQS protocol #1177
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
base: main
Are you sure you want to change the base?
Add SQS protocol #1177
Conversation
Signed-off-by: Yann Vigara <[email protected]>
Signed-off-by: Yann Vigara <[email protected]>
Hi @yvigara Disclosure: I work for AWS (EventBridge). Thank you for your contribution to the CloudEvents project! I noticed the EventBridge PR as well and was wondering if this is something you already discussed with the CloudEvents community e.g., in a community call? Can you elaborate more on the use case and need here? Since there's no official bindings for SQS/EventBridge for CloudEvents, I'm not sure we can incorporate this PR into the Go SDK yet. IMHO these vendor binding specs are usually owned (hosted) by the vendor, such as in the Google PubSub example. @duglin to share his perspective. |
I agree that proprietary specs are usually owned by the respective companies, and then we just point to them from here: https://github.com/cloudevents/spec/blob/main/cloudevents/proprietary-specs.md . But this isn't a spec, it's an implementation. Similar to what we have for Google pubsub ( https://github.com/cloudevents/sdk-go/tree/main/protocol/pubsub/v2 ). Isn't it? |
Correct, it's an implementation - of (a) non-existing spec(s). Related: #1178 |
Do we need a spec to use an existing protocol if the location of the CE attributes is obvious? I'm assuming https://github.com/cloudevents/sdk-go/pull/1177/files#diff-266fac21f6644a3685bcc063e9a25e3fa6fb3ec5a7cff0564edbc9f54c6d2d73R111-R125 is the spot of interest. Not being familiar with SQS, is there another way it would could been implemented? If a real spec is needed because there are "choices" that need to be stated to ensure interop when doing the mapping, then perhaps we could write one up and offer it to AWS to host it? If only we knew someone who worked there.... :-) We also have our "adapters" docs if something less formal than a spec is all that's needed. And at least some of those are proprietary mappings. |
@yvigara would you be willing to write down all of the options (and decisions) that you needed to consider when creating these two PRs? For example, things like:
This might help us decide if we need a "spec" vs an "adapter" vs "it's obvious so we can live with just code". I'm not looking for a 'spec', just a simple list of items that you thought about that might turn into a spec/adapter later on, but not yet. |
As a bit of background, I wrote these two protocol implementations for a project I've been working on. I couldn't find any implementation for SQS and EventBridge. In this project, we create CloudEvents and send them to an EventBridge, and they are "forwarded" to an SQS queue. I followed the AWS documentation for integrating CloudEvents with EventBridge. The EventBridge protocol only implements The SQS protocol supports sending and receiving CloudEvents in Binary and Structured mode and defaults to Structured mode with a JSON format if we don't find a version or format in the message attribute: https://github.com/cloudevents/sdk-go/pull/1177/files#diff-266fac21f6644a3685bcc063e9a25e3fa6fb3ec5a7cff0564edbc9f54c6d2d73R59 (this is to support EventBridge to SQS forwarding rules. Unless there's a way to set an attribute in an EventBridge Rule, but that's an AWS-specific question). I reused most of the code from the PubSub protocol, and it follows the same conventions. I would expect the behaviour to be the same as PubSub. To answer your questions:
In binary mode the attributes would be set in the
They would all be prefixed with
Structured mode expects to find the format of the event in
I'm not an expert on SQS, but from what I gathered from this documentation https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/quotas-messages.html:
There's an area of improvement: I think the Traceparent extension could be set/extracted from a system attribute. |
@yvigara sorry for the delay. Since there appears to be some "decisions" made in your implementation where someone might have made different choices it might be best to formalize it into a "spec". Now, there's another issue - where would this spec live? See: https://github.com/cloudevents/spec/blob/main/cloudevents/proprietary-specs.md - we normally don't put specs for proprietary (non-oss) protocols in the CE repo, but we will point to them as a convenience. I believe that @embano1 might have some thoughts here, at least for the AWS one. |
Hi and sorry for my delay, distracted with work. The reason a SPEC might be useful is that adapters might not help here as they're relevant in consumers/transports which can actively reshape data structures to CE. SQS doesn't qualify here, but I think a Lambda adapter might be a good candidate since it's often used with SQS. EventBridge also doesn't "understand" CloudEvents today, so users currently have to use custom input transformers to reshape JSON to CloudEvents, but those are limited when it comes to logic required to handle conditional formatting and error cases. Or @duglin are you saying that adapters can also be used to tell producers how to convert CloudEvents into the transport format (i.e., the reverse of the examples used in the adapters today which go from custom-to-CE)? I think the right approach here for SQS and EventBridge is to create SPECs and host them at AWS (similar to Google Pub/Sub). Especially the SPEC for EventBridge would be pretty lightweight currently (only structured mode support, mappings between EventBridge and CloudEvents fields, etc.) but allows us to evolve over time in case those services add native support for CloudEvents. As you call out, SQS has all the properties to model binary/structured encodings and so a SPEC makes sense to also document the limits. I can try to talk to the SQS and Lambda team and see if they'd be interested in supporting here (especially with repositories, reviews and maintenance). @duglin do we have a document which describes how SPECs can evolve e.g., a SPEC for a transport with limited/no support CE today? |
Resolves #377
Implement SQS protocol and provides examples.