qos-ch / logback

The reliable, generic, fast and flexible logging framework for Java.
http://logback.qos.ch
Other
2.97k stars 1.28k forks source link

No way to replace MDCAdapter when using Logback #799

Closed Yaytay closed 4 months ago

Yaytay commented 4 months ago

What I want to do is to have Vertx-context-specific data included in output from logback's json encoder. Vertx contexts are not thread bound, so I cannot use a thread-local MDC implementation.

It's easy to create an MDC adapter that stores data in the Vertx context, but there doesn't seem to be any way to make that available to both logback and slf4j. I can use loggerContext.setMDCAdapter, but this doesn't change the instance returned by LogbackServiceProvider.getMDCAdapter, so slf4j puts all the data into the logback MDCAdapter even though logback is reading from my MDCAdapter.

I also thought about subclassing LogbackServiceProvider, but then I'd have two providers. :(

I think the best solution would be to have org.slf4j.MDC.setMDCAdapter (or, to make that less public, another class in org.slf4j?) But equally I could solve my actual problem by having more flexibility in the Json encoder (a way to register a provider of key value pairs with a name?).

ceki commented 4 months ago

@Yaytay Logback version 1.5.1 allows setting and resetting the mdcAdapter whereas earlier versions did not.

Several users have asked to customize mdcAdapter functionality either by setting a new mdcAdapter in MDC (overriding the previous mdcAdapter), or by adding an extension to the existing mdcAdapter, that is to chain mdcAdapter instances.

Chaining mdcAdapterr instances would work nicely in case the additional instances can obtain their data from somewhere, for example from the thread context. However, in your use case, that is Vertx-context-spefic data, how would the relevant mdcAdaper obtain its data? Is there something like the following avaliable?

Context context = Vertx.currentContext();

Do not hesitate to provide sample code.

Yaytay commented 4 months ago

I'm using v1.5.3. My current state is that I can change the MDCAdapter that logback uses, but I can't find a way to change the one that slf4j uses, so logback is pulling from vertx context, but slf4j isn't writing to vertx context.

And now I've just realised I'm being a bit stupid: I don't need to use org.slf4j.MDC to write, I can write directly to my own MDCAdapter, and if you have chaining then users would have to know which MDCAdapter they wanted to write to. So I think chaining would work for me (and the current API works for me as long as no-one tries to write to org.slf4j.MDC).

This is an example of part of my current VertxMDCAdapter.

  private static final String VERTX_KEY = VertxMDCAdapter.class.getCanonicalName();
  public static final VertxMDCAdapter INSTANCE = new VertxMDCAdapter();

  private static class MdcData {
    private final Map<String, String> mdcReadWriteMap = new HashMap<>();
    private final Map<String, Deque<String>> mdcMapOfStacks = new HashMap<>();    
    private Map<String, String> mdcReadOnlyMap;
  }

  private MdcData getMdcData() {
    Context context = Vertx.currentContext();
    if (context != null) {
      return context.getLocal(VERTX_KEY);      
    }
    return null;
  }

  private static MdcData getOrCreateMdcData() {
    Context context = Vertx.currentContext();
    if (context != null) {
      MdcData data = context.getLocal(VERTX_KEY);
      if (data == null) {
        data = new MdcData();
        context.putLocal(VERTX_KEY, data);
      }
      return data;
    }
     return null;
  }

  @Override
  public void put(String key, String value) {
    MdcData data = getOrCreateMdcData();
    if (data != null) {
      data.mdcReadWriteMap.put(key, value);
      data.mdcReadOnlyMap = null;
    }
  }

  @Override
  public String get(String key) {
    MdcData data = getMdcData();
    if (data != null) {
      return data.mdcReadWriteMap.get(key);
    }
    return null;
  }
Yaytay commented 4 months ago

I'm happy to close this, I can work with what's there. If you'd like me to raise a more specific issue about chaining adapters I can do so, or I can just leave it with you if it's already in your plan.

ceki commented 4 months ago

@Yaytay org.slf4j.MDC delegates to the MDCAdapter provided by the org.slf4j.spi.SLF4JServiceProvider instance that was installed during the initialization of org.slf4.LoggerFactory.

If the provider is ch.qos.logback.classic.spi.LogbackServiceProvider the MDCAdapter used by SLF4J and logback will be the same instance. However, if the org.slf4j.spi.SLF4JServiceProvider instance is from a different logging backend, then overriding logback's MDCAdapter will not help as logging will go to a different logging backend by definition.

In short, there is not difference between slf4j's MDC and logback MDCAdapter if logback furnishes the org.slf4j.spi.SLF4JServiceProvider via a ch.qos.logback.classic.spi.LogbackServiceProvider instance.

Yaytay commented 4 months ago

The problem is that slf4j gets the MDCAdapter from logback when it starts up, which is the first time any static LoggerFactory is seen, which is before there is any opportunity to replace it. So almost any use of loggerContext.setMDCAdapter will result in logback having a different MDCAdapter from slf4j. Chaining will fix this, and until then I'm happy to explicitly use my own MDC instead of the one from slf4j.