prasanthj commented on a change in pull request #98:
URL: https://github.com/apache/tez/pull/98#discussion_r567605909
##########
File path: tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java
##########
@@ -248,6 +248,42 @@ public TezConfiguration(boolean loadDefaults) {
public static final String TEZ_TASK_LOG_LEVEL = TEZ_TASK_PREFIX +
"log.level";
public static final String TEZ_TASK_LOG_LEVEL_DEFAULT = "INFO";
+ /**
+ * By this option, user can easily override the logging pattern which is
applied in
+ * TezContainerLogAppender in AM, regardless of the environmental settings.
+ */
+ @ConfigurationScope(Scope.AM)
+ @ConfigurationProperty
+ public static final String TEZ_LOG_PATTERN_LAYOUT_AM = TEZ_PREFIX +
"log.pattern.layout.am";
+
+ /**
+ * By this option, user can easily override the logging pattern which is
applied in
+ * TezContainerLogAppender in tasks, regardless of the environmental
settings.
+ */
+ @ConfigurationScope(Scope.AM)
+ @ConfigurationProperty
+ public static final String TEZ_LOG_PATTERN_LAYOUT_TASK = TEZ_PREFIX +
"log.pattern.layout.task";
Review comment:
Use TEZ_TASK_PREFIX instead?
##########
File path: tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java
##########
@@ -248,6 +248,42 @@ public TezConfiguration(boolean loadDefaults) {
public static final String TEZ_TASK_LOG_LEVEL = TEZ_TASK_PREFIX +
"log.level";
public static final String TEZ_TASK_LOG_LEVEL_DEFAULT = "INFO";
+ /**
+ * By this option, user can easily override the logging pattern which is
applied in
+ * TezContainerLogAppender in AM, regardless of the environmental settings.
+ */
+ @ConfigurationScope(Scope.AM)
+ @ConfigurationProperty
+ public static final String TEZ_LOG_PATTERN_LAYOUT_AM = TEZ_PREFIX +
"log.pattern.layout.am";
Review comment:
Use TEZ_AM_PREFIX instead?
##########
File path: tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java
##########
@@ -248,6 +248,42 @@ public TezConfiguration(boolean loadDefaults) {
public static final String TEZ_TASK_LOG_LEVEL = TEZ_TASK_PREFIX +
"log.level";
public static final String TEZ_TASK_LOG_LEVEL_DEFAULT = "INFO";
+ /**
+ * By this option, user can easily override the logging pattern which is
applied in
+ * TezContainerLogAppender in AM, regardless of the environmental settings.
+ */
+ @ConfigurationScope(Scope.AM)
+ @ConfigurationProperty
+ public static final String TEZ_LOG_PATTERN_LAYOUT_AM = TEZ_PREFIX +
"log.pattern.layout.am";
+
+ /**
+ * By this option, user can easily override the logging pattern which is
applied in
+ * TezContainerLogAppender in tasks, regardless of the environmental
settings.
+ */
+ @ConfigurationScope(Scope.AM)
Review comment:
Scope should be Vertex since this task specific config
##########
File path: tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java
##########
@@ -248,6 +248,42 @@ public TezConfiguration(boolean loadDefaults) {
public static final String TEZ_TASK_LOG_LEVEL = TEZ_TASK_PREFIX +
"log.level";
public static final String TEZ_TASK_LOG_LEVEL_DEFAULT = "INFO";
+ /**
+ * By this option, user can easily override the logging pattern which is
applied in
+ * TezContainerLogAppender in AM, regardless of the environmental settings.
+ */
+ @ConfigurationScope(Scope.AM)
+ @ConfigurationProperty
+ public static final String TEZ_LOG_PATTERN_LAYOUT_AM = TEZ_PREFIX +
"log.pattern.layout.am";
+
+ /**
+ * By this option, user can easily override the logging pattern which is
applied in
+ * TezContainerLogAppender in tasks, regardless of the environmental
settings.
+ */
+ @ConfigurationScope(Scope.AM)
+ @ConfigurationProperty
+ public static final String TEZ_LOG_PATTERN_LAYOUT_TASK = TEZ_PREFIX +
"log.pattern.layout.task";
+
+ /**
+ * Comma separated list of keys, which can used for defining keys in MDC.
The corresponding values
+ * will be read from Configuration, see tez.mdc.custom.keys.values.from for
further details.
+ */
+ @ConfigurationScope(Scope.AM)
+ @ConfigurationProperty
+ public static final String TEZ_MDC_CUSTOM_KEYS = TEZ_PREFIX +
"mdc.custom.keys";
+ public static final String TEZ_MDC_CUSTOM_KEYS_DEFAULT = "";
+
+ /**
+ * Comma separated list of keys, which can used for defining keys in
Configuration. Tez will try
Review comment:
nit: list of values?
##########
File path: tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java
##########
@@ -248,6 +248,42 @@ public TezConfiguration(boolean loadDefaults) {
public static final String TEZ_TASK_LOG_LEVEL = TEZ_TASK_PREFIX +
"log.level";
public static final String TEZ_TASK_LOG_LEVEL_DEFAULT = "INFO";
+ /**
+ * By this option, user can easily override the logging pattern which is
applied in
+ * TezContainerLogAppender in AM, regardless of the environmental settings.
+ */
+ @ConfigurationScope(Scope.AM)
+ @ConfigurationProperty
+ public static final String TEZ_LOG_PATTERN_LAYOUT_AM = TEZ_PREFIX +
"log.pattern.layout.am";
+
+ /**
+ * By this option, user can easily override the logging pattern which is
applied in
+ * TezContainerLogAppender in tasks, regardless of the environmental
settings.
+ */
+ @ConfigurationScope(Scope.AM)
+ @ConfigurationProperty
+ public static final String TEZ_LOG_PATTERN_LAYOUT_TASK = TEZ_PREFIX +
"log.pattern.layout.task";
+
+ /**
+ * Comma separated list of keys, which can used for defining keys in MDC.
The corresponding values
+ * will be read from Configuration, see tez.mdc.custom.keys.values.from for
further details.
+ */
+ @ConfigurationScope(Scope.AM)
+ @ConfigurationProperty
+ public static final String TEZ_MDC_CUSTOM_KEYS = TEZ_PREFIX +
"mdc.custom.keys";
+ public static final String TEZ_MDC_CUSTOM_KEYS_DEFAULT = "";
+
+ /**
+ * Comma separated list of keys, which can used for defining keys in
Configuration. Tez will try
Review comment:
nit: list of values?
##########
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:
Isn't this configurable via isThreadContextMapInheritable system
property?
----------------------------------------------------------------
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]
