elastic / ecs-dotnet

https://www.elastic.co/guide/en/ecs-logging/dotnet/current/setup.html
Apache License 2.0
108 stars 54 forks source link

[BUG] Message generator property names are case sensitive #397

Open thompson-tomo opened 1 month ago

thompson-tomo commented 1 month ago

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

Elastic.CommonSchema.NLog = 8.11.0 but all will be impacted

.NET framework / OS: Net 8

Description of the problem, including expected versus actual behavior: When i pass ECS document fragments into my log generator the properties are only being added under label/metadata. But if i pass the same class with the property names changed to lowercase, the properties are overridden.

Steps to reproduce:

  1. Create a log generator

    public static partial class Log
    {
        [LoggerMessage(Level = LogLevel.Information, Message = "Message")]
        public static partial void UpdateEcsDocument(
            this ILogger logger, [LogProperties(SkipNullProperties = true)] Elastic.CommonSchema.Event @event);
    }
  1. Call the log generator
    _logger.UpdateEcsDocument(new Elastic.CommonSchema.Event() { Timezone = "testing});
  2. Observe the properties from event which were set are added as "event.Timezone" to the labels property.
  3. Create own event class with property being called timezone
  4. Observe timezone is being updated

Notes The issue is set the AssignField Method within the EcsDocument is performing a case sensitive comparison hence properties are not being found and results in metadata/labels being used.

thompson-tomo commented 1 month ago

@Mpdreamz any chance you could take a quick look at removing the case sensitive as it will be a massive gain for me and the library.

Mpdreamz commented 1 month ago

I took a slightly different approach and opened: https://github.com/elastic/ecs-dotnet/pull/401

To allow ECS entities to be logged directly, they will always overwrite what was set previously.

thompson-tomo commented 1 month ago

Thanks @Mpdreamz appreciate the extensive changes. I assume that if the message generator has ignore null properties, then it won't override it?

thompson-tomo commented 1 month ago

Another question is will the solution you have implemented support nested objects which was a big limitation with removing the case sensitivity logic?