mattwcole / gelf-extensions-logging

GELF provider for Microsoft.Extensions.Logging
MIT License
109 stars 42 forks source link

Support key customization for optional fields. #72

Closed ejball closed 1 year ago

ejball commented 1 year ago

We would like more precise control of how the optional fields are logged (logger, exception, event_id, and event_name). This change allows those fields to be renamed or omitted entirely.

mattwcole commented 1 year ago

Hi, I think we can make something like this work. Do you really need precise control over the fields, or would a general format option like this do the job?

builder.Logging.AddGelf(options =>
{
    options.LogSource = "MyApp";
    options.BuiltInFieldsFormat = GelfFieldsFormat.PascalCase;
});
ejball commented 1 year ago

Great question! That wouldn't meet our needs, no. Our specific requirements are to rename Logger, prettify the Exception call stack, and omit the two event fields.

mattwcole commented 1 year ago

OK I think understand the requirement. I might add support for this in a slightly different way to how you have done it.

If we add a new option IncludeDefaultFields and tweak AdditionalFieldsFactory, would something like this work?

builder.Logging.AddGelf(options =>
{
    options.LogSource = "MyApp";
    options.IncludeDefaultFields = false;
    options.AdditionalFieldsFactory = logContext => new Dictionary<string, object>
    {
        ["MyLogLevel"] = logContext.LogLevel,
        ["MyLoggerName"] = logContext.Logger,
        ["MyException"] = FormatMyException(logContext.Exception)
    };
});
ejball commented 1 year ago

Yes, that would definitely work. That design was actually my first thought, but I wasn't sure about introducing a breaking change to AdditionalFieldsFactory. Introducting a log context will at least protect it from needing another breaking change in the future.

mattwcole commented 1 year ago

Great, let's go with the change to AdditionalFieldsFactory. I'll gladly accept another PR, or if you can wait a week or so I can put something together.

ejball commented 1 year ago

Replaced by #73.