javaexceptionloggingthread-localmdc

Is exception handling mandatory in MDC.remove() or MDC.put()?


import org.slf4j.MDC;

            MDC.put(ID1, id1);
        MDC.put(ID2, id2);
        MDC.put(ID3, id3);
        try {
              return joinPoint.proceed();
        } finally {
            MDC.remove(ID1);
            MDC.remove(ID2);
            MDC.remove(ID3);
        }
        MDC.put(ID1, id1);
        MDC.put(ID2, id2);
        MDC.put(ID3, id3);
        try {
              return joinPoint.proceed();
        } finally {
                try{
                     MDC.remove(ID1); 
                } finally {
                           MDC.remove(ID2);
                   MDC.remove(ID3);
                }
           
          
        }

If there's anything lacking in the question, please point it out. I'll reflect it and upload it again


Solution

  • I assume that you are actually asking about the correct way to do the removals.

    If we assume that any of the put or remove calls can throw an exception, AND that not tidying up the MDC is unacceptable, then the correct way to write this would be:

    try {
        MDC.put(ID1, id1);
        try {
            MDC.put(ID2, id2);
            try {
                MDC.put(ID3, id3);
                return joinPoint.proceed();
            } finally {
                MDC.remove(ID3);
            }
        } finally {
            MDC.remove(ID2);
        }
    } finally {
        MDC.remove(ID1);
    }
    

    You can simplify this by using putCloseable instead of put as follows:

    try (
        MDC.MDCCloseable c1 = MDC.putCloseable(ID1, id1);
        MDC.MDCCloseable c2 = MDC.putCloseable(ID2, id2);
        MDC.MDCCloseable c3 = MDC.putCloseable(ID3, id2);
    ) {
        return joinPoint.proceed();
    }
    

    The putCloseable variant returns an adapter that implements Closeable where the close() method performs the required remove.

    On the other hand, if we treat the MDC javadocs as defining the behavior, then the only case where remove is documented to throw an exception is when the key is null. You should be able to guarantee that ID1, ID2 and ID3 are not null. If you can, the above code is unnecessary; i.e. your original version with a single finally should be fine.