[JAMES-3587] Deprecate MDCBuild::addContext

• Type: Improvement
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 3.6.0
• Fix Version/s: 3.7.0
• Component/s: James Core
• Assignee: Unassigned
• Reporter: Benoit Tellier
• Created: 19/May/21 14:15
• Updated: 25/May/21 06:50
• Resolved: 25/May/21 06:50

Description

Why?

MDCBuilder::addContext does an implicit call to Object::toString

There are two reasons:

• 1. Formatting. For instance ` {"user":"btellier@erugerkuf"}

  ` instead of simply `btellier@erugerkuf`.

• 2. Performance.

See the attached flame graphs: James spends ~1.20% of CPU calling Object::toString which correlate closely to MDCBuilder::addContext. This crosses the "valuable to optimize" threashold. Especially calls to MoreObject::toStringHelper(this) are not neglictible.

h3 How?

Deprecate the current API and replace it by an API that explicitly requires String or Optional<String> so that the callers understand that they have to pass a string.

If they decide to pass toString then they need to explicitly call it, and be fully aware of it. Nothing hidden anymore.

On the philosophy this mimics SLF4J MDC API.

Adapt all calls made in James code base to use this newer API.

Also, StructuredLogger API needs to be adapted too.

I don't think we should remove the deprecated calls

Issue Links

• links to: GitHub Pull Request #442