markmcdowell / NLog.Targets.ElasticSearch

NLog target for Elasticsearch
MIT License
176 stars 89 forks source link

Consider allowing to optionally use Elastic.CommonSchema.NLog.EcsLayout for providing Elastic Common Schema support #122

Closed xprzemekw closed 4 years ago

xprzemekw commented 4 years ago

Hi,

It looks Elastic as a part of Elastic Common Schema ('ECS') effort introduced NLog support in form of specialized NLog Layout (named 'EcsLayout') that renders LogEventInfo structure to proper JSON representation (as per ECS definition) which is ready to be submitted to Elastic via their Elastic Low Level Client (check https://github.com/elastic/ecs-dotnet/tree/master/src/Elastic.CommonSchema.NLog and https://github.com/elastic/ecs-dotnet/tree/master/src/Elastic.CommonSchema).

Can this be considered as a proper way of supporting ECS in NLog.Targets.ElasticSearch?

It seems we would need just a pass through option where RenderLogEvent(Layout, logEvent) output is just sent to Elastic (assuming Layout is properly set to EcsLayout).

snakefoot commented 4 years ago

The library is still BETA-stage:

https://github.com/elastic/ecs-dotnet/pull/65

But yes when the library can be considered stable, then it would make sense to inject pure Json into the LowLevel-Client.

Please make a PR that can take the output from the Target.Layout and pass as raw Json to the LowLevel-Client. Then you can use this Target together with Elastic.CommonSchema.NLog right now:

<extensions>
    <add assembly="NLog.Targets.ElasticSearch"/>
    <add assembly="Elastic.Apm.NLog"/>
    <add assembly="Elastic.CommonSchema.NLog"/>
</extensions>
<targets>
   <target xsi:type="ElasticSearch" enableJsonLayout="true">
       <layout xsi:type="EcsLayout" />
   </target>
</targets>

Then users can decide if they want to use NLog JsonLayout (high performance) or Elastic EcsLayout (beta) or the LowLevel-Client-JsonSerializer (current)

snakefoot commented 4 years ago

@xprzemekw Have created #125 that allows you to use JsonLayout (or EcsLayout) for generating document-payload.

snakefoot commented 4 years ago

@markmcdowell When #125 has been released, then this issue can be resolved.

markmcdowell commented 4 years ago

Closed by #125 thanks to @snakefoot in v7.3.0

snakefoot commented 4 years ago

Remember to update release notes for 7.3.0 about change in default DocumentType from logevent to _doc