Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating to 5.6.0 breaks compatibility with Serilog.Sinks.ElasticSearch 8.4.1 #339

Closed
RNBermudez opened this issue Feb 24, 2021 · 5 comments

Comments

@RNBermudez
Copy link

Issue summary

Updating Serilog.Sinks.MSSqlServer from 5.5.1 to 5.6.0 breaks compatibility with Serilog.Sinks.ElasticSearch 8.4.1.

It seems that in 5.6.0 the MSSqlServerSink class was deprecated in favor of MSSqlServerSinkOptions, however, the latter does not implement the ILogEventSink interface.

In 5.5.1 the MSSqlServerSink class inherits from PeriodicBatchingSink, which implements ILogEventSink. This is no longer the case with MSSqlServerSinkOptions. As result, it is no longer possible to set the FailureSink property.

Installed Serilog packages in the project:

  • Serilog.AspNetCore 3.4.0
  • Serilog.Settings.Configuration 3.1.0
  • Serilog.Settings.Configuration 3.1.0
  • Serilog.Sinks.MSSqlServer 5.5.1
  • Serilog.Sinks.Elasticsearch 8.4.1

Target framework and operating system:

  • .NET Core 5.0
  • OS Windows 10 (local environment)

Code

Log.Logger = new LoggerConfiguration()
    .ReadFrom.Configuration(Configuration)
    .Enrich.FromLogContext()
    .WriteTo.Elasticsearch(SerilogExtensions.ConfigureElasticSink(Configuration))
    .CreateLogger();
public static ElasticsearchSinkOptions ConfigureElasticSink(IConfiguration configuration)
{
    return new ElasticsearchSinkOptions(new Uri(configuration["MyConf:Uri"]))
    {
        ModifyConnectionSettings = conn =>
        {
            return conn.BasicAuthentication(configuration["MyConf:Username"], configuration["MyConf:Password"]);
        },
        FailureCallback = e => Console.WriteLine(Errors.ElasticSearch.ConnectionError(configuration["MyConf:Uri"])),
        EmitEventFailure = EmitEventFailureHandling.WriteToSelfLog |
                           EmitEventFailureHandling.WriteToFailureSink |
                           EmitEventFailureHandling.RaiseCallback,
        FailureSink = new MSSqlServerSink(configuration["MyDb:ConnString"], "Log", 30, TimeSpan.FromMinutes(1), null, true, new ColumnOptions(), "dbo"),
        MinimumLogEventLevel = Enum.Parse<LogEventLevel>(configuration["MyConf:MinimumLevel"]),
        AutoRegisterTemplate = true,
        IndexFormat = LogConstants.IndexName
    };
}

Is there a new class I should be using instead of MSSqlServerSink? Anything I may be missing here?

@jonorossi
Copy link
Member

Updating Serilog.Sinks.MSSqlServer from 5.5.1 to 5.6.0 breaks compatibility with Serilog.Sinks.ElasticSearch 8.4.1.

It seems that in 5.6.0 the MSSqlServerSink class was deprecated in favor of MSSqlServerSinkOptions, however, the latter does not implement the ILogEventSink interface.

The SinkOptions class was deprecated in favour of MSSqlServerSinkOptions (i.e. renamed), MSSqlServerSink use has not changed.

In 5.5.1 the MSSqlServerSink class inherits from PeriodicBatchingSink, which implements ILogEventSink. This is no longer the case with MSSqlServerSinkOptions. As result, it is no longer possible to set the FailureSink property.

This is different to the sink options class change.

As you said MSSqlServerSink used to inherit from PeriodicBatchingSink, but now just implements IBatchedLogEventSink and so lost being an ILogEventSink. This is the same change that was also made to the Email (serilog/serilog-sinks-email#78) and Seq (datalust/serilog-sinks-seq#132) sinks after the new PeriodicBatchingSink API.

Does the Elasticsearch sink need to handle batching sinks itself, not sure? The docs for batching sinks shows using IBatchedLogEventSink (https://github.com/serilog/serilog-sinks-periodicbatching).

@ckadluba
Copy link
Member

In addition to all the correct things @jonorossi said, I would like to ask if you could provide a full sample app to reproduce your problem.

@RNBermudez
Copy link
Author

@jonorossi thanks, appreciate the answer!

We may have been using the wrong class, to begin with. With version 5.5.1 it still works but shows a warning about MSSqlServerSink being obsolete.

image

I'll take a look at Serilog.Sinks.PeriodicBatching. Thanks!

@ckadluba
Copy link
Member

ckadluba commented Mar 1, 2021

Like @jonorossi already explained, we moved to the new API with 5.6.0 implementing IPeroidicBatchingSink instead of ILogEventSink. This should work with all sinks. The details are described here.

https://github.com/serilog/serilog-sinks-periodicbatching/#getting-started

and here

#191 (comment)

The obsole warning you get is because you are using the old constructor/config extension for initialization but the recommended way is the new set of methods consuming a MSSqlServerSinkOptions object instead of individual parameters.

You can find a lot of samples in the samples folder.

@RNBermudez
Copy link
Author

@ckadluba appreciate your reply. I'll update our application accordingly.

Many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants