Personally, I think you can make your life easier by not trying to replace JsonLayout. Then you don’t have to worry about backward compatibility. Alternatively, you could replace JsonLayout with a builder that uses the existing configuration elements but creates your new Layout. If you do that though the generated JSON would want to be the same as the current Layout (except I believe there is currently a bug in the JSON output).
If you just provide an “enhanced” JSON Layout and just have it be in addition to the existing one then the hardest thing you will have to do is decide on the name. Ralph > On Jan 21, 2020, at 2:54 AM, Volkan Yazıcı <[email protected]> wrote: > > While replacing JsonLayout, I am a little bit puzzled by a certain issue, > which I will try to exemplify below: > > LogstashLayout.Builder contains > - @PluginBuilderAttribute("prettyPrint") boolean prettyPrintEnabled; > - void isPrettyPrintEnabled() > - Builder setPrettyPrintEnabled(boolean prettyPrintEnabled) > > JsonLayout.Builder inherits from AbstractJacksonLayout.Builder > - @PluginBuilderAttribute boolean compact; > - void isCompact() > - Builder setCompact(boolean compact) > > Here I want to stick to the "pretty print" term rather than "compact", > since the former is clear in intent and widely used in JSON serialization > contexts. That said, the new builder should preserve the > backward-compatibility too. Here is my tentative attack plan: > > JsonLayout.Builder (extends from AbstractStringLayout.Builder) > - @PluginBuilderAttribute("prettyPrint") boolean prettyPrintEnabled; > - void isPrettyPrintEnabled() > - Builder setPrettyPrintEnabled(boolean prettyPrintEnabled) > - @Deprecated @PluginBuilderAttribute boolean compact; > - @Deprecated void isCompact() > - Builder setCompact(boolean compact) > > There are a couple of caveats with this approach and the ones I was able to > notice are: > > - The new JsonLayout.Builder doesn't extend from > AbstractJacksonLayout.Builder, but its parent > AbstractStringLayout.Builder. This is a deliberate decision > since I wanted to move away from AbstractJacksonLayout, > which is used for something totally different. Existing code > that is expecting a AbstractJacksonLayout.Builder will break. > > - Given we accept two attributes (prettyPrint & compact) to denote > the same thing, how shall I decide on which one to use? To the best > of my knowledge, at build()-site, there is no way to figure out which > one is configured by user and which one just points to its default. > > I would appreciate some help here. > > Further, these (and some other) issues put me down and make me consider > introducing a *new* JsonTemplateLayout and there provide JSON templates for > both existing GELF and JSON layouts. This will make the implementation > simpler and not induce any backward-compatibility issues. It will just be a > matter of users migrating to it. WDYT?
