eclipse / microprofile-reactive-messaging

Apache License 2.0
60 stars 36 forks source link

Should message contains headers? #10

Open cescoffier opened 6 years ago

cescoffier commented 6 years ago

We had a set of discussions where we mentioned headers attached to messages.

Right now, the Message interface does not have headers. However, we give the possibility to use Event which can have headers.

Looking at the current set of message-oriented middleware, most of them (if not all) support headers as a first-class citizen. So, I think we can add headers to the Message class. These headers would be represented as a Map<String, Object>. Whether or not the Object should be serializable, I don't have a strong opinion. It might be a limitation when we want to pass container-related objects (but should we?).

Would that reduce the interest of Event? I don't think so as typically the Events that have been studied so far (Cloud Event) are more about defining the header key and how the payload and header are encoded.

jroper commented 6 years ago

See https://github.com/eclipse/microprofile-reactive-messaging/issues/5 for a lot more discussion about this.

cescoffier commented 6 years ago

Except of the reactive call minute about this topic:

* Clement raised the issue of Header Propagation if we are to introduce headers to Messages—how to deal with anything which isn’t 1:1 propagation? Should we require manual propagation to make it clear that it needs to be done by the developer?
* What does the first version offer in terms of header management? Does the developer “just” clone/copy the header map by default?
* Clement raised the issue of what requirements header values have: what happens if someone put something non-serializable/non-copy/non-cloneable in the header map?
* Viktor mentioned possible issues with custom headers: confidentiality in the case of encrypted message payloads, whether something is safe to duplicate or not, what the lifetime of the individual header messages are etc.
* Emily, Clement, and Viktor discussed: How are duplicate headers handled? HTTP has more of a multimap approach where each header type can have multiple values. Are header names case-insensitive?

To summarise the questions that need to be answered:

  1. propagation: best effort, vs manual.
  2. copy - the header structure should be immutable, but should it allow copy. What about objects that cannot be cloned?
    • should the header structure be a multimap (a la HTTP) or a simple map.

The header structure needs to look like an immutable typed map, keys are case-insensitive.

jroper commented 6 years ago

I don't think we can say that keys are case-insensitive, that will be up to the underlying transport. For example, Kafka header keys are case sensitive. I'm not sure of this, but I think AMQP headers are also case sensitive. HTTP of course is case insensitive. Any attempt to make them case insensitive will introduce bugs and incompatibilities with transports that have case sensitive headers. I think we should make this something that the user has to be aware of with their selected transport, and write their code accordingly.

The same goes for duplicate headers, I think we should allow them, as this ensures compatibility with transports that use and even require duplicate headers (eg, imagine an SMTP transport, it makes heavy use of duplicate headers), but leave it up to the transport to decide how to handle duplicates.

For the structure to model headers, I think we should be unopinionated about this, offer an API that offers methods to get, set, remove and enumerate headers, but don't provide an implementation. Transports will provide their own implementation, and whether that's a map (as AMQP uses) or a list (as Kafka uses) is up to the transport.

For copying headers - I think we should specify that header values be immutable, at least effectively. This solves all the problems, this way there's no need to copy anything, you can just propagate object references directly.

jroper commented 6 years ago

Here's my suggestion re propagation - don't propagate any headers. Most headers should not be propagated, a content type or encoding header for example should not be. Security headers should be explicitly propagated by user code, it is dangerous to assume that because a principal is attached to an incoming message, that an outgoing message will have the same principal attached. The only headers that potentially should be propagated are correlation headers for tracing, and in that case, I think we should leave it up to implementations if they support any trace headers, and how to propagate them.

But what I think we should do is allow messages themselves to be correlated. What if Message had a List<Message>, or at least MessageMetadata has a List<MessageMetadata>, that represents the message(s) that caused this message? Then, messaging providers would be able to propagate whatever makes sense to them. This would also be a nice mechanism for passing acknowledgements, we can require that a messaging provider acknowledge all messages in the cause list.

hutchig commented 5 years ago

I also think metadata would be very useful for other context related things as touched on here: https://groups.google.com/d/msg/microprofile/Em24P1UCcPE/XaH_904lBAAJ

cescoffier commented 4 years ago

This is how it's done in Smallrye: https://github.com/smallrye/smallrye-reactive-messaging/pull/347