BrighterCommand / Brighter

A framework for building messaging apps with .NET and C#.
https://www.goparamore.io/
MIT License
2k stars 256 forks source link

[Feature] Support Cloud Events #2130

Open iancooper opened 2 years ago

iancooper commented 2 years ago

Discussed in https://github.com/BrighterCommand/Brighter/discussions/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

iancooper commented 1 year ago

Changed title from: Message.Id and Message.CorrelationId should be a string not a UUID as this is needed for CloudEvents

jeastham1993 commented 6 months ago

@iancooper can I support with this work at all?

iancooper commented 6 months ago

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.

iancooper commented 6 months ago

@jeastham1993 @preardon

For v10 we should move these identifiers to be a string type but default to providing a Guid to support the existing behavior.

We should pick this up in the V10 pre-release window as well though.

jeastham1993 commented 6 months ago

Thanks @iancooper. Just let me know on this thread if I can help in any way :)

iancooper commented 6 months ago

@jeastham1993 I think @preardon will share his thoughts, and the we can figure out what the ask is.

preardon commented 6 months ago

@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 ?

iancooper commented 6 months ago

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

iancooper commented 6 months ago

@jeastham1993 @preardon

This is the requirement I think:

@jeastham1993 are you volunteering to have a go at this?

preardon commented 6 months ago

@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

iancooper commented 6 months ago

@preardon and @jeastham1993

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.

iancooper commented 5 months ago

@preardon and @jeastham1993 I am going to pick this one up