-
Notifications
You must be signed in to change notification settings - Fork 1.1k
DistributedPubSub
: clearer logging when DeadLetter
publishing due to no subscribers
#7646
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
DistributedPubSub
: clearer logging when DeadLetter
publishing due to no subscribers
#7646
Conversation
…icate when there are no subscribers
…ss to clearly indicate no subscriber case in dead letter messages - Made DeadLetterWithNoSubscribers internal (class already existed) - Added test to verify dead letter messages are sent when no subscribers exist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Detailed my changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying out AMP code and this is the agent rules file for that
var testMessage = "test-message"; | ||
|
||
// act - publish to a topic that no one is subscribed to | ||
await EventFilter.Info(contains: "DeadLetterWithNoSubscribers") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validates that we get a more descriptive log message indicating that this message got DeadLetter
'd because there were no subscribers
// Use the specialized DeadLetterWithNoSubscribers class to clearly indicate | ||
// that the message was not delivered because there were no subscribers, | ||
// not because the mediator itself is dead. | ||
var deadLetter = new DeadLetterWithNoSubscribers(message, Sender, Context.Self); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uses the new custom dead letter type when we're routing messages to dead letters
/// DeadLetters because there were no subscribers for the topic in DistributedPubSub, | ||
/// NOT because the mediator itself is dead. | ||
/// </summary> | ||
internal sealed class DeadLetterWithNoSubscribers : AllDeadLetters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
internal
type, so no changes in public API
src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mentioned updates
if (_settings.SendToDeadLettersWhenNoSubscribers) | ||
Context.System.DeadLetters.Tell(new DeadLetter(message, Sender, Context.Self)); | ||
{ | ||
var topic = message switch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tries to extract the topic from the message so we can be as accurate as possible during the DeadLetter
logging
/// <returns>A string that represents the current object.</returns> | ||
public override string ToString() | ||
{ | ||
return $"DeadLetterWithNoSubscribers from {Sender} to {Recipient}: <{Message}> - No subscribers found for topic {Topic}"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Topic should never actually be null
- but that may change in the future if we start using IgnoreOrSendToDeadLetters
for anything other than Send
and Publish
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Changes
Fix #7626: Using
DeadLetterWithNoSubscribers
to clearly indicate when here are no subscribersChecklist
For significant changes, please ensure that the following have been completed (delete if not relevant):
DeadLetter
logging when no subscribers looks identical to theMediator
being dead #7626