TraceId and SpanId are saved as empty string instead of NULL

[marcominerva]

I have the following configuration with Serilog.Sinks.MSSqlServer v9.0.0:

{
    "Serilog": {
        "Using": [ "Serilog.Sinks.MSSqlServer" ],
        "MinimumLevel": {
            "Default": "Information"            
        },
        "WriteTo": [           
            {
                "Name": "MSSqlServer",
                "Args": {
                    "connectionString": "...",
                    "sinkOptionsSection": {
                        "tableName": "Logs",
                        "autoCreateSqlTable": true
                    },
                    "columnOptionsSection": {
                        "addStandardColumns": [ "LogEvent", "TraceId", "SpanId" ],
                        "removeStandardColumns": [ "MessageTemplate", "Properties" ],
                        "id": { "dataType": "bigint" },
                        "timeStamp": {
                            "columnName": "Timestamp",
                            "convertToUtc": true,
                            "nonClusteredIndex": true,
                            "nonClusteredIndexDirection": "Desc"
                        },
                        "logEvent": {
                            "excludeAdditionalProperties": false,
                            "excludeStandardColumns": false
                        }
                    }
                }
            }
        ],
        "Enrich": [ "FromLogContext" ]
    }
}

With this settings, I notice that when log messages don't contain SpanId or TraceId, in the database are written empty strings instead of NULL, even if the columns allow null:

[ckadluba]

Thank you for the fix PR @nanny07. After this was merged into dev, we now have a new 9.0.2-dev preview version published on nuget.org.

@marcominerva @nanny07 could you please take this build for a walk and see how the fix behaves? Especially I'm curious how it behaves if the columns TraceId and SpanId are non nullable and the properties are not supplied in the log event. I believe that we might still have an issue in this case.

I keep this item open until we have tested this.

Thank you!

[marcominerva]

I have tested the dev package and it correctly inserts NULL values for TraceId and SpanId when they aren't available.

I haven't yet tested changing the configuration to disallow nullable values for these columns (although I think it's quite unlikely that such a setting would be used).

[nanny07]

@ckadluba you are right but it was a problem even before with the field Exception

I'll have a look how to solve this

[ckadluba]

Awesome. Thank you.

[nanny07]

@ckadluba have a look at the new PR: I've used the default column value instead a "static" string.Empty

[nanny07]

@ckadluba apart from the latest PR for typos, I tested the edge case (non-nullable field and null value) and it works as expected without generating any exceptions.

[ckadluba]

Thank you very much for contributing and testing.

I will do some final tests and then create a stable release within the next days.

[marcominerva]

Hi @ckadluba!

I too have done some tests with nullable and not nullable columns and can confirm that it works as expected.

[ckadluba]

Awesome! Thank you so much guys.

I'll create a stable 9.0.2 version within the next days.