[Messenger] Pass sender details to SendMessageToTransportsEvent #40152
Conversation
|
Hey! I had a quick look at this PR, I think it is alright. I think @ruudk has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
There was a problem hiding this comment.
The SendMessageToTransportsEvent is about sending to one or multiple transports (hence plural transports).
The SentStamp is past tense, meaning it's already sent. When you would pass that to the subscribers listening on SendMessageToTransportsEvent that would be untrue, as the message is not sent yet.
It's different to the ReceivedStamp passed to the worker subscribers, in those cases, the message is actually received.
Therefore I think it would be better to do the following:
$senders = $this->sendersLocator->getSenders($envelope);
if (null !== $this->eventDispatcher) {
$event = new SendMessageToTransportsEvent($envelope, $senders);
$this->eventDispatcher->dispatch($event);
$envelope = $event->getEnvelope();
}
foreach ($senders as $alias => $sender) {
$this->logger->info('Sending message {class} with {alias} sender using {sender}', $context + ['alias' => $alias, 'sender' => \get_class($sender)]);
$envelope = $sender->send($envelope->with(new SentStamp(\get_class($sender), \is_string($alias) ? $alias : null)));
}This way:
- you still only emit 1 event
- the event now contains all the senders
- the envelope passed to the subscribers is still the same
|
@ruudk |
|
What's the status of this PR? @Jeroeny Are you still interested in moving it forward? |
|
Yes I'd be willing to move this forward. The last time I hit kind of a dead-end in which approach to take. Would ruudk's suggestion be desired for example? |
|
I think @ruudk's suggestion makes a lot of sense. |
|
In their snippet, |
|
What would be the use case for having two entries with the same alias? |
|
I don't think there is, but I thought it was technically supported and could cause issues. Anyway, I've implemented the suggested approach 👍 . |
|
Thank you @Jeroeny. |
This is a feature suggestion that will allow listeners for
SendMessageToTransportsEventto be able to read the sender and sender alias from a sent message.Use case: To create metrics for messages that go through the system (and especially where to and from). Listener(s) can be created to measure this. They would listen to
WorkerMessageHandledEvent,WorkerMessageFailedEventandSendMessageToTransportsEvent. The first two receive envelopes that contain aReceivedStamp, to determine where the message came from. However, the received envelopes for sent messages do not contain a stamp to read the sender. It would be great if this was possible to do with listener (instead of having to add middleware to each bus).Approach in this PR: stamp the envelope with
SentStampbefore dispatchingSendMessageToTransportsEvent.Alternative approach: pass the info to the event directly:
new SendMessageToTransportsEvent($envelope, get_class($sender), $alias). And keep the stamping afterwards.Edit: I just noticed that one event is dispatched regardless of the number of senders. In that case the event could be dispatched after all the senders have been called (and thus all the stamps set).