[Feature] Support Cloud Events
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
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.
@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.
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:
- For any given transport, support binary cloud events by including the values in the headers; however I think we are writing these anyway via Telemetry, so the question is how we want to rationalize between the telemetry code populating these and just making sure that all our transports write and read CE headers.
- Changing our headers so that we can capture the information that cloud events needs.
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
@jeastham1993 @preardon
This is the requirement I think:
- Design: What Brighter headers map to which Cloud Events headers; do any need to change type?
- Design: Which Cloud Events headers are missing from Brighter headers; we need to add these.
- Implementation: For each transport, we should use binary mode and when we map headers to the native header format, use the appropriate cloud events name for the header; when we map back we should look for cloud event headers.
- Implementation: When we map back, we should look for our old header name if the Cloud Events name is not there; this will allow consumers to consume messages produced under the old format for a while. We can drop this eventually.
- UpdateTelemetry no longer needs to use the bag to update the headers (@preardon check?).
@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
@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.
@preardon and @jeastham1993 I am going to pick this one up