elastic / ecs-dotnet

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

NLog EcsLayout - Added support for ProcessThreadName + MessageTemplate #348

Closed snakefoot closed 5 months ago

snakefoot commented 9 months ago

"Original"-field was removed from "Log"-section with #194 . Now trying to restore it as "MessageTemplate" property.

Curious where the recommended place to put the original "MesageTemplate". See also: https://discuss.elastic.co/t/log-original-field-lost-with-upgrade-8-6-1-from-1-5-3/348363

Have tried to follow the Elastic Extensions logging, so "MesageTemplate" becomes additional property. See also: https://www.elastic.co/guide/en/ecs-logging/dotnet/current/extensions-logging-data-shipper.html#_example_document

snakefoot commented 9 months ago

@Mpdreamz Really confused by this logic in PropDispatch.SetMetaOrLabel:

        public static void SetMetaOrLabel(EcsDocument document, string path, object value)
        {
            switch (value)
            {
                case string s:
                    document.Labels ??= new Labels();
                    document.Labels[path] = s;
                    break;
                case bool b:
                    document.Labels ??= new Labels();
                    document.Labels[path] = b.ToString(CultureInfo.InvariantCulture).ToLowerInvariant();
                    break;
                default:
                    document.Metadata ??= new MetadataDictionary();
                    document.Metadata[path] = value;
                    break;
            }
        }

Why does the field-datatype control whether it is MetaData or Labels ?

Notice NLog Layout has support for explict specifying both:

      <layout xsi:type="EcsLayout">
        <metadata name="MyProperty" layout="MyPropertyValue" /> <!-- repeated, optional -->
        <label name="MyLabel" layout="MyLabelValue" />          <!-- repeated, optional -->
        <tag layout="MyTagValue" />                             <!-- repeated, optional -->
      </layout>

Think it is confusing that if you have explictly specified <metadata>, then output becomes a label because datatype became string/bool.

Mpdreamz commented 5 months ago

@snakefoot this is to align with the ECS guidelines around specifying unknown fields: https://www.elastic.co/guide/en/ecs/current/ecs-custom-fields-in-ecs.html#_the_labels_field

labels is explicitly a Dictionary<string, string> metadata is a .NET extension to store unknown complex fields.

This is a step to move to labels.

I do think labels needs to allow complex fields in the ECS spec and be mapped as https://www.elastic.co/guide/en/elasticsearch/reference/current/flattened.html

However in that case in the long run we might run into a separation of labels into numeric_labels https://github.com/elastic/apm-server/issues/5963.

Will follow up in the ECS repository to discuss long term plans there.

Mpdreamz commented 5 months ago

run docs-build