From 6d1991fb6a577fa48b4effe6f5db4096a2237745 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Tue, 13 Jun 2023 10:36:40 +0200 Subject: [PATCH] cluster/logger: Fix leftover-log-rotation in multi-logger setups Populating log_metadata during zeek_init() is too late for the leftover-log-rotation functionality, so do it at script parse time. Also, prepend archiver_ to the log_metadata table and encoding function due to being in the global namespace and to align with the archiver_rotation_format_func. This hasn't been in a released version yet, so fine to rename still. Closes #3112 --- .../base/frameworks/cluster/nodes/logger.zeek | 31 +++++++------ .../out | 5 +++ .../leftover-log-rotation-multi-logger.zeek | 44 +++++++++++++++++++ 3 files changed, 64 insertions(+), 16 deletions(-) create mode 100644 testing/btest/Baseline/scripts.base.frameworks.cluster.leftover-log-rotation-multi-logger/out create mode 100644 testing/btest/scripts/base/frameworks/cluster/leftover-log-rotation-multi-logger.zeek diff --git a/scripts/base/frameworks/cluster/nodes/logger.zeek b/scripts/base/frameworks/cluster/nodes/logger.zeek index 1ea394913d..da4d892879 100644 --- a/scripts/base/frameworks/cluster/nodes/logger.zeek +++ b/scripts/base/frameworks/cluster/nodes/logger.zeek @@ -22,13 +22,21 @@ redef Log::default_rotation_interval = 1 hrs; ## Alarm summary mail interval. redef Log::default_mail_alarms_interval = 24 hrs; -## Generic log metadata rendered into the filename that zeek-archiver may interpret. -## This is populated with a log_suffix entry within zeek_init() when multiple -## logger nodes are defined in cluster-layout.zeek. -global log_metadata: table[string] of string; +## Generic log metadata rendered into filename that zeek-archiver may interpret. +global archiver_log_metadata: table[string] of string &redef; + +# Populate archiver_log_metadata with a "log_suffix" entry when multiple +# loggers are configured in Cluster::nodes. Need to evaluate at script +# loading time as leftover-log-rotation functionality is invoking +# archiver_rotation_format_func early on during InitPostScript(). +@if ( Cluster::get_node_count(Cluster::LOGGER) > 1 ) +redef archiver_log_metadata += { + ["log_suffix"] = Cluster::node, +}; +@endif ## Encode the given table as zeek-archiver understood metadata part. -function encode_log_metadata(tbl: table[string] of string): string +function archiver_encode_log_metadata(tbl: table[string] of string): string { local metadata_vec: vector of string; for ( k, v in tbl ) @@ -57,8 +65,8 @@ function archiver_rotation_format_func(ri: Log::RotationFmtInfo): Log::RotationP local close_str = strftime(Log::default_rotation_date_format, ri$close); local base = fmt("%s__%s__%s__", ri$path, open_str, close_str); - if ( |log_metadata| > 0 ) - base = fmt("%s%s__", base, encode_log_metadata(log_metadata)); + if ( |archiver_log_metadata| > 0 ) + base = fmt("%s%s__", base, archiver_encode_log_metadata(archiver_log_metadata)); local rval = Log::RotationPath($file_basename=base); return rval; @@ -71,15 +79,6 @@ redef Log::default_rotation_dir = "log-queue"; redef Log::rotation_format_func = archiver_rotation_format_func; redef LogAscii::enable_leftover_log_rotation = T; - -event zeek_init() - { - if ( "log_suffix" in log_metadata ) - return; - - if ( Cluster::get_node_count(Cluster::LOGGER) > 1 ) - log_metadata["log_suffix"] = Cluster::node; - } @else ## Use the cluster's archive logging script. diff --git a/testing/btest/Baseline/scripts.base.frameworks.cluster.leftover-log-rotation-multi-logger/out b/testing/btest/Baseline/scripts.base.frameworks.cluster.leftover-log-rotation-multi-logger/out new file mode 100644 index 0000000000..946ade5fb2 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.frameworks.cluster.leftover-log-rotation-multi-logger/out @@ -0,0 +1,5 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +./log-queue/conn__XXXX-XX-XX-XX-XX-XX__XXXX-XX-XX-XX-XX-XX__log_suffix=logger-2__.log +./log-queue/dns__XXXX-XX-XX-XX-XX-XX__XXXX-XX-XX-XX-XX-XX__log_suffix=logger-2__.log +leftover conn log +leftover dns log diff --git a/testing/btest/scripts/base/frameworks/cluster/leftover-log-rotation-multi-logger.zeek b/testing/btest/scripts/base/frameworks/cluster/leftover-log-rotation-multi-logger.zeek new file mode 100644 index 0000000000..d7b98df5aa --- /dev/null +++ b/testing/btest/scripts/base/frameworks/cluster/leftover-log-rotation-multi-logger.zeek @@ -0,0 +1,44 @@ +# @TEST-DOC: Ensure that left-over log rotation tags the logger name on as well. + +# @TEST-EXEC: echo ".log" >> .shadow.conn.log +# @TEST-EXEC: echo "archiver_rotation_format_func" >> .shadow.conn.log +# @TEST-EXEC: echo "leftover conn log" > conn.log + +# @TEST-EXEC: echo ".log" >> .shadow.dns.log +# @TEST-EXEC: echo "archiver_rotation_format_func" >> .shadow.dns.log +# @TEST-EXEC: echo "leftover dns log" > dns.log + +# Start Zeek as cluster node logger-2. +# @TEST-EXEC: CLUSTER_NODE=logger-2 zeek -b %INPUT > out + +# Ensure leftover files were removed. +# @TEST-EXEC: ! test -f .shadow.conn.log +# @TEST-EXEC: ! test -f conn.log +# @TEST-EXEC: ! test -f .shadow.dns.log +# @TEST-EXEC: ! test -f dns.log + +# Ensure the rotated files end-up in the default log-queue directory and have +# the logger-2 name encoded into them. +# @TEST-EXEC: ls ./log-queue/conn__*.log >>out +# @TEST-EXEC: ls ./log-queue/dns__*.log >>out +# @TEST-EXEC: cat ./log-queue/conn__*logger-2__.log ./log-queue/dns__*logger-2__.log >>out + +# @TEST-EXEC: TEST_DIFF_CANONIFIER='sed -r "s/[0-9]{2}/XX/g"' btest-diff out + +@TEST-START-FILE cluster-layout.zeek +redef Cluster::nodes = { + ["logger-1"] = [$node_type=Cluster::LOGGER, $ip=127.0.0.1, $p=1234/tcp], + ["logger-2"] = [$node_type=Cluster::LOGGER, $ip=127.0.0.1, $p=1235/tcp], +}; +@TEST-END-FILE + +# Switch settings into a supervisor/non-zeekctl setup +redef Log::default_rotation_dir = "log-queue"; +redef Log::rotation_format_func = archiver_rotation_format_func; +redef LogAscii::enable_leftover_log_rotation = T; +redef Log::default_rotation_postprocessor_cmd = ""; + +event zeek_init() + { + terminate(); + }