vy commented on PR #2419:
URL: https://github.com/apache/logging-log4j2/pull/2419#issuecomment-2034076685
**Disclaimer:** I did not completely read all the earlier discussions in
this PR.
### On using `ThreadLocal`s
I am against implementing `ScopedContext` using `ThreadLocal`s.
For one, Java introduced a specialized type for this: `ScopedValue`. It is
partially implemented in Java (in `Thread.java` and `ScopedValue.java`) and
partially in the JVM. See the following snippet from `Thread.java` of Java 22
sources:
```
// ScopedValue support:
@IntrinsicCandidate
static native Object[] scopedValueCache();
@IntrinsicCandidate
static native void setScopedValueCache(Object[] cache);
```
Second, as @rgoers is aware of, `ScopedContext` doesn't support thread jumps
out of the box, hence he introduced `ExecutorService`-specialized operators,
e.g., `R ScopedContext.callWhere(String,Object,ExecutorService,Callable<R>)`. I
can think of several examples where this won't be applicable:
```
ScopedContext.runWhere(food, "awesome", () -> {
aClassRunningCommandsInItsOwnExecutor.cook(() -> {
log.info("Food will not show up `awesome`.");
});
});
```
I find it unacceptable that we roll out our custom semi-working
`ScopedValue` using `ThreadLocal`s because we want to support Java 8. This is
exactly the reason we ended up having a `Supplier` in our code base.
### On the new `Renderable` interface
I am against adding yet another `String`-to-`Object` interface, I already
[stated this in the earlier
PR](https://github.com/apache/logging-log4j2/pull/2385/files#r1531746011):
> Please let's not introduce a yet another `Object`-to-`String` helper
interface ecosystem. I think `StringBuilderFormattable` fits the bill here.
@rgoers, responded as follows:
> How? There are no StringBuilders at all in ScopedContext. It is used to
convert an object into its String representation so it can be included in the
LogEvent's ContextMap as a single String item. While the Object's render method
certainly might use a StringBuilder internally the code storing the data into
the Map certainly doesn't want one.
Layout rendering the MDC/`ScopedContext`/etc. can take care of this. For
instance, `JsonTemplateLayout` already recursively walks over `Map`s and checks
if the entry value supports `StringBuilderFormattable`. Hence, no need to
introduce a new interface. Users who want to customize the output of their
MDC/`ScopedContext`/etc. values just need to extend from
`StringBuilderFormattable` and it will work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
