Looks good to me! As I was not the reporter of this issue, I cannot close it. Thanks!

I made the changes I mentioned and have committed it. None of the other projects require the javamail dependency. I believe this is because it is only referenced from the SMTPManager and not the appender now. Although I did quite a bit of refactoring I am still giving you the credit for this fix since all I really did was move code around.

Please verify and close.

The issue with the Filter is a good point. At the same time, if two LoggerContext's are sharing the same configuration you would end up two buffers each containing only some of the events. This can occur if you have a parent and a child ClassLoader (such as in Tomcat) and the Log4j jars are in Tomcat's directories, not the web apps. In that case, when an error occurs you would get an email but it would be missing either all the log entries from classes in Tomcat's ClassLoader or all the log entries from the web app ClassLoader. With a single manager they are all combined and you would get what I think is the expected behavior.

A similar problem occurs in the RollingFileAppender. In that case, once the manager has been created for a single file subsequent appenders will use it and whatever they specify for a policy or strategy will be ignored (to do otherwise would cause quite a mess).

In this case I would think the String value of the Filter, along with many of the Appender parameters, would become part of the manager's "name". Since it will be pretty long I'd probably use an MD5 like the Flume Embedded Appender does.

1. I don't believe that any of the changes from LOG4J2-136 are needed - it's entirely new code.
2. I pushed the JavaMail .jar files everywhere, as the tests in those projects were attempting to load all Appenders, including the new SMTPAppender. Without the JavaMail jars, multiple failures occurred. If there's a better solution (i.e. changing the subproject tests to not load SMTPAppender, please let me know.
3. I agree - SMTPSessionManager could do more work, but I wouldn't put the CyclicBuffer in there. Consistent with Log4J 1.x, the buffer is consumed/cleared when sending an event, and if instances have different Filters, they could unfortunately interfere with each other.

Thanks, and let me know if there's anything I can do to help.

I've spent a couple of hours reviewing the patch and am not going to be able to commit it as is for the following reasons:
1. The patch seems to require the changes from LOG4j2-136 to cleanly apply. I haven't reviewed that yet.
2. For some reason you have published the javamail jars to every subproject as an optional dependency. I'm not sure why that is needed.
3. Using a manager allows multiple appender instances to share resources so that they all behave as if they were a single appender. SMTPSessionManager doesn't do that. In fact, it is only used in a local variable in the factory method. I would expect that the CyclicBuffer as well as all the actual work to send the email would be in the Manager.

I plan to refactor your patch to address all of the above.

A port of Log4J 1.x's SMTPAppender, with significant cleanup/refactoring. Most previous features/configuration options are supported, with the exception of "sendOnClose". Includes unit tests, with Dumbster used for end-to-end testing of sending SMTP messages.

Should apply cleanly to trunk.

Might be interesting to aggregate a given time period's log events before sendi g the mail out. "here are the log events of the last 12 hours" type of thing