Skip to content
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

Should message contains headers? #10

Open
cescoffier opened this issue Aug 14, 2018 · 6 comments
Open

Should message contains headers? #10

cescoffier opened this issue Aug 14, 2018 · 6 comments

Comments

@cescoffier
Copy link
Contributor

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
Copy link
Member

jroper commented Aug 14, 2018

See #5 for a lot more discussion about this.

@cescoffier
Copy link
Contributor Author

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
Copy link
Member

jroper commented Sep 18, 2018

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
Copy link
Member

jroper commented Sep 18, 2018

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
Copy link
Member

hutchig commented Jan 15, 2019

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 cescoffier added this to the 1.1 milestone Apr 30, 2019
@hutchig hutchig added the context label Oct 9, 2019
@Emily-Jiang Emily-Jiang removed this from the 1.1 milestone Oct 16, 2019
lamtrhieu pushed a commit to lamtrhieu/microprofile-reactive-messaging that referenced this issue Nov 8, 2019
…containers-testcontainers-1.10.1

Bump testcontainers from 1.10.0 to 1.10.1
@Emily-Jiang Emily-Jiang added this to the 1.1 milestone Jul 22, 2020
@cescoffier
Copy link
Contributor Author

This is how it's done in Smallrye: smallrye/smallrye-reactive-messaging#347

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants