HADOOP-8865. Log warning message when loading deprecated properties

The goal is to emit the warning message when deprecated properties are loaded from XML configuration files to inform users that obsolete properties are in use. Before this patch we only inform users when a set/get operation is performed on a deprecated property but this is not sufficient cause if the users/apps are using the new key to obtain the value they will never see the warning. As a result when the property is finally removed or stops taking effect the application will fail or showcase different behavior.

With this patch deprecated properties may be logged twice:
* when a deprecated prop is loaded from a resource
* when a deprecated prop is accessed via get/set; if the deprecated key is not accessed no additional log is generated

Closes #7737

Signed-off-by: Chris Nauroth <[email protected]>

Author: zabetak

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java

@@ -3433,6 +3433,7 @@ void handleEndProperty() {
           deprecations.getDeprecatedKeyMap().get(confName);
 
       if (keyInfo != null) {
+        logDeprecation(keyInfo.getWarningMessage(confName, wrapper.toString()));
         keyInfo.clearAccessed();
         for (String key : keyInfo.newKeys) {
           // update new keys with deprecated key's value

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java

@@ -87,6 +87,7 @@
 import org.apache.log4j.AppenderSkeleton;
 import org.apache.log4j.Logger;
 import org.apache.log4j.spi.LoggingEvent;
+import org.junit.jupiter.api.io.TempDir;
 import org.mockito.Mockito;
 
 public class TestConfiguration {
@@ -120,7 +121,7 @@ public class TestConfiguration {
 
   @BeforeEach
   public void setUp() throws Exception {
-    conf = new Configuration();
+    conf = new Configuration(false);
   }
 
   @AfterEach
@@ -356,6 +357,33 @@ public void testFinalWarningsMultipleOverride() throws Exception {
     }
   }
 
+  @Test
+  public void testDeprecatedPropertyInXMLFileGeneratesLogMessage(@TempDir java.nio.file.Path tmp) throws IOException {
+    String oldProp = "test.deprecation.old.conf.a";
+    String newProp = "test.deprecation.new.conf.a";
+    Configuration.addDeprecation(oldProp, newProp);
+    java.nio.file.Path confFile = Files.createFile(tmp.resolve("TestConfiguration.xml"));
+    String confXml = "<configuration><property><name>" + oldProp + "</name><value>a</value></property></configuration>";
+    Files.write(confFile, confXml.getBytes());
+
+    TestAppender appender = new TestAppender();
+    Logger deprecationLogger = Logger.getLogger("org.apache.hadoop.conf.Configuration.deprecation");
+    deprecationLogger.addAppender(appender);
+
+    try {
+      conf.addResource(new Path(confFile.toUri()));
+      // Properties are lazily initialized so access them to trigger the loading of the resource
+      conf.getProps();
+    } finally {
+      deprecationLogger.removeAppender(appender);
+    }
+
+    Pattern deprecationMsgPattern = Pattern.compile(oldProp + " in file:" + confFile + " is deprecated");
+    boolean hasDeprecationMessage = appender.log.stream().map(LoggingEvent::getRenderedMessage)
+            .anyMatch(msg -> deprecationMsgPattern.matcher(msg).find());
+    assertTrue(hasDeprecationMessage);
+  }
+
   /**
    * A simple appender for white box testing.
    */
@@ -845,8 +873,7 @@ public void testToString() throws IOException {
     conf.addResource(fileResource);
 
     String expectedOutput =
-      "Configuration: core-default.xml, core-site.xml, " +
-      fileResource.toString();
+      "Configuration: " + fileResource;
     assertEquals(expectedOutput, conf.toString());
   }