Conversation
|
Thanks for opening this pull request! |
|
@ShadowDancer Thank you for the contribution, and finding all the locations where LogEvents are dropped. Excellent work. Any reason why the dropped-EventArgs is different from AsyncTargetWrapper ? (Not including the LogEvent) I recognize that it will give better performance, but curious if the inconsistency is good? |
|
@snakefoot QueuedNetworkSender operates on buffers of bytes, it does not have concept of LogEvent (it operates on buffers instead). I've taken simple approach and unified event across all call sites. I think LogEvent does not matter for this use case, as I want to know if there is backpressure on logshipper - in that case you probably cannot do anything with these logs anyway. |
This simplifies NetworkTarget logic as it does not make sense to have network target without buffer.
|
Kudos, SonarCloud Quality Gate passed! |
|
Not really, go ahead :) |
|
Hooray your first pull request is merged! Thanks for the contribution! Looking for more? 👼 Please check the up-for-grabs issues - thanks! |
Add LogEventDropped handler to the NetworkTarget. Resolves #5084
As proposed in the ticket copied implementation from
AsyncTargetWrapper, but had to make a few tweaks due to nature of NetworkTarget (it can discard messages on multiple occasions, due to message size or number of connections).This MR assumes that NetworkTarget owns and outlives NetworkSender.
Closes #5084