serilog-sinks-elasticsearch icon indicating copy to clipboard operation
serilog-sinks-elasticsearch copied to clipboard

`FailureCallback` is not used when buffer is enabled

Open PatrickLehnerXI opened this issue 5 years ago • 1 comments

A few questions before you begin:

Does this issue relate to a new feature or an existing bug?

  • [x] Bug
  • [ ] New Feature

What version of Serilog.Sinks.Elasticsearch is affected? Please list the related NuGet package. <PackageReference Include="Serilog.Sinks.Elasticsearch" Version="7.1.0" />

What is the target framework and operating system? See target frameworks & net standard matrix.

  • [x] netCore 2.0
  • [ ] netCore 1.0
  • [ ] 4.7
  • [ ] 4.6.x
  • [ ] 4.5.x

Please describe the current behavior? FailureCallback is not used when buffer is enabled.

Please describe the expected behavior? FailureCallback is used, no matter if buffer is enabled or not.

If the current behavior is a bug, please provide the steps to reproduce the issue and if possible a minimal demo of the problem

  • Configure serilog with ES Sink, with a FailureCallback (and usage of callback enabled):
// . . .
.WriteTo.Elasticsearch(
new ElasticsearchSinkOptions(new Uri("<ES server URL>")
    {
        BufferBaseFilename = "<filename>",
        FailureCallback = e => Console.WriteLine(">>> Unable to submit event " + e.RenderMessage()),
        EmitEventFailure = EmitEventFailureHandling.RaiseCallback,
    });
  • start the application without the ES server being reachable (to provoke errors)
  • => no output happens using the FailureCallback
  • Note: if SelfLog output is enabled, errors appear in SelfLog
  • Note: if buffer is not enabled, errors are output to FailureCallback
  • Note: I noticed that FailureCallback is used in ElasticSearchSink.cs, but not in ElasticSearchLogShipper.cs (the error message "Exception while emitting periodic batch from..." is one of the ones we observed in the SelfLog).

PatrickLehnerXI avatar Jul 11 '19 13:07 PatrickLehnerXI

Unfortunately the durable part is different from the main logic. Which is annoying and we are looking into options to remove it #254

mivano avatar Jul 17 '19 08:07 mivano