[BUG] Issue with scopes and known properties in latest alpha of Extensions.Logging library

[sleeuwen]

ECS integration/library project(s) (e.g. Elastic.CommonSchema.Serilog): Elasticsearch.Extensions.Logging

ECS schema version (e.g. 1.4.0): 8.4.0

ECS .NET assembly version (e.g. 1.4.2): 8.4.0-alpha3

Elasticsearch version (if applicable): -

.NET framework / OS: .NET 6 / linux

Description of the problem, including expected versus actual behavior: I've been using the 1.6.0-alpha release of the Elasticsearch.Extensions.Logging library for some time now. But when I wanted to try out the latest 8.4.0-alpha3 version I found a few issues/differences:

1. When you have a log message within a scope that uses a message format and args, the MessageTemplate parameter in the generated log document contains the template for the scope instead of the template of the log message

ILogger logger;

var x = "value";
using (var scope = logger.BeginScope("My Scope {Value}", x)
{
    logger.LogInformation("Log Message"); // The "message" property will contain "Log Message", but the "MessageTemplate" property contains "My Scope {Value}"
}

2. The value contained in the "Scopes" property no longer contains the ToString() representation of the scope message format + args. So when using logger.BeginScope("My Scope {Value}", x) in the previous example, in the 1.6.0-alpha version the generated log document contains "Scopes": ["My Scope value"], but in the latest version it contains "Scopes": ["[x, value], [{OriginalFormat}, My Scope {Value}]"]

3. In some cases I get a null reference exception originating from the AssignKnownHttpKeys method in the ElasticsearchLogger class, I believe this is because of the following part initializing the Http property if it is null but then assigning to a property on the Url property: https://github.com/elastic/ecs-dotnet/blob/af53e3cdb8d00858b62f6d071b442163a9bfad9f/src/Elasticsearch.Extensions.Logging/ElasticsearchLogger.cs#L147-L148

4. If I have a log message which has a Timestamp placeholder with DateTimeOffset parameter it would convert the parameter to a string and then try to set the @timestamp property on the log event based on that parameter, which throws an exception because the expected format is different from the format that was used in the ToString. I would expect that it just set the labels.Timestamp field instead of trying to overwrite the @timestamp field.

   logger.LogInformation("My Timestamp {Timestamp}", DateTimeOffset.Now.AddDays(-1));

Steps to reproduce:

1. Use the provided examples in the 8.4.0-alpha3 version and check the document that is indexed.

[Mpdreamz]

1. we have an integration test asserting MessageTemplate inside scopes.. However its not templated. Added another test to ensure we prefer the logs OriginalFormat over the scope's provided OriginalFormat

2. Thank you for this I added a few changes here that would include the type name of injected scope objects this broke Formatted strings through. https://github.com/elastic/ecs-dotnet/pull/290 addresses both points 1 and 2.

3. Thank you for catching this rather preventable NRE opened: https://github.com/elastic/ecs-dotnet/pull/289

4. This is currently by design, the abbility to asign ECS fields from message templates. Will investigate what can be done to remedy this.

[sleeuwen]

Thanks for the quick fixes!

re 4. If it's by design then that's fine for me, the only thing is then that if you try to set the timestamp currently it can't because the parameter is first serialized to a string with a format, and when trying to set it into the @timestamp field it is parsed with a different format so currently you can't use a DateTimeOffset to set the Timestamp field. I've changed my code to use a different name than Timestamp as I wanted to have it as a label anyway so it's not a problem for me.

as far as I'm concerned this issue can be closed, unless you want to keep it open for the timestamp issue.