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

JsonSerializerOptions - Added JsonStringEnumConverter #380

Closed snakefoot closed 2 months ago

snakefoot commented 2 months ago

Makes easier for humans to query data in Elastic when Enum-Properties are saved as String-Values (instead of numeric values)

sergiojrdotnet commented 2 months ago

@snakefoot do you know of any alternative methods to resolve this? I tried to set a custom event writer, but the JsonSerializerOptions isn't passed from EcsDocumentJsonConverter to EcsJsonConverterBase

snakefoot commented 2 months ago

Not sure why JsonStringEnumConverter is not enabled by default in Ecs-Dotnet, as it would give a better user-experience. Maybe @Mpdreamz knows why it is important that Enum-values are represented using their integer value?

Made a work-around with #369, where it adds basic conversion of Enum-properties for NLog:

https://github.com/elastic/ecs-dotnet/blob/8136f2ccfd90793fa3d6cff04faea1af64d86f1e/src/Elastic.CommonSchema.NLog/EcsLayout.cs#L729-L739

But this will not help on complex objects, with nested Enum-properties.

Mpdreamz commented 2 months ago

Its not important for storage in Elasticsearch. I just wanted to rock with as plain of System.Text.Json configuration as possible.

E.g its not the default for System.Text.Json so it isn't for us either.

I think the main reason STJ doesn't is that it allows you to refactor enum names freely if you are rigorous enough with each enums integer value.

I tried to set a custom event writer, but the JsonSerializerOptions isn't passed from EcsDocumentJsonConverter to EcsJsonConverterBase

I think this is to prevent recursion into the same writer but let me double check.

sergiojrdotnet commented 2 months ago

I think this is to prevent recursion into the same writer but let me double check.

As far as I can see, they are independent instances. I've made the proposal to #388