Skip to content

Remove binary / varbinary support (reverse PR 117) #165

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

Closed
MV10 opened this issue Oct 13, 2018 · 0 comments · Fixed by #166
Closed

Remove binary / varbinary support (reverse PR 117) #165

MV10 opened this issue Oct 13, 2018 · 0 comments · Fixed by #166
Assignees
Labels

Comments

@MV10
Copy link
Contributor

MV10 commented Oct 13, 2018

PR #117 added support for binary and varbinary SQL column data types, but it doesn't work -- and can't work.

If you try to actually create a SQL binary column and populate it, the sink throws an exception because Serilog's internal property-handling has already converted the byte array property data to a clipped string stored as a ScalarValue object. The sink never sees the byte array and the string data is a mismatch for the column type.

The sink couldn't support this type of data unless Serilog's own internal property-handling changed, which seems unlikely...

image

The code to reproduce this is simple enough:

static void Main(string[] args)
{
    var appConfig = new ConfigurationBuilder().SetBasePath(Directory.GetCurrentDirectory()).AddJsonFile("appconfig.json").Build();
    Log.Logger = new LoggerConfiguration().ReadFrom.Configuration(appConfig).CreateLogger();
    var smaller = new byte[60];
    var larger = new byte[1024];
    Random random = new Random();
    random.NextBytes(smaller);
    random.NextBytes(larger);
    SelfLog.Enable(Console.Out);
    Log.Information("smaller {ba_binary}", smaller);
    Log.Information("larger {ba_binary)", larger);
    Log.Information("var smaller {ba_varbinary}", smaller);
    Log.Information("var larger {ba_varbinary)", larger);
    Log.CloseAndFlush();
    Console.WriteLine("Press any key...");
    Console.ReadKey();
}
{
  "ConnectionStrings": {
    "DevTest": "Data Source=(LocalDB)\\MSSQLLocalDB;Initial Catalog=SerilogTest;Integrated Security=true"
  },
  "Serilog": {
    "Using": [ "Serilog.Sinks.Console" ],
    "WriteTo": [
      { "Name": "Console" },
      {
        "Name": "MSSqlServer",
        "Args": {
          "connectionString": "DevTest",
          "tableName": "PR117",
          "autoCreateSqlTable": true,
          "columnOptionsSection": {
            "customColumns": [
              {
                "ColumnName": "ba_binary",
                "DataType": "binary",
                "DataLength": 512
              },
              {
                "ColumnName": "ba_varbinary",
                "DataType": "varbinary",
                "DataLength": 2048
              }
            ]
          }
        }
      }
    ]
  }
}
@MV10 MV10 added the bug label Oct 13, 2018
@MV10 MV10 self-assigned this Oct 13, 2018
@MV10 MV10 closed this as completed in #166 Oct 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant