abstractdog commented on a change in pull request #98: URL: https://github.com/apache/tez/pull/98#discussion_r571444465
########## File path: tez-common/src/main/java/org/apache/tez/util/LoggingUtils.java ########## @@ -0,0 +1,147 @@ +/** +* Licensed to the Apache Software Foundation (ASF) under one +* or more contributor license agreements. See the NOTICE file +* distributed with this work for additional information +* regarding copyright ownership. The ASF licenses this file +* to you under the Apache License, Version 2.0 (the +* "License"); you may not use this file except in compliance +* with the License. You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +package org.apache.tez.util; + +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.util.Arrays; +import java.util.Hashtable; + +import org.apache.hadoop.conf.Configuration; +import org.apache.log4j.helpers.ThreadLocalMap; +import org.apache.tez.dag.api.TezConfiguration; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class LoggingUtils { + private static final Logger LOG = LoggerFactory.getLogger(LoggingUtils.class); + + private LoggingUtils() {} + + @SuppressWarnings("unchecked") + public static void initLoggingContext(ThreadLocalMap threadLocalMap, Configuration conf, + String dagId, String taskAttemptId) { + Hashtable<String, String> data = (Hashtable<String, String>) threadLocalMap.get(); + if (data == null) { + data = new NonClonableHashtable<String, String>(); + threadLocalMap.set(data); + } + data.put("dagId", dagId == null ? "" : dagId); + data.put("taskAttemptId", taskAttemptId == null ? "" : taskAttemptId); + + String[] mdcKeys = conf.getStrings(TezConfiguration.TEZ_MDC_CUSTOM_KEYS, + TezConfiguration.TEZ_MDC_CUSTOM_KEYS_DEFAULT); + + if (mdcKeys.length == 0) { + return; + } + + String[] mdcKeysValuesFrom = conf.getStrings(TezConfiguration.TEZ_MDC_CUSTOM_KEYS_VALUES_FROM, + TezConfiguration.TEZ_MDC_CUSTOM_KEYS_VALUES_FROM_DEFAULT); + LOG.info("MDC_LOGGING: setting up MDC keys: keys: {} / conf: {}", Arrays.asList(mdcKeys), + Arrays.asList(mdcKeysValuesFrom)); + + int i = 0; + for (String mdcKey : mdcKeys) { + // don't want to fail on incorrect mdc key settings, but warn in app logs + if (mdcKey.isEmpty() || mdcKeysValuesFrom.length < i + 1) { + LOG.warn("cannot set mdc key: {}", mdcKey); + break; + } + + String mdcValue = mdcKeysValuesFrom[i] == null ? "" : conf.get(mdcKeysValuesFrom[i]); + // MDC is backed by a Hashtable, let's prevent NPE because of null values + if (mdcValue != null) { + data.put(mdcKey, mdcValue); + } else { + LOG.warn("MDC_LOGGING: mdc value is null for key: {}, config key: {}", mdcKey, + mdcKeysValuesFrom[i]); + } + + i++; + } + } + + public static String getPatternForAM(Configuration conf) { + return conf.get(TezConfiguration.TEZ_LOG_PATTERN_LAYOUT_AM, null); + } + + public static String getPatternForTask(Configuration conf) { + return conf.get(TezConfiguration.TEZ_LOG_PATTERN_LAYOUT_TASK, null); + } + + /** + * This method is for setting a NonClonableHashtable into log4j's mdc. Reflection hacks are + * needed, because MDC.mdc is well protected (final static MDC mdc = new MDC();). The logic below + * is supposed to be called once per JVM, so it's not a subject to performance bottlenecks. For + * further details of this solution, please check NonClonableHashtable class, which is set into + * the ThreadLocalMap. A wrong outcome of this method (any kind of runtime/reflection problems) + * should not affect the DAGAppMaster/TezChild. In case of an exception a ThreadLocalMap is + * returned, but it won't affect the content of the MDC. + */ + @SuppressWarnings("unchecked") + public static ThreadLocalMap setupLog4j() { + ThreadLocalMap mdcContext = new ThreadLocalMap(); + mdcContext.set(new NonClonableHashtable<String, String>()); + + try { + final Constructor<?>[] constructors = org.apache.log4j.MDC.class.getDeclaredConstructors(); + for (Constructor<?> c : constructors) { + c.setAccessible(true); + } + + org.apache.log4j.MDC mdc = (org.apache.log4j.MDC) constructors[0].newInstance(); + Field tlmField = org.apache.log4j.MDC.class.getDeclaredField("tlm"); + tlmField.setAccessible(true); + tlmField.set(mdc, mdcContext); + + Field mdcField = org.apache.log4j.MDC.class.getDeclaredField("mdc"); + mdcField.setAccessible(true); + + Field modifiers = Field.class.getDeclaredField("modifiers"); + modifiers.setAccessible(true); + modifiers.setInt(mdcField, mdcField.getModifiers() & ~Modifier.FINAL); + + mdcField.set(null, mdc); + + } catch (Exception e) { + LOG.warn("Cannot set log4j global MDC, mdcContext won't be applied to log4j's MDC class", e); + } + + return mdcContext; + } + + /** + * NonClonableHashtable is a special class for hacking the log4j MDC context. By design, log4j's + * MDC uses a ThreadLocalMap, which clones parent thread's context before propagating it to child Review comment: unfortunately, isThreadContextMapInheritable is slightly different from what I needed: ``` Set the system property `log4j2.isThreadContextMapInheritable` to `true` to enable child threads to inherit the Thread Context Map. ``` this is used for inheritance, which is needed, child thread inherits parent's context, this is fine, the problem is that it inherits **and clones** the map, which is against my implementation... my implementation makes it possible to define a global MDC context in DAGAppMaster and TezChild in the main thread, and all threads inherit that, but when a new dag (DAGAppMaster) or task attempt (TezChild) comes, modifying the global thread's context should have an effect on all threads' context, but with cloning, it won't have: https://github.com/apache/log4j/blob/trunk/src/main/java/org/apache/log4j/helpers/ThreadLocalMap.java#L34-L41 instead, I choose to define 1 context in the main thread and propagating it to all child threads (which is automatic due to ThreadLocalMap behavior by default), but I only need to init the logging context once for every dag/taskattempt. Without cloning, a single change will change the MDC contents of all threads in the JVM. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
