search

serilog-mssql / serilog-sinks-mssqlserver

A Serilog sink that writes events to Microsoft SQL Server and Azure SQL
Apache License 2.0
276 stars 147 forks source link

Failed db login exception on db creation #533

Open alexsrdev opened 4 months ago

alexsrdev commented 4 months ago

Bug Report / Support Request Template

If you are opening a feature request, you can ignore this template. Bug reports and requests for assistance usually require the same basic information described below. This will help us more quickly reproduce and investigate the problem you're reporting. (If you are using Serilog.Sinks.MSSqlServerCore, that package is deprecated, please switch to Serilog.Sinks.MSSqlServer before reporting an issue.)

Please clearly describe what the SQL Sink is doing incorrectly: When trying to create the mssql db using package manager console executing a update-database, the mssql sink generates a login exception.

Please clearly describe the expected behavior:

List the names and versions of all Serilog packages used in the project:

Target framework and operating system:

[x] .NET 8 OS: Running on top of WSL and dotnet/aspnet image

Provide a simple reproduction of your Serilog configuration code: "Serilog": { "Using": [ "Serilog.Sinks.Console" ], "EnableUi": true, "MinimumLevel": "Information", "WriteTo": [ { "Name": "Console", "Args": { "outputTemplate": "[{Timestamp:u} {Level:u3}] {Message:lj}{NewLine}{Exception}" } }, { "Name": "MSSqlServer", "Args": { "connectionString": "DefaultConnection", "sinkOptionsSection": { "tableName": "ServiceLogs", "schemaName": "dbo", "autoCreateSqlTable": true }, "restrictedToMinimumLevel": "Information" } } ] },

Provide a simple reproduction of your Serilog configuration file, if any:

Provide a simple reproduction of your application code: Line throwing the exception: builder.Host.UseSerilog((ctx, lc) => { lc.ReadFrom.Configuration(ctx.Configuration); });

I noticed that since mssqk sink version 6.4.0, the sql executor is re-throwing the exception (see here): https://github.com/serilog-mssql/serilog-sinks-mssqlserver/commit/bf1f5e67baaa3231d91ca6d05e7fba51c1441e8f https://github.com/serilog-mssql/serilog-sinks-mssqlserver/commit/bf1f5e67baaa3231d91ca6d05e7fba51c1441e8f#diff-f8c67af6a71f2771abeac7d9eb21caee190c7f6530085394bd37f24c60c9e65a

Not sure why the change, but this sink should not be trying to connect to database on a db migration execution (e.g. update-database).

When I downgrade to 6.3.0, everything works as expected (db creation is successful).

C-Coretex commented 4 months ago

Same for me with Serilog.Sinks.MSSqlServer version 6.6.0 A downgrade to 6.3.0 helped

ckadluba commented 4 months ago

Hi @alexsrdev and @C-Coretex!

I'll take a look at this as soon as I can. Thanks for letting us know.

Christian

rmacfadyen commented 4 months ago

This issue also impacted me for a net8 API project.

It was particularly tricky to track down because in our environment the database is only missing during certain test stages via docker-compose (spins up new/fresh/empty sql server and spins up the API who's efcore migrations then create the database... or in this case the Startup.cs crashes in builder.Build()).

A lot of effort to track this down. And only tracked down by detailed examination of the stack trace and noticing that Serilog was in the middle of it.

Workarounds:

rmacfadyen commented 4 months ago

Note that the autoCreateSqlDatabase flag may require access to the "master" database, regardless of whether or not the logging database exists or not. This fails on Azure SQL (the master database is not user accessible).

So... not much of a work around if your app/api actual deployment uses Azure SQL,

Here's the error you get for Azure SQL when autoCreateSqlDatabase is true:

[hostname]-6686d6cf7f-fh9c4  ---> Microsoft.Data.SqlClient.SqlException (0x80131904): The server principal "[hostname]" is not able to access the database "master" under the current security context.
[hostname]-6686d6cf7f-fh9c4 Cannot open user default database. Login failed.
ckadluba commented 4 months ago

Hi!

Thank you for reporting this. Please be aware that the auto create log table and log database features were intended for convenient local testing and development purposes. Those features are not recommended for production deployments. I think we should state this clearly in the documentation.

The rethrow of exceptions in SQL executor class was added because when using the audit sink (AuditTo()) the calling code needs to have a way of knowing when a log attempt failed and be able to handle this situation. The batched sink (WriteTo()) does not rethrow such exceptions because it writes log events delayed in the background and not syncronously like audit.

Regarding the access to master db in Azure SQL, I think you are just lacking the correct permissions for the identity which the SQL sink is running with. According to this documentation, access to master db is possible in Azure SQL https://learn.microsoft.com/en-us/azure/azure-sql/database/logins-create-manage?view=azuresql#create-additional-logins-and-users-having-administrative-permissions.

Since some of you might experience this issue, can someone please share a complete but simple sample application which we can use to reproduce the problem (using a local SQL instance)?

Thank you.

Christian