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?
