Use proxy.config.log.hostname for rotated log filenames (#7943)

When ATS rotates logs it includes the hostname in the rotated log
filename.  The user has the ability to set the hostname via
'proxy.config.log.hostname', however ATS did not respected this
configuration parameter when naming the rotated log file.  Rather, ATS
always used the result of gethostname().  This fixes the log rotation
mechanism so that if the user sets 'proxy.config.log.hostname' to a
non-default value, then ATS will use that when naming the rotated log
filename.

Author: bneradt

proxy/logging/LogConfig.cc

@@ -23,6 +23,7 @@
 
 #include "tscore/ink_platform.h"
 #include "tscore/I_Layout.h"
+#include "I_Machine.h"
 
 #ifdef HAVE_SYS_PARAM_H
 #include <sys/param.h>
@@ -68,13 +69,7 @@
 void
 LogConfig::setup_default_values()
 {
-  const unsigned int bufSize = 512;
-  char name[bufSize];
-  if (!gethostname(name, bufSize)) {
-    ink_strlcpy(name, "unknown_host_name", sizeof(name));
-  }
-  hostname = ats_strdup(name);
-
+  hostname              = ats_strdup(Machine::instance()->hostname);
   log_buffer_size       = static_cast<int>(10 * LOG_KILOBYTE);
   max_secs_per_buffer   = 5;
   max_space_mb_for_logs = 100;
@@ -160,7 +155,7 @@ LogConfig::read_configuration_variables()
   ats_free(ptr);
 
   ptr = REC_ConfigReadString("proxy.config.log.hostname");
-  if (ptr != nullptr) {
+  if (ptr != nullptr && std::string_view(ptr) != "localhost") {
     ats_free(hostname);
     hostname = ptr;
   }

proxy/logging/LogFile.cc

@@ -73,7 +73,9 @@ LogFile::LogFile(const char *name, const char *header, LogFileFormat format, uin
 {
   if (m_file_format != LOG_FILE_PIPE) {
     m_log = new BaseLogFile(name, m_signature);
-    m_log->set_hostname(Machine::instance()->hostname);
+    // Use Log::config->hostname rather than Machine::instance()->hostname
+    // because the former is reloadable.
+    m_log->set_hostname(Log::config->hostname);
   } else {
     m_log = nullptr;
   }

proxy/shared/UglyLogStubs.cc

@@ -66,13 +66,31 @@ ConfigUpdateCbTable::invoke(const char * /* name ATS_UNUSED */)
 }
 
 struct Machine {
+  Machine();
+  ~Machine();
   static Machine *instance();
+  char *hostname = nullptr;
+
+private:
+  static Machine _instance;
 };
+
+Machine Machine::_instance;
+
+Machine::Machine()
+{
+  hostname = ats_strdup("test.host.com");
+}
+
+Machine::~Machine()
+{
+  ats_free(hostname);
+}
+
 Machine *
 Machine::instance()
 {
-  ink_release_assert(false);
-  return nullptr;
+  return &_instance;
 }
 
 NetAccept *

src/traffic_server/traffic_server.cc

@@ -1943,7 +1943,13 @@ main(int /* argc ATS_UNUSED */, const char **argv)
   } else if (HttpConfig::m_master.inbound_ip6.isValid()) {
     machine_addr.assign(HttpConfig::m_master.inbound_ip6);
   }
-  Machine::init(nullptr, &machine_addr.sa);
+  char *hostname = REC_ConfigReadString("proxy.config.log.hostname");
+  if (hostname != nullptr && std::string_view(hostname) == "localhost") {
+    // The default value was used. Let Machine::init derive the hostname.
+    hostname = nullptr;
+  }
+  Machine::init(hostname, &machine_addr.sa);
+  ats_free(hostname);
 
   RecRegisterStatString(RECT_PROCESS, "proxy.process.version.server.uuid", (char *)Machine::instance()->uuid.getString(),
                         RECP_NON_PERSISTENT);

tests/gold_tests/logging/log_retention.test.py

@@ -18,6 +18,7 @@
 #  limitations under the License.
 
 import os
+import socket
 
 Test.Summary = '''
 Test the enforcement of proxy.config.log.max_space_mb_for_logs.
@@ -46,7 +47,7 @@ class TestLogRetention:
     }
 
     __server = None
-    __ts_counter = 1
+    __ts_counter = 0
     __server_is_started = False
 
     def __init__(self, records_config, run_description, command="traffic_manager"):
@@ -144,14 +145,17 @@ def get_command_to_rotate_thrice(self):
 
 
 #
-# Run 1: Verify that log deletion happens when no min_count is specified.
+# Test 0: Verify that log deletion happens when no min_count is specified.
 #
+specified_hostname = 'my_hostname'
 twelve_meg_log_space = {
     # The following configures a 12 MB log cap with a required 2 MB head room.
     # Thus the rotated log of just over 10 MB should be deleted because it
     # will not leave enough head room.
     'proxy.config.log.max_space_mb_headroom': 2,
     'proxy.config.log.max_space_mb_for_logs': 12,
+    # Verify that setting a hostname changes the hostname used in rolled logs.
+    'proxy.config.log.hostname': specified_hostname,
 }
 test = TestLogRetention(twelve_meg_log_space,
                         "Verify log rotation and deletion of the configured log file with no min_count.")
@@ -187,7 +191,7 @@ def get_command_to_rotate_thrice(self):
     "Verify manager.log auto-delete configuration")
 # Verify test_deletion was rotated and deleted.
 test.ts.Streams.stderr += Testers.ContainsExpression(
-    "The rolled logfile.*test_deletion.log_.*was auto-deleted.*bytes were reclaimed",
+    f"The rolled logfile.*test_deletion.log_{specified_hostname}.*was auto-deleted.*bytes were reclaimed",
     "Verify that space was reclaimed")
 
 test.tr.Processes.Default.Command = test.get_command_to_rotate_once()
@@ -198,7 +202,7 @@ def get_command_to_rotate_thrice(self):
 
 
 #
-# Test 2: Verify log deletion happens with a min_count of 1.
+# Test 1: Verify log deletion happens with a min_count of 1.
 #
 test = TestLogRetention(twelve_meg_log_space,
                         "Verify log rotation and deletion of the configured log file with a min_count of 1.")
@@ -236,7 +240,7 @@ def get_command_to_rotate_thrice(self):
     "Verify manager.log auto-delete configuration")
 # Verify test_deletion was rotated and deleted.
 test.ts.Streams.stderr += Testers.ContainsExpression(
-    "The rolled logfile.*test_deletion.log_.*was auto-deleted.*bytes were reclaimed",
+    f"The rolled logfile.*test_deletion.log_{specified_hostname}.*was auto-deleted.*bytes were reclaimed",
     "Verify that space was reclaimed")
 
 test.tr.Processes.Default.Command = test.get_command_to_rotate_once()
@@ -246,7 +250,7 @@ def get_command_to_rotate_thrice(self):
 
 
 #
-# Test 3: Verify log deletion happens for a plugin's logs.
+# Test 2: Verify log deletion happens for a plugin's logs.
 #
 test = TestLogRetention(twelve_meg_log_space,
                         "Verify log rotation and deletion of plugin logs.")
@@ -279,7 +283,7 @@ def get_command_to_rotate_thrice(self):
 test.tr.StillRunningAfter = test.server
 
 #
-# Test 4: Verify log deletion priority behavior.
+# Test 3: Verify log deletion priority behavior.
 #
 twenty_two_meg_log_space = {
     # The following configures a 22 MB log cap with a required 2 MB head room.
@@ -332,8 +336,12 @@ def get_command_to_rotate_thrice(self):
 test.ts.Streams.stderr += Testers.ExcludesExpression(
     "The rolled logfile.*test_low_priority_deletion.log_.*was auto-deleted.*bytes were reclaimed",
     "Verify that space was reclaimed from test_high_priority_deletion")
+
+# Verify that ATS derives the hostname correctly if the user does not specify a
+# hostname via 'proxy.config.log.hostname'.
+hostname = socket.gethostname()
 test.ts.Streams.stderr += Testers.ContainsExpression(
-    "The rolled logfile.*test_high_priority_deletion.log_.*was auto-deleted.*bytes were reclaimed",
+    f"The rolled logfile.*test_high_priority_deletion.log_{hostname}.*was auto-deleted.*bytes were reclaimed",
     "Verify that space was reclaimed from test_high_priority_deletion")
 
 test.tr.Processes.Default.Command = test.get_command_to_rotate_once()
@@ -342,7 +350,7 @@ def get_command_to_rotate_thrice(self):
 test.tr.StillRunningAfter = test.server
 
 #
-# Test 5: Verify min_count configuration overrides.
+# Test 4: Verify min_count configuration overrides.
 #
 various_min_count_overrides = {
     'proxy.config.log.max_space_mb_for_logs': 22,
@@ -381,11 +389,13 @@ def get_command_to_rotate_thrice(self):
 
 
 #
-# Test 6: Verify log deletion does not happen when it is disabled.
+# Test 5: Verify log deletion does not happen when it is disabled.
 #
 auto_delete_disabled = twelve_meg_log_space.copy()
 auto_delete_disabled.update({
     'proxy.config.log.auto_delete_rolled_files': 0,
+    # Verify that setting a hostname changes the hostname used in rolled logs.
+    'proxy.config.log.hostname': 'my_hostname',
 })
 test = TestLogRetention(auto_delete_disabled,
                         "Verify log deletion does not happen when auto-delet is disabled.")
@@ -432,7 +442,7 @@ def get_command_to_rotate_thrice(self):
 test.tr.StillRunningAfter = test.server
 
 #
-# Test 7: Verify that max_roll_count is respected.
+# Test 6: Verify that max_roll_count is respected.
 #
 max_roll_count_of_2 = {
     'proxy.config.diags.debug.tags': 'log-file',
@@ -472,7 +482,7 @@ def get_command_to_rotate_thrice(self):
 test.tr.StillRunningAfter = test.server
 
 #
-# Test 8: Verify log deletion happens after a config reload.
+# Test 7: Verify log deletion happens after a config reload.
 #
 test = TestLogRetention(twelve_meg_log_space,
                         "Verify log rotation and deletion after a config reload.")