Implement batching of emails

[kll]

This is a significant change, and is essentially untested on a real Seq server. I did add unit tests, and it looks ok as far as I can tell, but I will hook it up to a real server and test it out over the next few days. I wanted to go ahead and send a PR now to get some feedback. Not only is it significantly different, but it is also not backwards compatible with templates for previous versions due to having an array of events rather than a single event. For those reasons I was not sure about modifying the existing Email+ app versus creating a new separate app. If you would rather it be a different app, let me know and I'll update this PR.

It should work essentially the same as before with one email per event if you do not configure any of the new settings. If you set a delay, then it will start a batch of events based on the templated subject. As long a new event arrives once every delay period, it will keep waiting and add that event to the batch. Once a full delay period has elapsed, or the max delay or max event count has been reached it will construct an email with the details of each event in the body and send it.

Let me know what you think. I'll post a note here after I'm done testing it out and am confident it is working correctly.

[nblumhardt]

Wow - that's a pretty impressive changeset! :-)

I would guess the downsides might be:

• (Slightly) more complicated configuration for new users
• The possibility of unexpected memory use when events are large (or if we hold on to anything we're not expecting to)

But the upsides are pretty great - I can imagine this making the email app applicable to a significantly wider range of scenarios.

Whether we ship this as an updated Email+ or initially as a variation (Batched Email?) it'd be great to see this out there. Thanks very much for sending the PR!

Cheers,
Nick

[kll]

So far this seems to be working well on our production Seq server which is a t2.small instance at AWS. I've got it set to email everything that is at least level Warning. That isn't my long term plan, but I'm using it to help transition people who are used to everything landing directly in their inbox. So far about 2k events have been sent through this version of the plugin.

Memory use is for sure a concern if you set it with large max value for batch size or delay, or if you have an extreme number of events going through it. I've been testing it with a batch delay of 60s, max delay of 300s, and max size of 50.

[kll]

Our production Seq server crashed today, and after investigating it looks like the source of it was an SmtpException thrown from this app. There was not any exception handling in the original app so I had left it that way assuming the Seq server would catch the exceptions. However, based on what happened today I decided to go in and wrap the message send with a try/catch to handle that case.

Is it expected that exceptions from a seq-app can bring down the entire Seq server? It doesn't seem like that should be the case. If so, I probably need to add a lot more error handling to this. :)

[nblumhardt]

Interesting - thanks for the info! Do you still have a stack trace handy?

All of the code in the Seq server that interacts with user apps does it in a sandboxed app domain, with try/catch on both sides of the boundary, so in theory simple exceptions from user apps shouldn't be able to bring down Seq.

In practice, there are probably still some holes - I'm guessing this one may have been an unhandled exception on a threapool thread or within a task. The trace should shine a light on it :-)

[kll]

Sorry it took a few days, but here is the stack trace finally.

Application: seq.exe
Framework Version: v4.0.30319
Description: The process was terminated due to an unhandled exception.
Exception Info: System.Net.Mail.SmtpException
Stack:
   at System.Net.Mail.SmtpClient.Send(System.Net.Mail.MailMessage)
   at Seq.App.EmailPlus.EmailReactor.Send(System.Collections.Generic.ICollection`1<Seq.Apps.Event`1<Seq.Apps.LogEvents.LogEventData>>)
   at System.Reactive.AnonymousSafeObserver`1[[System.__Canon, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]].OnNext(System.__Canon)
   at System.Reactive.Linq.ObservableImpl.SelectMany`3+_+Iter[[System.__Canon, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089],[System.__Canon, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089],[System.__Canon, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]].OnNext(System.__Canon)
   at System.Reactive.Linq.ObservableImpl.ToList`1+_[[System.__Canon, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]].OnCompleted()
   at System.Reactive.AutoDetachObserver`1[[System.__Canon, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]].OnCompletedCore()
   at System.Reactive.Observer`1[[System.__Canon, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]].OnCompleted()
   at System.Reactive.Linq.ObservableImpl.GroupByUntil`4+_+Delta[[System.__Canon, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089],[System.__Canon, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089],[System.__Canon, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089],[System.Int64, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]].OnCompleted()
   at System.Reactive.Linq.ObservableImpl.Take`1+_[[System.Int64, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]].OnNext(Int64)
   at System.Reactive.Linq.ObservableImpl.Merge`1+_+Iter[[System.Int64, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]].OnNext(Int64)
   at System.Reactive.Linq.ObservableImpl.Merge`1+_+Iter[[System.Int64, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]].OnNext(Int64)
   at System.Reactive.Linq.ObservableImpl.Throttle`1+_[[System.Int64, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]].Propagate(System.Reactive.Concurrency.IScheduler, UInt64)
   at System.Reactive.Concurrency.DefaultScheduler+<>c__DisplayClass4`1[[System.UInt64, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]].<Schedule>b__3(System.Object)
   at System.Reactive.Concurrency.ConcurrencyAbstractionLayerImpl+Timer.Tick(System.Object)
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.TimerQueueTimer.CallCallback()
   at System.Threading.TimerQueueTimer.Fire()
   at System.Threading.TimerQueue.FireNextTimers()

[nblumhardt]

Thanks for the additional info. At first glance, it doesn't seem like there's a lot we can do in the host to prevent unhandled exceptions in the app from bringing it down (since this one is on a task-pool thread and not one we can wrap a try/catch block around). Will investigate options further.

I take it you've been able to handle this successfully in the app itself now?

Thanks again!

[kll]

So far so good, just that one crash ever. I added a try catch with commit https://github.com/kll/seq-apps/commit/46fc29ab4540d4d231b705e5381b5593b6ba0892 and deployed that and nothing else since.

[nblumhardt]

Hi Kevin!

Unfortunately I think I dropped the ball on this one. The codebase has moved on quite a bit in the year since we were on this thread; I've just posted a very, very basic implementation of batched email to: https://github.com/datalust/seq-app-digestemail. It could use some improvement but I needed to scratch an itch and decided to try doing it with the least possible changes atop Email+.

If you're keen to check it out there's a package on NuGet, but I gather you're getting along fine with this implementation :-)

Just thought I'd send a heads-up; it's probably time we closed this one.

Cheers,
Nick