fluency icon indicating copy to clipboard operation
fluency copied to clipboard

Provide an additional way to configure the RecordFormatter when using the FluencyBuilderForFluentd

Open frosiere opened this issue 2 years ago • 4 comments

In some situations, it may be required to customize the RecordFormatter (additional Jackson modules, etc). There is currently a method taking a RecordFormatter and an Ingester (FluencyBuilderForFluentd#buildFromIngester(RecordFormatter recordFormatter, Ingester ingester)) which is nice is some cases.

The configuration becomes complex and requires a bunch of copy/paste when used in combination with the FluencyLogbackAppender

Therefore, to ease the configuration, and dissociate the customization of the recorder from the ingester, it would be great to update the FluencyBuilderForFluentd as follow

public class FluencyBuilderForFluentd {
   ...
   private RecordFormatter recordFormatter = new FluentdRecordFormatter(); // default recorder like now - see buildRecordFormatter
   
   public void setRecordFormatter(RecordFormatter recordFormatter) {
      this.recordFormatter = recordFormatter;
   }
   ...
}

At appender level, this would allow the following customization

public class CustomFluencyLogbackAppender extends FluencyLogbackAppender<ILoggingEvent> {

    @Override
    protected FluencyBuilderForFluentd configureFluency() {
        final var builder = super.configureFluency();
        final var config = new FluentdRecordFormatter.Config();
        config.setJacksonModules(Lists.newArrayList(
                new Jdk8Module(),
                new JavaTimeModule()
        ));
        builder.setRecordFormatter(new FluentdRecordFormatter(config));
        return builder;
    }
}

Any other suggestions to support this use case would be appreciated.

Thanks a lot for your help and for the Fluency support.

frosiere avatar Jan 06 '22 09:01 frosiere

@frosiere Even with setRecordFormatter(), it looks still a bit complicated to inject Jackson modules. And I have another concern that a wrong RecordFormatter can be passed (e.g. passing JsonlRecordFormatter to FluencyBuilderForFluentd.)

How about this?

public class CustomFluencyLogbackAppender extends FluencyLogbackAppender<ILoggingEvent> {

    @Override
    protected FluencyBuilderForFluentd configureFluency() {
        final var builder = new FluencyBuilderForFluentd();
        // "setJacksonModulesForRecordFormatter" is too verbose...?
        builder.setJacksonModulesForRecordFormatter(Lists.newArrayList(
            new Jdk8Module(),
            new JavaTimeModule()
        ));
        return builder;
    }
}

komamitsu avatar Jan 10 '22 12:01 komamitsu

Good point, proposal is nice and prevents using a wrong formatter. So, this may be the way to go.

An alternative would either be a method taking the FluentdRecordFormatter like builder.setRecordFormatter(FluentdRecordFormatter formatter) Or a method taking the record formatter config like builder.setRecordFormatterConfig(RecordFormatter.Config config) This would support possible extensions of the config in future releases.

From a more general point of view, only the solution exposing the formatter gives a way to "setup" (enable/disable feature, etc) the internal ObjectMapper by overriding the method AbstractRecordFormatter#registerObjectMapperModules.

Thanks a lot for your reply.

frosiere avatar Jan 10 '22 15:01 frosiere

Yeah, setRecordFormatter() is more powerful. I'll go with it. I'm a bit busy these days and it would take some time until the feature is released.

komamitsu avatar Jan 11 '22 13:01 komamitsu

Perfect. If I drop a PR, would it be possible to have a 2.6.1 release soon? Thanks a lot.

frosiere avatar Jan 11 '22 15:01 frosiere