search

serilog / serilog-sinks-email

A Serilog sink that writes events to SMTP email
Apache License 2.0
70 stars 68 forks source link

Update dependencies and add overloads to allow custom SMTP Server Ports #48

Closed hellfirehd closed 6 years ago

hellfirehd commented 6 years ago

See #33 for more.

Did not change any existing API, just added overloads to preserve backwards compatibility.

nblumhardt commented 6 years ago

Thanks for the PR, Doug 👍

The backwards-compatible overload is sound, but over time if we use this approach we end up having to create a lot of potentially overlapping method signatures to cope with new options (i.e. each overload needs a new overload for each new feature).

There's an alternative documented in this comment that should work here - would it be possible to switch to that approach to add an int port = 25 optional parameter?

Thanks again!

hellfirehd commented 6 years ago

I'll definitely take a look.

hellfirehd commented 6 years ago

All right... that took a lot more effort than I anticipated. I followed the example suggested and I think I've got something that works that will also allow for the future.

But the more that I worked with this, the more that I think it needs a significant rewrite and version bump.

hellfirehd commented 6 years ago

The more I work with this sink the more frustrated I become. This issue is a complete showstopper for me as fixing risks breaking existing usages.

The suggestion here of splitting the sink has a lot of merit and I think is the correct way to go.

I'm going to close this PR...

nblumhardt commented 6 years ago

Yep, know the feeling 👍

Logging directly to SMTP email is often a pretty awkward experience - email has too many points of failure of its own to be a good channel except in the most dire circumstances.

If you have access to a monitoring tool that can do email alerting with some kind of aggregation of events, plus rate limiting, that's the way to go.