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

Enable CorrelationMetadata Emission on SenderMessageResults, Similar to Reactor-Kafka #109

Closed
Sage-Pierce opened this issue Sep 24, 2019 · 3 comments
Labels
effort-low type/enhancement A general enhancement
Milestone

Comments

@Sage-Pierce
Copy link

Sage-Pierce commented Sep 24, 2019

The similar-minded project Reactor Kafka allows propagation of "Correlation Metadata" with sent Records: https://github.com/reactor/reactor-kafka/blob/master/src/main/java/reactor/kafka/sender/internals/DefaultKafkaSender.java#L116

It would be great to get this functionality implemented on Reactor RabbitMQ's Sender and OutboundMessage here:
https://github.com/reactor/reactor-rabbitmq/blob/master/src/main/java/reactor/rabbitmq/Sender.java#L187

Motivation

It is sometimes desired to be able to propagate information that is not strictly a subset of the information serialized on RabbitMQ messages. This information may be logical in nature, for example, a callback. There is not yet a place on OutboundMessage to propagate such information.

Reactor Kafka provides this ability as "correlation metadata" on sent SenderRecords. It should be fairly straightforward to add this functionality to OutboundMessage

Note that there is a TODO as a placeholder for this functionality:
https://github.com/reactor/reactor-rabbitmq/blob/master/src/main/java/reactor/rabbitmq/OutboundMessage.java#L36-L37

Desired solution

OutboundMessage will have generically parameterized correlationMetadata associated with each Message. The resulting send methods on Sender will be parameterized with this type information

Considered alternatives

N/A

Additional context

We have a project that propagates callbacks to originating message emitters in order to notify "message processing has been completed or failed" (in this case, with an ack or nack). We don't have this ability for usage with RabbitMQ in the absence of correlationMetadata on OutboundMessage

@acogoluegnes
Copy link
Contributor

@Sage-Pierce Thanks for the suggestion and the PR.

This is an interesting feature to have, I'd like to integrate it as smoothly as possible into the library.

  • ideally, it should be optional. Application-specific correlation metadata is useful, but not in every case. Having to declare Flux<OutboundMessage<Void>> when there is no need of correlation metadata seems a bit cumbersome to me.
  • the implementation should be backward-compatible if we want to see it released anytime soon. There's no real plan for 2.0 now, where all breaking changes will go. A possible solution is to add the feature in 2 steps: the first being backward-compatible (e.g. by addition, not modification of the existing API), the second adding the feature with breaking changes (e.g. by merging the addition of the first step into the existing API). Just a thought.

Maybe introducing new sub-classes of OutboundMessage and OutboundMessageResult with the correlation metadata and a new sendWithPublishConfirms method could to the trick.

@Sage-Pierce
Copy link
Author

Thanks for the feedback, @acogoluegnes ! I agree with all of it. I like the idea of a two-step process with the addition of a new method that has the generic parameterization. Compiled, it will be the same thing as the current sendWithPublishConfirms, but should be backward compatible. I believe it will also not require new extension of OutboundMessage.

Will revise these changes

acogoluegnes added a commit that referenced this issue Sep 26, 2019
acogoluegnes added a commit that referenced this issue Sep 27, 2019
[#109]Implement Correlation metadata on Sender OutboundMessages
@acogoluegnes acogoluegnes added effort-low type/enhancement A general enhancement labels Sep 27, 2019
@acogoluegnes acogoluegnes added this to the 1.4.0 milestone Sep 27, 2019
acogoluegnes added a commit that referenced this issue Sep 27, 2019
References #109
@acogoluegnes
Copy link
Contributor

Fixed in #110.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort-low type/enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants