Make sure that DeadLetters published by DistributedPubSubMediator contain full context of topic
#6209
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… contain full context of topic This is a usability improvement aimed at making sure we contain the full context of what topic we were trying to publish to or which `Put`-ed actor we were trying to publish to when there are no subscribers available on the local node.
Aaronontheweb
commented
Oct 24, 2022
| @@ -500,7 +500,7 @@ private void IgnoreOrSendToDeadLetters(object message) | |||
| Context.System.DeadLetters.Tell(new DeadLetter(message, Sender, Context.Self)); | |||
| } | |||
|
|
|||
| private void PublishMessage(string path, object message, bool allButSelf = false) | |||
| private void PublishMessage(string path, IWrappedMessage publish, bool allButSelf = false) | |||
There was a problem hiding this comment.
Rather than send just the object Message to these methods, send the full IWrappedMessage payload which we'll pass directly to the IgnoreOrSendToDeadLetters method - as that wrapper message includes all of the useful debugging context a developer might need.
| { | ||
| var prefix = path + "/"; | ||
| var lastKey = path + "0"; // '0' is the next char of '/' | ||
|
|
||
| var groups = ExtractGroups(prefix, lastKey).GroupBy(kv => kv.Key).ToList(); | ||
| var wrappedMessage = new SendToOneSubscriber(message); | ||
| var wrappedMessage = new SendToOneSubscriber(publish.Message); |
There was a problem hiding this comment.
Forward only the payload to the subscriber still, preserving the same behavior as before.
Aaronontheweb
commented
Oct 24, 2022
|
|
||
| if (groups.Count == 0) | ||
| { | ||
| IgnoreOrSendToDeadLetters(message); | ||
| IgnoreOrSendToDeadLetters(publish); |
There was a problem hiding this comment.
Send the full, debuggable payload to DeadLetters
Arkatufus
pushed a commit
to Arkatufus/akka.net
that referenced
this pull request
Oct 24, 2022
… contain full context of topic (akkadotnet#6209) This is a usability improvement aimed at making sure we contain the full context of what topic we were trying to publish to or which `Put`-ed actor we were trying to publish to when there are no subscribers available on the local node. (cherry picked from commit 9f84438)
Aaronontheweb
added a commit
that referenced
this pull request
Oct 24, 2022
… contain full context of topic (#6209) (#6212) This is a usability improvement aimed at making sure we contain the full context of what topic we were trying to publish to or which `Put`-ed actor we were trying to publish to when there are no subscribers available on the local node. (cherry picked from commit 9f84438) Co-authored-by: Aaron Stannard <aaron@petabridge.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
This is a usability improvement aimed at making sure we contain the full context of what topic we were trying to publish to or which
Put-ed actor we were trying to publish to when there are no subscribers available on the local node.Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):