-
Notifications
You must be signed in to change notification settings - Fork 259
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
[Feature] Support Cloud Events #2130
Comments
Changed title from: Message.Id and Message.CorrelationId should be a string not a UUID as this is needed for CloudEvents |
@iancooper can I support with this work at all? |
Hi @jeastham1993 I want to check in with @preardon because the telemetry work has given us cloud events headers, but perhaps via a weird route. I can ping him on a back channel too. That way we can establish what the requirements are for this work. |
We should pick this up in the V10 pre-release window as well though. |
Thanks @iancooper. Just let me know on this thread if I can help in any way :) |
@jeastham1993 I think @preardon will share his thoughts, and the we can figure out what the ask is. |
@jeastham1993 I am happy to get this in for V10, If my memory serves me correctly we were also going to look at allows Message ID to not be a Guid @iancooper ? |
i think there are several things here:
We discussed making our message id a string, because in interop other messaging platforms were not necessarily using a UUID; it happens that would mean were more aligned with CE as well |
This is the requirement I think:
@jeastham1993 are you volunteering to have a go at this? |
@iancooper they sounds good to me, I think we should also consider changeing our RequestIds to Strings (I would still use GUIDs to generate them but I feel we support more with them as string), this will flow down to our Message Ids ect. @jeastham1993 if you are volunteering to do this I'm happy to see what time I can make to give you a hand |
Cloud Events does have a C# SDK: https://github.com/cloudevents/sdk-csharp I don't think we want to derive our message from CloudEvent. I'm not necessarily a fan of backing properties with a dictionary of values, but it may help us understand types and header values. More usefully it does show us how to map to individual protocols. I don't think we want to loose our Option type, HeaderResult etc, but it might be useful to see if we their code when we understand what our support needs to offer. |
@preardon and @jeastham1993 I am going to pick this one up |
Discussed in #2067
Originally posted by IlSocio April 7, 2022
Hello everyone...
I just noticed that Brighter is forcing these two fields to be Guid, while when we use directly the transports ASB or Rabbit, it doesn't exists this limitation.
Would you accept a PR to use the more generic "string" type instead of Guid?
In case that the string is not provided, it could still fallback to the default value of Guid.NewGuid().ToString(), but, at least, we have the opportunity to provide any string value for the MessageId and CorrelationId, without any particular restriction.
Actions
For v10 we should move these identifiers to be a string type but default to providing a Guid to support the existing behavior. This may help with some issues around serialization where it can be difficult to determine if a field is a Guid without trying to parse it anyway. If we just treat it as a unique string this becomes easier
The text was updated successfully, but these errors were encountered: