search

serilog-mssql / serilog-sinks-mssqlserver

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

UseSqlBulkCopy=false has no effect #526

Closed chrisg920 closed 8 months ago

chrisg920 commented 8 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: The sink was not logging anything to my database after moving its configuration settings from "AuditTo" to "WriteTo" in my appSettings.json file. Enabling Serilog diagnostic logging showed that this error was occurring:

Unable to write batch of 10 log events to the database due to the following error: System.InvalidOperationException: The given ColumnMapping does not match up with any column in the source or destination. at Microsoft.DataSqlClient.SqlBulkCopy.WriteToServerInternalRestContinuedAsync...

Since I did not have this problem when using AuditTo, I figured it must be as problem when using SqlBullkCopy indtead of using INSERT statements, so I decided to take advantage of the configuration setting for turning off SqlBulkCopy as I did not have time to troubleshoot the issue with bulk logging. This is what my configuration looks like:

"WriteTo": [
  {
    "Name": "MSSqlServer",
    "Args": {
      "connectionString": "<Removed>",
      "tableName": "Jobs",
      "schemaName": "Logging",
      "restrictedToMinimumLevel": "Information",
      "autoCreateSqlTable": true,
      "batchPostingLimit": 10,
      "useSqlBulkCopy": false
    }
  }

After adding the useSqlBulkCopy setting, I still got the same error from above, telling me that it was still in fact using bulk copy instead of insert. I found that if I modified the code in SinkDependenciesFactory.Create to force it to use SqlInsertStatementWriter as follows:

        var sinkDependencies = new SinkDependencies
        {
            DataTableCreator = dataTableCreator,
            SqlDatabaseCreator = new SqlDatabaseCreator(
                sqlCreateDatabaseWriter, sqlConnectionFactoryNoDb),
            SqlTableCreator = new SqlTableCreator(
                sqlCreateTableWriter, sqlConnectionFactory),
            //SqlBulkBatchWriter = sinkOptions.UseSqlBulkCopy
            //    ? (ISqlBulkBatchWriter)new SqlBulkBatchWriter(
            //        sinkOptions.TableName, sinkOptions.SchemaName, columnOptions.DisableTriggers,
            //        sqlConnectionFactory, logEventDataGenerator)
            //    : (ISqlBulkBatchWriter)new SqlInsertStatementWriter(
            //        sinkOptions.TableName, sinkOptions.SchemaName,
            //        sqlConnectionFactory, logEventDataGenerator),
            SqlBulkBatchWriter = new SqlInsertStatementWriter(
                sinkOptions.TableName, sinkOptions.SchemaName,
                sqlConnectionFactory, logEventDataGenerator),
            SqlLogEventWriter = new SqlInsertStatementWriter(
                sinkOptions.TableName, sinkOptions.SchemaName,
                sqlConnectionFactory, logEventDataGenerator)
        };

it eliminated the error I was getting, telling me that it was now indeed not using bulk copy as I desired.

Note that all other settings work fine.

Please clearly describe the expected behavior: It should not use bulk copy if useSqlBulkCopy=false is specified in the application configuration file.

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

Target framework and operating system:

[X ] .NET 6 [ ] .NET Framework 4.8 [ ] .NET Framework 4.7 [ ] .NET Framework 4.6 OS: Windows 8.0

Provide a simple reproduction of your Serilog configuration code:

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

Provide a simple reproduction of your application code:

ckadluba commented 8 months ago

Hi @chrisg920!

Your config structure is wrong. The UseSqlBulkCopy option is part of the MSSqlServerSinkOptions object as documented here: https://github.com/serilog-mssql/serilog-sinks-mssqlserver?tab=readme-ov-file#usesqlbulkcopy.

Therefore your appsettings should look something like this.

"WriteTo": [
  {
    "Name": "MSSqlServer",
    "Args": {
      "connectionString": "<Removed>",
      "restrictedToMinimumLevel": "Information",
      "sinkOptionsSection": {
        "tableName": "Jobs",
        "schemaName": "Logging",
        "autoCreateSqlTable": true,
        "batchPostingLimit": 10,
        "useSqlBulkCopy": false
      }
    }
  }

Please refer to our sample apps for some inspiration: https://github.com/serilog-mssql/serilog-sinks-mssqlserver/blob/dev/sample/WorkerServiceDemo/appsettings.json.

chrisg920 commented 8 months ago

Thanks for setting me straight on this. I was fooled by the fact that the other options worked just fine, but I think that was because they were part of the old configuration system and useSqlBulkCopy was not.

From: Christian Kadluba @.> Sent: Friday, March 15, 2024 7:35 AM To: serilog-mssql/serilog-sinks-mssqlserver @.> Cc: Glasser, Chris @.>; Mention @.> Subject: Re: [serilog-mssql/serilog-sinks-mssqlserver] UseSqlBulkCopy=false has no effect (Issue #526)

EXTERNAL SENDER

CAUTION: This email originated from outside your organization. Exercise caution when opening attachments or clicking links, especially from unknown senders.

Hi @chrisg920https://github.com/chrisg920!

Your config structure is wrong. The UseSqlBulkCopy option is part of the MSSqlServerSinkOptions object as documented here: https://github.com/serilog-mssql/serilog-sinks-mssqlserver?tab=readme-ov-file#usesqlbulkcopy.

Therefore your appsettings should look something like this.

"WriteTo": [

{

"Name": "MSSqlServer",

"Args": {

  "connectionString": "<Removed>",

  "restrictedToMinimumLevel": "Information",

  "sinkOptionsSection": {

    "tableName": "Jobs",

    "schemaName": "Logging",

    "autoCreateSqlTable": true,

    "batchPostingLimit": 10,

    "useSqlBulkCopy": false

  }

}

}

Please refer to our sample apps for some inspiration: https://github.com/serilog-mssql/serilog-sinks-mssqlserver/blob/dev/sample/WorkerServiceDemo/appsettings.json.

— Reply to this email directly, view it on GitHubhttps://github.com/serilog-mssql/serilog-sinks-mssqlserver/issues/526#issuecomment-1999469640, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AR2UASM7QBRCWVNZHZJS3J3YYLME3AVCNFSM6AAAAABES3IXUWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJZGQ3DSNRUGA. You are receiving this because you were mentioned.Message ID: @.***> IMPORTANT NOTICES: This electronic transmission may contain confidential or privileged information. If you have received this message in error, please notify the sender by replying and delete the message without copying or disclosing it. If this email requests to transfer funds, DO NOT respond to this email and contact us immediately via known contact information.

dcanning commented 3 months ago

Can you provide an XML config version of this, I tried: <add key="serilog:write-to:MSSqlServer.sinkOptions:useSqlBulkCopy" value="false" /> and <add key="serilog:write-to:MSSqlServer.sinkOptions:UseSqlBulkCopy" value="false" /> and <add key="serilog:write-to:MSSqlServer.sinkOptions.useSqlBulkCopy" value="false" />

neither works and I don't see UseSqlBulkCopy in the example .configs

ckadluba commented 1 month ago

@dcanning Could you get the UseSqlBulkCopy option working for you or are you still facing this issue?