ecs-dotnet icon indicating copy to clipboard operation
ecs-dotnet copied to clipboard

[FEATURE] Allow custom JSON converter

Open cwuethrich opened this issue 4 years ago • 4 comments

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

Is your feature request related to a problem? Please describe. As many other companies as well, we do have a own implementation of a Date type. This type should be serialized in ISO date format (yyyy-MM-dd). The type is implemented in .Net Standard and we also use it in projects with .Net Framework 4.7.2. Because of the .Net Framework and older projects without nuget references, we can't add a json converter attribute to specify a own implementation of a converter. All the implementations related to the json serialization are implemented as internal. Even the JsonSerializerOptions is internal and we can't extend it with our own converter.

Describe the solution you'd like Json serialization should be public or at least the serializer options should be a public class.

public static class JsonConfiguration
{
	public static readonly JsonSerializerOptions SerializerOptions = new JsonSerializerOptions
	{
		...
	};
	...
}

With this change we could easily add our own converter.

public class ContosoEcsTextFormatter : EcsTextFormatter
{
	static ContosoEcsTextFormatter() => JsonConfiguration.SerializerOptions.Converters.Add(new MyConverter());
	...
}

Or we can use the converter of this library in own serializations.

  var serializerOptions= new JsonSerializerOptions(JsonConfiguration.SerializerOptions)
  {
      Converters =
      {
          new MyConverter()
      }
  };

Describe alternatives you've considered An other way to get this serializer options could be a method on the Base class.

public partial class Base
{
	public virtual JsonSerializerOptions GetSerializerOptions() => JsonConfiguration.SerializerOptions;
	...
}

Additional context

What do you think about this change? Is it worth it to create a pull request?

cwuethrich avatar Oct 28 '21 15:10 cwuethrich

+1 on this request. We want to format our log fields following the standard here: https://www.elastic.co/guide/en/ecs/master/ecs-tracing.html To be more specific, we want to get trace.id and span.id fields, but the current ecs-dotnet implementation formats the fields to trace_id and span_id because it's using snake case and doesn't allow any overwrites for the serializationOptions.

Allowing custom Json converter solves this issue for us.

zzou-cn avatar Jan 25 '22 01:01 zzou-cn

What's the appropriate way to push the feature request forward? Can we have an approver to take a look at this soon? thanks in advance

wanshoupu avatar Jan 25 '22 15:01 wanshoupu

I used a work around with reflection:

public class ContosoEcsTextFormatter : EcsTextFormatter
{
    private static bool _initialized = false;
    static ContosoEcsTextFormatter()
    {
        if (!_initialized)
        {
            _initialized = true;

            var type = typeof(Base).Assembly.GetType("Elastic.CommonSchema.Serialization.JsonConfiguration");
            var fieldInfo = type.GetField("SerializerOptions", BindingFlags.NonPublic | BindingFlags.Static);
            if (null != fieldInfo)
            {
                var options = fieldInfo.GetValue(null) as JsonSerializerOptions;
                if (null != options)
                {
                    options.Converters.Add(new DateConverter());
                }
            }
        }
    }
}

cwuethrich avatar Jun 20 '22 14:06 cwuethrich

@cwuethrich addressed this in #177 and this should ship as soon as we settle on #194 and #110

Mpdreamz avatar Jul 29 '22 12:07 Mpdreamz