Make MdcJsonProvider.writeMdcEntry() protected

[jug]

Follow up of #957 to make the writeMdcEntry() method protected.

In my former proposal the intention for the protected on the method in question was to allow an easier way to write a custom extension by extending from MdcJsonProvider and only overwriting what one wants to override.

Though there's no obligation to do so for a custom handler, it would be nicer as it already provides useful functions that a custom MdcJsonProvider would need as well.

A dependency is there regardless if the methods are private or protected. However, opening up some of the (private) methods would make customer handlers easier as some of the inner framework methods are very hard to reach otherwise.

Having it private would enforce to copy a redundant version of some of the inner workings. Though from the perspective of the framework, the inner workings are often black-boxed. But from the perspective of a client writing a custom handler it's more cumbersome to rewrite part of the code to fulfill a similar or exact purpose.

[codecov]

Codecov Report

Merging #973 (5e379c3) into main (1ac92af) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main     #973   +/-   ##
=========================================
  Coverage     71.78%   71.78%           
  Complexity     1417     1417           
=========================================
  Files           177      177           
  Lines          5178     5178           
  Branches        534      534           
=========================================
  Hits           3717     3717           
  Misses         1197     1197           
  Partials        264      264           

Impacted Files	Coverage Δ
...ogback/composite/loggingevent/MdcJsonProvider.java	87.27% <ø> (ø)

[philsttr]

It's not clear to me why logstash-logback-encoder needs to provide two extension points for customizing the writing of MDC entries:

1. override MdcJsonProvider.writeMdcEntry
   AND
2. custom implementation of MdcEntryWriter

What can be done with overriding MdcJsonProvider.writeMdcEntry that can't be done with a custom implementation of MdcEntryWriter ?

For background, I prefer to keep the internal logic as encapsulated as possible to allow the library to evolve without constantly breaking compatibility for folks trying to reach into the guts of the library. There are well defined extension points that are kept compatible (In this case, JsonProvider and MdcEntryWriter). MdcJsonProvider was not really meant to be extended. In fact, I overlooked that the fields switched from private to protected in the previous change. Now they can never change without breaking compatibility, which puts an additional maintenance burden on the library by making future changes more difficult.

I realize it's a tradeoff, because you might need to write a little bit more code, but that code will be much more resilient to future library changes if it sticks to the public interfaces.

[jug]

@philsttr

philsttr> It's not clear to me why logstash-logback-encoder needs to provide two extension points for customizing the writing of MDC entries: 1. override MdcJsonProvider.writeMdcEntry AND 2. custom implementation of MdcEntryWriter [...]
philsttr> I overlooked that the fields switched from private to protected in the previous change. Now they can never change without breaking compatibility [...]

Sorry, the private-to-protected change was the 1st commit in the other pull request (178b29e).

It was trying to open an easy way to some of the inner methods before I came up with the idea of the MDC entry writers, so that a new custom MdcJsonProvider was possible without having to rewrite/copy the writeTo()-function only manipulating the way of the output.

I have to admit I'm not used to write or contribute to frameworks (yet).

Wouldn't it be possible to directly follow-up with a change back to private again (as it was an "accident")? If it comes shortly after the last release, clients tend to use the higher available release version ... so then not having much time to use the "exposed" protected methods yet.

philsttr> For background, I prefer to keep the internal logic as encapsulated as possible to allow the library to evolve without constantly breaking compatibility for folks trying to reach into the guts of the library.

Sure, that's natural ... I think I can quote from "Clean Code" ;-)

Clean Code, chapter 8 "Using Third-Party Code"> "There is a natural tension between the provider of an interface and the user of an interface. [...] Users [...] want an interface that is focused on their particular needs."

As you've said it's a trade-off, but also a matter of personal preference focusing on ones own need more than the others point of view, in the tension field between framework provider and framework user.

I can certainly also see your point.

I suggest to change the modifiers back to private if you prefer, just wanted to give the background thinking behind it.

[philsttr]

Closing this in favor of using a custom MdcEntryWriter